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

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

[David Jander]

 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 <david@protonic.nl>
---
 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 {
+		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>;
+		};
+	};
+	
+	soc@80000000 {
+		compatible = "fsl,mpc5121-immr", "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		#interrupt-cells = <2>;
+		ranges = <0x0 0x80000000 0x400000>;
+		reg = <0x80000000 0x400000>;
+		bus-frequency = <66000000>;	// 66 MHz ips bus
+
+
+		// IPIC
+		// interrupts cell = <intr #, sense>
+		// sense values match linux IORESOURCE_IRQ_* defines:
+		// sense == 8: Level, low assertion
+		// sense == 2: Edge, high-to-low change
+		//
+		ipic: interrupt-controller@c00 {
+			compatible = "fsl,mpc5121-ipic", "fsl,ipic";
+			interrupt-controller;
+			#address-cells = <0>;
+			#interrupt-cells = <2>;
+			reg = <0xc00 0x100>;
+		};
+
+		// 512x PSCs are not 52xx PSCs compatible
+		// PSC0 serial port aka ttyPSC0
+		serial@11000 {
+			device_type = "serial";
+			compatible = "fsl,mpc5121-psc-uart";
+			port-number = <0>;
+			cell-index = <0>;
+			reg = <0x11000 0x100>;
+			interrupts = <0x28 0x8>; // actually the fifo irq
+			interrupt-parent = < &ipic >;
+		};
+
+		// PSC1 serial port aka ttyPSC1
+		serial@11100 {
+			device_type = "serial";
+			compatible = "fsl,mpc5121-psc-uart";
+			port-number = <1>;
+			cell-index = <1>;
+			reg = <0x11100 0x100>;
+			interrupts = <0x28 0x8>; // actually the fifo irq
+			interrupt-parent = < &ipic >;
+		};
+
+		// PSC2 serial port aka ttyPSC2
+		serial@11200 {
+			device_type = "serial";
+			compatible = "fsl,mpc5121-psc-uart";
+			port-number = <2>;
+			cell-index = <2>;
+			reg = <0x11200 0x100>;
+			interrupts = <0x28 0x8>; // actually the fifo irq
+			interrupt-parent = < &ipic >;
+		};
+		
+		// PSC3 serial port aka ttyPSC3
+		serial@11300 {
+			device_type = "serial";
+			compatible = "fsl,mpc5121-psc-uart";
+			port-number = <3>;
+			cell-index = <3>;
+			reg = <0x11300 0x100>;
+			interrupts = <0x28 0x8>; // actually the fifo irq
+			interrupt-parent = < &ipic >;
+		};
+
+		pscsfifo@11f00 {
+			compatible = "fsl,mpc5121-psc-fifo";
+			reg = <0x11f00 0x100>;
+			interrupts = <0x28 0x8>;
+			interrupt-parent = < &ipic >;
+		};
+
+		i2c@1700 {
+			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
+			reg = <0x1700 0x20>;
+			interrupts = <0x9 0x8>;
+			interrupt-parent = < &ipic >;
+			fsl5200-clocking;
+		};
+
+		i2c@1720 {
+			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
+			reg = <0x1720 0x20>;
+			interrupts = <0xa 0x8>;
+			interrupt-parent = < &ipic >;
+			fsl5200-clocking;
+		};
+
+		i2c@1740 {
+			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
+			reg = <0x1740 0x20>;
+			interrupts = <0xb 0x8>;
+			interrupt-parent = < &ipic >;
+			fsl5200-clocking;
+		};
+
+		i2ccontrol@1760 {
+			compatible = "fsl,mpc5121-i2c-ctrl";
+			reg = <0x1760 0x8>;
+		};
+
+		diu@2100 {
+			compatible = "fsl,mpc5121-diu", "fsl-diu";
+			reg = <0x2100 0x100>;
+			interrupts = <0x40 0x8>;
+			interrupt-parent = < &ipic >;
+			clk-name = "diu_clk";
+			clk-parent = "ips_clk";
+			clk-ctrl = <0x1 0x1f>; // sccr2 bit 31
+		};
+
+		mdio@2800 {
+			compatible = "fsl,mpc5121-fec-mdio";
+			reg = <0x2800 0x800>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			phy: ethernet-phy@0 {
+				reg = <1>;
+				device_type = "ethernet-phy";
+			};
+		};
+
+		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 >;
+		};
+
+		// port1 using extern ULPI PHY
+		usb@3000 {
+			compatible = "fsl,mpc5121-usb2-dr", "fsl-usb2-dr";
+			reg = <0x3000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupt-parent = < &ipic >;
+			interrupts = <0x2c 0x8>;
+			dr_mode = "otg";
+			phy_type = "ulpi";
+			port1;
+		};
+
+		// port0 using internal UTMI PHY
+		usb@4000 {
+			compatible = "fsl,mpc5121-usb2-dr", "fsl-usb2-dr";
+			reg = <0x4000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupt-parent = < &ipic >;
+			interrupts = <0x2b 0x8>;
+			dr_mode = "otg";
+			phy_type = "utmi";
+			port0;
+		};
+
+		// PSC4 in i2s mode
+		i2s@11500 {
+			compatible = "fsl,mpc5121-psc-i2s";
+			cell-index = <4>;
+			reg = <0x11400 0x100>;
+			interrupts = <0x20 8>;
+			interrupt-parent = < &ipic >;
+		};
+
+		// PSC5 in SPI mode for touchscreen
+		spi@11500 {
+			compatible = "fsl,mpc5121-psc-spi";
+			cell-index = <5>;
+			reg = <0x11500 0x100>;
+			interrupts = <0x21 8>;
+			interrupt-parent = < &ipic >;
+		};
+
+		// This dma controller is not compatible with fsldma
+		dma2@14000 {
+			compatible = "fsl,mpc5121-dma2";
+			reg = <0x14000 0x1800>;
+			interrupts = <0x41 0x8>;
+			interrupt-parent = < &ipic >;
+		};
+
+		sata@20000 {
+			compatible = "fsl,mpc5121-sata";
+			reg = <0x20000 0x2000>;
+			interrupts = <0x2d 0x8 0x56 0x8>;
+			interrupt-parent = < &ipic >;
+		};
+	};
+};
diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
index 4c0da0c..57b3912 100644
--- a/arch/powerpc/platforms/512x/Kconfig
+++ b/arch/powerpc/platforms/512x/Kconfig
@@ -2,18 +2,20 @@ config PPC_MPC512x
 	bool
 	select FSL_SOC
 	select IPIC
-	default n
 
 config PPC_MPC5121
 	bool
 	select PPC_MPC512x
-	default n
 
-config MPC5121_ADS
-	bool "Freescale MPC5121E ADS"
+config MPC5121_GENERIC
+	bool "Generic support for simple MPC5121 based boards"
 	depends on PPC_MULTIPLATFORM && PPC32
 	select DEFAULT_UIMAGE
 	select PPC_MPC5121
 	help
-	  This option enables support for the MPC5121E ADS board.
-	default n
+	  This option enables support for a simple MPC5121 based boards which
+	  do not need a custom platform specific setup.
+
+	  Boards that are compatible with this generic platform support
+	  are: Freescale MPC5121 ADS and Protonic LVT based boards (ZANMCU
+	  and VICVT2).
diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
index 232c89f..9d40a2e 100644
--- a/arch/powerpc/platforms/512x/Makefile
+++ b/arch/powerpc/platforms/512x/Makefile
@@ -1,4 +1,4 @@
 #
 # Makefile for the Freescale PowerPC 512x linux kernel.
 #
-obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o
+obj-$(CONFIG_MPC5121_GENERIC)	+= mpc5121_generic.o
diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
similarity index 73%
rename from arch/powerpc/platforms/512x/mpc5121_ads.c
rename to arch/powerpc/platforms/512x/mpc5121_generic.c
index 50bd3a3..824ddbb 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads.c
+++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
@@ -4,7 +4,7 @@
  * Author: John Rigby, <jrigby@freescale.com>, Thur Mar 29 2007
  *
  * Description:
- * MPC5121 ADS board setup
+ * MPC5121 SoC setup
  *
  * This is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by
@@ -58,7 +58,7 @@ static struct of_device_id __initdata of_bus_ids[] = {
 	{},
 };
 
-static void __init mpc5121_ads_declare_of_platform_devices(void)
+static void __init mpc5121_generic_declare_of_platform_devices(void)
 {
 	/* Find every child of the SOC node and add it to of_platform */
 	if (of_platform_bus_probe(NULL, of_bus_ids, NULL))
@@ -66,7 +66,7 @@ static void __init mpc5121_ads_declare_of_platform_devices(void)
 			"Error while probing of_platform bus\n");
 }
 
-static void __init mpc5121_ads_init_IRQ(void)
+static void __init mpc5121_generic_init_IRQ(void)
 {
 	struct device_node *np;
 
@@ -84,21 +84,35 @@ static void __init mpc5121_ads_init_IRQ(void)
 	ipic_set_default_priority();
 }
 
+/* list of the supported boards */
+static char *board[] __initdata = {
+	"fsl,mpc5121ads",
+	"prt,prtlvt",
+	NULL
+};
+
 /*
  * Called very early, MMU is off, device-tree isn't unflattened
  */
-static int __init mpc5121_ads_probe(void)
+static int __init mpc5121_generic_probe(void)
 {
-	unsigned long root = of_get_flat_dt_root();
+	unsigned long node = of_get_flat_dt_root();
+	int i = 0;
 
-	return of_flat_dt_is_compatible(root, "fsl,mpc5121ads");
+	while (board[i]) {
+		if (of_flat_dt_is_compatible(node, board[i]))
+			break;
+		i++;
+	}
+	
+	return (board[i] != NULL);
 }
 
-define_machine(mpc5121_ads) {
-	.name			= "MPC5121 ADS",
-	.probe			= mpc5121_ads_probe,
-	.init			= mpc5121_ads_declare_of_platform_devices,
-	.init_IRQ		= mpc5121_ads_init_IRQ,
+define_machine(mpc5121_generic) {
+	.name			= "MPC5121 generic",
+	.probe			= mpc5121_generic_probe,
+	.init			= mpc5121_generic_declare_of_platform_devices,
+	.init_IRQ		= mpc5121_generic_init_IRQ,
 	.get_irq		= ipic_get_irq,
-	.calibrate_decr		= generic_calibrate_decr,
+	.calibrate_decr	= generic_calibrate_decr,
 };
-- 
1.5.4.3

[PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB to current head

[David Jander]

Signed-off-by: David Jander <david@protonic.nl>
---
 arch/powerpc/platforms/Kconfig     |    2 +-
 drivers/net/fec.h                  |   43 ++++++++++++++++++++
 drivers/net/fs_enet/Kconfig        |   22 +++++++++-
 drivers/net/fs_enet/fs_enet-main.c |   76 ++++++++++++++++++++++++++++++------
 drivers/net/fs_enet/fs_enet.h      |   17 +++++---
 drivers/net/fs_enet/mac-fec.c      |   22 +++++++++-
 drivers/net/fs_enet/mii-fec.c      |   10 ++++-
 7 files changed, 167 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 87454c5..a96937f 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -288,7 +288,7 @@ config CPM2
 
 config PPC_CPM_NEW_BINDING
 	bool
-	depends on CPM1 || CPM2
+	depends on CPM1 || CPM2 || FS_ENET_MPC5121_FEC
 	default y
 
 config AXON_RAM
diff --git a/drivers/net/fec.h b/drivers/net/fec.h
index 292719d..5c9fe34 100644
--- a/drivers/net/fec.h
+++ b/drivers/net/fec.h
@@ -59,6 +59,7 @@ typedef struct fec {
 } fec_t;
 
 #else
+#if !defined(CONFIG_FS_ENET_MPC5121_FEC)
 
 /*
  *	Define device register set address map.
@@ -97,6 +98,48 @@ typedef struct fec {
 	unsigned long	fec_fifo_ram[112];	/* FIFO RAM buffer */
 } fec_t;
 
+#else /* CONFIG_FS_ENET_MPC5121_FEC */
+
+typedef struct fec {
+	u32 fec_reserved0;
+	u32 fec_ievent;		/* Interrupt event reg */
+	u32 fec_imask;		/* Interrupt mask reg */
+	u32 fec_reserved1;
+	u32 fec_r_des_active;	/* Receive descriptor reg */
+	u32 fec_x_des_active;	/* Transmit descriptor reg */
+	u32 fec_reserved2[3];
+	u32 fec_ecntrl;		/* Ethernet control reg */
+	u32 fec_reserved3[6];
+	u32 fec_mii_data;		/* MII manage frame reg */
+	u32 fec_mii_speed;		/* MII speed control reg */
+	u32 fec_reserved4[7];
+	u32 fec_mib_ctrlstat;	/* MIB control/status reg */
+	u32 fec_reserved5[7];
+	u32 fec_r_cntrl;		/* Receive control reg */
+	u32 fec_reserved6[15];
+	u32 fec_x_cntrl;		/* Transmit Control reg */
+	u32 fec_reserved7[7];
+	u32 fec_addr_low;		/* Low 32bits MAC address */
+	u32 fec_addr_high;		/* High 16bits MAC address */
+	u32 fec_opd;		/* Opcode + Pause duration */
+	u32 fec_reserved8[10];
+	u32 fec_hash_table_high;	/* High 32bits hash table */
+	u32 fec_hash_table_low;	/* Low 32bits hash table */
+	u32 fec_grp_hash_table_high;/* High 32bits hash table */
+	u32 fec_grp_hash_table_low;	/* Low 32bits hash table */
+	u32 fec_reserved9[7];
+	u32 fec_x_wmrk;		/* FIFO transmit water mark */
+	u32 fec_reserved10;
+	u32 fec_r_bound;		/* FIFO receive bound reg */
+	u32 fec_r_fstart;		/* FIFO receive start reg */
+	u32 fec_reserved11[11];
+	u32 fec_r_des_start;	/* Receive descriptor ring */
+	u32 fec_x_des_start;	/* Transmit descriptor ring */
+	u32 fec_r_buff_size;	/* Maximum receive buff size */
+	u32 fec_dma_control;	/* DMA Endian and other ctrl */
+} fec_t;
+
+#endif /* CONFIG_FS_ENET_MPC5121_FEC */
 #endif /* CONFIG_M5272 */
 
 
diff --git a/drivers/net/fs_enet/Kconfig b/drivers/net/fs_enet/Kconfig
index 562ea68..5e2520b 100644
--- a/drivers/net/fs_enet/Kconfig
+++ b/drivers/net/fs_enet/Kconfig
@@ -1,9 +1,23 @@
 config FS_ENET
        tristate "Freescale Ethernet Driver"
-       depends on CPM1 || CPM2
+       depends on CPM1 || CPM2 || FS_ENET_MPC5121_FEC
        select MII
        select PHYLIB
 
+config FS_ENET_MPC5121_FEC
+	bool "Freescale MPC512x FEC driver"
+	depends on PPC_MPC512x
+	select FS_ENET
+	select FS_ENET_HAS_FEC
+
+config FS_ENET_TX_ALIGN_WORKAROUND
+	bool "MPC5121 FEC driver TX alignment workaround"
+	depends on FS_ENET_MPC5121_FEC
+	help
+	  Workaround for a problem with early Freescale MPC5121 chips.
+	  If unsure say 'y'
+	default y
+
 config FS_ENET_HAS_SCC
 	bool "Chip has an SCC usable for ethernet"
 	depends on FS_ENET && (CPM1 || CPM2)
@@ -16,13 +30,15 @@ config FS_ENET_HAS_FCC
 
 config FS_ENET_HAS_FEC
 	bool "Chip has an FEC usable for ethernet"
-	depends on FS_ENET && CPM1
+	depends on FS_ENET && (CPM1 || FS_ENET_MPC5121_FEC)
 	select FS_ENET_MDIO_FEC
 	default y
 
+
 config FS_ENET_MDIO_FEC
 	tristate "MDIO driver for FEC"
-	depends on FS_ENET && CPM1
+	depends on FS_ENET && (CPM1 || FS_ENET_MPC5121_FEC)
+
 
 config FS_ENET_MDIO_FCC
 	tristate "MDIO driver for FCC"
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 31c9693..54f0079 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -592,6 +592,31 @@ void fs_cleanup_bds(struct net_device *dev)
 
 /**********************************************************************************/
 
+#ifdef CONFIG_FS_ENET_TX_ALIGN_WORKAROUND
+static struct sk_buff *aligntxskb(struct net_device *dev, struct sk_buff *skb)
+{
+	struct sk_buff *skbn;
+	skbn = dev_alloc_skb(ENET_RX_FRSIZE+0x20);
+	if (skbn)
+		skb_align(skbn, 0x20);
+
+	if (!skbn) { 
+	    printk(KERN_WARNING DRV_MODULE_NAME
+		    ": %s Memory squeeze, dropping tx packet.\n",
+		    dev->name);
+		dev_kfree_skb_any(skb);
+		return NULL;
+	}
+
+	skb_copy_from_linear_data(skb, skbn->data, skb->len);
+	skb_put(skbn, skb->len);
+	dev_kfree_skb_any(skb);
+	return skbn;
+}
+#else
+#define aligntxskb(skb) skb
+#endif
+
 static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct fs_enet_private *fep = netdev_priv(dev);
@@ -600,6 +625,7 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u16 sc;
 	unsigned long flags;
 
+	skb = aligntxskb(dev, skb);
 	spin_lock_irqsave(&fep->tx_lock, flags);
 
 	/*
@@ -1370,10 +1396,17 @@ static struct of_device_id fs_enet_match[] = {
 	},
 #endif
 #ifdef CONFIG_FS_ENET_HAS_FEC
+#ifndef CONFIG_FS_ENET_MPC5121_FEC
 	{
 		.compatible = "fsl,pq1-fec-enet",
 		.data = (void *)&fs_fec_ops,
 	},
+#else
+	{
+		.compatible = "fsl,mpc5121-fec",
+		.data = (void *)&fs_fec_ops,
+	},
+#endif
 #endif
 	{}
 };
@@ -1431,15 +1464,38 @@ static int fs_enet_remove(struct device *dev)
 	return fs_cleanup_instance(dev_get_drvdata(dev));
 }
 
+#ifdef CONFIG_PM
+static int fs_enet_suspend(struct device *dev, pm_message_t state)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+
+	if (netif_running(ndev))
+		fs_enet_close(ndev);
+
+	return 0;
+}
+
+static int fs_enet_resume(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+
+	if (netif_running(ndev))
+		fs_enet_open(ndev);
+
+	return 0;
+}
+#else
+#define fs_enet_suspend	NULL
+#define fs_enet_resume	NULL
+#endif
+
 static struct device_driver fs_enet_fec_driver = {
 	.name	  	= "fsl-cpm-fec",
 	.bus		= &platform_bus_type,
 	.probe		= fs_enet_probe,
 	.remove		= fs_enet_remove,
-#ifdef CONFIG_PM
-/*	.suspend	= fs_enet_suspend,	TODO */
-/*	.resume		= fs_enet_resume,	TODO */
-#endif
+	.suspend	= fs_enet_suspend,
+	.resume		= fs_enet_resume,
 };
 
 static struct device_driver fs_enet_scc_driver = {
@@ -1447,10 +1503,8 @@ static struct device_driver fs_enet_scc_driver = {
 	.bus		= &platform_bus_type,
 	.probe		= fs_enet_probe,
 	.remove		= fs_enet_remove,
-#ifdef CONFIG_PM
-/*	.suspend	= fs_enet_suspend,	TODO */
-/*	.resume		= fs_enet_resume,	TODO */
-#endif
+	.suspend	= fs_enet_suspend,
+	.resume		= fs_enet_resume,
 };
 
 static struct device_driver fs_enet_fcc_driver = {
@@ -1458,10 +1512,8 @@ static struct device_driver fs_enet_fcc_driver = {
 	.bus		= &platform_bus_type,
 	.probe		= fs_enet_probe,
 	.remove		= fs_enet_remove,
-#ifdef CONFIG_PM
-/*	.suspend	= fs_enet_suspend,	TODO */
-/*	.resume		= fs_enet_resume,	TODO */
-#endif
+	.suspend	= fs_enet_suspend,
+	.resume		= fs_enet_resume,
 };
 
 static int __init fs_init(void)
diff --git a/drivers/net/fs_enet/fs_enet.h b/drivers/net/fs_enet/fs_enet.h
index e05389c..ea3cf2f 100644
--- a/drivers/net/fs_enet/fs_enet.h
+++ b/drivers/net/fs_enet/fs_enet.h
@@ -10,10 +10,15 @@
 
 #include <linux/fs_enet_pd.h>
 #include <asm/fs_pd.h>
+#ifdef CONFIG_FS_ENET_MPC5121_FEC
+#include "../fec.h"
+#endif
 
 #ifdef CONFIG_CPM1
 #include <asm/cpm1.h>
+#endif
 
+#if defined(CONFIG_CPM1) || defined(CONFIG_FS_ENET_MPC5121_FEC)
 struct fec_info {
 	fec_t __iomem *fecp;
 	u32 mii_speed;
@@ -177,14 +182,14 @@ void fs_enet_platform_cleanup(void);
 #endif
 
 /* write */
-#define CBDW_SC(_cbd, _sc) 		__cbd_out16(&(_cbd)->cbd_sc, (_sc))
-#define CBDW_DATLEN(_cbd, _datlen)	__cbd_out16(&(_cbd)->cbd_datlen, (_datlen))
-#define CBDW_BUFADDR(_cbd, _bufaddr)	__cbd_out32(&(_cbd)->cbd_bufaddr, (_bufaddr))
+#define CBDW_SC(_cbd, _sc) 		__cbd_out16((volatile void __iomem *)&(_cbd)->cbd_sc, (_sc))
+#define CBDW_DATLEN(_cbd, _datlen)	__cbd_out16((volatile void __iomem *)&(_cbd)->cbd_datlen, (_datlen))
+#define CBDW_BUFADDR(_cbd, _bufaddr)	__cbd_out32((volatile void __iomem *)&(_cbd)->cbd_bufaddr, (_bufaddr))
 
 /* read */
-#define CBDR_SC(_cbd) 			__cbd_in16(&(_cbd)->cbd_sc)
-#define CBDR_DATLEN(_cbd)		__cbd_in16(&(_cbd)->cbd_datlen)
-#define CBDR_BUFADDR(_cbd)		__cbd_in32(&(_cbd)->cbd_bufaddr)
+#define CBDR_SC(_cbd) 			__cbd_in16((volatile void __iomem *)&(_cbd)->cbd_sc)
+#define CBDR_DATLEN(_cbd)		__cbd_in16((volatile void __iomem *)&(_cbd)->cbd_datlen)
+#define CBDR_BUFADDR(_cbd)		__cbd_in32((volatile void __iomem *)&(_cbd)->cbd_bufaddr)
 
 /* set bits */
 #define CBDS_SC(_cbd, _sc) 		CBDW_SC(_cbd, CBDR_SC(_cbd) | (_sc))
diff --git a/drivers/net/fs_enet/mac-fec.c b/drivers/net/fs_enet/mac-fec.c
index 8a311d1..ddbe548 100644
--- a/drivers/net/fs_enet/mac-fec.c
+++ b/drivers/net/fs_enet/mac-fec.c
@@ -42,6 +42,9 @@
 #include <asm/mpc8xx.h>
 #include <asm/cpm1.h>
 #endif
+#ifdef CONFIG_FS_ENET_MPC5121_FEC
+#include "../fec.h"
+#endif
 
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 #include <asm/of_device.h>
@@ -306,7 +309,9 @@ static void restart(struct net_device *dev)
 	 * Set maximum receive buffer size.
 	 */
 	FW(fecp, r_buff_size, PKT_MAXBLR_SIZE);
+#ifndef CONFIG_FS_ENET_MPC5121_FEC
 	FW(fecp, r_hash, PKT_MAXBUF_SIZE);
+#endif
 
 	/* get physical address */
 	rx_bd_base_phys = fep->ring_mem_addr;
@@ -320,10 +325,17 @@ static void restart(struct net_device *dev)
 
 	fs_init_bds(dev);
 
+#ifndef CONFIG_FS_ENET_MPC5121_FEC
 	/*
 	 * Enable big endian and don't care about SDMA FC.
 	 */
 	FW(fecp, fun_code, 0x78000000);
+#else
+	/*
+	 * Set DATA_BO and DESC_BO and leave the rest unchanged
+	 */
+	FS(fecp, dma_control, 0xc0000000);
+#endif
 
 	/*
 	 * Set MII speed.
@@ -334,11 +346,13 @@ static void restart(struct net_device *dev)
 	 * Clear any outstanding interrupt.
 	 */
 	FW(fecp, ievent, 0xffc0);
+#ifndef CONFIG_FS_ENET_MPC5121_FEC
 #ifndef CONFIG_PPC_MERGE
 	FW(fecp, ivec, (fep->interrupt / 2) << 29);
 #else
 	FW(fecp, ivec, (virq_to_hw(fep->interrupt) / 2) << 29);
 #endif
+#endif
 
 	/*
 	 * adjust to speed (only for DUET & RMII)
@@ -368,9 +382,13 @@ static void restart(struct net_device *dev)
 		out_be32(&immap->im_cpm.cp_cptr, cptr);
 	}
 #endif
-
-
+#ifdef CONFIG_FS_ENET_MPC5121_FEC
+	FW(fecp, r_cntrl, PKT_MAXBUF_SIZE<<16);	/* max frame size */
+	FS(fecp, r_cntrl, FEC_RCNTRL_MII_MODE); /* MII enable */
+#else
 	FW(fecp, r_cntrl, FEC_RCNTRL_MII_MODE);	/* MII enable */
+#endif
+
 	/*
 	 * adjust to duplex mode
 	 */
diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.c
index f0014cf..5d9b846 100644
--- a/drivers/net/fs_enet/mii-fec.c
+++ b/drivers/net/fs_enet/mii-fec.c
@@ -38,6 +38,7 @@
 
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 #include <asm/of_platform.h>
+#include <asm/time.h>
 #endif
 
 #include "fs_enet.h"
@@ -71,7 +72,7 @@ static int fs_mii_fec_init(struct fec_info* fec, struct fs_mii_fec_platform_info
 {
 	struct resource *r;
 	fec_t __iomem *fecp;
-	char* name = "fsl-cpm-fec";
+	char *name = "fsl-cpm-fec";
 
 	/* we need fec in order to be useful */
 	struct platform_device *fec_pdev =
@@ -261,9 +262,16 @@ static int fs_enet_mdio_remove(struct of_device *ofdev)
 }
 
 static struct of_device_id fs_enet_mdio_fec_match[] = {
+#ifdef CONFIG_FS_ENET_FEC
 	{
 		.compatible = "fsl,pq1-fec-mdio",
 	},
+#endif
+#ifdef CONFIG_FS_ENET_MPC5121_FEC
+	{
+		.compatible = "fsl,mpc5121-fec-mdio",
+	},
+#endif
 	{},
 };
 
-- 
1.5.4.3

Re: [PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB to current head

[Kumar Gala]

On Jun 12, 2008, at 6:45 AM, David Jander wrote:

Your commit message isn't exactly helpful as most people dont know  
what LTIB is and its not terribly relevant.  It just seems like you  
are adding support for the FEC on MPC5121 and this point.

>
> Signed-off-by: David Jander <david@protonic.nl>
> ---
> arch/powerpc/platforms/Kconfig     |    2 +-
> drivers/net/fec.h                  |   43 ++++++++++++++++++++
> drivers/net/fs_enet/Kconfig        |   22 +++++++++-
> drivers/net/fs_enet/fs_enet-main.c |   76 +++++++++++++++++++++++++++ 
> +++------
> drivers/net/fs_enet/fs_enet.h      |   17 +++++---
> drivers/net/fs_enet/mac-fec.c      |   22 +++++++++-
> drivers/net/fs_enet/mii-fec.c      |   10 ++++-
> 7 files changed, 167 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/ 
> Kconfig
> index 87454c5..a96937f 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -288,7 +288,7 @@ config CPM2
>
> config PPC_CPM_NEW_BINDING
> 	bool
> -	depends on CPM1 || CPM2
> +	depends on CPM1 || CPM2 || FS_ENET_MPC5121_FEC
> 	default y
>
> config AXON_RAM
> diff --git a/drivers/net/fec.h b/drivers/net/fec.h
> index 292719d..5c9fe34 100644
> --- a/drivers/net/fec.h
> +++ b/drivers/net/fec.h
> @@ -59,6 +59,7 @@ typedef struct fec {
> } fec_t;
>
> #else
> +#if !defined(CONFIG_FS_ENET_MPC5121_FEC)
>
> /*
>  *	Define device register set address map.
> @@ -97,6 +98,48 @@ typedef struct fec {
> 	unsigned long	fec_fifo_ram[112];	/* FIFO RAM buffer */
> } fec_t;
>
> +#else /* CONFIG_FS_ENET_MPC5121_FEC */
> +
> +typedef struct fec {
> +	u32 fec_reserved0;
> +	u32 fec_ievent;		/* Interrupt event reg */
> +	u32 fec_imask;		/* Interrupt mask reg */
> +	u32 fec_reserved1;
> +	u32 fec_r_des_active;	/* Receive descriptor reg */
> +	u32 fec_x_des_active;	/* Transmit descriptor reg */
> +	u32 fec_reserved2[3];
> +	u32 fec_ecntrl;		/* Ethernet control reg */
> +	u32 fec_reserved3[6];
> +	u32 fec_mii_data;		/* MII manage frame reg */
> +	u32 fec_mii_speed;		/* MII speed control reg */
> +	u32 fec_reserved4[7];
> +	u32 fec_mib_ctrlstat;	/* MIB control/status reg */
> +	u32 fec_reserved5[7];
> +	u32 fec_r_cntrl;		/* Receive control reg */
> +	u32 fec_reserved6[15];
> +	u32 fec_x_cntrl;		/* Transmit Control reg */
> +	u32 fec_reserved7[7];
> +	u32 fec_addr_low;		/* Low 32bits MAC address */
> +	u32 fec_addr_high;		/* High 16bits MAC address */
> +	u32 fec_opd;		/* Opcode + Pause duration */
> +	u32 fec_reserved8[10];
> +	u32 fec_hash_table_high;	/* High 32bits hash table */
> +	u32 fec_hash_table_low;	/* Low 32bits hash table */
> +	u32 fec_grp_hash_table_high;/* High 32bits hash table */
> +	u32 fec_grp_hash_table_low;	/* Low 32bits hash table */
> +	u32 fec_reserved9[7];
> +	u32 fec_x_wmrk;		/* FIFO transmit water mark */
> +	u32 fec_reserved10;
> +	u32 fec_r_bound;		/* FIFO receive bound reg */
> +	u32 fec_r_fstart;		/* FIFO receive start reg */
> +	u32 fec_reserved11[11];
> +	u32 fec_r_des_start;	/* Receive descriptor ring */
> +	u32 fec_x_des_start;	/* Transmit descriptor ring */
> +	u32 fec_r_buff_size;	/* Maximum receive buff size */
> +	u32 fec_dma_control;	/* DMA Endian and other ctrl */
> +} fec_t;
> +
> +#endif /* CONFIG_FS_ENET_MPC5121_FEC */
> #endif /* CONFIG_M5272 */

I'm not exactly clear as to why this was done this way but this not  
acceptable as it means we can't build a multiplatform kernel that  
needs this driver.

I'm also not clear to me if the MPC5121 FEC is really the same device  
or close to it that it should be sharing this driver or have its own.

- k

Re: [PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB to current head

[Grant Likely]

On Thu, Jun 12, 2008 at 6:12 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Jun 12, 2008, at 6:45 AM, David Jander wrote:
>
> Your commit message isn't exactly helpful as most people dont know what LTIB
> is and its not terribly relevant.  It just seems like you are adding support
> for the FEC on MPC5121 and this point.

Agreed.  Also, you need to CC: the netdev mailing list for network drivers.

>> Signed-off-by: David Jander <david@protonic.nl>
>> ---
>> arch/powerpc/platforms/Kconfig     |    2 +-
>> drivers/net/fec.h                  |   43 ++++++++++++++++++++
>> drivers/net/fs_enet/Kconfig        |   22 +++++++++-
>> drivers/net/fs_enet/fs_enet-main.c |   76
>> ++++++++++++++++++++++++++++++------
>> drivers/net/fs_enet/fs_enet.h      |   17 +++++---
>> drivers/net/fs_enet/mac-fec.c      |   22 +++++++++-
>> drivers/net/fs_enet/mii-fec.c      |   10 ++++-
>> 7 files changed, 167 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/Kconfig
>> b/arch/powerpc/platforms/Kconfig
>> index 87454c5..a96937f 100644
>> --- a/arch/powerpc/platforms/Kconfig
>> +++ b/arch/powerpc/platforms/Kconfig
>> @@ -288,7 +288,7 @@ config CPM2
>>
>> config PPC_CPM_NEW_BINDING
>>        bool
>> -       depends on CPM1 || CPM2
>> +       depends on CPM1 || CPM2 || FS_ENET_MPC5121_FEC
>>        default y
>>
>> config AXON_RAM
>> diff --git a/drivers/net/fec.h b/drivers/net/fec.h
>> index 292719d..5c9fe34 100644
>> --- a/drivers/net/fec.h
>> +++ b/drivers/net/fec.h
>> @@ -59,6 +59,7 @@ typedef struct fec {
>> } fec_t;
>>
>> #else
>> +#if !defined(CONFIG_FS_ENET_MPC5121_FEC)
>>
>> /*
>>  *      Define device register set address map.
>> @@ -97,6 +98,48 @@ typedef struct fec {
>>        unsigned long   fec_fifo_ram[112];      /* FIFO RAM buffer */
>> } fec_t;
>>
>> +#else /* CONFIG_FS_ENET_MPC5121_FEC */
>> +
>> +typedef struct fec {
>> +       u32 fec_reserved0;
>> +       u32 fec_ievent;         /* Interrupt event reg */
>> +       u32 fec_imask;          /* Interrupt mask reg */
>> +       u32 fec_reserved1;
>> +       u32 fec_r_des_active;   /* Receive descriptor reg */
>> +       u32 fec_x_des_active;   /* Transmit descriptor reg */
>> +       u32 fec_reserved2[3];
>> +       u32 fec_ecntrl;         /* Ethernet control reg */
>> +       u32 fec_reserved3[6];
>> +       u32 fec_mii_data;               /* MII manage frame reg */
>> +       u32 fec_mii_speed;              /* MII speed control reg */
>> +       u32 fec_reserved4[7];
>> +       u32 fec_mib_ctrlstat;   /* MIB control/status reg */
>> +       u32 fec_reserved5[7];
>> +       u32 fec_r_cntrl;                /* Receive control reg */
>> +       u32 fec_reserved6[15];
>> +       u32 fec_x_cntrl;                /* Transmit Control reg */
>> +       u32 fec_reserved7[7];
>> +       u32 fec_addr_low;               /* Low 32bits MAC address */
>> +       u32 fec_addr_high;              /* High 16bits MAC address */
>> +       u32 fec_opd;            /* Opcode + Pause duration */
>> +       u32 fec_reserved8[10];
>> +       u32 fec_hash_table_high;        /* High 32bits hash table */
>> +       u32 fec_hash_table_low; /* Low 32bits hash table */
>> +       u32 fec_grp_hash_table_high;/* High 32bits hash table */
>> +       u32 fec_grp_hash_table_low;     /* Low 32bits hash table */
>> +       u32 fec_reserved9[7];
>> +       u32 fec_x_wmrk;         /* FIFO transmit water mark */
>> +       u32 fec_reserved10;
>> +       u32 fec_r_bound;                /* FIFO receive bound reg */
>> +       u32 fec_r_fstart;               /* FIFO receive start reg */
>> +       u32 fec_reserved11[11];
>> +       u32 fec_r_des_start;    /* Receive descriptor ring */
>> +       u32 fec_x_des_start;    /* Transmit descriptor ring */
>> +       u32 fec_r_buff_size;    /* Maximum receive buff size */
>> +       u32 fec_dma_control;    /* DMA Endian and other ctrl */
>> +} fec_t;
>> +
>> +#endif /* CONFIG_FS_ENET_MPC5121_FEC */
>> #endif /* CONFIG_M5272 */
>
> I'm not exactly clear as to why this was done this way but this not
> acceptable as it means we can't build a multiplatform kernel that needs this
> driver.
>
> I'm also not clear to me if the MPC5121 FEC is really the same device or
> close to it that it should be sharing this driver or have its own.

Indeed.  If it is not easy to isolate the differences for runtime
binding then it may be best to just clone the driver as a starting
point and make the 5121 specific changes.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB to current head

[Grant Likely]

On Thu, Jun 12, 2008 at 5:45 AM, David Jander <david.jander@protonic.nl> wrote:
>
>
>  /* write */
> -#define CBDW_SC(_cbd, _sc)             __cbd_out16(&(_cbd)->cbd_sc, (_sc))
> -#define CBDW_DATLEN(_cbd, _datlen)     __cbd_out16(&(_cbd)->cbd_datlen, (_datlen))
> -#define CBDW_BUFADDR(_cbd, _bufaddr)   __cbd_out32(&(_cbd)->cbd_bufaddr, (_bufaddr))
> +#define CBDW_SC(_cbd, _sc)             __cbd_out16((volatile void __iomem *)&(_cbd)->cbd_sc, (_sc))
> +#define CBDW_DATLEN(_cbd, _datlen)     __cbd_out16((volatile void __iomem *)&(_cbd)->cbd_datlen, (_datlen))
> +#define CBDW_BUFADDR(_cbd, _bufaddr)   __cbd_out32((volatile void __iomem *)&(_cbd)->cbd_bufaddr, (_bufaddr))
>
>  /* read */
> -#define CBDR_SC(_cbd)                  __cbd_in16(&(_cbd)->cbd_sc)
> -#define CBDR_DATLEN(_cbd)              __cbd_in16(&(_cbd)->cbd_datlen)
> -#define CBDR_BUFADDR(_cbd)             __cbd_in32(&(_cbd)->cbd_bufaddr)
> +#define CBDR_SC(_cbd)                  __cbd_in16((volatile void __iomem *)&(_cbd)->cbd_sc)
> +#define CBDR_DATLEN(_cbd)              __cbd_in16((volatile void __iomem *)&(_cbd)->cbd_datlen)
> +#define CBDR_BUFADDR(_cbd)             __cbd_in32((volatile void __iomem *)&(_cbd)->cbd_bufaddr)

Another comment: This really doesn't look right.  The _cbd pointer
passed in should already be tagged with __iomem.  Trying to fix it
here is a band-aid and unsafe.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

[Grant Likely]

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 <david.jander@protonic.nl> wrote:
>  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 <david@protonic.nl>
> ---
>  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.

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

[David Gibson]

On Thu, Jun 12, 2008 at 08:10:47AM -0600, Grant Likely wrote:
[snip]
> > +               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.

If this is a video display device, the recommended generic name is
"display@XX", not "video@XX".

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

[Grant Likely]

On Thu, Jun 12, 2008 at 10:19 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:
> On Thu, Jun 12, 2008 at 08:10:47AM -0600, Grant Likely wrote:
> [snip]
>> > +               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.
>
> If this is a video display device, the recommended generic name is
> "display@XX", not "video@XX".

heh; oops.  I should know better and I stand corrected.  :-)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB to current head

[David Jander]

On Thursday 12 June 2008 14:12:15 you wrote:
> On Jun 12, 2008, at 6:45 AM, David Jander wrote:
>
> Your commit message isn't exactly helpful as most people dont know
> what LTIB is and its not terribly relevant.  It just seems like you
> are adding support for the FEC on MPC5121 and this point.
>
>[...]
> > --- a/drivers/net/fec.h
> > +++ b/drivers/net/fec.h
> > @@ -59,6 +59,7 @@ typedef struct fec {
> > } fec_t;
> >
> > #else
> > +#if !defined(CONFIG_FS_ENET_MPC5121_FEC)
> >
> > /*
> >  *	Define device register set address map.
> > @@ -97,6 +98,48 @@ typedef struct fec {
> > 	unsigned long	fec_fifo_ram[112];	/* FIFO RAM buffer */
> > } fec_t;
> >
> > +#else /* CONFIG_FS_ENET_MPC5121_FEC */
> > +
> > +typedef struct fec {
> > [...]
> > +} fec_t;
> > +
> > +#endif /* CONFIG_FS_ENET_MPC5121_FEC */
> > #endif /* CONFIG_M5272 */
>
> I'm not exactly clear as to why this was done this way but this not
> acceptable as it means we can't build a multiplatform kernel that
> needs this driver.

Well, it wouldn't be possible either, since CONFIG_M5272 is a Cold-fire 
processor, and CONFIG_FS_ENET_MPC5121_FEC is for a PowerPC processor.
In this case.
Otherwise you are right, the driver breaks MPC83xx/MPC5121 multiplatform 
builds.

> I'm also not clear to me if the MPC5121 FEC is really the same device
> or close to it that it should be sharing this driver or have its own.

I am coming to the conclusion that it should have its own driver. 
Altough a lot of code could be shared, there are still enough differences, so 
that writing just ONE driver without some #ifdef's that would break 
multiplatform builds, would instead end up with a much bigger amount of if's, 
that would make it unreadable, unmaintainable and inefficient.
Here's why: The above struct fet_t for instance is mapped to a set of 
registers in the FEC. For processors with a CPM1, a CPM2 or without CPM (i.e. 
MPC5121) the register mapping seems to be significantly different, 
nevertheless the structs are all called "struct fec_t". How can one fix this 
at runtime without changing the name of the structs and then just use a lot 
of "if's" or a combination of macro's and if's everywhere a register of the 
FEC is accessed? I fear it will be a mess.

So I think it's either a separate driver, or break multiplatform builds.

Since I am learning from you that breaking multiplatform builds is a no-go, 
I'll settle for splitting up the driver.

Any suggestion on where to put that split-off? How to name it?
I would suggest drivers/net/fec_mpc512x/*

I just resubmitted PATCH 1/2 again without part 2 (which hasn't much to do 
with it anyway), so Grant may have a final look at it (hopefully I did it 
right this time). Part 2 (MPC5121_FEC) will have to wait until monday or so, 
since it will take me a while, and I have to do other things in between.

Any suggestions on how to solve the puzzle are of course welcome...

Thanks a lot for reviewing.

Best regards,

-- 
David Jander
Protonic Holland.