Thread (9 messages) flat view 9 messages, 4 authors, 2008-06-13

Re: [PATCH 1/2] Added support for PRTLVT based boards (MPC5121)

From: Grant Likely <hidden>
Date: 2008-06-12 14:10:48

Looking even better.  Just a few more comments.  I'll probably be able
to pick up the next one for inclusion in 2.6.27.

On Thu, Jun 12, 2008 at 5:44 AM, David Jander [off-list ref] wrote:
quoted hunk ↗ jump to hunk
 Made MPC5121_ADS board support generic:
 Renamed arch/powerpc/platforms/512x/mpc5121_ads.c and added list of supported
 boards.
 For both MPC5121 ADS or PRTLVT support, just select "MPC5121_GENERIC" and use
 the corresponding device-tree.

Signed-off-by: David Jander <redacted>
---
 arch/powerpc/boot/dts/prtlvt.dts                   |  272 ++++++++++++++++++++
 arch/powerpc/platforms/512x/Kconfig                |   14 +-
 arch/powerpc/platforms/512x/Makefile               |    2 +-
 .../512x/{mpc5121_ads.c => mpc5121_generic.c}      |   38 ++-
 4 files changed, 307 insertions(+), 19 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/prtlvt.dts
 rename arch/powerpc/platforms/512x/{mpc5121_ads.c => mpc5121_generic.c} (73%)
diff --git a/arch/powerpc/boot/dts/prtlvt.dts b/arch/powerpc/boot/dts/prtlvt.dts
new file mode 100644
index 0000000..a011c8c
--- /dev/null
+++ b/arch/powerpc/boot/dts/prtlvt.dts
@@ -0,0 +1,272 @@
+/*
+ * Device tree source for PRTLVT based boards, base on:
+ * MPC5121E MDS Device Tree Source
+ *
+ * Copyright 2007 Freescale Semiconductor Inc.
+ * Copyright 2008 Protonic Holland
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+ /* compile with: ./dtc -p 10240 -R 20 -I dts -o prtlvt.dtb -O dtb -b 0 dts/prtlvt.dts */
+
+/dts-v1/;
+
+/ {
+       model = "prtlvt";
+       compatible = "prt,prtlvt";
+       #address-cells = <1>;
+       #size-cells = <1>;
+
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               PowerPC,5121@0 {
+                       device_type = "cpu";
+                       reg = <0>;
+                       d-cache-line-size = <0x20>;     // 32 bytes
+                       i-cache-line-size = <0x20>;     // 32 bytes
+                       d-cache-size = <0x8000>;        // L1, 32K
+                       i-cache-size = <0x8000>;        // L1, 32K
+                       timebase-frequency = <50000000>;// 50 MHz (csb/4)
+                       bus-frequency = <200000000>;    // 200 MHz csb bus
+                       clock-frequency = <400000000>;  // 400 MHz ppc core
+               };
+       };
+
+       memory {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000>;  // 256MB at 0
+       };
+
+       flash@fe000000 {
There should probably be a node to describe the local bus that the
flash is attached to and this flash node should be a child of the bus.
+               compatible = "amd,s29gl256n", "cfi-flash";
+               reg = <0xfe000000 0x02000000>;
+               bank-width = <2>;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               rootfs@0 {
+                       label = "rootfs";
+                       reg = <0x00000000 0x01800000>;
+               };
+               config@1800000 {
+                       label ="config0";
+                       reg = <0x01800000 0x00200000>;
+               };
+               config@1a00000 {
+                       label ="config1";
+                       reg = <0x01a00000 0x00200000>;
+               };
+               kernel@1c00000 {
+                       label ="kernel";
+                       reg = <0x01c00000 0x002e0000>;
+               };
+               dt@1ee0000 {
+                       label ="devicetree";
+                       reg = <0x01ee0000 0x00020000>;
+               };
+               uboot@1ee0000 {
+                       label ="uboot";
+                       reg = <0x01f00000 0x00100000>;
+               };
+       };
I'm still not all that keen on encoding the partition information into
the 'stock' device tree included with the kernel as it is more of a
configuration description that is more properly supplied by the
bootloader.  This is a debate that has been going back and forth over
the last few months, so there isn't a solid concensus yet, but my
preference is to remove or comment out the partition information for
now.
+               i2ccontrol@1760 {
+                       compatible = "fsl,mpc5121-i2c-ctrl";
+                       reg = <0x1760 0x8>;
+               };
+
+               diu@2100 {
(nitpick) There is a recommended practice that says node names should
be generic as much as possible, so I think this should probably be
video@2100.  The compatible value is fine.
+               ethernet@2800 {
+                       compatible = "fsl,mpc5121-fec";
+                       reg = <0x2800 0x800>;
+                       local-mac-address = [ 00 00 00 00 00 00 ];
+                       interrupts = <0x4 0x8 >;
+                       interrupt-parent = < &ipic >;
+                       phy-handle = < &phy >;
+               };
+
+               // This dma controller is not compatible with fsldma
+               dma2@14000 {
Also for generic names; 'dma@14000' makes more sense than 'dma2@14000'.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help