On 07/09/2013 03:35:43 AM, Haijun Zhang wrote:
Overview of P1020RDB-PD device:
- DDR3 2GB
- NOR flash 64MB
- NAND flash 128MB
- SPI flash 16MB
- I2C EEPROM 256Kb
- eTSEC1 (RGMII PHY) connected to VSC7385 L2 switch
- eTSEC2 (SGMII PHY)
- eTSEC3 (RGMII PHY)
- SDHC
- 2 USB ports
- 4 TDM ports
- PCIe
Signed-off-by: Haijun Zhang <haijun.zh...@freescale.com>
Signed-off-by: Jerry Huang <chang-ming.hu...@freescale.com>
Signed-off-by: Xie Xiaobo-R63061 <x....@freescale.com>
CC: Scott Wood <scottw...@freescale.com>
---
changes for v3:
- Remove some blank and changed the usb node
- Renamed the dts file
- change the cpld name of pd board
arch/powerpc/boot/dts/p1020rdb-pd.dts | 90 ++++++++++++
arch/powerpc/boot/dts/p1020rdb-pd.dtsi | 257
+++++++++++++++++++++++++++++++++
2 files changed, 347 insertions(+)
create mode 100644 arch/powerpc/boot/dts/p1020rdb-pd.dts
create mode 100644 arch/powerpc/boot/dts/p1020rdb-pd.dtsi
Again, why do you need a separate .dtsi? If this isn't for a 32/36-bit
split, what is the criteria for what goes in the .dts versus what goes
in the .dtsi?
+ partition@600000 {
+ /* 4MB for Compressed Root file System Image */
+ reg = <0x00600000 0x00400000>;
+ label = "NAND Compressed RFS Image";
+ };
+
+ partition@a00000 {
+ /* 22MB for JFFS2 based Root file System */
+ reg = <0x00a00000 0x01600000>;
+ label = "NAND JFFS2 Root File System";
+ };
Don't refer to JFFS2. It's bad enough that we specify partition layout
here -- no need to specify the filesystem type, especially when it's a
fs type that is no longer recommended.
+ partition@2000000 {
+ /* 96MB for RAMDISK based Root file System */
+ reg = <0x02000000 0x06000000>;
+ label = "NAND Writable User area";
+ };
Wouldn't it be better to combine these last three partitions? Why do
you need three root filesystems?
+ cpld@2,0 {
+ compatible = "fsl,p1020rdb-pd-cpld";
+ reg = <0x2 0x0 0x20000>;
+ read-only;
+ };
Remove read-only.
+ spi@7000 {
+ flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "spansion,s25sl12801";
+ reg = <0>;
+ spi-max-frequency = <40000000>; /* input clock
*/
+
+ partition@u-boot {
+ /* 512KB for u-boot Bootloader Image */
+ reg = <0x0 0x00080000>;
+ label = "u-boot";
+ read-only;
+ };
+
+ partition@dtb {
+ /* 512KB for DTB Image*/
+ reg = <0x00080000 0x00080000>;
+ label = "dtb";
+ };
+
+ partition@kernel {
+ /* 4MB for Linux Kernel Image */
+ reg = <0x00100000 0x00400000>;
+ label = "kernel";
+ };
These unit addresses are not appropriate. They should match reg, not
label.
+ partition@fs {
+ /* 4MB for Compressed RFS Image */
+ reg = <0x00500000 0x00400000>;
+ label = "file system";
+ };
+
+ partition@jffs-fs {
+ /* 7MB for JFFS2 based RFS */
+ reg = <0x00900000 0x00700000>;
+ label = "file system jffs2";
+ };
As with NAND flash, please combine these and don't reference JFFS2.
+ };
+
+ slic@0 {
+ compatible = "zarlink,le88266";
+ reg = <1>;
+ spi-max-frequency = <8000000>;
+ };
+
+ slic@1 {
+ compatible = "zarlink,le88266";
+ reg = <2>;
+ spi-max-frequency = <8000000>;
+ };
+ };
+
+ mdio@24000 {
+ phy0: ethernet-phy@0 {
+ interrupts = <3 1 0 0>;
+ reg = <0x0>;
+ };
+ phy1: ethernet-phy@1 {
+ interrupts = <2 1 0 0>;
+ reg = <0x1>;
+ };
+ };
Again, leave a blank line between nodes. If I comment about a style
issue in one place, you should also fix the same issue in other places
where it occurs.
+ /*
+ * USB2 is shared with localbus, so it must be disabled
+ * by default. We can't put 'status = "disabled";' here
+ * since U-Boot doesn't clear the status property when
+ * it enables USB2. OTOH, U-Boot does create a new node
+ * when there isn't any. So, just comment it out.
+ * usb@23000 {
+ * status = "disabled";
+ * phy_type = "ulpi";
+ * };
+ */
No. Fix U-Boot to do whatever updating it needs to do based on runtime
configuration. Do not add commented out nodes to the tree.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
