All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 14:34 ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

Fixed all comments.

* Dropped csb1724 defconfig.
* Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
* Dropped un-necessary D-T irq fixup code


Ian Molton (7):
  Initial csb1724 board support (FDT)
  mv643xx.c: Remove magic numbers.
  mv643xx.c: Add basic device tree support.
  kirkwood: Add fixups for DT based mv643xx ethernet.
  csb1724: Enable device tree based mv643xx ethernet support.
  DT: Convert all kirkwood boards with mv643xx that use DT
  NET: mv643xx: remove device name macro.

 Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++
 arch/arm/boot/dts/kirkwood-csb1724.dts            |   49 ++++++++++
 arch/arm/boot/dts/kirkwood-dnskw.dtsi             |    9 ++
 arch/arm/boot/dts/kirkwood-dreamplug.dts          |   18 ++++
 arch/arm/boot/dts/kirkwood-goflexnet.dts          |    8 ++
 arch/arm/boot/dts/kirkwood-ib62x0.dts             |   10 ++
 arch/arm/boot/dts/kirkwood-iconnect.dts           |   10 ++
 arch/arm/boot/dts/kirkwood-lsxl.dtsi              |   17 ++++
 arch/arm/boot/dts/kirkwood-ts219-6281.dts         |    8 +-
 arch/arm/boot/dts/kirkwood-ts219-6282.dts         |    8 +-
 arch/arm/boot/dts/kirkwood-ts219.dtsi             |    3 +
 arch/arm/boot/dts/kirkwood.dtsi                   |   33 +++++++
 arch/arm/mach-kirkwood/Kconfig                    |    7 ++
 arch/arm/mach-kirkwood/Makefile                   |    1 +
 arch/arm/mach-kirkwood/Makefile.boot              |    1 +
 arch/arm/mach-kirkwood/board-csb1724.c            |   60 ++++++++++++
 arch/arm/mach-kirkwood/board-dnskw.c              |    7 +-
 arch/arm/mach-kirkwood/board-dreamplug.c          |   13 +--
 arch/arm/mach-kirkwood/board-dt.c                 |   11 +++
 arch/arm/mach-kirkwood/board-goflexnet.c          |    7 +-
 arch/arm/mach-kirkwood/board-ib62x0.c             |    7 +-
 arch/arm/mach-kirkwood/board-iconnect.c           |    7 +-
 arch/arm/mach-kirkwood/board-lsxl.c               |   13 +--
 arch/arm/mach-kirkwood/board-ts219.c              |   10 +-
 arch/arm/mach-kirkwood/common.c                   |   26 +++++-
 arch/arm/mach-kirkwood/common.h                   |    9 ++
 arch/arm/plat-orion/common.c                      |   24 ++---
 arch/powerpc/platforms/chrp/pegasos_eth.c         |    4 +-
 arch/powerpc/sysdev/mv64x60_dev.c                 |    5 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c        |  104 ++++++++++++++++++---
 include/linux/mv643xx_eth.h                       |    2 -
 31 files changed, 476 insertions(+), 90 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt
 create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
 create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 14:34 ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

Fixed all comments.

* Dropped csb1724 defconfig.
* Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
* Dropped un-necessary D-T irq fixup code


Ian Molton (7):
  Initial csb1724 board support (FDT)
  mv643xx.c: Remove magic numbers.
  mv643xx.c: Add basic device tree support.
  kirkwood: Add fixups for DT based mv643xx ethernet.
  csb1724: Enable device tree based mv643xx ethernet support.
  DT: Convert all kirkwood boards with mv643xx that use DT
  NET: mv643xx: remove device name macro.

 Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++
 arch/arm/boot/dts/kirkwood-csb1724.dts            |   49 ++++++++++
 arch/arm/boot/dts/kirkwood-dnskw.dtsi             |    9 ++
 arch/arm/boot/dts/kirkwood-dreamplug.dts          |   18 ++++
 arch/arm/boot/dts/kirkwood-goflexnet.dts          |    8 ++
 arch/arm/boot/dts/kirkwood-ib62x0.dts             |   10 ++
 arch/arm/boot/dts/kirkwood-iconnect.dts           |   10 ++
 arch/arm/boot/dts/kirkwood-lsxl.dtsi              |   17 ++++
 arch/arm/boot/dts/kirkwood-ts219-6281.dts         |    8 +-
 arch/arm/boot/dts/kirkwood-ts219-6282.dts         |    8 +-
 arch/arm/boot/dts/kirkwood-ts219.dtsi             |    3 +
 arch/arm/boot/dts/kirkwood.dtsi                   |   33 +++++++
 arch/arm/mach-kirkwood/Kconfig                    |    7 ++
 arch/arm/mach-kirkwood/Makefile                   |    1 +
 arch/arm/mach-kirkwood/Makefile.boot              |    1 +
 arch/arm/mach-kirkwood/board-csb1724.c            |   60 ++++++++++++
 arch/arm/mach-kirkwood/board-dnskw.c              |    7 +-
 arch/arm/mach-kirkwood/board-dreamplug.c          |   13 +--
 arch/arm/mach-kirkwood/board-dt.c                 |   11 +++
 arch/arm/mach-kirkwood/board-goflexnet.c          |    7 +-
 arch/arm/mach-kirkwood/board-ib62x0.c             |    7 +-
 arch/arm/mach-kirkwood/board-iconnect.c           |    7 +-
 arch/arm/mach-kirkwood/board-lsxl.c               |   13 +--
 arch/arm/mach-kirkwood/board-ts219.c              |   10 +-
 arch/arm/mach-kirkwood/common.c                   |   26 +++++-
 arch/arm/mach-kirkwood/common.h                   |    9 ++
 arch/arm/plat-orion/common.c                      |   24 ++---
 arch/powerpc/platforms/chrp/pegasos_eth.c         |    4 +-
 arch/powerpc/sysdev/mv64x60_dev.c                 |    5 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c        |  104 ++++++++++++++++++---
 include/linux/mv643xx_eth.h                       |    2 -
 31 files changed, 476 insertions(+), 90 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt
 create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
 create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 1/7] Initial csb1724 board support (FDT)
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

This patch adds support for the csb1724 SoM.

It includes serial and SATA support.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   30 ++++++++++++++++
 arch/arm/mach-kirkwood/Kconfig         |    7 ++++
 arch/arm/mach-kirkwood/Makefile        |    1 +
 arch/arm/mach-kirkwood/Makefile.boot   |    1 +
 arch/arm/mach-kirkwood/board-csb1724.c |   59 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-dt.c      |    4 +++
 arch/arm/mach-kirkwood/common.h        |    6 ++++
 7 files changed, 108 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
 create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c

diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
new file mode 100644
index 0000000..44dfe9a
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -0,0 +1,30 @@
+/dts-v1/;
+
+/include/ "kirkwood.dtsi"
+
+/ {
+	model = "Cogent CSB1724-88F-628X SoM";
+	compatible = "cogent,csb1724", "marvell,kirkwood-88f6281", "marvell,kirkwood";
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8 earlyprintk";
+	};
+
+	ocp@f1000000 {
+		serial@12000 {
+			clock-frequency = <200000000>;
+			status = "ok";
+		};
+
+		sata@80000 {
+			nr-ports = <2>;
+			status = "ok";
+		};
+	};
+
+};
diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index ca5c15a..6d51c50 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -109,6 +109,13 @@ config MACH_LSXL_DT
 	  Buffalo Linkstation LS-XHL & LS-CHLv2 devices, using
 	  Flattened Device Tree.
 
+config MACH_CSB1724_DT
+	bool "Cogent CSB1724 SoM (Flattened Device Tree)"
+	select ARCH_KIRKWOOD_DT
+	help
+	  Say 'Y' here if you want your kernel to support the
+	  Cogent CSB1724 SoM, using Flattened Device Tree.
+
 config MACH_TS219
 	bool "QNAP TS-110, TS-119, TS-119P+, TS-210, TS-219, TS-219P and TS-219P+ Turbo NAS"
 	help
diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index 055c85a..665ed63 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MACH_IB62X0_DT)		+= board-ib62x0.o
 obj-$(CONFIG_MACH_TS219_DT)		+= board-ts219.o tsx1x-common.o
 obj-$(CONFIG_MACH_GOFLEXNET_DT)		+= board-goflexnet.o
 obj-$(CONFIG_MACH_LSXL_DT)		+= board-lsxl.o
+obj-$(CONFIG_MACH_CSB1724_DT)		+= board-csb1724.o
diff --git a/arch/arm/mach-kirkwood/Makefile.boot b/arch/arm/mach-kirkwood/Makefile.boot
index 2a576ab..899bc80 100644
--- a/arch/arm/mach-kirkwood/Makefile.boot
+++ b/arch/arm/mach-kirkwood/Makefile.boot
@@ -2,6 +2,7 @@
 params_phys-y	:= 0x00000100
 initrd_phys-y	:= 0x00800000
 
+dtb-$(CONFIG_MACH_CSB1724_DT) += kirkwood-csb1724.dtb
 dtb-$(CONFIG_MACH_DREAMPLUG_DT) += kirkwood-dreamplug.dtb
 dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns320.dtb
 dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns325.dtb
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
new file mode 100644
index 0000000..979112d
--- /dev/null
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2012 (C), Ian Molton <ian.molton@codethink.co.uk>
+ *
+ * arch/arm/mach-kirkwood/board-csb1724.c
+ *
+ * Cogent csb1724 Board Init for drivers not converted to
+ * flattened device tree yet.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include "mpp.h"
+
+static unsigned int csb1724_mpp_config[] __initdata = {
+	MPP0_NF_IO2,
+	MPP1_NF_IO3,
+	MPP2_NF_IO4,
+	MPP3_NF_IO5,
+	MPP4_NF_IO6,
+	MPP5_NF_IO7,
+	MPP8_TW0_SDA,
+	MPP9_TW0_SCK,
+	MPP12_SD_CLK,
+	MPP13_SD_CMD,
+	MPP14_SD_D0,
+	MPP15_SD_D1,
+	MPP16_SD_D2,
+	MPP17_SD_D3,
+	MPP18_NF_IO0,
+	MPP19_NF_IO1,
+	MPP20_GE1_TXD0,
+	MPP21_GE1_TXD1,
+	MPP22_GE1_TXD2,
+	MPP23_GE1_TXD3,
+	MPP24_GE1_RXD0,
+	MPP25_GE1_RXD1,
+	MPP26_GE1_RXD2,
+	MPP27_GE1_RXD3,
+	MPP30_GE1_RXCTL,
+	MPP31_GE1_RXCLK,
+	MPP32_GE1_TCLKOUT,
+	MPP33_GE1_TXCTL,
+	MPP34_SATA1_ACTn,
+	MPP35_SATA0_ACTn,
+	MPP36_TW1_SDA,
+	MPP37_TW1_SCK,
+};
+
+void __init csb1724_init(void)
+{
+	/*
+	 * Basic setup. Needs to be called early.
+	 */
+	kirkwood_mpp_conf(csb1724_mpp_config);
+}
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index e4eb450..7679f7f 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -87,6 +87,9 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("buffalo,lsxl"))
 		lsxl_init();
 
+	if (of_machine_is_compatible("cogent,csb1724"))
+		csb1724_init();
+
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);
 }
@@ -100,6 +103,7 @@ static const char *kirkwood_dt_board_compat[] = {
 	"qnap,ts219",
 	"seagate,goflexnet",
 	"buffalo,lsxl",
+	"cogent,csb1724",
 	NULL
 };
 
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 304dd1a..8aab1ae 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -94,6 +94,12 @@ void lsxl_init(void);
 static inline void lsxl_init(void) {};
 #endif
 
+#ifdef CONFIG_MACH_CSB1724_DT
+void csb1724_init(void);
+#else
+static inline void csb1724_init(void) {};
+#endif
+
 /* early init functions not converted to fdt yet */
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 1/7] Initial csb1724 board support (FDT)
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds support for the csb1724 SoM.

It includes serial and SATA support.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   30 ++++++++++++++++
 arch/arm/mach-kirkwood/Kconfig         |    7 ++++
 arch/arm/mach-kirkwood/Makefile        |    1 +
 arch/arm/mach-kirkwood/Makefile.boot   |    1 +
 arch/arm/mach-kirkwood/board-csb1724.c |   59 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-dt.c      |    4 +++
 arch/arm/mach-kirkwood/common.h        |    6 ++++
 7 files changed, 108 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
 create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c

diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
new file mode 100644
index 0000000..44dfe9a
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -0,0 +1,30 @@
+/dts-v1/;
+
+/include/ "kirkwood.dtsi"
+
+/ {
+	model = "Cogent CSB1724-88F-628X SoM";
+	compatible = "cogent,csb1724", "marvell,kirkwood-88f6281", "marvell,kirkwood";
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8 earlyprintk";
+	};
+
+	ocp at f1000000 {
+		serial at 12000 {
+			clock-frequency = <200000000>;
+			status = "ok";
+		};
+
+		sata at 80000 {
+			nr-ports = <2>;
+			status = "ok";
+		};
+	};
+
+};
diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index ca5c15a..6d51c50 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -109,6 +109,13 @@ config MACH_LSXL_DT
 	  Buffalo Linkstation LS-XHL & LS-CHLv2 devices, using
 	  Flattened Device Tree.
 
+config MACH_CSB1724_DT
+	bool "Cogent CSB1724 SoM (Flattened Device Tree)"
+	select ARCH_KIRKWOOD_DT
+	help
+	  Say 'Y' here if you want your kernel to support the
+	  Cogent CSB1724 SoM, using Flattened Device Tree.
+
 config MACH_TS219
 	bool "QNAP TS-110, TS-119, TS-119P+, TS-210, TS-219, TS-219P and TS-219P+ Turbo NAS"
 	help
diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index 055c85a..665ed63 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MACH_IB62X0_DT)		+= board-ib62x0.o
 obj-$(CONFIG_MACH_TS219_DT)		+= board-ts219.o tsx1x-common.o
 obj-$(CONFIG_MACH_GOFLEXNET_DT)		+= board-goflexnet.o
 obj-$(CONFIG_MACH_LSXL_DT)		+= board-lsxl.o
+obj-$(CONFIG_MACH_CSB1724_DT)		+= board-csb1724.o
diff --git a/arch/arm/mach-kirkwood/Makefile.boot b/arch/arm/mach-kirkwood/Makefile.boot
index 2a576ab..899bc80 100644
--- a/arch/arm/mach-kirkwood/Makefile.boot
+++ b/arch/arm/mach-kirkwood/Makefile.boot
@@ -2,6 +2,7 @@
 params_phys-y	:= 0x00000100
 initrd_phys-y	:= 0x00800000
 
+dtb-$(CONFIG_MACH_CSB1724_DT) += kirkwood-csb1724.dtb
 dtb-$(CONFIG_MACH_DREAMPLUG_DT) += kirkwood-dreamplug.dtb
 dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns320.dtb
 dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns325.dtb
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
new file mode 100644
index 0000000..979112d
--- /dev/null
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2012 (C), Ian Molton <ian.molton@codethink.co.uk>
+ *
+ * arch/arm/mach-kirkwood/board-csb1724.c
+ *
+ * Cogent csb1724 Board Init for drivers not converted to
+ * flattened device tree yet.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include "mpp.h"
+
+static unsigned int csb1724_mpp_config[] __initdata = {
+	MPP0_NF_IO2,
+	MPP1_NF_IO3,
+	MPP2_NF_IO4,
+	MPP3_NF_IO5,
+	MPP4_NF_IO6,
+	MPP5_NF_IO7,
+	MPP8_TW0_SDA,
+	MPP9_TW0_SCK,
+	MPP12_SD_CLK,
+	MPP13_SD_CMD,
+	MPP14_SD_D0,
+	MPP15_SD_D1,
+	MPP16_SD_D2,
+	MPP17_SD_D3,
+	MPP18_NF_IO0,
+	MPP19_NF_IO1,
+	MPP20_GE1_TXD0,
+	MPP21_GE1_TXD1,
+	MPP22_GE1_TXD2,
+	MPP23_GE1_TXD3,
+	MPP24_GE1_RXD0,
+	MPP25_GE1_RXD1,
+	MPP26_GE1_RXD2,
+	MPP27_GE1_RXD3,
+	MPP30_GE1_RXCTL,
+	MPP31_GE1_RXCLK,
+	MPP32_GE1_TCLKOUT,
+	MPP33_GE1_TXCTL,
+	MPP34_SATA1_ACTn,
+	MPP35_SATA0_ACTn,
+	MPP36_TW1_SDA,
+	MPP37_TW1_SCK,
+};
+
+void __init csb1724_init(void)
+{
+	/*
+	 * Basic setup. Needs to be called early.
+	 */
+	kirkwood_mpp_conf(csb1724_mpp_config);
+}
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index e4eb450..7679f7f 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -87,6 +87,9 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("buffalo,lsxl"))
 		lsxl_init();
 
+	if (of_machine_is_compatible("cogent,csb1724"))
+		csb1724_init();
+
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);
 }
@@ -100,6 +103,7 @@ static const char *kirkwood_dt_board_compat[] = {
 	"qnap,ts219",
 	"seagate,goflexnet",
 	"buffalo,lsxl",
+	"cogent,csb1724",
 	NULL
 };
 
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 304dd1a..8aab1ae 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -94,6 +94,12 @@ void lsxl_init(void);
 static inline void lsxl_init(void) {};
 #endif
 
+#ifdef CONFIG_MACH_CSB1724_DT
+void csb1724_init(void);
+#else
+static inline void csb1724_init(void) {};
+#endif
+
 /* early init functions not converted to fdt yet */
 char *kirkwood_id(void);
 void kirkwood_l2_init(void);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 2/7] mv643xx.c: Remove magic numbers.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

replace magic number with RX_CSUM_WITH_HEADER.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 4fbba57..92497eb 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -86,6 +86,7 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * port #0, 0x0800 for port #1, and 0x0c00 for port #2.
  */
 #define PORT_CONFIG			0x0000
+#define  RX_CSUM_WITH_HEADER		0x02000000
 #define  UNICAST_PROMISCUOUS_MODE	0x00000001
 #define PORT_CONFIG_EXT			0x0004
 #define MAC_ADDR_LOW			0x0014
@@ -1607,7 +1608,7 @@ mv643xx_eth_set_features(struct net_device *dev, netdev_features_t features)
 	struct mv643xx_eth_private *mp = netdev_priv(dev);
 	bool rx_csum = features & NETIF_F_RXCSUM;
 
-	wrlp(mp, PORT_CONFIG, rx_csum ? 0x02000000 : 0x00000000);
+	wrlp(mp, PORT_CONFIG, rx_csum ? RX_CSUM_WITH_HEADER : 0x00000000);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 2/7] mv643xx.c: Remove magic numbers.
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

replace magic number with RX_CSUM_WITH_HEADER.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 4fbba57..92497eb 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -86,6 +86,7 @@ static char mv643xx_eth_driver_version[] = "1.4";
  * port #0, 0x0800 for port #1, and 0x0c00 for port #2.
  */
 #define PORT_CONFIG			0x0000
+#define  RX_CSUM_WITH_HEADER		0x02000000
 #define  UNICAST_PROMISCUOUS_MODE	0x00000001
 #define PORT_CONFIG_EXT			0x0004
 #define MAC_ADDR_LOW			0x0014
@@ -1607,7 +1608,7 @@ mv643xx_eth_set_features(struct net_device *dev, netdev_features_t features)
 	struct mv643xx_eth_private *mp = netdev_priv(dev);
 	bool rx_csum = features & NETIF_F_RXCSUM;
 
-	wrlp(mp, PORT_CONFIG, rx_csum ? 0x02000000 : 0x00000000);
+	wrlp(mp, PORT_CONFIG, rx_csum ? RX_CSUM_WITH_HEADER : 0x00000000);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

    This patch adds basic device tree support to the mv643xx ethernet driver.

    It should be enough for most current users of the device, and should allow
    a painless migration.

    Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++++
 drivers/net/ethernet/marvell/mv643xx_eth.c        |   93 +++++++++++++++++++--
 2 files changed, 161 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt

diff --git a/Documentation/devicetree/bindings/net/mv643xx.txt b/Documentation/devicetree/bindings/net/mv643xx.txt
new file mode 100644
index 0000000..2727f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mv643xx.txt
@@ -0,0 +1,75 @@
+mv643xx related nodes.
+
+marvell,mdio-mv643xx:
+
+Required properties:
+
+ - interrupts : <a> where a is the SMI interrupt number.
+ - reg : the base address and size of the controllers register space.
+
+Optional properties:
+ - shared_smi : on some chips, the second PHY is "shared", meaning it is
+	really accessed via the first SMI controller. It is passed in this
+	way due to the present structure of the driver, which requires the
+	base address for the MAC to be passed in via the SMI controllers
+	platform data.
+ - tx_csum_limit : on some devices, this option is required for proper
+	operation wrt. jumbo frames.
+
+
+Example:
+
+smi0: mdio@72000 {
+	compatible = "marvell,mdio-mv643xx";
+	reg = <0x72000 0x4000>;
+	interrupts = <46>;
+	tx_csum_limit = <1600>;
+	status = "disabled";
+};
+
+smi1: mdio@76000 {
+	compatible = "marvell,mdio-mv643xx";
+	reg = <0x76000 0x4000>;
+	interrupts = <47>;
+	shared_smi = <&smi0>;
+	tx_csum_limit = <1600>;
+	status = "disabled";
+};
+
+
+
+marvell,mv643xx-eth:
+
+Required properties:
+ - interrupts : the port interrupt number.
+ - mdio : phandle of the smi device as drescribed above
+
+Optional properties:
+ - port_number : the port number on this bus.
+ - phy_addr : the PHY address.
+ - reg : should match the mdio reg this device is attached to.
+	this is a required hack for now due to the way the
+	driver is constructed. This allows the device clock to be
+	kept running so that the MAC is not lost after boot.
+
+
+Example:
+
+egiga0 {
+	compatible = "marvell,mv643xx-eth";
+	reg = <0x72000 0x4000>;
+	mdio = <&smi0>;
+	port_number = <0>;
+	phy_addr = <0x80>;
+	interrupts = <11>;
+};
+
+egiga1 {
+	compatible = "marvell,mv643xx-eth";
+	reg = <0x76000 0x4000>;
+	mdio = <&smi1>;
+	port_number = <0>;
+	phy_addr = <0x81>;
+	interrupts = <15>;
+};
+
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 92497eb..bb80050 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -48,6 +48,9 @@
 #include <linux/ethtool.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -2601,7 +2604,7 @@ static void infer_hw_params(struct mv643xx_eth_shared_private *msp)
 static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 {
 	static int mv643xx_eth_version_printed;
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
+	struct mv643xx_eth_shared_platform_data *pd;
 	struct mv643xx_eth_shared_private *msp;
 	const struct mbus_dram_target_info *dram;
 	struct resource *res;
@@ -2625,6 +2628,26 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	if (msp->base == NULL)
 		goto out_free;
 
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			goto out_free;
+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"tx_csum_limit", &pd->tx_csum_limit);
+
+		np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
+		if (np)
+			pd->shared_smi = of_find_device_by_node(np);
+
+	} else {
+		pd = pdev->dev.platform_data;
+	}
 	/*
 	 * Set up and register SMI bus.
 	 */
@@ -2657,7 +2680,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res != NULL) {
 		int err;
-
 		err = request_irq(res->start, mv643xx_eth_err_irq,
 				  IRQF_SHARED, "mv643xx_eth", msp);
 		if (!err) {
@@ -2675,6 +2697,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
 					pd->tx_csum_limit : 9 * 1024;
+
+	if (pdev->dev.of_node)
+		kfree(pd);  /* If we created a fake pd, free it now */
+
 	infer_hw_params(msp);
 
 	platform_set_drvdata(pdev, msp);
@@ -2708,12 +2734,21 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mdio-mv643xx", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_shared_driver = {
 	.probe		= mv643xx_eth_shared_probe,
 	.remove		= mv643xx_eth_shared_remove,
 	.driver = {
 		.name	= MV643XX_ETH_SHARED_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_mdio_dt_ids),
 	},
 };
 
@@ -2873,7 +2908,36 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;
+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number", &pd->port_number);
+
+		if (!of_property_read_u32(pdev->dev.of_node,
+				"phy_addr", &pd->phy_addr))
+			pd->phy_addr = MV643XX_ETH_PHY_ADDR(pd->phy_addr);
+		else
+			pd->phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT;
+
+		np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+		if (np) {
+			pd->shared = of_find_device_by_node(np);
+		} else {
+			kfree(pd);
+			return -ENODEV;
+		}
+	} else {
+		pd = pdev->dev.platform_data;
+	}
+
 	if (pd == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
 		return -ENODEV;
@@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	if (pd->shared == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data->shared\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto out_free_pd;
 	}
 
 	dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
-	if (!dev)
-		return -ENOMEM;
+	if (!dev) {
+		err = -ENOMEM;
+		goto out_free_pd;
+	}
 
 	mp = netdev_priv(dev);
 	platform_set_drvdata(pdev, mp);
@@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	init_pscr(mp, pd->speed, pd->duplex);
 
+	if (pdev->dev.of_node)
+		kfree(pd); /* If we created a fake pd, free it now */
 
 	mib_counters_clear(mp);
 
@@ -2942,7 +3011,6 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	mp->rx_oom.data = (unsigned long)mp;
 	mp->rx_oom.function = oom_timer_wrapper;
 
-
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	BUG_ON(!res);
 	dev->irq = res->start;
@@ -2991,6 +3059,8 @@ out:
 	}
 #endif
 	free_netdev(dev);
+out_free_pd:
+	kfree(pd);
 
 	return err;
 }
@@ -3030,6 +3100,14 @@ static void mv643xx_eth_shutdown(struct platform_device *pdev)
 		port_reset(mp);
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mv643xx-eth", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_driver = {
 	.probe		= mv643xx_eth_probe,
 	.remove		= mv643xx_eth_remove,
@@ -3037,6 +3115,7 @@ static struct platform_driver mv643xx_eth_driver = {
 	.driver = {
 		.name	= MV643XX_ETH_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_eth_dt_ids),
 	},
 };
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

    This patch adds basic device tree support to the mv643xx ethernet driver.

    It should be enough for most current users of the device, and should allow
    a painless migration.

    Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++++
 drivers/net/ethernet/marvell/mv643xx_eth.c        |   93 +++++++++++++++++++--
 2 files changed, 161 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt

diff --git a/Documentation/devicetree/bindings/net/mv643xx.txt b/Documentation/devicetree/bindings/net/mv643xx.txt
new file mode 100644
index 0000000..2727f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mv643xx.txt
@@ -0,0 +1,75 @@
+mv643xx related nodes.
+
+marvell,mdio-mv643xx:
+
+Required properties:
+
+ - interrupts : <a> where a is the SMI interrupt number.
+ - reg : the base address and size of the controllers register space.
+
+Optional properties:
+ - shared_smi : on some chips, the second PHY is "shared", meaning it is
+	really accessed via the first SMI controller. It is passed in this
+	way due to the present structure of the driver, which requires the
+	base address for the MAC to be passed in via the SMI controllers
+	platform data.
+ - tx_csum_limit : on some devices, this option is required for proper
+	operation wrt. jumbo frames.
+
+
+Example:
+
+smi0: mdio at 72000 {
+	compatible = "marvell,mdio-mv643xx";
+	reg = <0x72000 0x4000>;
+	interrupts = <46>;
+	tx_csum_limit = <1600>;
+	status = "disabled";
+};
+
+smi1: mdio at 76000 {
+	compatible = "marvell,mdio-mv643xx";
+	reg = <0x76000 0x4000>;
+	interrupts = <47>;
+	shared_smi = <&smi0>;
+	tx_csum_limit = <1600>;
+	status = "disabled";
+};
+
+
+
+marvell,mv643xx-eth:
+
+Required properties:
+ - interrupts : the port interrupt number.
+ - mdio : phandle of the smi device as drescribed above
+
+Optional properties:
+ - port_number : the port number on this bus.
+ - phy_addr : the PHY address.
+ - reg : should match the mdio reg this device is attached to.
+	this is a required hack for now due to the way the
+	driver is constructed. This allows the device clock to be
+	kept running so that the MAC is not lost after boot.
+
+
+Example:
+
+egiga0 {
+	compatible = "marvell,mv643xx-eth";
+	reg = <0x72000 0x4000>;
+	mdio = <&smi0>;
+	port_number = <0>;
+	phy_addr = <0x80>;
+	interrupts = <11>;
+};
+
+egiga1 {
+	compatible = "marvell,mv643xx-eth";
+	reg = <0x76000 0x4000>;
+	mdio = <&smi1>;
+	port_number = <0>;
+	phy_addr = <0x81>;
+	interrupts = <15>;
+};
+
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 92497eb..bb80050 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -48,6 +48,9 @@
 #include <linux/ethtool.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -2601,7 +2604,7 @@ static void infer_hw_params(struct mv643xx_eth_shared_private *msp)
 static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 {
 	static int mv643xx_eth_version_printed;
-	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
+	struct mv643xx_eth_shared_platform_data *pd;
 	struct mv643xx_eth_shared_private *msp;
 	const struct mbus_dram_target_info *dram;
 	struct resource *res;
@@ -2625,6 +2628,26 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	if (msp->base == NULL)
 		goto out_free;
 
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			goto out_free;
+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"tx_csum_limit", &pd->tx_csum_limit);
+
+		np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
+		if (np)
+			pd->shared_smi = of_find_device_by_node(np);
+
+	} else {
+		pd = pdev->dev.platform_data;
+	}
 	/*
 	 * Set up and register SMI bus.
 	 */
@@ -2657,7 +2680,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res != NULL) {
 		int err;
-
 		err = request_irq(res->start, mv643xx_eth_err_irq,
 				  IRQF_SHARED, "mv643xx_eth", msp);
 		if (!err) {
@@ -2675,6 +2697,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
 
 	msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
 					pd->tx_csum_limit : 9 * 1024;
+
+	if (pdev->dev.of_node)
+		kfree(pd);  /* If we created a fake pd, free it now */
+
 	infer_hw_params(msp);
 
 	platform_set_drvdata(pdev, msp);
@@ -2708,12 +2734,21 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mdio-mv643xx", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_shared_driver = {
 	.probe		= mv643xx_eth_shared_probe,
 	.remove		= mv643xx_eth_shared_remove,
 	.driver = {
 		.name	= MV643XX_ETH_SHARED_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_mdio_dt_ids),
 	},
 };
 
@@ -2873,7 +2908,36 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	pd = pdev->dev.platform_data;
+	if (pdev->dev.of_node) {
+		struct device_node *np = NULL;
+
+		/* when all users of this driver use FDT, we can remove this */
+		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+			return -ENOMEM;
+		}
+
+		of_property_read_u32(pdev->dev.of_node,
+			"port_number", &pd->port_number);
+
+		if (!of_property_read_u32(pdev->dev.of_node,
+				"phy_addr", &pd->phy_addr))
+			pd->phy_addr = MV643XX_ETH_PHY_ADDR(pd->phy_addr);
+		else
+			pd->phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT;
+
+		np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+		if (np) {
+			pd->shared = of_find_device_by_node(np);
+		} else {
+			kfree(pd);
+			return -ENODEV;
+		}
+	} else {
+		pd = pdev->dev.platform_data;
+	}
+
 	if (pd == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
 		return -ENODEV;
@@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	if (pd->shared == NULL) {
 		dev_err(&pdev->dev, "no mv643xx_eth_platform_data->shared\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto out_free_pd;
 	}
 
 	dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
-	if (!dev)
-		return -ENOMEM;
+	if (!dev) {
+		err = -ENOMEM;
+		goto out_free_pd;
+	}
 
 	mp = netdev_priv(dev);
 	platform_set_drvdata(pdev, mp);
@@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 
 	init_pscr(mp, pd->speed, pd->duplex);
 
+	if (pdev->dev.of_node)
+		kfree(pd); /* If we created a fake pd, free it now */
 
 	mib_counters_clear(mp);
 
@@ -2942,7 +3011,6 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	mp->rx_oom.data = (unsigned long)mp;
 	mp->rx_oom.function = oom_timer_wrapper;
 
-
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	BUG_ON(!res);
 	dev->irq = res->start;
@@ -2991,6 +3059,8 @@ out:
 	}
 #endif
 	free_netdev(dev);
+out_free_pd:
+	kfree(pd);
 
 	return err;
 }
@@ -3030,6 +3100,14 @@ static void mv643xx_eth_shutdown(struct platform_device *pdev)
 		port_reset(mp);
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
+	{ .compatible = "marvell,mv643xx-eth", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
+#endif
+
 static struct platform_driver mv643xx_eth_driver = {
 	.probe		= mv643xx_eth_probe,
 	.remove		= mv643xx_eth_remove,
@@ -3037,6 +3115,7 @@ static struct platform_driver mv643xx_eth_driver = {
 	.driver = {
 		.name	= MV643XX_ETH_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mv_eth_dt_ids),
 	},
 };
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 4/7] kirkwood: Add fixups for DT based mv643xx ethernet.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

This patch adds auxdata for kirkwood ethernet and an ethernet clock setup
helper function allowing the mv643xx clock to be kept enabled after boot so
that the MAC address(es) are not lost.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/mach-kirkwood/board-dt.c |    7 +++++++
 arch/arm/mach-kirkwood/common.c   |   22 ++++++++++++++++++++++
 arch/arm/mach-kirkwood/common.h   |    3 +++
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 7679f7f..aa213b6 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -14,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/mv643xx_eth.h>
 #include <linux/kexec.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
 	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
 	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth_port.0",
+			NULL),
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth_port.1",
+			NULL),
 	{},
 };
 
@@ -92,6 +97,8 @@ static void __init kirkwood_dt_init(void)
 
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);
+
+	kirkwood_eth_clock_fixup();
 }
 
 static const char *kirkwood_dt_board_compat[] = {
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index c4b64ad..57b91cf 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -18,6 +18,7 @@
 #include <linux/clk-provider.h>
 #include <linux/spinlock.h>
 #include <linux/mv643xx_i2c.h>
+#include <linux/of.h>
 #include <net/dsa.h>
 #include <asm/page.h>
 #include <asm/timex.h>
@@ -293,6 +294,27 @@ void __init kirkwood_ehci_init(void)
 	orion_ehci_init(USB_PHYS_BASE, IRQ_KIRKWOOD_USB, EHCI_PHY_NA);
 }
 
+/* Fixup ethernet clocks for DT based kirkwood platforms.
+ * This is required because if the clock is not kept running, the
+ * Interface will forget its MAC address.
+ */
+#ifdef CONFIG_OF
+void __init kirkwood_eth_clock_fixup(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_name(NULL, "egiga0");
+	if (np && of_device_is_available(np))
+		clk_prepare_enable(ge0);
+	of_node_put(np);
+
+	np = of_find_node_by_name(NULL, "egiga1");
+	if (np && of_device_is_available(np))
+		clk_prepare_enable(ge1);
+	of_node_put(np);
+
+}
+#endif
 
 /*****************************************************************************
  * GE00
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 8aab1ae..7183718 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -36,6 +36,9 @@ void kirkwood_enable_pcie(void);
 void kirkwood_pcie_id(u32 *dev, u32 *rev);
 
 void kirkwood_ehci_init(void);
+#ifdef CONFIG_OF
+void kirkwood_eth_clock_fixup(void);
+#endif
 void kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data);
 void kirkwood_ge01_init(struct mv643xx_eth_platform_data *eth_data);
 void kirkwood_ge00_switch_init(struct dsa_platform_data *d, int irq);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 4/7] kirkwood: Add fixups for DT based mv643xx ethernet.
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds auxdata for kirkwood ethernet and an ethernet clock setup
helper function allowing the mv643xx clock to be kept enabled after boot so
that the MAC address(es) are not lost.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/mach-kirkwood/board-dt.c |    7 +++++++
 arch/arm/mach-kirkwood/common.c   |   22 ++++++++++++++++++++++
 arch/arm/mach-kirkwood/common.h   |    3 +++
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 7679f7f..aa213b6 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -14,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/mv643xx_eth.h>
 #include <linux/kexec.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
 	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
 	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth_port.0",
+			NULL),
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth_port.1",
+			NULL),
 	{},
 };
 
@@ -92,6 +97,8 @@ static void __init kirkwood_dt_init(void)
 
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);
+
+	kirkwood_eth_clock_fixup();
 }
 
 static const char *kirkwood_dt_board_compat[] = {
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index c4b64ad..57b91cf 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -18,6 +18,7 @@
 #include <linux/clk-provider.h>
 #include <linux/spinlock.h>
 #include <linux/mv643xx_i2c.h>
+#include <linux/of.h>
 #include <net/dsa.h>
 #include <asm/page.h>
 #include <asm/timex.h>
@@ -293,6 +294,27 @@ void __init kirkwood_ehci_init(void)
 	orion_ehci_init(USB_PHYS_BASE, IRQ_KIRKWOOD_USB, EHCI_PHY_NA);
 }
 
+/* Fixup ethernet clocks for DT based kirkwood platforms.
+ * This is required because if the clock is not kept running, the
+ * Interface will forget its MAC address.
+ */
+#ifdef CONFIG_OF
+void __init kirkwood_eth_clock_fixup(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_name(NULL, "egiga0");
+	if (np && of_device_is_available(np))
+		clk_prepare_enable(ge0);
+	of_node_put(np);
+
+	np = of_find_node_by_name(NULL, "egiga1");
+	if (np && of_device_is_available(np))
+		clk_prepare_enable(ge1);
+	of_node_put(np);
+
+}
+#endif
 
 /*****************************************************************************
  * GE00
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 8aab1ae..7183718 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -36,6 +36,9 @@ void kirkwood_enable_pcie(void);
 void kirkwood_pcie_id(u32 *dev, u32 *rev);
 
 void kirkwood_ehci_init(void);
+#ifdef CONFIG_OF
+void kirkwood_eth_clock_fixup(void);
+#endif
 void kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data);
 void kirkwood_ge01_init(struct mv643xx_eth_platform_data *eth_data);
 void kirkwood_ge00_switch_init(struct dsa_platform_data *d, int irq);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 5/7] csb1724: Enable device tree based mv643xx ethernet support.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

        This patch enables mv643xx based ethernet built into the SoM on the
        csb1724, via flattened device tree.

        Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   19 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi        |   33 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-csb1724.c |    1 +
 3 files changed, 53 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
index 44dfe9a..f43f8dd 100644
--- a/arch/arm/boot/dts/kirkwood-csb1724.dts
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -25,6 +25,25 @@
 			nr-ports = <2>;
 			status = "ok";
 		};
+
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		smi1: mdio@76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <1>;
+			status = "ok";
+		};
+
 	};
 
 };
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index cef9616..f5f1f92 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -76,6 +76,39 @@
 			status = "okay";
 		};
 
+		smi0: mdio@72000 {
+			compatible = "marvell,mdio-mv643xx";
+			reg = <0x72000 0x4000>;
+			interrupts = <46>;
+			tx_csum_limit = <1600>;
+			status = "disabled";
+		};
+
+		egiga0 {
+			compatible = "marvell,mv643xx-eth";
+			reg = <0x72000 0x4000>;
+			mdio = <&smi0>;
+			interrupts = <11>;
+			status = "disabled";
+		};
+
+		smi1: mdio@76000 {
+			compatible = "marvell,mdio-mv643xx";
+			reg = <0x76000 0x4000>;
+			interrupts = <47>;
+			shared_smi = <&smi0>;
+			tx_csum_limit = <1600>;
+			status = "disabled";
+		};
+
+		egiga1 {
+			compatible = "marvell,mv643xx-eth";
+			reg = <0x76000 0x4000>;
+			mdio = <&smi1>;
+			interrupts = <15>;
+			status = "disabled";
+		};
+
 		sata@80000 {
 			compatible = "marvell,orion-sata";
 			reg = <0x80000 0x5000>;
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
index 979112d..9c58b92 100644
--- a/arch/arm/mach-kirkwood/board-csb1724.c
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include "common.h"
 #include "mpp.h"
 
 static unsigned int csb1724_mpp_config[] __initdata = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 5/7] csb1724: Enable device tree based mv643xx ethernet support.
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

        This patch enables mv643xx based ethernet built into the SoM on the
        csb1724, via flattened device tree.

        Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-csb1724.dts |   19 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi        |   33 ++++++++++++++++++++++++++++++++
 arch/arm/mach-kirkwood/board-csb1724.c |    1 +
 3 files changed, 53 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
index 44dfe9a..f43f8dd 100644
--- a/arch/arm/boot/dts/kirkwood-csb1724.dts
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -25,6 +25,25 @@
 			nr-ports = <2>;
 			status = "ok";
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		smi1: mdio at 76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <1>;
+			status = "ok";
+		};
+
 	};
 
 };
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index cef9616..f5f1f92 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -76,6 +76,39 @@
 			status = "okay";
 		};
 
+		smi0: mdio at 72000 {
+			compatible = "marvell,mdio-mv643xx";
+			reg = <0x72000 0x4000>;
+			interrupts = <46>;
+			tx_csum_limit = <1600>;
+			status = "disabled";
+		};
+
+		egiga0 {
+			compatible = "marvell,mv643xx-eth";
+			reg = <0x72000 0x4000>;
+			mdio = <&smi0>;
+			interrupts = <11>;
+			status = "disabled";
+		};
+
+		smi1: mdio at 76000 {
+			compatible = "marvell,mdio-mv643xx";
+			reg = <0x76000 0x4000>;
+			interrupts = <47>;
+			shared_smi = <&smi0>;
+			tx_csum_limit = <1600>;
+			status = "disabled";
+		};
+
+		egiga1 {
+			compatible = "marvell,mv643xx-eth";
+			reg = <0x76000 0x4000>;
+			mdio = <&smi1>;
+			interrupts = <15>;
+			status = "disabled";
+		};
+
 		sata at 80000 {
 			compatible = "marvell,orion-sata";
 			reg = <0x80000 0x5000>;
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
index 979112d..9c58b92 100644
--- a/arch/arm/mach-kirkwood/board-csb1724.c
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include "common.h"
 #include "mpp.h"
 
 static unsigned int csb1724_mpp_config[] __initdata = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 6/7] DT: Convert all kirkwood boards with mv643xx that use DT
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

This patch converts all present DT capable kirkwood board configurations
to use DT to configure the mv643xx ethernet controller.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-dnskw.dtsi     |    9 +++++++++
 arch/arm/boot/dts/kirkwood-dreamplug.dts  |   18 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood-goflexnet.dts  |    8 ++++++++
 arch/arm/boot/dts/kirkwood-ib62x0.dts     |   10 ++++++++++
 arch/arm/boot/dts/kirkwood-iconnect.dts   |   10 ++++++++++
 arch/arm/boot/dts/kirkwood-lsxl.dtsi      |   17 +++++++++++++++++
 arch/arm/boot/dts/kirkwood-ts219-6281.dts |    8 +++++++-
 arch/arm/boot/dts/kirkwood-ts219-6282.dts |    8 +++++++-
 arch/arm/boot/dts/kirkwood-ts219.dtsi     |    3 +++
 arch/arm/mach-kirkwood/board-dnskw.c      |    7 +------
 arch/arm/mach-kirkwood/board-dreamplug.c  |   13 ++-----------
 arch/arm/mach-kirkwood/board-goflexnet.c  |    7 +------
 arch/arm/mach-kirkwood/board-ib62x0.c     |    7 +------
 arch/arm/mach-kirkwood/board-iconnect.c   |    7 +------
 arch/arm/mach-kirkwood/board-lsxl.c       |   13 ++-----------
 arch/arm/mach-kirkwood/board-ts219.c      |   10 +---------
 16 files changed, 98 insertions(+), 57 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-dnskw.dtsi b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
index 7408655..214fe0b 100644
--- a/arch/arm/boot/dts/kirkwood-dnskw.dtsi
+++ b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
@@ -65,5 +65,14 @@
 				reg = <0x7b00000 0x500000>;
 			};
 		};
+
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/kirkwood-dreamplug.dts b/arch/arm/boot/dts/kirkwood-dreamplug.dts
index 26e281f..c27ed1c 100644
--- a/arch/arm/boot/dts/kirkwood-dreamplug.dts
+++ b/arch/arm/boot/dts/kirkwood-dreamplug.dts
@@ -53,6 +53,24 @@
 			status = "okay";
 			nr-ports = <1>;
 		};
+
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		smi1: mdio@76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <1>;
+			status = "ok";
+		};
 	};
 
 	gpio-leds {
diff --git a/arch/arm/boot/dts/kirkwood-goflexnet.dts b/arch/arm/boot/dts/kirkwood-goflexnet.dts
index 7c8238f..f03dbd0 100644
--- a/arch/arm/boot/dts/kirkwood-goflexnet.dts
+++ b/arch/arm/boot/dts/kirkwood-goflexnet.dts
@@ -50,6 +50,14 @@
 			nr-ports = <2>;
 		};
 
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
 	};
 	gpio-leds {
 		compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-ib62x0.dts b/arch/arm/boot/dts/kirkwood-ib62x0.dts
index 66794ed..8c462a1 100644
--- a/arch/arm/boot/dts/kirkwood-ib62x0.dts
+++ b/arch/arm/boot/dts/kirkwood-ib62x0.dts
@@ -45,6 +45,16 @@
 			};
 
 		};
+
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
+
 	};
 
 	gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-iconnect.dts b/arch/arm/boot/dts/kirkwood-iconnect.dts
index 52d9470..9fc82be 100644
--- a/arch/arm/boot/dts/kirkwood-iconnect.dts
+++ b/arch/arm/boot/dts/kirkwood-iconnect.dts
@@ -30,6 +30,16 @@
 			clock-frequency = <200000000>;
 			status = "ok";
 		};
+
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <b>;
+			status = "ok";
+		};
+
 	};
 	gpio-leds {
 		compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-lsxl.dtsi b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
index 8ac51c0..2f47661 100644
--- a/arch/arm/boot/dts/kirkwood-lsxl.dtsi
+++ b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
@@ -40,6 +40,23 @@
 				};
 			};
 		};
+		smi0: mdio@72000 {
+			status = "ok";
+		};
+
+		smi1: mdio@76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <8>;
+			status = "ok";
+		};
 	};
 
 	gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6281.dts b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
index ccbf327..4ca49b5 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6281.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
@@ -3,6 +3,12 @@
 /include/ "kirkwood-ts219.dtsi"
 
 / {
+	ocp@f1000000 {
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
+	};
 	gpio_keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
@@ -18,4 +24,4 @@
 			gpios = <&gpio0 16 1>;
 		};
 	};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6282.dts b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
index fbe9932..40f3c61 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6282.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
@@ -3,6 +3,12 @@
 /include/ "kirkwood-ts219.dtsi"
 
 / {
+	ocp@f1000000 {
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+	};
 	gpio_keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
@@ -18,4 +24,4 @@
 			gpios = <&gpio1 5 1>;
 		};
 	};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219.dtsi b/arch/arm/boot/dts/kirkwood-ts219.dtsi
index 64ea27c..06caf41 100644
--- a/arch/arm/boot/dts/kirkwood-ts219.dtsi
+++ b/arch/arm/boot/dts/kirkwood-ts219.dtsi
@@ -74,5 +74,8 @@
 			status = "okay";
 			nr-ports = <2>;
 		};
+		smi0: mdio@72000 {
+			status = "ok";
+		};
 	};
 };
diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c
index 4ab3506..4d8216b 100644
--- a/arch/arm/mach-kirkwood/board-dnskw.c
+++ b/arch/arm/mach-kirkwood/board-dnskw.c
@@ -15,7 +15,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
@@ -29,10 +28,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data dnskw_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int dnskw_mpp_config[] __initdata = {
 	MPP13_UART1_TXD,	/* Custom ... */
 	MPP14_UART1_RXD,	/* ... Controller (DNS-320 only) */
@@ -112,7 +107,7 @@ void __init dnskw_init(void)
 	kirkwood_mpp_conf(dnskw_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&dnskw_ge00_data);
+	kirkwood_ge00_init(NULL);
 
 	platform_device_register(&dnskw_fan_device);
 
diff --git a/arch/arm/mach-kirkwood/board-dreamplug.c b/arch/arm/mach-kirkwood/board-dreamplug.c
index aeb234d..b97a112 100644
--- a/arch/arm/mach-kirkwood/board-dreamplug.c
+++ b/arch/arm/mach-kirkwood/board-dreamplug.c
@@ -15,7 +15,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_fdt.h>
@@ -34,14 +33,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data dreamplug_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data dreamplug_ge01_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(1),
-};
-
 static struct mvsdio_platform_data dreamplug_mvsdio_data = {
 	/* unfortunately the CD signal has not been connected */
 };
@@ -65,7 +56,7 @@ void __init dreamplug_init(void)
 	kirkwood_mpp_conf(dreamplug_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&dreamplug_ge00_data);
-	kirkwood_ge01_init(&dreamplug_ge01_data);
+	kirkwood_ge00_init(NULL);
+	kirkwood_ge01_init(NULL);
 	kirkwood_sdio_init(&dreamplug_mvsdio_data);
 }
diff --git a/arch/arm/mach-kirkwood/board-goflexnet.c b/arch/arm/mach-kirkwood/board-goflexnet.c
index 413e2c8..be7437d 100644
--- a/arch/arm/mach-kirkwood/board-goflexnet.c
+++ b/arch/arm/mach-kirkwood/board-goflexnet.c
@@ -20,7 +20,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_fdt.h>
@@ -36,10 +35,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data goflexnet_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
 static unsigned int goflexnet_mpp_config[] __initdata = {
 	MPP29_GPIO,	/* USB Power Enable */
 	MPP47_GPIO,	/* LED Orange */
@@ -67,5 +62,5 @@ void __init goflexnet_init(void)
 		pr_err("can't setup GPIO 29 (USB Power Enable)\n");
 	kirkwood_ehci_init();
 
-	kirkwood_ge00_init(&goflexnet_ge00_data);
+	kirkwood_ge00_init(NULL);
 }
diff --git a/arch/arm/mach-kirkwood/board-ib62x0.c b/arch/arm/mach-kirkwood/board-ib62x0.c
index cfc47f8..0a29183 100644
--- a/arch/arm/mach-kirkwood/board-ib62x0.c
+++ b/arch/arm/mach-kirkwood/board-ib62x0.c
@@ -16,7 +16,6 @@
 #include <linux/platform_device.h>
 #include <linux/mtd/partitions.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <asm/mach-types.h>
@@ -27,10 +26,6 @@
 
 #define IB62X0_GPIO_POWER_OFF	24
 
-static struct mv643xx_eth_platform_data ib62x0_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int ib62x0_mpp_config[] __initdata = {
 	MPP0_NF_IO2,
 	MPP1_NF_IO3,
@@ -62,7 +57,7 @@ void __init ib62x0_init(void)
 	kirkwood_mpp_conf(ib62x0_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&ib62x0_ge00_data);
+	kirkwood_ge00_init(NULL);
 	if (gpio_request(IB62X0_GPIO_POWER_OFF, "ib62x0:power:off") == 0 &&
 	    gpio_direction_output(IB62X0_GPIO_POWER_OFF, 0) == 0)
 		pm_power_off = ib62x0_power_off;
diff --git a/arch/arm/mach-kirkwood/board-iconnect.c b/arch/arm/mach-kirkwood/board-iconnect.c
index d7a9198..220f0d4 100644
--- a/arch/arm/mach-kirkwood/board-iconnect.c
+++ b/arch/arm/mach-kirkwood/board-iconnect.c
@@ -17,7 +17,6 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/mtd/partitions.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <linux/gpio_keys.h>
@@ -26,10 +25,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data iconnect_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(11),
-};
-
 static unsigned int iconnect_mpp_config[] __initdata = {
 	MPP12_GPIO,
 	MPP35_GPIO,
@@ -92,7 +87,7 @@ void __init iconnect_init(void)
 	kirkwood_nand_init(ARRAY_AND_SIZE(iconnect_nand_parts), 25);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&iconnect_ge00_data);
+	kirkwood_ge00_init(NULL);
 
 	platform_device_register(&iconnect_button_device);
 }
diff --git a/arch/arm/mach-kirkwood/board-lsxl.c b/arch/arm/mach-kirkwood/board-lsxl.c
index 83d8975..60331d1 100644
--- a/arch/arm/mach-kirkwood/board-lsxl.c
+++ b/arch/arm/mach-kirkwood/board-lsxl.c
@@ -18,7 +18,6 @@
 #include <linux/ata_platform.h>
 #include <linux/spi/flash.h>
 #include <linux/spi/spi.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/gpio-fan.h>
 #include <linux/input.h>
@@ -28,14 +27,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data lsxl_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data lsxl_ge01_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int lsxl_mpp_config[] __initdata = {
 	MPP10_GPO,	/* HDD Power Enable */
 	MPP11_GPIO,	/* USB Vbus Enable */
@@ -126,8 +117,8 @@ void __init lsxl_init(void)
 	gpio_set_value(LSXL_GPIO_HDD_POWER, 1);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&lsxl_ge00_data);
-	kirkwood_ge01_init(&lsxl_ge01_data);
+	kirkwood_ge00_init(NULL);
+	kirkwood_ge01_init(NULL);
 	platform_device_register(&lsxl_fan_device);
 
 	/* register power-off method */
diff --git a/arch/arm/mach-kirkwood/board-ts219.c b/arch/arm/mach-kirkwood/board-ts219.c
index 1750e68..7e7fe6c 100644
--- a/arch/arm/mach-kirkwood/board-ts219.c
+++ b/arch/arm/mach-kirkwood/board-ts219.c
@@ -18,7 +18,6 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/ata_platform.h>
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
@@ -29,10 +28,6 @@
 #include "mpp.h"
 #include "tsx1x-common.h"
 
-static struct mv643xx_eth_platform_data qnap_ts219_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int qnap_ts219_mpp_config[] __initdata = {
 	MPP0_SPI_SCn,
 	MPP1_SPI_MOSI,
@@ -62,10 +57,7 @@ void __init qnap_dt_ts219_init(void)
 	kirkwood_mpp_conf(qnap_ts219_mpp_config);
 
 	kirkwood_pcie_id(&dev, &rev);
-	if (dev == MV88F6282_DEV_ID)
-		qnap_ts219_ge00_data.phy_addr = MV643XX_ETH_PHY_ADDR(0);
-
-	kirkwood_ge00_init(&qnap_ts219_ge00_data);
+	kirkwood_ge00_init(NULL);
 	kirkwood_ehci_init();
 
 	pm_power_off = qnap_tsx1x_power_off;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 6/7] DT: Convert all kirkwood boards with mv643xx that use DT
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

This patch converts all present DT capable kirkwood board configurations
to use DT to configure the mv643xx ethernet controller.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/boot/dts/kirkwood-dnskw.dtsi     |    9 +++++++++
 arch/arm/boot/dts/kirkwood-dreamplug.dts  |   18 ++++++++++++++++++
 arch/arm/boot/dts/kirkwood-goflexnet.dts  |    8 ++++++++
 arch/arm/boot/dts/kirkwood-ib62x0.dts     |   10 ++++++++++
 arch/arm/boot/dts/kirkwood-iconnect.dts   |   10 ++++++++++
 arch/arm/boot/dts/kirkwood-lsxl.dtsi      |   17 +++++++++++++++++
 arch/arm/boot/dts/kirkwood-ts219-6281.dts |    8 +++++++-
 arch/arm/boot/dts/kirkwood-ts219-6282.dts |    8 +++++++-
 arch/arm/boot/dts/kirkwood-ts219.dtsi     |    3 +++
 arch/arm/mach-kirkwood/board-dnskw.c      |    7 +------
 arch/arm/mach-kirkwood/board-dreamplug.c  |   13 ++-----------
 arch/arm/mach-kirkwood/board-goflexnet.c  |    7 +------
 arch/arm/mach-kirkwood/board-ib62x0.c     |    7 +------
 arch/arm/mach-kirkwood/board-iconnect.c   |    7 +------
 arch/arm/mach-kirkwood/board-lsxl.c       |   13 ++-----------
 arch/arm/mach-kirkwood/board-ts219.c      |   10 +---------
 16 files changed, 98 insertions(+), 57 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-dnskw.dtsi b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
index 7408655..214fe0b 100644
--- a/arch/arm/boot/dts/kirkwood-dnskw.dtsi
+++ b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
@@ -65,5 +65,14 @@
 				reg = <0x7b00000 0x500000>;
 			};
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/kirkwood-dreamplug.dts b/arch/arm/boot/dts/kirkwood-dreamplug.dts
index 26e281f..c27ed1c 100644
--- a/arch/arm/boot/dts/kirkwood-dreamplug.dts
+++ b/arch/arm/boot/dts/kirkwood-dreamplug.dts
@@ -53,6 +53,24 @@
 			status = "okay";
 			nr-ports = <1>;
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		smi1: mdio at 76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <1>;
+			status = "ok";
+		};
 	};
 
 	gpio-leds {
diff --git a/arch/arm/boot/dts/kirkwood-goflexnet.dts b/arch/arm/boot/dts/kirkwood-goflexnet.dts
index 7c8238f..f03dbd0 100644
--- a/arch/arm/boot/dts/kirkwood-goflexnet.dts
+++ b/arch/arm/boot/dts/kirkwood-goflexnet.dts
@@ -50,6 +50,14 @@
 			nr-ports = <2>;
 		};
 
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
 	};
 	gpio-leds {
 		compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-ib62x0.dts b/arch/arm/boot/dts/kirkwood-ib62x0.dts
index 66794ed..8c462a1 100644
--- a/arch/arm/boot/dts/kirkwood-ib62x0.dts
+++ b/arch/arm/boot/dts/kirkwood-ib62x0.dts
@@ -45,6 +45,16 @@
 			};
 
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
+
 	};
 
 	gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-iconnect.dts b/arch/arm/boot/dts/kirkwood-iconnect.dts
index 52d9470..9fc82be 100644
--- a/arch/arm/boot/dts/kirkwood-iconnect.dts
+++ b/arch/arm/boot/dts/kirkwood-iconnect.dts
@@ -30,6 +30,16 @@
 			clock-frequency = <200000000>;
 			status = "ok";
 		};
+
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <b>;
+			status = "ok";
+		};
+
 	};
 	gpio-leds {
 		compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-lsxl.dtsi b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
index 8ac51c0..2f47661 100644
--- a/arch/arm/boot/dts/kirkwood-lsxl.dtsi
+++ b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
@@ -40,6 +40,23 @@
 				};
 			};
 		};
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
+
+		smi1: mdio at 76000 {
+			status = "ok";
+		};
+
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+
+		egiga1 {
+			phy_addr = <8>;
+			status = "ok";
+		};
 	};
 
 	gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6281.dts b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
index ccbf327..4ca49b5 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6281.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
@@ -3,6 +3,12 @@
 /include/ "kirkwood-ts219.dtsi"
 
 / {
+	ocp at f1000000 {
+		egiga0 {
+			phy_addr = <8>;
+			status = "ok";
+		};
+	};
 	gpio_keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
@@ -18,4 +24,4 @@
 			gpios = <&gpio0 16 1>;
 		};
 	};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6282.dts b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
index fbe9932..40f3c61 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6282.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
@@ -3,6 +3,12 @@
 /include/ "kirkwood-ts219.dtsi"
 
 / {
+	ocp at f1000000 {
+		egiga0 {
+			phy_addr = <0>;
+			status = "ok";
+		};
+	};
 	gpio_keys {
 		compatible = "gpio-keys";
 		#address-cells = <1>;
@@ -18,4 +24,4 @@
 			gpios = <&gpio1 5 1>;
 		};
 	};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219.dtsi b/arch/arm/boot/dts/kirkwood-ts219.dtsi
index 64ea27c..06caf41 100644
--- a/arch/arm/boot/dts/kirkwood-ts219.dtsi
+++ b/arch/arm/boot/dts/kirkwood-ts219.dtsi
@@ -74,5 +74,8 @@
 			status = "okay";
 			nr-ports = <2>;
 		};
+		smi0: mdio at 72000 {
+			status = "ok";
+		};
 	};
 };
diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c
index 4ab3506..4d8216b 100644
--- a/arch/arm/mach-kirkwood/board-dnskw.c
+++ b/arch/arm/mach-kirkwood/board-dnskw.c
@@ -15,7 +15,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
@@ -29,10 +28,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data dnskw_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int dnskw_mpp_config[] __initdata = {
 	MPP13_UART1_TXD,	/* Custom ... */
 	MPP14_UART1_RXD,	/* ... Controller (DNS-320 only) */
@@ -112,7 +107,7 @@ void __init dnskw_init(void)
 	kirkwood_mpp_conf(dnskw_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&dnskw_ge00_data);
+	kirkwood_ge00_init(NULL);
 
 	platform_device_register(&dnskw_fan_device);
 
diff --git a/arch/arm/mach-kirkwood/board-dreamplug.c b/arch/arm/mach-kirkwood/board-dreamplug.c
index aeb234d..b97a112 100644
--- a/arch/arm/mach-kirkwood/board-dreamplug.c
+++ b/arch/arm/mach-kirkwood/board-dreamplug.c
@@ -15,7 +15,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_fdt.h>
@@ -34,14 +33,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data dreamplug_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data dreamplug_ge01_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(1),
-};
-
 static struct mvsdio_platform_data dreamplug_mvsdio_data = {
 	/* unfortunately the CD signal has not been connected */
 };
@@ -65,7 +56,7 @@ void __init dreamplug_init(void)
 	kirkwood_mpp_conf(dreamplug_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&dreamplug_ge00_data);
-	kirkwood_ge01_init(&dreamplug_ge01_data);
+	kirkwood_ge00_init(NULL);
+	kirkwood_ge01_init(NULL);
 	kirkwood_sdio_init(&dreamplug_mvsdio_data);
 }
diff --git a/arch/arm/mach-kirkwood/board-goflexnet.c b/arch/arm/mach-kirkwood/board-goflexnet.c
index 413e2c8..be7437d 100644
--- a/arch/arm/mach-kirkwood/board-goflexnet.c
+++ b/arch/arm/mach-kirkwood/board-goflexnet.c
@@ -20,7 +20,6 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_fdt.h>
@@ -36,10 +35,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data goflexnet_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
 static unsigned int goflexnet_mpp_config[] __initdata = {
 	MPP29_GPIO,	/* USB Power Enable */
 	MPP47_GPIO,	/* LED Orange */
@@ -67,5 +62,5 @@ void __init goflexnet_init(void)
 		pr_err("can't setup GPIO 29 (USB Power Enable)\n");
 	kirkwood_ehci_init();
 
-	kirkwood_ge00_init(&goflexnet_ge00_data);
+	kirkwood_ge00_init(NULL);
 }
diff --git a/arch/arm/mach-kirkwood/board-ib62x0.c b/arch/arm/mach-kirkwood/board-ib62x0.c
index cfc47f8..0a29183 100644
--- a/arch/arm/mach-kirkwood/board-ib62x0.c
+++ b/arch/arm/mach-kirkwood/board-ib62x0.c
@@ -16,7 +16,6 @@
 #include <linux/platform_device.h>
 #include <linux/mtd/partitions.h>
 #include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <asm/mach-types.h>
@@ -27,10 +26,6 @@
 
 #define IB62X0_GPIO_POWER_OFF	24
 
-static struct mv643xx_eth_platform_data ib62x0_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int ib62x0_mpp_config[] __initdata = {
 	MPP0_NF_IO2,
 	MPP1_NF_IO3,
@@ -62,7 +57,7 @@ void __init ib62x0_init(void)
 	kirkwood_mpp_conf(ib62x0_mpp_config);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&ib62x0_ge00_data);
+	kirkwood_ge00_init(NULL);
 	if (gpio_request(IB62X0_GPIO_POWER_OFF, "ib62x0:power:off") == 0 &&
 	    gpio_direction_output(IB62X0_GPIO_POWER_OFF, 0) == 0)
 		pm_power_off = ib62x0_power_off;
diff --git a/arch/arm/mach-kirkwood/board-iconnect.c b/arch/arm/mach-kirkwood/board-iconnect.c
index d7a9198..220f0d4 100644
--- a/arch/arm/mach-kirkwood/board-iconnect.c
+++ b/arch/arm/mach-kirkwood/board-iconnect.c
@@ -17,7 +17,6 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/mtd/partitions.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <linux/gpio_keys.h>
@@ -26,10 +25,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data iconnect_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(11),
-};
-
 static unsigned int iconnect_mpp_config[] __initdata = {
 	MPP12_GPIO,
 	MPP35_GPIO,
@@ -92,7 +87,7 @@ void __init iconnect_init(void)
 	kirkwood_nand_init(ARRAY_AND_SIZE(iconnect_nand_parts), 25);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&iconnect_ge00_data);
+	kirkwood_ge00_init(NULL);
 
 	platform_device_register(&iconnect_button_device);
 }
diff --git a/arch/arm/mach-kirkwood/board-lsxl.c b/arch/arm/mach-kirkwood/board-lsxl.c
index 83d8975..60331d1 100644
--- a/arch/arm/mach-kirkwood/board-lsxl.c
+++ b/arch/arm/mach-kirkwood/board-lsxl.c
@@ -18,7 +18,6 @@
 #include <linux/ata_platform.h>
 #include <linux/spi/flash.h>
 #include <linux/spi/spi.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/gpio.h>
 #include <linux/gpio-fan.h>
 #include <linux/input.h>
@@ -28,14 +27,6 @@
 #include "common.h"
 #include "mpp.h"
 
-static struct mv643xx_eth_platform_data lsxl_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data lsxl_ge01_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int lsxl_mpp_config[] __initdata = {
 	MPP10_GPO,	/* HDD Power Enable */
 	MPP11_GPIO,	/* USB Vbus Enable */
@@ -126,8 +117,8 @@ void __init lsxl_init(void)
 	gpio_set_value(LSXL_GPIO_HDD_POWER, 1);
 
 	kirkwood_ehci_init();
-	kirkwood_ge00_init(&lsxl_ge00_data);
-	kirkwood_ge01_init(&lsxl_ge01_data);
+	kirkwood_ge00_init(NULL);
+	kirkwood_ge01_init(NULL);
 	platform_device_register(&lsxl_fan_device);
 
 	/* register power-off method */
diff --git a/arch/arm/mach-kirkwood/board-ts219.c b/arch/arm/mach-kirkwood/board-ts219.c
index 1750e68..7e7fe6c 100644
--- a/arch/arm/mach-kirkwood/board-ts219.c
+++ b/arch/arm/mach-kirkwood/board-ts219.c
@@ -18,7 +18,6 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <linux/mv643xx_eth.h>
 #include <linux/ata_platform.h>
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
@@ -29,10 +28,6 @@
 #include "mpp.h"
 #include "tsx1x-common.h"
 
-static struct mv643xx_eth_platform_data qnap_ts219_ge00_data = {
-	.phy_addr	= MV643XX_ETH_PHY_ADDR(8),
-};
-
 static unsigned int qnap_ts219_mpp_config[] __initdata = {
 	MPP0_SPI_SCn,
 	MPP1_SPI_MOSI,
@@ -62,10 +57,7 @@ void __init qnap_dt_ts219_init(void)
 	kirkwood_mpp_conf(qnap_ts219_mpp_config);
 
 	kirkwood_pcie_id(&dev, &rev);
-	if (dev == MV88F6282_DEV_ID)
-		qnap_ts219_ge00_data.phy_addr = MV643XX_ETH_PHY_ADDR(0);
-
-	kirkwood_ge00_init(&qnap_ts219_ge00_data);
+	kirkwood_ge00_init(NULL);
 	kirkwood_ehci_init();
 
 	pm_power_off = qnap_tsx1x_power_off;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 7/7] NET: mv643xx: remove device name macro.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 14:34   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev

Coding style: remove the macros:

MV643XX_ETH_NAME and
MV643XX_ETH_SHARED_NAME

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/mach-kirkwood/board-dt.c          |    4 ++--
 arch/arm/mach-kirkwood/common.c            |    4 ++--
 arch/arm/plat-orion/common.c               |   24 ++++++++++++------------
 arch/powerpc/platforms/chrp/pegasos_eth.c  |    4 ++--
 arch/powerpc/sysdev/mv64x60_dev.c          |    5 ++---
 drivers/net/ethernet/marvell/mv643xx_eth.c |    8 ++++----
 include/linux/mv643xx_eth.h                |    2 --
 7 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index aa213b6..91784d9 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -34,9 +34,9 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
 	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
 	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
-	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth_port.0",
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth.0",
 			NULL),
-	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth_port.1",
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth.1",
 			NULL),
 	{},
 };
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index 57b91cf..eb0a253 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -264,8 +264,8 @@ void __init kirkwood_clk_init(void)
 	/* clkdev entries, mapping clks to devices */
 	orion_clkdev_add(NULL, "orion_spi.0", runit);
 	orion_clkdev_add(NULL, "orion_spi.1", runit);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".0", ge0);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".1", ge1);
+	orion_clkdev_add(NULL, "mv643xx_eth.0", ge0);
+	orion_clkdev_add(NULL, "mv643xx_eth.1", ge1);
 	orion_clkdev_add(NULL, "orion_wdt", tclk);
 	orion_clkdev_add("0", "sata_mv.0", sata0);
 	orion_clkdev_add("1", "sata_mv.0", sata1);
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index d245a87..d4a6467 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -42,10 +42,10 @@ void __init orion_clkdev_init(struct clk *tclk)
 {
 	orion_clkdev_add(NULL, "orion_spi.0", tclk);
 	orion_clkdev_add(NULL, "orion_spi.1", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".0", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".1", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".2", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".3", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.0", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.1", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.2", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.3", tclk);
 	orion_clkdev_add(NULL, "orion_wdt", tclk);
 	orion_clkdev_add(NULL, MV64XXX_I2C_CTLR_NAME ".0", tclk);
 }
@@ -264,7 +264,7 @@ static struct resource orion_ge00_shared_resources[] = {
 };
 
 static struct platform_device orion_ge00_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 0,
 	.dev		= {
 		.platform_data	= &orion_ge00_shared_data,
@@ -279,7 +279,7 @@ static struct resource orion_ge00_resources[] = {
 };
 
 static struct platform_device orion_ge00 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 0,
 	.num_resources	= 1,
 	.resource	= orion_ge00_resources,
@@ -316,7 +316,7 @@ static struct resource orion_ge01_shared_resources[] = {
 };
 
 static struct platform_device orion_ge01_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge01_shared_data,
@@ -331,7 +331,7 @@ static struct resource orion_ge01_resources[] = {
 };
 
 static struct platform_device orion_ge01 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 1,
 	.resource	= orion_ge01_resources,
@@ -368,7 +368,7 @@ static struct resource orion_ge10_shared_resources[] = {
 };
 
 static struct platform_device orion_ge10_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge10_shared_data,
@@ -383,7 +383,7 @@ static struct resource orion_ge10_resources[] = {
 };
 
 static struct platform_device orion_ge10 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 2,
 	.resource	= orion_ge10_resources,
@@ -420,7 +420,7 @@ static struct resource orion_ge11_shared_resources[] = {
 };
 
 static struct platform_device orion_ge11_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge11_shared_data,
@@ -435,7 +435,7 @@ static struct resource orion_ge11_resources[] = {
 };
 
 static struct platform_device orion_ge11 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 2,
 	.resource	= orion_ge11_resources,
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..1832127 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -41,7 +41,7 @@ static struct resource mv643xx_eth_shared_resources[] = {
 };
 
 static struct platform_device mv643xx_eth_shared_device = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth_shared_resources),
 	.resource	= mv643xx_eth_shared_resources,
@@ -71,7 +71,7 @@ static struct mv643xx_eth_platform_data eth_port1_pd = {
 };
 
 static struct platform_device eth_port1_device = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth_port1_resources),
 	.resource	= mv643xx_eth_port1_resources,
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..ca404ec 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -221,8 +221,7 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 	if (err)
 		return ERR_PTR(err);
 
-	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+	pdev = platform_device_register_simple("mdio-mv643xx", id, i r, 1);
 	return pdev;
 }
 
@@ -296,7 +295,7 @@ static int __init mv64x60_eth_device_setup(struct device_node *np, int id,
 
 	of_node_put(phy);
 
-	pdev = platform_device_alloc(MV643XX_ETH_NAME, id);
+	pdev = platform_device_alloc("mv643xx_eth", id);
 	if (!pdev)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index bb80050..9371601 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2746,7 +2746,7 @@ static struct platform_driver mv643xx_eth_shared_driver = {
 	.probe		= mv643xx_eth_shared_probe,
 	.remove		= mv643xx_eth_shared_remove,
 	.driver = {
-		.name	= MV643XX_ETH_SHARED_NAME,
+		.name	= "mdio-mv643xx",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(mv_mdio_dt_ids),
 	},
@@ -3113,7 +3113,7 @@ static struct platform_driver mv643xx_eth_driver = {
 	.remove		= mv643xx_eth_remove,
 	.shutdown	= mv643xx_eth_shutdown,
 	.driver = {
-		.name	= MV643XX_ETH_NAME,
+		.name	= "mv643xx_eth",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(mv_eth_dt_ids),
 	},
@@ -3145,5 +3145,5 @@ MODULE_AUTHOR("Rabeeh Khoury, Assaf Hoffman, Matthew Dharm, "
 	      "Manish Lachwani, Dale Farnsworth and Lennert Buytenhek");
 MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" MV643XX_ETH_SHARED_NAME);
-MODULE_ALIAS("platform:" MV643XX_ETH_NAME);
+MODULE_ALIAS("platform:" "mdio-mv643xx");
+MODULE_ALIAS("platform:" "mv643xx_eth");
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 51bf8ad..33dc6f4 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -7,8 +7,6 @@
 
 #include <linux/mbus.h>
 
-#define MV643XX_ETH_SHARED_NAME		"mv643xx_eth"
-#define MV643XX_ETH_NAME		"mv643xx_eth_port"
 #define MV643XX_ETH_SHARED_REGS		0x2000
 #define MV643XX_ETH_SHARED_REGS_SIZE	0x2000
 #define MV643XX_ETH_BAR_4		0x2220
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v3 7/7] NET: mv643xx: remove device name macro.
@ 2012-08-07 14:34   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

Coding style: remove the macros:

MV643XX_ETH_NAME and
MV643XX_ETH_SHARED_NAME

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 arch/arm/mach-kirkwood/board-dt.c          |    4 ++--
 arch/arm/mach-kirkwood/common.c            |    4 ++--
 arch/arm/plat-orion/common.c               |   24 ++++++++++++------------
 arch/powerpc/platforms/chrp/pegasos_eth.c  |    4 ++--
 arch/powerpc/sysdev/mv64x60_dev.c          |    5 ++---
 drivers/net/ethernet/marvell/mv643xx_eth.c |    8 ++++----
 include/linux/mv643xx_eth.h                |    2 --
 7 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index aa213b6..91784d9 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -34,9 +34,9 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
 	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
 	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
-	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth_port.0",
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, "mv643xx_eth.0",
 			NULL),
-	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth_port.1",
+	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, "mv643xx_eth.1",
 			NULL),
 	{},
 };
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index 57b91cf..eb0a253 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -264,8 +264,8 @@ void __init kirkwood_clk_init(void)
 	/* clkdev entries, mapping clks to devices */
 	orion_clkdev_add(NULL, "orion_spi.0", runit);
 	orion_clkdev_add(NULL, "orion_spi.1", runit);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".0", ge0);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".1", ge1);
+	orion_clkdev_add(NULL, "mv643xx_eth.0", ge0);
+	orion_clkdev_add(NULL, "mv643xx_eth.1", ge1);
 	orion_clkdev_add(NULL, "orion_wdt", tclk);
 	orion_clkdev_add("0", "sata_mv.0", sata0);
 	orion_clkdev_add("1", "sata_mv.0", sata1);
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index d245a87..d4a6467 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -42,10 +42,10 @@ void __init orion_clkdev_init(struct clk *tclk)
 {
 	orion_clkdev_add(NULL, "orion_spi.0", tclk);
 	orion_clkdev_add(NULL, "orion_spi.1", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".0", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".1", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".2", tclk);
-	orion_clkdev_add(NULL, MV643XX_ETH_NAME ".3", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.0", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.1", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.2", tclk);
+	orion_clkdev_add(NULL, "mv643xx_eth.3", tclk);
 	orion_clkdev_add(NULL, "orion_wdt", tclk);
 	orion_clkdev_add(NULL, MV64XXX_I2C_CTLR_NAME ".0", tclk);
 }
@@ -264,7 +264,7 @@ static struct resource orion_ge00_shared_resources[] = {
 };
 
 static struct platform_device orion_ge00_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 0,
 	.dev		= {
 		.platform_data	= &orion_ge00_shared_data,
@@ -279,7 +279,7 @@ static struct resource orion_ge00_resources[] = {
 };
 
 static struct platform_device orion_ge00 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 0,
 	.num_resources	= 1,
 	.resource	= orion_ge00_resources,
@@ -316,7 +316,7 @@ static struct resource orion_ge01_shared_resources[] = {
 };
 
 static struct platform_device orion_ge01_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge01_shared_data,
@@ -331,7 +331,7 @@ static struct resource orion_ge01_resources[] = {
 };
 
 static struct platform_device orion_ge01 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 1,
 	.resource	= orion_ge01_resources,
@@ -368,7 +368,7 @@ static struct resource orion_ge10_shared_resources[] = {
 };
 
 static struct platform_device orion_ge10_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge10_shared_data,
@@ -383,7 +383,7 @@ static struct resource orion_ge10_resources[] = {
 };
 
 static struct platform_device orion_ge10 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 2,
 	.resource	= orion_ge10_resources,
@@ -420,7 +420,7 @@ static struct resource orion_ge11_shared_resources[] = {
 };
 
 static struct platform_device orion_ge11_shared = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 1,
 	.dev		= {
 		.platform_data	= &orion_ge11_shared_data,
@@ -435,7 +435,7 @@ static struct resource orion_ge11_resources[] = {
 };
 
 static struct platform_device orion_ge11 = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= 2,
 	.resource	= orion_ge11_resources,
diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..1832127 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -41,7 +41,7 @@ static struct resource mv643xx_eth_shared_resources[] = {
 };
 
 static struct platform_device mv643xx_eth_shared_device = {
-	.name		= MV643XX_ETH_SHARED_NAME,
+	.name		= "mdio-mv643xx",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth_shared_resources),
 	.resource	= mv643xx_eth_shared_resources,
@@ -71,7 +71,7 @@ static struct mv643xx_eth_platform_data eth_port1_pd = {
 };
 
 static struct platform_device eth_port1_device = {
-	.name		= MV643XX_ETH_NAME,
+	.name		= "mv643xx_eth",
 	.id		= 1,
 	.num_resources	= ARRAY_SIZE(mv643xx_eth_port1_resources),
 	.resource	= mv643xx_eth_port1_resources,
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..ca404ec 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -221,8 +221,7 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
 	if (err)
 		return ERR_PTR(err);
 
-	pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
-					       r, 1);
+	pdev = platform_device_register_simple("mdio-mv643xx", id, i r, 1);
 	return pdev;
 }
 
@@ -296,7 +295,7 @@ static int __init mv64x60_eth_device_setup(struct device_node *np, int id,
 
 	of_node_put(phy);
 
-	pdev = platform_device_alloc(MV643XX_ETH_NAME, id);
+	pdev = platform_device_alloc("mv643xx_eth", id);
 	if (!pdev)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index bb80050..9371601 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2746,7 +2746,7 @@ static struct platform_driver mv643xx_eth_shared_driver = {
 	.probe		= mv643xx_eth_shared_probe,
 	.remove		= mv643xx_eth_shared_remove,
 	.driver = {
-		.name	= MV643XX_ETH_SHARED_NAME,
+		.name	= "mdio-mv643xx",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(mv_mdio_dt_ids),
 	},
@@ -3113,7 +3113,7 @@ static struct platform_driver mv643xx_eth_driver = {
 	.remove		= mv643xx_eth_remove,
 	.shutdown	= mv643xx_eth_shutdown,
 	.driver = {
-		.name	= MV643XX_ETH_NAME,
+		.name	= "mv643xx_eth",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(mv_eth_dt_ids),
 	},
@@ -3145,5 +3145,5 @@ MODULE_AUTHOR("Rabeeh Khoury, Assaf Hoffman, Matthew Dharm, "
 	      "Manish Lachwani, Dale Farnsworth and Lennert Buytenhek");
 MODULE_DESCRIPTION("Ethernet driver for Marvell MV643XX");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" MV643XX_ETH_SHARED_NAME);
-MODULE_ALIAS("platform:" MV643XX_ETH_NAME);
+MODULE_ALIAS("platform:" "mdio-mv643xx");
+MODULE_ALIAS("platform:" "mv643xx_eth");
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 51bf8ad..33dc6f4 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -7,8 +7,6 @@
 
 #include <linux/mbus.h>
 
-#define MV643XX_ETH_SHARED_NAME		"mv643xx_eth"
-#define MV643XX_ETH_NAME		"mv643xx_eth_port"
 #define MV643XX_ETH_SHARED_REGS		0x2000
 #define MV643XX_ETH_SHARED_REGS_SIZE	0x2000
 #define MV643XX_ETH_BAR_4		0x2220
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
  2012-08-07 14:34   ` Ian Molton
@ 2012-08-07 14:56     ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-07 14:56 UTC (permalink / raw)
  To: Ian Molton; +Cc: linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks, netdev

On Tuesday 07 August 2012, Ian Molton wrote:
>     This patch adds basic device tree support to the mv643xx ethernet driver.
> 
>     It should be enough for most current users of the device, and should allow
>     a painless migration.
> 
>     Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
> ---
>  Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++++
>  drivers/net/ethernet/marvell/mv643xx_eth.c        |   93 +++++++++++++++++++--
>  2 files changed, 161 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt

Hi Ian,

Have you had a look at Documentation/devicetree/bindings/marvell.txt ?

I think it documents some of the same thing, so it would be good
to keep all of that in one place. We might also want to move
some of the code from arch/powerpc/sysdev/mv64x60_dev.c
to live in the same place as the device driver.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 14:56     ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-07 14:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 07 August 2012, Ian Molton wrote:
>     This patch adds basic device tree support to the mv643xx ethernet driver.
> 
>     It should be enough for most current users of the device, and should allow
>     a painless migration.
> 
>     Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
> ---
>  Documentation/devicetree/bindings/net/mv643xx.txt |   75 +++++++++++++++++
>  drivers/net/ethernet/marvell/mv643xx_eth.c        |   93 +++++++++++++++++++--
>  2 files changed, 161 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt

Hi Ian,

Have you had a look at Documentation/devicetree/bindings/marvell.txt ?

I think it documents some of the same thing, so it would be good
to keep all of that in one place. We might also want to move
some of the code from arch/powerpc/sysdev/mv64x60_dev.c
to live in the same place as the device driver.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
  2012-08-07 14:56     ` Arnd Bergmann
@ 2012-08-07 15:56       ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 15:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks, netdev

On 07/08/12 15:56, Arnd Bergmann wrote:

> Hi Ian,
>
> Have you had a look at Documentation/devicetree/bindings/marvell.txt
> ?

Nope. I had no idea it was hiding there.

> I think it documents some of the same thing,

Not really. It documents some godawful hack that recycled the platform
device -based driver and provided a DT binding for it, just for PPC.

I cant even *find* anything that implements code for whatever
"marvell,mv64360-mdio" might be. I'm sure it might exist somewhere.

> We might also want to move some of the
> code from arch/powerpc/sysdev/mv64x60_dev.c to live in the same place
> as the device driver.

I hope not. I don't really want to touch that stuff at all. If it works
the way it
is, then it can stay that way. If the PPC folk want to send patches to
add the
properties they use to the driver, then they can do. I'll send an email
their
way and see if they want to join in.

>From my perspective, the next thing that needs to happen to the driver is
for it to be broken up into ethernet and mdio drivers, so that we can
get rid
of all this shared_smi craziness... But that's for another patch series.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 15:56       ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-07 15:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/08/12 15:56, Arnd Bergmann wrote:

> Hi Ian,
>
> Have you had a look at Documentation/devicetree/bindings/marvell.txt
> ?

Nope. I had no idea it was hiding there.

> I think it documents some of the same thing,

Not really. It documents some godawful hack that recycled the platform
device -based driver and provided a DT binding for it, just for PPC.

I cant even *find* anything that implements code for whatever
"marvell,mv64360-mdio" might be. I'm sure it might exist somewhere.

> We might also want to move some of the
> code from arch/powerpc/sysdev/mv64x60_dev.c to live in the same place
> as the device driver.

I hope not. I don't really want to touch that stuff at all. If it works
the way it
is, then it can stay that way. If the PPC folk want to send patches to
add the
properties they use to the driver, then they can do. I'll send an email
their
way and see if they want to join in.

>From my perspective, the next thing that needs to happen to the driver is
for it to be broken up into ethernet and mdio drivers, so that we can
get rid
of all this shared_smi craziness... But that's for another patch series.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
  2012-08-07 15:56       ` Ian Molton
  (?)
@ 2012-08-07 20:25           ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-07 20:25 UTC (permalink / raw)
  To: Ian Molton, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Dale Farnsworth
  Cc: thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	andrew-g2DYL2Zd6BY, ben.dooks-4yDnlxn2s6sWdaTGBSpHTA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA

On Tuesday 07 August 2012, Ian Molton wrote:
> > I think it documents some of the same thing,
> 
> Not really. It documents some godawful hack that recycled the platform
> device -based driver and provided a DT binding for it, just for PPC.
> 
> I cant even find anything that implements code for whatever
> "marvell,mv64360-mdio" might be. I'm sure it might exist somewhere.

The code dates back to when we had separate buses for platform devices
and of devices, and then it was decided not to add support for both
bus types to each of the marvell drivers. In hindsight it would have
been better to do that, but that was impossible to tell back then.

> > We might also want to move some of the
> > code from arch/powerpc/sysdev/mv64x60_dev.c to live in the same place
> > as the device driver.
> 
> I hope not. I don't really want to touch that stuff at all. If it works
> the way it
> is, then it can stay that way. If the PPC folk want to send patches to
> add the
> properties they use to the driver, then they can do. I'll send an email
> their
> way and see if they want to join in.
> 
> From my perspective, the next thing that needs to happen to the driver is
> for it to be broken up into ethernet and mdio drivers, so that we can
> get rid
> of all this shared_smi craziness... But that's for another patch series.

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

I would hope that at least some of the properties that are used on
powerpc can be reused in the same way for other architectures.
The method to find the phy address on powerpc does indeed make
more sense than the "port_number" property you suggested, and the
phandle for the phy node is usually called "phy" not "mdio".

I'm not sure if the ethernet-group is required on ARM as well, but
it does sound a lot like what you actually want instead of the
shared_smi property.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 20:25           ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-07 20:25 UTC (permalink / raw)
  To: Ian Molton, linuxppc-dev, devicetree-discuss, Dale Farnsworth
  Cc: thomas.petazzoni, andrew, ben.dooks, linux-arm-kernel, netdev

On Tuesday 07 August 2012, Ian Molton wrote:
> > I think it documents some of the same thing,
> 
> Not really. It documents some godawful hack that recycled the platform
> device -based driver and provided a DT binding for it, just for PPC.
> 
> I cant even find anything that implements code for whatever
> "marvell,mv64360-mdio" might be. I'm sure it might exist somewhere.

The code dates back to when we had separate buses for platform devices
and of devices, and then it was decided not to add support for both
bus types to each of the marvell drivers. In hindsight it would have
been better to do that, but that was impossible to tell back then.

> > We might also want to move some of the
> > code from arch/powerpc/sysdev/mv64x60_dev.c to live in the same place
> > as the device driver.
> 
> I hope not. I don't really want to touch that stuff at all. If it works
> the way it
> is, then it can stay that way. If the PPC folk want to send patches to
> add the
> properties they use to the driver, then they can do. I'll send an email
> their
> way and see if they want to join in.
> 
> From my perspective, the next thing that needs to happen to the driver is
> for it to be broken up into ethernet and mdio drivers, so that we can
> get rid
> of all this shared_smi craziness... But that's for another patch series.

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

I would hope that at least some of the properties that are used on
powerpc can be reused in the same way for other architectures.
The method to find the phy address on powerpc does indeed make
more sense than the "port_number" property you suggested, and the
phandle for the phy node is usually called "phy" not "mdio".

I'm not sure if the ethernet-group is required on ARM as well, but
it does sound a lot like what you actually want instead of the
shared_smi property.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 3/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 20:25           ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-07 20:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 07 August 2012, Ian Molton wrote:
> > I think it documents some of the same thing,
> 
> Not really. It documents some godawful hack that recycled the platform
> device -based driver and provided a DT binding for it, just for PPC.
> 
> I cant even find anything that implements code for whatever
> "marvell,mv64360-mdio" might be. I'm sure it might exist somewhere.

The code dates back to when we had separate buses for platform devices
and of devices, and then it was decided not to add support for both
bus types to each of the marvell drivers. In hindsight it would have
been better to do that, but that was impossible to tell back then.

> > We might also want to move some of the
> > code from arch/powerpc/sysdev/mv64x60_dev.c to live in the same place
> > as the device driver.
> 
> I hope not. I don't really want to touch that stuff at all. If it works
> the way it
> is, then it can stay that way. If the PPC folk want to send patches to
> add the
> properties they use to the driver, then they can do. I'll send an email
> their
> way and see if they want to join in.
> 
> From my perspective, the next thing that needs to happen to the driver is
> for it to be broken up into ethernet and mdio drivers, so that we can
> get rid
> of all this shared_smi craziness... But that's for another patch series.

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

I would hope that at least some of the properties that are used on
powerpc can be reused in the same way for other architectures.
The method to find the phy address on powerpc does indeed make
more sense than the "port_number" property you suggested, and the
phandle for the phy node is usually called "phy" not "mdio".

I'm not sure if the ethernet-group is required on ARM as well, but
it does sound a lot like what you actually want instead of the
shared_smi property.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-07 14:34 ` Ian Molton
@ 2012-08-07 23:29   ` David Miller
  -1 siblings, 0 replies; 71+ messages in thread
From: David Miller @ 2012-08-07 23:29 UTC (permalink / raw)
  To: ian.molton
  Cc: linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks, arnd, netdev

From: Ian Molton <ian.molton@codethink.co.uk>
Date: Tue,  7 Aug 2012 15:34:45 +0100

> Fixed all comments.
> 
> * Dropped csb1724 defconfig.
> * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
> * Dropped un-necessary D-T irq fixup code

Who is going to take this series?

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-07 23:29   ` David Miller
  0 siblings, 0 replies; 71+ messages in thread
From: David Miller @ 2012-08-07 23:29 UTC (permalink / raw)
  To: linux-arm-kernel

From: Ian Molton <ian.molton@codethink.co.uk>
Date: Tue,  7 Aug 2012 15:34:45 +0100

> Fixed all comments.
> 
> * Dropped csb1724 defconfig.
> * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
> * Dropped un-necessary D-T irq fixup code

Who is going to take this series?

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-07 23:29   ` David Miller
@ 2012-08-08  0:31     ` Matt Sealey
  -1 siblings, 0 replies; 71+ messages in thread
From: Matt Sealey @ 2012-08-08  0:31 UTC (permalink / raw)
  To: David Miller
  Cc: ian.molton, thomas.petazzoni, andrew, arnd, netdev, ben.dooks,
	linux-arm-kernel

On Tue, Aug 7, 2012 at 6:29 PM, David Miller <davem@davemloft.net> wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> Date: Tue,  7 Aug 2012 15:34:45 +0100
>
>> Fixed all comments.
>>
>> * Dropped csb1724 defconfig.
>> * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
>> * Dropped un-necessary D-T irq fixup code
>
> Who is going to take this series?

Would anyone mind too much if I *didn't* break out a Pegasos II and
test it? Our platform has a Marvell northbridge (Discovery II)
implementing this, with a Marvell PHY, and it's OpenFirmware (as in,
REAL OpenFirmware) so the device tree isn't about to change to fit new
bindings. But I'm not sure we even have one in the office that boots
anymore.. there may be users out there but they're well beyond
warranty support (early 2005 or so was the last time we sold one).

If anyone needs the original device tree entries to compare and
contrast I may be able to provide them such that any parsing and
initializing of the driver take into account this old
board/northbridge/implementation. I'm just curious if anyone cares
enough..

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08  0:31     ` Matt Sealey
  0 siblings, 0 replies; 71+ messages in thread
From: Matt Sealey @ 2012-08-08  0:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 7, 2012 at 6:29 PM, David Miller <davem@davemloft.net> wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> Date: Tue,  7 Aug 2012 15:34:45 +0100
>
>> Fixed all comments.
>>
>> * Dropped csb1724 defconfig.
>> * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
>> * Dropped un-necessary D-T irq fixup code
>
> Who is going to take this series?

Would anyone mind too much if I *didn't* break out a Pegasos II and
test it? Our platform has a Marvell northbridge (Discovery II)
implementing this, with a Marvell PHY, and it's OpenFirmware (as in,
REAL OpenFirmware) so the device tree isn't about to change to fit new
bindings. But I'm not sure we even have one in the office that boots
anymore.. there may be users out there but they're well beyond
warranty support (early 2005 or so was the last time we sold one).

If anyone needs the original device tree entries to compare and
contrast I may be able to provide them such that any parsing and
initializing of the driver take into account this old
board/northbridge/implementation. I'm just curious if anyone cares
enough..

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-07 23:29   ` David Miller
@ 2012-08-08  8:16     ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-08  8:16 UTC (permalink / raw)
  To: David Miller
  Cc: ian.molton, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

On Tuesday 07 August 2012, David Miller wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> Date: Tue,  7 Aug 2012 15:34:45 +0100
> 
> > Fixed all comments.
> > 
> > * Dropped csb1724 defconfig.
> > * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
> > * Dropped un-necessary D-T irq fixup code
> 
> Who is going to take this series?

I'd prefer to take the entire series through the arm-soc tree from
the kirkwood maintainers. We first have to work out the bindings
though, since the current patch introduces a new one that is
incompatible with the one we were using on powerpc with
open firmware before.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08  8:16     ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-08  8:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 07 August 2012, David Miller wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> Date: Tue,  7 Aug 2012 15:34:45 +0100
> 
> > Fixed all comments.
> > 
> > * Dropped csb1724 defconfig.
> > * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
> > * Dropped un-necessary D-T irq fixup code
> 
> Who is going to take this series?

I'd prefer to take the entire series through the arm-soc tree from
the kirkwood maintainers. We first have to work out the bindings
though, since the current patch introduces a new one that is
incompatible with the one we were using on powerpc with
open firmware before.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08  8:16     ` Arnd Bergmann
@ 2012-08-08  8:59       ` David Miller
  -1 siblings, 0 replies; 71+ messages in thread
From: David Miller @ 2012-08-08  8:59 UTC (permalink / raw)
  To: arnd
  Cc: ian.molton, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Aug 2012 08:16:28 +0000

> On Tuesday 07 August 2012, David Miller wrote:
>> From: Ian Molton <ian.molton@codethink.co.uk>
>> Date: Tue,  7 Aug 2012 15:34:45 +0100
>> 
>> > Fixed all comments.
>> > 
>> > * Dropped csb1724 defconfig.
>> > * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
>> > * Dropped un-necessary D-T irq fixup code
>> 
>> Who is going to take this series?
> 
> I'd prefer to take the entire series through the arm-soc tree from
> the kirkwood maintainers. We first have to work out the bindings
> though, since the current patch introduces a new one that is
> incompatible with the one we were using on powerpc with
> open firmware before.

Ok.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08  8:59       ` David Miller
  0 siblings, 0 replies; 71+ messages in thread
From: David Miller @ 2012-08-08  8:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Aug 2012 08:16:28 +0000

> On Tuesday 07 August 2012, David Miller wrote:
>> From: Ian Molton <ian.molton@codethink.co.uk>
>> Date: Tue,  7 Aug 2012 15:34:45 +0100
>> 
>> > Fixed all comments.
>> > 
>> > * Dropped csb1724 defconfig.
>> > * Added patch to remove MV643XX_ETH_SHARED_NAME and MV643XX_ETH_NAME
>> > * Dropped un-necessary D-T irq fixup code
>> 
>> Who is going to take this series?
> 
> I'd prefer to take the entire series through the arm-soc tree from
> the kirkwood maintainers. We first have to work out the bindings
> though, since the current patch introduces a new one that is
> incompatible with the one we were using on powerpc with
> open firmware before.

Ok.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08  8:16     ` Arnd Bergmann
@ 2012-08-08  9:40       ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08  9:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

On 08/08/12 09:16, Arnd Bergmann wrote:

> I'd prefer to take the entire series through the arm-soc tree from
> the kirkwood maintainers. We first have to work out the bindings
> though, since the current patch introduces a new one that is
> incompatible with the one we were using on powerpc with open firmware
> before.

Looking at the ethernet-group stuff, specifically from
arch/powerpc/boot/dts/prpmc2800.dts, which I've taken as a base for the
below:

I think we can (and should) do something similar.

Sadly, there is no code present to describe marvell,mv64360-mdio,
however the device tree looks basically sane.



        mdio@2000 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        device_type = "mdio";
                        compatible = "marvell,mv643xx-mdio";
                        PHY0: ethernet-phy@1 {
                                device_type = "ethernet-phy";
                                compatible = "broadcom,bcm5421";
                                interrupts = <76>;      /* GPP 12 */
                                interrupt-parent = <&PIC>;
                                reg = <1>;
                        };
                        PHY1: ethernet-phy@3 {
                                device_type = "ethernet-phy";
                                compatible = "broadcom,bcm5421";
                                interrupts = <76>;      /* GPP 12 */
                                interrupt-parent = <&PIC>;
                                reg = <3>;
                        };
                };

                ethernet-group@2400 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "marvell,mv64360-eth-group";
                        reg = <0x2400 0x2000>;
                        ethernet@0 {
                                device_type = "network";
                                compatible = "marvell,mv64360-eth";
                                reg = <0>;
                                interrupts = <32>;
                                interrupt-parent = <&mpic>;
                                phy = <&phy0>;
                                local-mac-address = [ 00 00 00 00 00 00 ];
                        };
                        ethernet@1 {
                                device_type = "network";
                                compatible = "marvell,mv64360-eth";
                                reg = <1>;
                                interrupts = <33>;
                                interrupt-parent = <&mpic>;
                                phy = <&phy1>;
                                local-mac-address = [ 00 00 00 00 00 00 ];
                        };
                };

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08  9:40       ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/08/12 09:16, Arnd Bergmann wrote:

> I'd prefer to take the entire series through the arm-soc tree from
> the kirkwood maintainers. We first have to work out the bindings
> though, since the current patch introduces a new one that is
> incompatible with the one we were using on powerpc with open firmware
> before.

Looking at the ethernet-group stuff, specifically from
arch/powerpc/boot/dts/prpmc2800.dts, which I've taken as a base for the
below:

I think we can (and should) do something similar.

Sadly, there is no code present to describe marvell,mv64360-mdio,
however the device tree looks basically sane.



        mdio at 2000 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        device_type = "mdio";
                        compatible = "marvell,mv643xx-mdio";
                        PHY0: ethernet-phy at 1 {
                                device_type = "ethernet-phy";
                                compatible = "broadcom,bcm5421";
                                interrupts = <76>;      /* GPP 12 */
                                interrupt-parent = <&PIC>;
                                reg = <1>;
                        };
                        PHY1: ethernet-phy at 3 {
                                device_type = "ethernet-phy";
                                compatible = "broadcom,bcm5421";
                                interrupts = <76>;      /* GPP 12 */
                                interrupt-parent = <&PIC>;
                                reg = <3>;
                        };
                };

                ethernet-group at 2400 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "marvell,mv64360-eth-group";
                        reg = <0x2400 0x2000>;
                        ethernet at 0 {
                                device_type = "network";
                                compatible = "marvell,mv64360-eth";
                                reg = <0>;
                                interrupts = <32>;
                                interrupt-parent = <&mpic>;
                                phy = <&phy0>;
                                local-mac-address = [ 00 00 00 00 00 00 ];
                        };
                        ethernet at 1 {
                                device_type = "network";
                                compatible = "marvell,mv64360-eth";
                                reg = <1>;
                                interrupts = <33>;
                                interrupt-parent = <&mpic>;
                                phy = <&phy1>;
                                local-mac-address = [ 00 00 00 00 00 00 ];
                        };
                };

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08  9:40       ` Ian Molton
@ 2012-08-08  9:42         ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08  9:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

Ignore, Hit send whilst editing, by mistake. Sorry for the noise guys.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08  9:42         ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08  9:42 UTC (permalink / raw)
  To: linux-arm-kernel

Ignore, Hit send whilst editing, by mistake. Sorry for the noise guys.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08  9:40       ` Ian Molton
@ 2012-08-08 11:51         ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08 11:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

On 08/08/12 10:40, Ian Molton wrote:
> On 08/08/12 09:16, Arnd Bergmann wrote:
>
>> I'd prefer to take the entire series through the arm-soc tree from
>> the kirkwood maintainers. We first have to work out the bindings
>> though, since the current patch introduces a new one that is
>> incompatible with the one we were using on powerpc with open
>> firmware before.

Looking at the ethernet-group stuff, specifically from
arch/powerpc/boot/dts/prpmc2800.dts, which I've taken as a base for
the below:


The SMI / PHY stuff should look very similar, so I'm happy with something
like:

mdio@2000 {
                #address-cells = <1>;
                #size-cells = <1>;
                device_type = "mdio";
                compatible = "marvell,mv643xx-mdio";
                phy0: ethernet-phy@0 {
                        device_type = "ethernet-phy";
                        compatible = "marvell,whatever";
                        interrupts = <76>;
                        interrupt-parent = <&mpic>;
                        reg = <0 32>;          // Auto probed phy addr
                };

                phy1: ethernet-phy@3 {
                        device_type = "ethernet-phy";
                        compatible = "marvell,whatever";
                        interrupts = <77>;
                        interrupt-parent = <&mpic>;
                        reg = <3 1>;            // specified phy addr
                };

                ... and so on.
}

Where we can use the reg parameter to allow auto-probing, by
specifying a size of 32 (32 phy addrs max).


The ethernet driver itself is more complicated:

We have the following considerations:

* we have one MDIO bus, typically, shared between all the MACs / PHYs.
* each ethernet device can multiple ports (up to three), each with its
  own MAC/PHY.
* MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
  mix of the two.
* existing D-T users, albeit not well documented / code complete.
* some port address ranges overlap (MIB counters, MCAST / UNICAST
  tables, etc.

The existing ethernet-group idea only works because the current
platform-device based driver doesnt really do proper resource
management, and thus the MAC registers are actually mapped by
the MDIO driver.

I don't think that preserving this bad behaviour is a good idea, which
leaves us with two choices:

1) My preferred solution - allow each device to specify up to three
interrupts, MACs, and PHYs. This is clean in that it doesnt require
multiply instantiating a driver three times over the same address
space.

ethernet@2400 {
                compatible = "marvell,mv643xx-eth";
                reg = <0x2400 0x1c00>
                interrupt_parent = <&mpic>;
                ports = <3>;
                interrupts = <4>, <5>, <6>;
                phys = <&phy0>, <&phy1>, <&phy2>;
};

ethernet@6400 {
                compatible = "marvell,mv643xx-eth";
                reg = <0x6400 0x1c00>
                interrupt_parent = <&mpic>;
                ports = <1>;
                interrupts = <4>;
                phys = <&phy3>;
};

Note that the address is 2400, not 2000 - since this driver no longer
would share its address range with the MDIO driver.

This method would require a small amount of rework in the driver to
set up <n> ports, rather than just one.

2) Create some kind of pseudo-ethernet group device that manages
all the work for some sort of lightweight ethernet device, one per
port. This can never be done cleanly since the port address ranges
overlap:

pseudo_eth@2400 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "marvell,mv643xx-shared-eth"
        reg = <0x2400 0x1c00>;

        ethernet@0 {
                compatible = "marvell,mv643xx-port";
                interrupts = <4>;
                interrupt_parent = <&mpic>;
                phy = <&phy0>;
        };

        ethernet@1 {
                compatible = "marvell,mv643xx-port";
                interrupts = <5>;
                interrupt_parent = <&mpic>;
                phy = <&phy1>;
        };

        ethernet@2 {
                compatible = "marvell,mv643xx-port";
                interrupts = <6>;
                interrupt_parent = <&mpic>;
                phy = <&phy2>;
        };
}
pseudo_eth@6400 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "marvell,mv643xx-shared-eth"
        reg = <0x6400 0x1c00>;

        ethernet@0 {
                compatible = "marvell,mv643xx-port";
                interrupts = <4>;
                interrupt_parent = <&mpic>;
                phy = <&phy3>;
        };
};

Thoughts?

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08 11:51         ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/08/12 10:40, Ian Molton wrote:
> On 08/08/12 09:16, Arnd Bergmann wrote:
>
>> I'd prefer to take the entire series through the arm-soc tree from
>> the kirkwood maintainers. We first have to work out the bindings
>> though, since the current patch introduces a new one that is
>> incompatible with the one we were using on powerpc with open
>> firmware before.

Looking at the ethernet-group stuff, specifically from
arch/powerpc/boot/dts/prpmc2800.dts, which I've taken as a base for
the below:


The SMI / PHY stuff should look very similar, so I'm happy with something
like:

mdio at 2000 {
                #address-cells = <1>;
                #size-cells = <1>;
                device_type = "mdio";
                compatible = "marvell,mv643xx-mdio";
                phy0: ethernet-phy at 0 {
                        device_type = "ethernet-phy";
                        compatible = "marvell,whatever";
                        interrupts = <76>;
                        interrupt-parent = <&mpic>;
                        reg = <0 32>;          // Auto probed phy addr
                };

                phy1: ethernet-phy at 3 {
                        device_type = "ethernet-phy";
                        compatible = "marvell,whatever";
                        interrupts = <77>;
                        interrupt-parent = <&mpic>;
                        reg = <3 1>;            // specified phy addr
                };

                ... and so on.
}

Where we can use the reg parameter to allow auto-probing, by
specifying a size of 32 (32 phy addrs max).


The ethernet driver itself is more complicated:

We have the following considerations:

* we have one MDIO bus, typically, shared between all the MACs / PHYs.
* each ethernet device can multiple ports (up to three), each with its
  own MAC/PHY.
* MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
  mix of the two.
* existing D-T users, albeit not well documented / code complete.
* some port address ranges overlap (MIB counters, MCAST / UNICAST
  tables, etc.

The existing ethernet-group idea only works because the current
platform-device based driver doesnt really do proper resource
management, and thus the MAC registers are actually mapped by
the MDIO driver.

I don't think that preserving this bad behaviour is a good idea, which
leaves us with two choices:

1) My preferred solution - allow each device to specify up to three
interrupts, MACs, and PHYs. This is clean in that it doesnt require
multiply instantiating a driver three times over the same address
space.

ethernet at 2400 {
                compatible = "marvell,mv643xx-eth";
                reg = <0x2400 0x1c00>
                interrupt_parent = <&mpic>;
                ports = <3>;
                interrupts = <4>, <5>, <6>;
                phys = <&phy0>, <&phy1>, <&phy2>;
};

ethernet at 6400 {
                compatible = "marvell,mv643xx-eth";
                reg = <0x6400 0x1c00>
                interrupt_parent = <&mpic>;
                ports = <1>;
                interrupts = <4>;
                phys = <&phy3>;
};

Note that the address is 2400, not 2000 - since this driver no longer
would share its address range with the MDIO driver.

This method would require a small amount of rework in the driver to
set up <n> ports, rather than just one.

2) Create some kind of pseudo-ethernet group device that manages
all the work for some sort of lightweight ethernet device, one per
port. This can never be done cleanly since the port address ranges
overlap:

pseudo_eth at 2400 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "marvell,mv643xx-shared-eth"
        reg = <0x2400 0x1c00>;

        ethernet at 0 {
                compatible = "marvell,mv643xx-port";
                interrupts = <4>;
                interrupt_parent = <&mpic>;
                phy = <&phy0>;
        };

        ethernet at 1 {
                compatible = "marvell,mv643xx-port";
                interrupts = <5>;
                interrupt_parent = <&mpic>;
                phy = <&phy1>;
        };

        ethernet at 2 {
                compatible = "marvell,mv643xx-port";
                interrupts = <6>;
                interrupt_parent = <&mpic>;
                phy = <&phy2>;
        };
}
pseudo_eth at 6400 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "marvell,mv643xx-shared-eth"
        reg = <0x6400 0x1c00>;

        ethernet at 0 {
                compatible = "marvell,mv643xx-port";
                interrupts = <4>;
                interrupt_parent = <&mpic>;
                phy = <&phy3>;
        };
};

Thoughts?

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08 11:51         ` Ian Molton
@ 2012-08-08 12:39           ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-08 12:39 UTC (permalink / raw)
  To: Ian Molton
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

On Wednesday 08 August 2012, Ian Molton wrote:
> The SMI / PHY stuff should look very similar, so I'm happy with something
> like:
> 
> mdio@2000 {
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 device_type = "mdio";
>                 compatible = "marvell,mv643xx-mdio";
>                 phy0: ethernet-phy@0 {
>                         device_type = "ethernet-phy";
>                         compatible = "marvell,whatever";
>                         interrupts = <76>;
>                         interrupt-parent = <&mpic>;
>                         reg = <0 32>;          // Auto probed phy addr
>                 };
> 
>                 phy1: ethernet-phy@3 {
>                         device_type = "ethernet-phy";
>                         compatible = "marvell,whatever";
>                         interrupts = <77>;
>                         interrupt-parent = <&mpic>;
>                         reg = <3 1>;            // specified phy addr
>                 };
> 
>                 ... and so on.
> }
> 
> Where we can use the reg parameter to allow auto-probing, by
> specifying a size of 32 (32 phy addrs max).

I don't understand the auto-probed phy address. What is the purpose of that?

If possible, I think we should keep using #size-cells=<0>, which would
make the method you describe impossible. It might still work if you just
leave out the "reg" property for that node.

I also don't understand how the phy driver would locate ethernet-phy@0
on the bus if it does not know the address.

> The ethernet driver itself is more complicated:
> 
> We have the following considerations:
> 
> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
> * each ethernet device can multiple ports (up to three), each with its
>   own MAC/PHY.
> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>   mix of the two.
> * existing D-T users, albeit not well documented / code complete.
> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>   tables, etc.
> 
> The existing ethernet-group idea only works because the current
> platform-device based driver doesnt really do proper resource
> management, and thus the MAC registers are actually mapped by
> the MDIO driver.
> 
> I don't think that preserving this bad behaviour is a good idea, which
> leaves us with two choices:
> 
> 1) My preferred solution - allow each device to specify up to three
> interrupts, MACs, and PHYs. This is clean in that it doesnt require
> multiply instantiating a driver three times over the same address
> space.
> 
> ethernet@2400 {
>                 compatible = "marvell,mv643xx-eth";
>                 reg = <0x2400 0x1c00>
>                 interrupt_parent = <&mpic>;
>                 ports = <3>;
>                 interrupts = <4>, <5>, <6>;
>                 phys = <&phy0>, <&phy1>, <&phy2>;
> };
> 
> ethernet@6400 {
>                 compatible = "marvell,mv643xx-eth";
>                 reg = <0x6400 0x1c00>
>                 interrupt_parent = <&mpic>;
>                 ports = <1>;
>                 interrupts = <4>;
>                 phys = <&phy3>;
> };
> 
> Note that the address is 2400, not 2000 - since this driver no longer
> would share its address range with the MDIO driver.
> 
> This method would require a small amount of rework in the driver to
> set up <n> ports, rather than just one.

This looks quite nice, but it is still very much incompatible with the
existing binding. Obviously we can abandon an existing binding and
introduce a second one for the same hardware, but that should not
be taken lightly.

> 2) Create some kind of pseudo-ethernet group device that manages
> all the work for some sort of lightweight ethernet device, one per
> port. This can never be done cleanly since the port address ranges
> overlap:
> 
> pseudo_eth@2400 {
>         #address-cells = <1>;
>         #size-cells = <0>;
>         compatible = "marvell,mv643xx-shared-eth"
>         reg = <0x2400 0x1c00>;
> 
>         ethernet@0 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <4>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy0>;
>         };
> 
>         ethernet@1 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <5>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy1>;
>         };
> 
>         ethernet@2 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <6>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy2>;
>         };
> }
> pseudo_eth@6400 {
>         #address-cells = <1>;
>         #size-cells = <0>;
>         compatible = "marvell,mv643xx-shared-eth"
>         reg = <0x6400 0x1c00>;
> 
>         ethernet@0 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <4>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy3>;
>         };
> };

This looks almost compatible with the existing binding, which is
good. I would in fact recommend to use the actual "compatible"
strings from the binding. More generally speaking, you should not
use wildcards in those strings anyway, so always use
"marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
"marvell,mv643xx-eth". If you have multiple chips that are
completely compatible, put use the identifier for the older one.

I don't fully understand your concern with the overlapping
registers, mostly because I still don't know all the combinations
that are actually valid here. Let me try to say what I understood
so far, and you can correct me if that's wrong:

* A system can have multiple instances of an mv64360 ethernet
block, with a register area of 0x2000 bytes.
* Each such block can have three MACs and three PHYs.
* The first 0x400 bytes in the register space control the three
  PHYs and the remaining registers control the MACs.
* While this is meant to be used in a way that you assign
  the each of the three PHYs to one of the MACs, this is not
  always done, and sometimes you use a different PHY (?), or
  one from a different instance of the mv64360 ethernet block
  on the same SoC?.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08 12:39           ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-08 12:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 08 August 2012, Ian Molton wrote:
> The SMI / PHY stuff should look very similar, so I'm happy with something
> like:
> 
> mdio at 2000 {
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 device_type = "mdio";
>                 compatible = "marvell,mv643xx-mdio";
>                 phy0: ethernet-phy at 0 {
>                         device_type = "ethernet-phy";
>                         compatible = "marvell,whatever";
>                         interrupts = <76>;
>                         interrupt-parent = <&mpic>;
>                         reg = <0 32>;          // Auto probed phy addr
>                 };
> 
>                 phy1: ethernet-phy at 3 {
>                         device_type = "ethernet-phy";
>                         compatible = "marvell,whatever";
>                         interrupts = <77>;
>                         interrupt-parent = <&mpic>;
>                         reg = <3 1>;            // specified phy addr
>                 };
> 
>                 ... and so on.
> }
> 
> Where we can use the reg parameter to allow auto-probing, by
> specifying a size of 32 (32 phy addrs max).

I don't understand the auto-probed phy address. What is the purpose of that?

If possible, I think we should keep using #size-cells=<0>, which would
make the method you describe impossible. It might still work if you just
leave out the "reg" property for that node.

I also don't understand how the phy driver would locate ethernet-phy at 0
on the bus if it does not know the address.

> The ethernet driver itself is more complicated:
> 
> We have the following considerations:
> 
> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
> * each ethernet device can multiple ports (up to three), each with its
>   own MAC/PHY.
> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>   mix of the two.
> * existing D-T users, albeit not well documented / code complete.
> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>   tables, etc.
> 
> The existing ethernet-group idea only works because the current
> platform-device based driver doesnt really do proper resource
> management, and thus the MAC registers are actually mapped by
> the MDIO driver.
> 
> I don't think that preserving this bad behaviour is a good idea, which
> leaves us with two choices:
> 
> 1) My preferred solution - allow each device to specify up to three
> interrupts, MACs, and PHYs. This is clean in that it doesnt require
> multiply instantiating a driver three times over the same address
> space.
> 
> ethernet at 2400 {
>                 compatible = "marvell,mv643xx-eth";
>                 reg = <0x2400 0x1c00>
>                 interrupt_parent = <&mpic>;
>                 ports = <3>;
>                 interrupts = <4>, <5>, <6>;
>                 phys = <&phy0>, <&phy1>, <&phy2>;
> };
> 
> ethernet at 6400 {
>                 compatible = "marvell,mv643xx-eth";
>                 reg = <0x6400 0x1c00>
>                 interrupt_parent = <&mpic>;
>                 ports = <1>;
>                 interrupts = <4>;
>                 phys = <&phy3>;
> };
> 
> Note that the address is 2400, not 2000 - since this driver no longer
> would share its address range with the MDIO driver.
> 
> This method would require a small amount of rework in the driver to
> set up <n> ports, rather than just one.

This looks quite nice, but it is still very much incompatible with the
existing binding. Obviously we can abandon an existing binding and
introduce a second one for the same hardware, but that should not
be taken lightly.

> 2) Create some kind of pseudo-ethernet group device that manages
> all the work for some sort of lightweight ethernet device, one per
> port. This can never be done cleanly since the port address ranges
> overlap:
> 
> pseudo_eth at 2400 {
>         #address-cells = <1>;
>         #size-cells = <0>;
>         compatible = "marvell,mv643xx-shared-eth"
>         reg = <0x2400 0x1c00>;
> 
>         ethernet at 0 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <4>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy0>;
>         };
> 
>         ethernet at 1 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <5>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy1>;
>         };
> 
>         ethernet at 2 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <6>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy2>;
>         };
> }
> pseudo_eth at 6400 {
>         #address-cells = <1>;
>         #size-cells = <0>;
>         compatible = "marvell,mv643xx-shared-eth"
>         reg = <0x6400 0x1c00>;
> 
>         ethernet at 0 {
>                 compatible = "marvell,mv643xx-port";
>                 interrupts = <4>;
>                 interrupt_parent = <&mpic>;
>                 phy = <&phy3>;
>         };
> };

This looks almost compatible with the existing binding, which is
good. I would in fact recommend to use the actual "compatible"
strings from the binding. More generally speaking, you should not
use wildcards in those strings anyway, so always use
"marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
"marvell,mv643xx-eth". If you have multiple chips that are
completely compatible, put use the identifier for the older one.

I don't fully understand your concern with the overlapping
registers, mostly because I still don't know all the combinations
that are actually valid here. Let me try to say what I understood
so far, and you can correct me if that's wrong:

* A system can have multiple instances of an mv64360 ethernet
block, with a register area of 0x2000 bytes.
* Each such block can have three MACs and three PHYs.
* The first 0x400 bytes in the register space control the three
  PHYs and the remaining registers control the MACs.
* While this is meant to be used in a way that you assign
  the each of the three PHYs to one of the MACs, this is not
  always done, and sometimes you use a different PHY (?), or
  one from a different instance of the mv64360 ethernet block
  on the same SoC?.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08 12:39           ` Arnd Bergmann
@ 2012-08-08 13:19             ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08 13:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev

On 08/08/12 13:39, Arnd Bergmann wrote:
> On Wednesday 08 August 2012, Ian Molton wrote:
>> The SMI / PHY stuff should look very similar, so I'm happy with something
>> like:
>>
>> mdio@2000 {
>>                 #address-cells = <1>;
>>                 #size-cells = <1>;
>>                 device_type = "mdio";
>>                 compatible = "marvell,mv643xx-mdio";
>>                 phy0: ethernet-phy@0 {
>>                         device_type = "ethernet-phy";
>>                         compatible = "marvell,whatever";
>>                         interrupts = <76>;
>>                         interrupt-parent = <&mpic>;
>>                         reg = <0 32>;          // Auto probed phy addr
>>                 };
>>
>>                 phy1: ethernet-phy@3 {
>>                         device_type = "ethernet-phy";
>>                         compatible = "marvell,whatever";
>>                         interrupts = <77>;
>>                         interrupt-parent = <&mpic>;
>>                         reg = <3 1>;            // specified phy addr
>>                 };
>>
>>                 ... and so on.
>> }
>>
>> Where we can use the reg parameter to allow auto-probing, by
>> specifying a size of 32 (32 phy addrs max).
> I don't understand the auto-probed phy address. What is the purpose of that?

Personally, I think it should die - but the existing driver and a number
of its users actually scan the bus for their PHY.

I doubt the PHY really moves about or is hotplugged by any of them,
and its actually quite a slow process.

> If possible, I think we should keep using #size-cells=<0>, which would
> make the method you describe impossible. It might still work if you just
> leave out the "reg" property for that node.

I can certainly investigate that. I couldn't see any good evidence that
it was a supported mechanism when I looked.

> I also don't understand how the phy driver would locate ethernet-phy@0
> on the bus if it does not know the address.
>
>> The ethernet driver itself is more complicated:
>>
>> We have the following considerations:
>>
>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>> * each ethernet device can multiple ports (up to three), each with its
>>   own MAC/PHY.
>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>>   mix of the two.
>> * existing D-T users, albeit not well documented / code complete.
>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>>   tables, etc.
>>
>> The existing ethernet-group idea only works because the current
>> platform-device based driver doesnt really do proper resource
>> management, and thus the MAC registers are actually mapped by
>> the MDIO driver.
>>
>> I don't think that preserving this bad behaviour is a good idea, which
>> leaves us with two choices:
>>
>> 1) My preferred solution - allow each device to specify up to three
>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>> multiply instantiating a driver three times over the same address
>> space.
>>
>> ethernet@2400 {
>>                 compatible = "marvell,mv643xx-eth";
>>                 reg = <0x2400 0x1c00>
>>                 interrupt_parent = <&mpic>;
>>                 ports = <3>;
>>                 interrupts = <4>, <5>, <6>;
>>                 phys = <&phy0>, <&phy1>, <&phy2>;
>> };
>>
>> ethernet@6400 {
>>                 compatible = "marvell,mv643xx-eth";
>>                 reg = <0x6400 0x1c00>
>>                 interrupt_parent = <&mpic>;
>>                 ports = <1>;
>>                 interrupts = <4>;
>>                 phys = <&phy3>;
>> };
>>
>> Note that the address is 2400, not 2000 - since this driver no longer
>> would share its address range with the MDIO driver.
>>
>> This method would require a small amount of rework in the driver to
>> set up <n> ports, rather than just one.
> This looks quite nice, but it is still very much incompatible with the
> existing binding. Obviously we can abandon an existing binding and
> introduce a second one for the same hardware, but that should not
> be taken lightly.

Fair, however the existing users aren't anywhere near as
numerous as the new ones.

>> 2) Create some kind of pseudo-ethernet group device that manages
>> all the work for some sort of lightweight ethernet device, one per
>> port. This can never be done cleanly since the port address ranges
>> overlap:
>>
>> pseudo_eth@2400 {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>         compatible = "marvell,mv643xx-shared-eth"
>>         reg = <0x2400 0x1c00>;
>>
>>         ethernet@0 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <4>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy0>;
>>         };
>>
>>         ethernet@1 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <5>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy1>;
>>         };
>>
>>         ethernet@2 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <6>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy2>;
>>         };
>> }
>> pseudo_eth@6400 {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>         compatible = "marvell,mv643xx-shared-eth"
>>         reg = <0x6400 0x1c00>;
>>
>>         ethernet@0 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <4>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy3>;
>>         };
>> };
> This looks almost compatible with the existing binding, which is
> good.

Well, I'm not sure about that - if the existing bindings are really
baked into firmware, then "almost" wont be any use at all.

>  I would in fact recommend to use the actual "compatible"
> strings from the binding. More generally speaking, you should not
> use wildcards in those strings anyway, so always use
> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
> "marvell,mv643xx-eth". If you have multiple chips that are
> completely compatible, put use the identifier for the older one.
Noted.

> I don't fully understand your concern with the overlapping
> registers, mostly because I still don't know all the combinations
> that are actually valid here. Let me try to say what I understood
> so far, and you can correct me if that's wrong:
>
> * A system can have multiple instances of an mv64360 ethernet
> block, with a register area of 0x2000 bytes.
> * Each such block can have three MACs and three PHYs.
> * The first 0x400 bytes in the register space control the three
>   PHYs and the remaining registers control the MACs.
> * While this is meant to be used in a way that you assign
>   the each of the three PHYs to one of the MACs, this is not
>   always done, and sometimes you use a different PHY (?), or
>   one from a different instance of the mv64360 ethernet block
>   on the same SoC?.

Nearly - the whole block is 0x2000 in size, yes. And each one
can have 3 MACs and PHYs, as you say.

There is SMI @ 0x2000 - just one for all ports, and in many
(all?) cases, for all all the other controllers on the SoC to
share. On the armadaXP SoC, for example, each ethernet
block has its own alias of the same bas SMI reg. (there are
4 blocks)

ethernet0@ 0x2400
## regs in order: Main regs, MIB counters, Special mcast table, Mcast
table, Unicast table.
   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
   port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
   port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
ethernet1@ 0x6400
  port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
...

As you can see, instead of putting port1 at +0x1700 or so,
marvell have overlapped the register files - in fact, doubly
so, since port1 + 0x1080 is right in the middle of
(port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
either.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-08 13:19             ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-08 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/08/12 13:39, Arnd Bergmann wrote:
> On Wednesday 08 August 2012, Ian Molton wrote:
>> The SMI / PHY stuff should look very similar, so I'm happy with something
>> like:
>>
>> mdio at 2000 {
>>                 #address-cells = <1>;
>>                 #size-cells = <1>;
>>                 device_type = "mdio";
>>                 compatible = "marvell,mv643xx-mdio";
>>                 phy0: ethernet-phy at 0 {
>>                         device_type = "ethernet-phy";
>>                         compatible = "marvell,whatever";
>>                         interrupts = <76>;
>>                         interrupt-parent = <&mpic>;
>>                         reg = <0 32>;          // Auto probed phy addr
>>                 };
>>
>>                 phy1: ethernet-phy at 3 {
>>                         device_type = "ethernet-phy";
>>                         compatible = "marvell,whatever";
>>                         interrupts = <77>;
>>                         interrupt-parent = <&mpic>;
>>                         reg = <3 1>;            // specified phy addr
>>                 };
>>
>>                 ... and so on.
>> }
>>
>> Where we can use the reg parameter to allow auto-probing, by
>> specifying a size of 32 (32 phy addrs max).
> I don't understand the auto-probed phy address. What is the purpose of that?

Personally, I think it should die - but the existing driver and a number
of its users actually scan the bus for their PHY.

I doubt the PHY really moves about or is hotplugged by any of them,
and its actually quite a slow process.

> If possible, I think we should keep using #size-cells=<0>, which would
> make the method you describe impossible. It might still work if you just
> leave out the "reg" property for that node.

I can certainly investigate that. I couldn't see any good evidence that
it was a supported mechanism when I looked.

> I also don't understand how the phy driver would locate ethernet-phy at 0
> on the bus if it does not know the address.
>
>> The ethernet driver itself is more complicated:
>>
>> We have the following considerations:
>>
>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>> * each ethernet device can multiple ports (up to three), each with its
>>   own MAC/PHY.
>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>>   mix of the two.
>> * existing D-T users, albeit not well documented / code complete.
>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>>   tables, etc.
>>
>> The existing ethernet-group idea only works because the current
>> platform-device based driver doesnt really do proper resource
>> management, and thus the MAC registers are actually mapped by
>> the MDIO driver.
>>
>> I don't think that preserving this bad behaviour is a good idea, which
>> leaves us with two choices:
>>
>> 1) My preferred solution - allow each device to specify up to three
>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>> multiply instantiating a driver three times over the same address
>> space.
>>
>> ethernet at 2400 {
>>                 compatible = "marvell,mv643xx-eth";
>>                 reg = <0x2400 0x1c00>
>>                 interrupt_parent = <&mpic>;
>>                 ports = <3>;
>>                 interrupts = <4>, <5>, <6>;
>>                 phys = <&phy0>, <&phy1>, <&phy2>;
>> };
>>
>> ethernet at 6400 {
>>                 compatible = "marvell,mv643xx-eth";
>>                 reg = <0x6400 0x1c00>
>>                 interrupt_parent = <&mpic>;
>>                 ports = <1>;
>>                 interrupts = <4>;
>>                 phys = <&phy3>;
>> };
>>
>> Note that the address is 2400, not 2000 - since this driver no longer
>> would share its address range with the MDIO driver.
>>
>> This method would require a small amount of rework in the driver to
>> set up <n> ports, rather than just one.
> This looks quite nice, but it is still very much incompatible with the
> existing binding. Obviously we can abandon an existing binding and
> introduce a second one for the same hardware, but that should not
> be taken lightly.

Fair, however the existing users aren't anywhere near as
numerous as the new ones.

>> 2) Create some kind of pseudo-ethernet group device that manages
>> all the work for some sort of lightweight ethernet device, one per
>> port. This can never be done cleanly since the port address ranges
>> overlap:
>>
>> pseudo_eth at 2400 {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>         compatible = "marvell,mv643xx-shared-eth"
>>         reg = <0x2400 0x1c00>;
>>
>>         ethernet at 0 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <4>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy0>;
>>         };
>>
>>         ethernet at 1 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <5>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy1>;
>>         };
>>
>>         ethernet at 2 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <6>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy2>;
>>         };
>> }
>> pseudo_eth at 6400 {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>         compatible = "marvell,mv643xx-shared-eth"
>>         reg = <0x6400 0x1c00>;
>>
>>         ethernet at 0 {
>>                 compatible = "marvell,mv643xx-port";
>>                 interrupts = <4>;
>>                 interrupt_parent = <&mpic>;
>>                 phy = <&phy3>;
>>         };
>> };
> This looks almost compatible with the existing binding, which is
> good.

Well, I'm not sure about that - if the existing bindings are really
baked into firmware, then "almost" wont be any use at all.

>  I would in fact recommend to use the actual "compatible"
> strings from the binding. More generally speaking, you should not
> use wildcards in those strings anyway, so always use
> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
> "marvell,mv643xx-eth". If you have multiple chips that are
> completely compatible, put use the identifier for the older one.
Noted.

> I don't fully understand your concern with the overlapping
> registers, mostly because I still don't know all the combinations
> that are actually valid here. Let me try to say what I understood
> so far, and you can correct me if that's wrong:
>
> * A system can have multiple instances of an mv64360 ethernet
> block, with a register area of 0x2000 bytes.
> * Each such block can have three MACs and three PHYs.
> * The first 0x400 bytes in the register space control the three
>   PHYs and the remaining registers control the MACs.
> * While this is meant to be used in a way that you assign
>   the each of the three PHYs to one of the MACs, this is not
>   always done, and sometimes you use a different PHY (?), or
>   one from a different instance of the mv64360 ethernet block
>   on the same SoC?.

Nearly - the whole block is 0x2000 in size, yes. And each one
can have 3 MACs and PHYs, as you say.

There is SMI @ 0x2000 - just one for all ports, and in many
(all?) cases, for all all the other controllers on the SoC to
share. On the armadaXP SoC, for example, each ethernet
block has its own alias of the same bas SMI reg. (there are
4 blocks)

ethernet0@ 0x2400
## regs in order: Main regs, MIB counters, Special mcast table, Mcast
table, Unicast table.
   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
   port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
   port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
ethernet1@ 0x6400
  port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
...

As you can see, instead of putting port1 at +0x1700 or so,
marvell have overlapped the register files - in fact, doubly
so, since port1 + 0x1080 is right in the middle of
(port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
either.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-08 13:19             ` Ian Molton
  (?)
@ 2012-08-09 10:59               ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 10:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Miller, linux-arm-kernel, andrew, thomas.petazzoni,
	ben.dooks, netdev, devicetree-discuss, linuxppc-dev, dale

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

On 08/08/12 14:19, Ian Molton wrote:
> On 08/08/12 13:39, Arnd Bergmann wrote:
>> On Wednesday 08 August 2012, Ian Molton wrote:
>>> The SMI / PHY stuff should look very similar, so I'm happy with something
>>> like:
>>>
>>> mdio@2000 {
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 device_type = "mdio";
>>>                 compatible = "marvell,mv643xx-mdio";
>>>                 phy0: ethernet-phy@0 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <76>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <0 32>;          // Auto probed phy addr
>>>                 };
>>>
>>>                 phy1: ethernet-phy@3 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <77>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <3 1>;            // specified phy addr
>>>                 };
>>>
>>>                 ... and so on.
>>> }
>>>
>>> Where we can use the reg parameter to allow auto-probing, by
>>> specifying a size of 32 (32 phy addrs max).
>> I don't understand the auto-probed phy address. What is the purpose of that?
> Personally, I think it should die - but the existing driver and a number
> of its users actually scan the bus for their PHY.
>
> I doubt the PHY really moves about or is hotplugged by any of them,
> and its actually quite a slow process.
>
>> If possible, I think we should keep using #size-cells=<0>, which would
>> make the method you describe impossible. It might still work if you just
>> leave out the "reg" property for that node.
> I can certainly investigate that. I couldn't see any good evidence that
> it was a supported mechanism when I looked.
>
>> I also don't understand how the phy driver would locate ethernet-phy@0
>> on the bus if it does not know the address.
>>
>>> The ethernet driver itself is more complicated:
>>>
>>> We have the following considerations:
>>>
>>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>>> * each ethernet device can multiple ports (up to three), each with its
>>>   own MAC/PHY.
>>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>>>   mix of the two.
>>> * existing D-T users, albeit not well documented / code complete.
>>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>>>   tables, etc.
>>>
>>> The existing ethernet-group idea only works because the current
>>> platform-device based driver doesnt really do proper resource
>>> management, and thus the MAC registers are actually mapped by
>>> the MDIO driver.
>>>
>>> I don't think that preserving this bad behaviour is a good idea, which
>>> leaves us with two choices:
>>>
>>> 1) My preferred solution - allow each device to specify up to three
>>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>>> multiply instantiating a driver three times over the same address
>>> space.
>>>
>>> ethernet@2400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x2400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <3>;
>>>                 interrupts = <4>, <5>, <6>;
>>>                 phys = <&phy0>, <&phy1>, <&phy2>;
>>> };
>>>
>>> ethernet@6400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x6400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <1>;
>>>                 interrupts = <4>;
>>>                 phys = <&phy3>;
>>> };
>>>
>>> Note that the address is 2400, not 2000 - since this driver no longer
>>> would share its address range with the MDIO driver.
>>>
>>> This method would require a small amount of rework in the driver to
>>> set up <n> ports, rather than just one.
>> This looks quite nice, but it is still very much incompatible with the
>> existing binding. Obviously we can abandon an existing binding and
>> introduce a second one for the same hardware, but that should not
>> be taken lightly.
> Fair, however the existing users aren't anywhere near as
> numerous as the new ones.
>
>>> 2) Create some kind of pseudo-ethernet group device that manages
>>> all the work for some sort of lightweight ethernet device, one per
>>> port. This can never be done cleanly since the port address ranges
>>> overlap:
>>>
>>> pseudo_eth@2400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x2400 0x1c00>;
>>>
>>>         ethernet@0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy0>;
>>>         };
>>>
>>>         ethernet@1 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <5>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy1>;
>>>         };
>>>
>>>         ethernet@2 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <6>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy2>;
>>>         };
>>> }
>>> pseudo_eth@6400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x6400 0x1c00>;
>>>
>>>         ethernet@0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy3>;
>>>         };
>>> };
>> This looks almost compatible with the existing binding, which is
>> good.
> Well, I'm not sure about that - if the existing bindings are really
> baked into firmware, then "almost" wont be any use at all.
>
>>  I would in fact recommend to use the actual "compatible"
>> strings from the binding. More generally speaking, you should not
>> use wildcards in those strings anyway, so always use
>> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
>> "marvell,mv643xx-eth". If you have multiple chips that are
>> completely compatible, put use the identifier for the older one.
> Noted.
>
>> I don't fully understand your concern with the overlapping
>> registers, mostly because I still don't know all the combinations
>> that are actually valid here. Let me try to say what I understood
>> so far, and you can correct me if that's wrong:
>>
>> * A system can have multiple instances of an mv64360 ethernet
>> block, with a register area of 0x2000 bytes.
>> * Each such block can have three MACs and three PHYs.
>> * The first 0x400 bytes in the register space control the three
>>   PHYs and the remaining registers control the MACs.
>> * While this is meant to be used in a way that you assign
>>   the each of the three PHYs to one of the MACs, this is not
>>   always done, and sometimes you use a different PHY (?), or
>>   one from a different instance of the mv64360 ethernet block
>>   on the same SoC?.
> Nearly - the whole block is 0x2000 in size, yes. And each one
> can have 3 MACs and PHYs, as you say.
>
> There is SMI @ 0x2000 - just one for all ports, and in many
> (all?) cases, for all all the other controllers on the SoC to
> share. On the armadaXP SoC, for example, each ethernet
> block has its own alias of the same bas SMI reg. (there are
> 4 blocks)
>
> ethernet0@ 0x2400
> ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
> table, Unicast table.
>    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
>    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
>    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
> ethernet1@ 0x6400
>   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
> ...
>
> As you can see, instead of putting port1 at +0x1700 or so,
> marvell have overlapped the register files - in fact, doubly
> so, since port1 + 0x1080 is right in the middle of
> (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
> sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
> either.
>
> -Ian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 10:59               ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 10:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

On 08/08/12 14:19, Ian Molton wrote:
> On 08/08/12 13:39, Arnd Bergmann wrote:
>> On Wednesday 08 August 2012, Ian Molton wrote:
>>> The SMI / PHY stuff should look very similar, so I'm happy with something
>>> like:
>>>
>>> mdio@2000 {
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 device_type = "mdio";
>>>                 compatible = "marvell,mv643xx-mdio";
>>>                 phy0: ethernet-phy@0 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <76>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <0 32>;          // Auto probed phy addr
>>>                 };
>>>
>>>                 phy1: ethernet-phy@3 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <77>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <3 1>;            // specified phy addr
>>>                 };
>>>
>>>                 ... and so on.
>>> }
>>>
>>> Where we can use the reg parameter to allow auto-probing, by
>>> specifying a size of 32 (32 phy addrs max).
>> I don't understand the auto-probed phy address. What is the purpose of that?
> Personally, I think it should die - but the existing driver and a number
> of its users actually scan the bus for their PHY.
>
> I doubt the PHY really moves about or is hotplugged by any of them,
> and its actually quite a slow process.
>
>> If possible, I think we should keep using #size-cells=<0>, which would
>> make the method you describe impossible. It might still work if you just
>> leave out the "reg" property for that node.
> I can certainly investigate that. I couldn't see any good evidence that
> it was a supported mechanism when I looked.
>
>> I also don't understand how the phy driver would locate ethernet-phy@0
>> on the bus if it does not know the address.
>>
>>> The ethernet driver itself is more complicated:
>>>
>>> We have the following considerations:
>>>
>>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>>> * each ethernet device can multiple ports (up to three), each with its
>>>   own MAC/PHY.
>>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>>>   mix of the two.
>>> * existing D-T users, albeit not well documented / code complete.
>>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>>>   tables, etc.
>>>
>>> The existing ethernet-group idea only works because the current
>>> platform-device based driver doesnt really do proper resource
>>> management, and thus the MAC registers are actually mapped by
>>> the MDIO driver.
>>>
>>> I don't think that preserving this bad behaviour is a good idea, which
>>> leaves us with two choices:
>>>
>>> 1) My preferred solution - allow each device to specify up to three
>>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>>> multiply instantiating a driver three times over the same address
>>> space.
>>>
>>> ethernet@2400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x2400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <3>;
>>>                 interrupts = <4>, <5>, <6>;
>>>                 phys = <&phy0>, <&phy1>, <&phy2>;
>>> };
>>>
>>> ethernet@6400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x6400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <1>;
>>>                 interrupts = <4>;
>>>                 phys = <&phy3>;
>>> };
>>>
>>> Note that the address is 2400, not 2000 - since this driver no longer
>>> would share its address range with the MDIO driver.
>>>
>>> This method would require a small amount of rework in the driver to
>>> set up <n> ports, rather than just one.
>> This looks quite nice, but it is still very much incompatible with the
>> existing binding. Obviously we can abandon an existing binding and
>> introduce a second one for the same hardware, but that should not
>> be taken lightly.
> Fair, however the existing users aren't anywhere near as
> numerous as the new ones.
>
>>> 2) Create some kind of pseudo-ethernet group device that manages
>>> all the work for some sort of lightweight ethernet device, one per
>>> port. This can never be done cleanly since the port address ranges
>>> overlap:
>>>
>>> pseudo_eth@2400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x2400 0x1c00>;
>>>
>>>         ethernet@0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy0>;
>>>         };
>>>
>>>         ethernet@1 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <5>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy1>;
>>>         };
>>>
>>>         ethernet@2 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <6>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy2>;
>>>         };
>>> }
>>> pseudo_eth@6400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x6400 0x1c00>;
>>>
>>>         ethernet@0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy3>;
>>>         };
>>> };
>> This looks almost compatible with the existing binding, which is
>> good.
> Well, I'm not sure about that - if the existing bindings are really
> baked into firmware, then "almost" wont be any use at all.
>
>>  I would in fact recommend to use the actual "compatible"
>> strings from the binding. More generally speaking, you should not
>> use wildcards in those strings anyway, so always use
>> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
>> "marvell,mv643xx-eth". If you have multiple chips that are
>> completely compatible, put use the identifier for the older one.
> Noted.
>
>> I don't fully understand your concern with the overlapping
>> registers, mostly because I still don't know all the combinations
>> that are actually valid here. Let me try to say what I understood
>> so far, and you can correct me if that's wrong:
>>
>> * A system can have multiple instances of an mv64360 ethernet
>> block, with a register area of 0x2000 bytes.
>> * Each such block can have three MACs and three PHYs.
>> * The first 0x400 bytes in the register space control the three
>>   PHYs and the remaining registers control the MACs.
>> * While this is meant to be used in a way that you assign
>>   the each of the three PHYs to one of the MACs, this is not
>>   always done, and sometimes you use a different PHY (?), or
>>   one from a different instance of the mv64360 ethernet block
>>   on the same SoC?.
> Nearly - the whole block is 0x2000 in size, yes. And each one
> can have 3 MACs and PHYs, as you say.
>
> There is SMI @ 0x2000 - just one for all ports, and in many
> (all?) cases, for all all the other controllers on the SoC to
> share. On the armadaXP SoC, for example, each ethernet
> block has its own alias of the same bas SMI reg. (there are
> 4 blocks)
>
> ethernet0@ 0x2400
> ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
> table, Unicast table.
>    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
>    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
>    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
> ethernet1@ 0x6400
>   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
> ...
>
> As you can see, instead of putting port1 at +0x1700 or so,
> marvell have overlapped the register files - in fact, doubly
> so, since port1 + 0x1080 is right in the middle of
> (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
> sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
> either.
>
> -Ian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 10:59               ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

Adding devicetree-discuss and linuxppc-dev, as well as Dale Farnsworth,
who initially added the bindings for mv643xx.

On 08/08/12 14:19, Ian Molton wrote:
> On 08/08/12 13:39, Arnd Bergmann wrote:
>> On Wednesday 08 August 2012, Ian Molton wrote:
>>> The SMI / PHY stuff should look very similar, so I'm happy with something
>>> like:
>>>
>>> mdio at 2000 {
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 device_type = "mdio";
>>>                 compatible = "marvell,mv643xx-mdio";
>>>                 phy0: ethernet-phy at 0 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <76>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <0 32>;          // Auto probed phy addr
>>>                 };
>>>
>>>                 phy1: ethernet-phy at 3 {
>>>                         device_type = "ethernet-phy";
>>>                         compatible = "marvell,whatever";
>>>                         interrupts = <77>;
>>>                         interrupt-parent = <&mpic>;
>>>                         reg = <3 1>;            // specified phy addr
>>>                 };
>>>
>>>                 ... and so on.
>>> }
>>>
>>> Where we can use the reg parameter to allow auto-probing, by
>>> specifying a size of 32 (32 phy addrs max).
>> I don't understand the auto-probed phy address. What is the purpose of that?
> Personally, I think it should die - but the existing driver and a number
> of its users actually scan the bus for their PHY.
>
> I doubt the PHY really moves about or is hotplugged by any of them,
> and its actually quite a slow process.
>
>> If possible, I think we should keep using #size-cells=<0>, which would
>> make the method you describe impossible. It might still work if you just
>> leave out the "reg" property for that node.
> I can certainly investigate that. I couldn't see any good evidence that
> it was a supported mechanism when I looked.
>
>> I also don't understand how the phy driver would locate ethernet-phy at 0
>> on the bus if it does not know the address.
>>
>>> The ethernet driver itself is more complicated:
>>>
>>> We have the following considerations:
>>>
>>> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
>>> * each ethernet device can multiple ports (up to three), each with its
>>>   own MAC/PHY.
>>> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
>>>   mix of the two.
>>> * existing D-T users, albeit not well documented / code complete.
>>> * some port address ranges overlap (MIB counters, MCAST / UNICAST
>>>   tables, etc.
>>>
>>> The existing ethernet-group idea only works because the current
>>> platform-device based driver doesnt really do proper resource
>>> management, and thus the MAC registers are actually mapped by
>>> the MDIO driver.
>>>
>>> I don't think that preserving this bad behaviour is a good idea, which
>>> leaves us with two choices:
>>>
>>> 1) My preferred solution - allow each device to specify up to three
>>> interrupts, MACs, and PHYs. This is clean in that it doesnt require
>>> multiply instantiating a driver three times over the same address
>>> space.
>>>
>>> ethernet at 2400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x2400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <3>;
>>>                 interrupts = <4>, <5>, <6>;
>>>                 phys = <&phy0>, <&phy1>, <&phy2>;
>>> };
>>>
>>> ethernet at 6400 {
>>>                 compatible = "marvell,mv643xx-eth";
>>>                 reg = <0x6400 0x1c00>
>>>                 interrupt_parent = <&mpic>;
>>>                 ports = <1>;
>>>                 interrupts = <4>;
>>>                 phys = <&phy3>;
>>> };
>>>
>>> Note that the address is 2400, not 2000 - since this driver no longer
>>> would share its address range with the MDIO driver.
>>>
>>> This method would require a small amount of rework in the driver to
>>> set up <n> ports, rather than just one.
>> This looks quite nice, but it is still very much incompatible with the
>> existing binding. Obviously we can abandon an existing binding and
>> introduce a second one for the same hardware, but that should not
>> be taken lightly.
> Fair, however the existing users aren't anywhere near as
> numerous as the new ones.
>
>>> 2) Create some kind of pseudo-ethernet group device that manages
>>> all the work for some sort of lightweight ethernet device, one per
>>> port. This can never be done cleanly since the port address ranges
>>> overlap:
>>>
>>> pseudo_eth at 2400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x2400 0x1c00>;
>>>
>>>         ethernet at 0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy0>;
>>>         };
>>>
>>>         ethernet at 1 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <5>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy1>;
>>>         };
>>>
>>>         ethernet at 2 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <6>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy2>;
>>>         };
>>> }
>>> pseudo_eth at 6400 {
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>         compatible = "marvell,mv643xx-shared-eth"
>>>         reg = <0x6400 0x1c00>;
>>>
>>>         ethernet at 0 {
>>>                 compatible = "marvell,mv643xx-port";
>>>                 interrupts = <4>;
>>>                 interrupt_parent = <&mpic>;
>>>                 phy = <&phy3>;
>>>         };
>>> };
>> This looks almost compatible with the existing binding, which is
>> good.
> Well, I'm not sure about that - if the existing bindings are really
> baked into firmware, then "almost" wont be any use at all.
>
>>  I would in fact recommend to use the actual "compatible"
>> strings from the binding. More generally speaking, you should not
>> use wildcards in those strings anyway, so always use
>> "marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
>> "marvell,mv643xx-eth". If you have multiple chips that are
>> completely compatible, put use the identifier for the older one.
> Noted.
>
>> I don't fully understand your concern with the overlapping
>> registers, mostly because I still don't know all the combinations
>> that are actually valid here. Let me try to say what I understood
>> so far, and you can correct me if that's wrong:
>>
>> * A system can have multiple instances of an mv64360 ethernet
>> block, with a register area of 0x2000 bytes.
>> * Each such block can have three MACs and three PHYs.
>> * The first 0x400 bytes in the register space control the three
>>   PHYs and the remaining registers control the MACs.
>> * While this is meant to be used in a way that you assign
>>   the each of the three PHYs to one of the MACs, this is not
>>   always done, and sometimes you use a different PHY (?), or
>>   one from a different instance of the mv64360 ethernet block
>>   on the same SoC?.
> Nearly - the whole block is 0x2000 in size, yes. And each one
> can have 3 MACs and PHYs, as you say.
>
> There is SMI @ 0x2000 - just one for all ports, and in many
> (all?) cases, for all all the other controllers on the SoC to
> share. On the armadaXP SoC, for example, each ethernet
> block has its own alias of the same bas SMI reg. (there are
> 4 blocks)
>
> ethernet0@ 0x2400
> ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
> table, Unicast table.
>    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
>    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
>    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
> ethernet1@ 0x6400
>   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
> ...
>
> As you can see, instead of putting port1 at +0x1700 or so,
> marvell have overlapped the register files - in fact, doubly
> so, since port1 + 0x1080 is right in the middle of
> (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
> sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
> either.
>
> -Ian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-09 10:59               ` Ian Molton
  (?)
@ 2012-08-09 11:43                 ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-09 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Ian Molton, thomas.petazzoni, andrew, netdev, devicetree-discuss,
	ben.dooks, dale, linuxppc-dev, David Miller

 On 08/08/12 14:19, Ian Molton wrote:
 > On 08/08/12 13:39, Arnd Bergmann wrote:
 >> On Wednesday 08 August 2012, Ian Molton wrote:
 >>> This method would require a small amount of rework in the driver to
 >>> set up <n> ports, rather than just one.
 >> This looks quite nice, but it is still very much incompatible with the
 >> existing binding. Obviously we can abandon an existing binding and
 >> introduce a second one for the same hardware, but that should not
 >> be taken lightly.
 > Fair, however the existing users aren't anywhere near as
 > numerous as the new ones.

Depends on how you count the numbers. I see at least three machines
supported in the kernel with the old binding and none with the new one
so far ;-)

 >> I don't fully understand your concern with the overlapping
 >> registers, mostly because I still don't know all the combinations
 >> that are actually valid here. Let me try to say what I understood
 >> so far, and you can correct me if that's wrong:
 >>
 >> * A system can have multiple instances of an mv64360 ethernet
 >> block, with a register area of 0x2000 bytes.
 >> * Each such block can have three MACs and three PHYs.
 >> * The first 0x400 bytes in the register space control the three
 >>   PHYs and the remaining registers control the MACs.
 >> * While this is meant to be used in a way that you assign
 >>   the each of the three PHYs to one of the MACs, this is not
 >>   always done, and sometimes you use a different PHY (?), or
 >>   one from a different instance of the mv64360 ethernet block
 >>   on the same SoC?.
 > Nearly - the whole block is 0x2000 in size, yes. And each one
 > can have 3 MACs and PHYs, as you say.
 >
 > There is SMI @ 0x2000 - just one for all ports, and in many
 > (all?) cases, for all all the other controllers on the SoC to
 > share. On the armadaXP SoC, for example, each ethernet
 > block has its own alias of the same bas SMI reg. (there are
 > 4 blocks)
 >
 > ethernet0@ 0x2400
 > ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
 > table, Unicast table.
 >    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 >    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
 >    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
 > ethernet1@ 0x6400
 >   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 > ...
 >
 > As you can see, instead of putting port1 at +0x1700 or so,
 > marvell have overlapped the register files - in fact, doubly
 > so, since port1 + 0x1080 is right in the middle of
 > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
 > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
 > either.

This could theoretically be dealt with by having 5 register ranges
per device, but that would cause extra overhead and also be
incompatible with the existing binding. I think showing one
parent device with children at address 0, 1 and 2 is ok. The driver
already knows all those offsets and they are always the same
for all variants of mv643xx, right?

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 11:43                 ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-09 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss,
	linuxppc-dev, ben.dooks, Ian Molton, David Miller

 On 08/08/12 14:19, Ian Molton wrote:
 > On 08/08/12 13:39, Arnd Bergmann wrote:
 >> On Wednesday 08 August 2012, Ian Molton wrote:
 >>> This method would require a small amount of rework in the driver to
 >>> set up <n> ports, rather than just one.
 >> This looks quite nice, but it is still very much incompatible with the
 >> existing binding. Obviously we can abandon an existing binding and
 >> introduce a second one for the same hardware, but that should not
 >> be taken lightly.
 > Fair, however the existing users aren't anywhere near as
 > numerous as the new ones.

Depends on how you count the numbers. I see at least three machines
supported in the kernel with the old binding and none with the new one
so far ;-)

 >> I don't fully understand your concern with the overlapping
 >> registers, mostly because I still don't know all the combinations
 >> that are actually valid here. Let me try to say what I understood
 >> so far, and you can correct me if that's wrong:
 >>
 >> * A system can have multiple instances of an mv64360 ethernet
 >> block, with a register area of 0x2000 bytes.
 >> * Each such block can have three MACs and three PHYs.
 >> * The first 0x400 bytes in the register space control the three
 >>   PHYs and the remaining registers control the MACs.
 >> * While this is meant to be used in a way that you assign
 >>   the each of the three PHYs to one of the MACs, this is not
 >>   always done, and sometimes you use a different PHY (?), or
 >>   one from a different instance of the mv64360 ethernet block
 >>   on the same SoC?.
 > Nearly - the whole block is 0x2000 in size, yes. And each one
 > can have 3 MACs and PHYs, as you say.
 >
 > There is SMI @ 0x2000 - just one for all ports, and in many
 > (all?) cases, for all all the other controllers on the SoC to
 > share. On the armadaXP SoC, for example, each ethernet
 > block has its own alias of the same bas SMI reg. (there are
 > 4 blocks)
 >
 > ethernet0@ 0x2400
 > ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
 > table, Unicast table.
 >    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 >    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
 >    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
 > ethernet1@ 0x6400
 >   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 > ...
 >
 > As you can see, instead of putting port1 at +0x1700 or so,
 > marvell have overlapped the register files - in fact, doubly
 > so, since port1 + 0x1080 is right in the middle of
 > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
 > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
 > either.

This could theoretically be dealt with by having 5 register ranges
per device, but that would cause extra overhead and also be
incompatible with the existing binding. I think showing one
parent device with children at address 0, 1 and 2 is ok. The driver
already knows all those offsets and they are always the same
for all variants of mv643xx, right?

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 11:43                 ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-09 11:43 UTC (permalink / raw)
  To: linux-arm-kernel

 On 08/08/12 14:19, Ian Molton wrote:
 > On 08/08/12 13:39, Arnd Bergmann wrote:
 >> On Wednesday 08 August 2012, Ian Molton wrote:
 >>> This method would require a small amount of rework in the driver to
 >>> set up <n> ports, rather than just one.
 >> This looks quite nice, but it is still very much incompatible with the
 >> existing binding. Obviously we can abandon an existing binding and
 >> introduce a second one for the same hardware, but that should not
 >> be taken lightly.
 > Fair, however the existing users aren't anywhere near as
 > numerous as the new ones.

Depends on how you count the numbers. I see at least three machines
supported in the kernel with the old binding and none with the new one
so far ;-)

 >> I don't fully understand your concern with the overlapping
 >> registers, mostly because I still don't know all the combinations
 >> that are actually valid here. Let me try to say what I understood
 >> so far, and you can correct me if that's wrong:
 >>
 >> * A system can have multiple instances of an mv64360 ethernet
 >> block, with a register area of 0x2000 bytes.
 >> * Each such block can have three MACs and three PHYs.
 >> * The first 0x400 bytes in the register space control the three
 >>   PHYs and the remaining registers control the MACs.
 >> * While this is meant to be used in a way that you assign
 >>   the each of the three PHYs to one of the MACs, this is not
 >>   always done, and sometimes you use a different PHY (?), or
 >>   one from a different instance of the mv64360 ethernet block
 >>   on the same SoC?.
 > Nearly - the whole block is 0x2000 in size, yes. And each one
 > can have 3 MACs and PHYs, as you say.
 >
 > There is SMI @ 0x2000 - just one for all ports, and in many
 > (all?) cases, for all all the other controllers on the SoC to
 > share. On the armadaXP SoC, for example, each ethernet
 > block has its own alias of the same bas SMI reg. (there are
 > 4 blocks)
 >
 > ethernet0@ 0x2400
 > ## regs in order: Main regs, MIB counters, Special mcast table, Mcast
 > table, Unicast table.
 >    port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 >    port1 has regs at +0x0400 *0x1080 +0x1800 +0x1900 +0x1a00
 >    port2 has regs at +0x0800 *0x1100 +0x1c00 +0x1d00 +0x1e00
 > ethernet1@ 0x6400
 >   port0 has regs at +0x0000 *0x1000 +0x1400 +0x1500 +0x1600
 > ...
 >
 > As you can see, instead of putting port1 at +0x1700 or so,
 > marvell have overlapped the register files - in fact, doubly
 > so, since port1 + 0x1080 is right in the middle of
 > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
 > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
 > either.

This could theoretically be dealt with by having 5 register ranges
per device, but that would cause extra overhead and also be
incompatible with the existing binding. I think showing one
parent device with children at address 0, 1 and 2 is ok. The driver
already knows all those offsets and they are always the same
for all variants of mv643xx, right?

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-09 11:43                 ` Arnd Bergmann
  (?)
@ 2012-08-09 15:21                   ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 15:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
	devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller

On 09/08/12 12:43, Arnd Bergmann wrote:
>  On 08/08/12 14:19, Ian Molton wrote:
>  > On 08/08/12 13:39, Arnd Bergmann wrote:
>  >> On Wednesday 08 August 2012, Ian Molton wrote:
>  >>> This method would require a small amount of rework in the driver to
>  >>> set up <n> ports, rather than just one.
>  >> This looks quite nice, but it is still very much incompatible with the
>  >> existing binding. Obviously we can abandon an existing binding and
>  >> introduce a second one for the same hardware, but that should not
>  >> be taken lightly.
>  > Fair, however the existing users aren't anywhere near as
>  > numerous as the new ones.
>
> Depends on how you count the numbers. I see at least three machines
> supported in the kernel with the old binding and none with the new one
> so far ;-)

I'm curious as to how any of those actually work, given the
apparent total lack of a mv64360-mdio device binding...
>  > As you can see, instead of putting port1 at +0x1700 or so,
>  > marvell have overlapped the register files - in fact, doubly
>  > so, since port1 + 0x1080 is right in the middle of
>  > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
>  > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
>  > either.
>
> This could theoretically be dealt with by having 5 register ranges

I make that three...

> per device, but that would cause extra overhead and also be
> incompatible with the existing binding.

Indeed.

>  I think showing one
> parent device with children at address 0, 1 and 2 is ok.
Is it acceptable for the child devices to directly access the
parents register space? because there would be no other
way for that to work.

>  The driver
> already knows all those offsets and they are always the same
> for all variants of mv643xx, right?
Yes, but its not clean. And no amount of refactoring is
really going to make a nice driver that also fits the ancient
(and badly thought out) OF bindings.

If we have to break things, we can at least go for a nice
clean design, surely?

The ports arent really child devices of the MAC. The MAC
just has 3 ports.

Luckily, it looks like the existing users don't actually use
the device tree to set up the driver at all, preferring to
translate their D-T bindings to calls to
platform_device_register() so all we'd need to do to
support them is completely ignore them.

We're going to have to maintain a legacy
platform_device -> DT bindings hack somewhere anyway,
at least until the remaining other users of the driver
convert to D-T.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 15:21                   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 15:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

On 09/08/12 12:43, Arnd Bergmann wrote:
>  On 08/08/12 14:19, Ian Molton wrote:
>  > On 08/08/12 13:39, Arnd Bergmann wrote:
>  >> On Wednesday 08 August 2012, Ian Molton wrote:
>  >>> This method would require a small amount of rework in the driver to
>  >>> set up <n> ports, rather than just one.
>  >> This looks quite nice, but it is still very much incompatible with the
>  >> existing binding. Obviously we can abandon an existing binding and
>  >> introduce a second one for the same hardware, but that should not
>  >> be taken lightly.
>  > Fair, however the existing users aren't anywhere near as
>  > numerous as the new ones.
>
> Depends on how you count the numbers. I see at least three machines
> supported in the kernel with the old binding and none with the new one
> so far ;-)

I'm curious as to how any of those actually work, given the
apparent total lack of a mv64360-mdio device binding...
>  > As you can see, instead of putting port1 at +0x1700 or so,
>  > marvell have overlapped the register files - in fact, doubly
>  > so, since port1 + 0x1080 is right in the middle of
>  > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
>  > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
>  > either.
>
> This could theoretically be dealt with by having 5 register ranges

I make that three...

> per device, but that would cause extra overhead and also be
> incompatible with the existing binding.

Indeed.

>  I think showing one
> parent device with children at address 0, 1 and 2 is ok.
Is it acceptable for the child devices to directly access the
parents register space? because there would be no other
way for that to work.

>  The driver
> already knows all those offsets and they are always the same
> for all variants of mv643xx, right?
Yes, but its not clean. And no amount of refactoring is
really going to make a nice driver that also fits the ancient
(and badly thought out) OF bindings.

If we have to break things, we can at least go for a nice
clean design, surely?

The ports arent really child devices of the MAC. The MAC
just has 3 ports.

Luckily, it looks like the existing users don't actually use
the device tree to set up the driver at all, preferring to
translate their D-T bindings to calls to
platform_device_register() so all we'd need to do to
support them is completely ignore them.

We're going to have to maintain a legacy
platform_device -> DT bindings hack somewhere anyway,
at least until the remaining other users of the driver
convert to D-T.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-09 15:21                   ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-09 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/08/12 12:43, Arnd Bergmann wrote:
>  On 08/08/12 14:19, Ian Molton wrote:
>  > On 08/08/12 13:39, Arnd Bergmann wrote:
>  >> On Wednesday 08 August 2012, Ian Molton wrote:
>  >>> This method would require a small amount of rework in the driver to
>  >>> set up <n> ports, rather than just one.
>  >> This looks quite nice, but it is still very much incompatible with the
>  >> existing binding. Obviously we can abandon an existing binding and
>  >> introduce a second one for the same hardware, but that should not
>  >> be taken lightly.
>  > Fair, however the existing users aren't anywhere near as
>  > numerous as the new ones.
>
> Depends on how you count the numbers. I see at least three machines
> supported in the kernel with the old binding and none with the new one
> so far ;-)

I'm curious as to how any of those actually work, given the
apparent total lack of a mv64360-mdio device binding...
>  > As you can see, instead of putting port1 at +0x1700 or so,
>  > marvell have overlapped the register files - in fact, doubly
>  > so, since port1 + 0x1080 is right in the middle of
>  > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
>  > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
>  > either.
>
> This could theoretically be dealt with by having 5 register ranges

I make that three...

> per device, but that would cause extra overhead and also be
> incompatible with the existing binding.

Indeed.

>  I think showing one
> parent device with children at address 0, 1 and 2 is ok.
Is it acceptable for the child devices to directly access the
parents register space? because there would be no other
way for that to work.

>  The driver
> already knows all those offsets and they are always the same
> for all variants of mv643xx, right?
Yes, but its not clean. And no amount of refactoring is
really going to make a nice driver that also fits the ancient
(and badly thought out) OF bindings.

If we have to break things, we can at least go for a nice
clean design, surely?

The ports arent really child devices of the MAC. The MAC
just has 3 ports.

Luckily, it looks like the existing users don't actually use
the device tree to set up the driver at all, preferring to
translate their D-T bindings to calls to
platform_device_register() so all we'd need to do to
support them is completely ignore them.

We're going to have to maintain a legacy
platform_device -> DT bindings hack somewhere anyway,
at least until the remaining other users of the driver
convert to D-T.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-09 15:21                   ` Ian Molton
  (?)
@ 2012-08-10 10:49                     ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-10 10:49 UTC (permalink / raw)
  To: Ian Molton
  Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
	devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller

On Thursday 09 August 2012, Ian Molton wrote:
> >  I think showing one
> > parent device with children at address 0, 1 and 2 is ok.
> Is it acceptable for the child devices to directly access the
> parents register space? because there would be no other
> way for that to work.

Yes, I see no problem with that. As long as all the drivers
agree on who can access what.

> >  The driver
> > already knows all those offsets and they are always the same
> > for all variants of mv643xx, right?
> Yes, but its not clean. And no amount of refactoring is
> really going to make a nice driver that also fits the ancient
> (and badly thought out) OF bindings.

In what way is it badly though out, or not clean? The use of
underscores in the properties, and the way that the sram
is configured is problematic, I agree. But The way that
the three ports are addressed and how the PHY is found
seems quite clever.

> If we have to break things, we can at least go for a nice
> clean design, surely?
> 
> The ports arent really child devices of the MAC. The MAC
> just has 3 ports.

I don't see the difference between those two things.

> Luckily, it looks like the existing users don't actually use
> the device tree to set up the driver at all, preferring to
> translate their D-T bindings to calls to
> platform_device_register() so all we'd need to do to
> support them is completely ignore them.
> 
> We're going to have to maintain a legacy
> platform_device -> DT bindings hack somewhere anyway,
> at least until the remaining other users of the driver
> convert to D-T.

I don't understand why you describe the method used in
powerpc as a hack. It was the normal way to introduce
DT support for platform devices back when it was implemented.
It also had the advantage of not requiring any modifications
to the generic driver, because it was shared between one
architecture using DT (powerpc) and one that didn't (ARM).

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-10 10:49                     ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-10 10:49 UTC (permalink / raw)
  To: Ian Molton
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

On Thursday 09 August 2012, Ian Molton wrote:
> >  I think showing one
> > parent device with children at address 0, 1 and 2 is ok.
> Is it acceptable for the child devices to directly access the
> parents register space? because there would be no other
> way for that to work.

Yes, I see no problem with that. As long as all the drivers
agree on who can access what.

> >  The driver
> > already knows all those offsets and they are always the same
> > for all variants of mv643xx, right?
> Yes, but its not clean. And no amount of refactoring is
> really going to make a nice driver that also fits the ancient
> (and badly thought out) OF bindings.

In what way is it badly though out, or not clean? The use of
underscores in the properties, and the way that the sram
is configured is problematic, I agree. But The way that
the three ports are addressed and how the PHY is found
seems quite clever.

> If we have to break things, we can at least go for a nice
> clean design, surely?
> 
> The ports arent really child devices of the MAC. The MAC
> just has 3 ports.

I don't see the difference between those two things.

> Luckily, it looks like the existing users don't actually use
> the device tree to set up the driver at all, preferring to
> translate their D-T bindings to calls to
> platform_device_register() so all we'd need to do to
> support them is completely ignore them.
> 
> We're going to have to maintain a legacy
> platform_device -> DT bindings hack somewhere anyway,
> at least until the remaining other users of the driver
> convert to D-T.

I don't understand why you describe the method used in
powerpc as a hack. It was the normal way to introduce
DT support for platform devices back when it was implemented.
It also had the advantage of not requiring any modifications
to the generic driver, because it was shared between one
architecture using DT (powerpc) and one that didn't (ARM).

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-10 10:49                     ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-10 10:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 09 August 2012, Ian Molton wrote:
> >  I think showing one
> > parent device with children at address 0, 1 and 2 is ok.
> Is it acceptable for the child devices to directly access the
> parents register space? because there would be no other
> way for that to work.

Yes, I see no problem with that. As long as all the drivers
agree on who can access what.

> >  The driver
> > already knows all those offsets and they are always the same
> > for all variants of mv643xx, right?
> Yes, but its not clean. And no amount of refactoring is
> really going to make a nice driver that also fits the ancient
> (and badly thought out) OF bindings.

In what way is it badly though out, or not clean? The use of
underscores in the properties, and the way that the sram
is configured is problematic, I agree. But The way that
the three ports are addressed and how the PHY is found
seems quite clever.

> If we have to break things, we can at least go for a nice
> clean design, surely?
> 
> The ports arent really child devices of the MAC. The MAC
> just has 3 ports.

I don't see the difference between those two things.

> Luckily, it looks like the existing users don't actually use
> the device tree to set up the driver at all, preferring to
> translate their D-T bindings to calls to
> platform_device_register() so all we'd need to do to
> support them is completely ignore them.
> 
> We're going to have to maintain a legacy
> platform_device -> DT bindings hack somewhere anyway,
> at least until the remaining other users of the driver
> convert to D-T.

I don't understand why you describe the method used in
powerpc as a hack. It was the normal way to introduce
DT support for platform devices back when it was implemented.
It also had the advantage of not requiring any modifications
to the generic driver, because it was shared between one
architecture using DT (powerpc) and one that didn't (ARM).

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-10 10:49                     ` Arnd Bergmann
  (?)
@ 2012-08-13 10:00                       ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-13 10:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
	devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller

On 10/08/12 11:49, Arnd Bergmann wrote:
> On Thursday 09 August 2012, Ian Molton wrote:
>>>  The driver
>>> already knows all those offsets and they are always the same
>>> for all variants of mv643xx, right?
>> Yes, but its not clean. And no amount of refactoring is
>> really going to make a nice driver that also fits the ancient
>> (and badly thought out) OF bindings.
> In what way is it badly though out, or not clean? The use of
> underscores in the properties, and the way that the sram
> is configured is problematic, I agree. But The way that
> the three ports are addressed and how the PHY is found
> seems quite clever.

It forces one to load the MDIO driver first, because it maps ALL the
registers for both itself and all the ports, and the MDIO driver has no
way of knowing how many ethernet blocks are present (I have a device
here with two, and another with four). Thats anywhere from 1 to 12
ports, split across 1 to 4 address ranges, and theres a big gap in the
address range between controllers 0,1 and 2,3. *ALL* the devices on the
board are sharing ethernet block 0's MDIO bus. By pure luck it happens
to work, because the blocks 2,3 have an alias of the MDIO registers from
blocks 0,1.

Having the MDIO driver map the ethernet drivers memory is a terrible
solution, IMO. Ethernet drivers should map their own memory, and that
introduces the n-ports-per-block problem, because their address ranges
overlap.

I think the best solution is to make each ethernet block register 3 ports.

the PPC code can simply generate different fixups so that instead of
creating 3 devices, it creates one, with three ports.

>> If we have to break things, we can at least go for a nice
>> clean design, surely?
>>
>> The ports arent really child devices of the MAC. The MAC
>> just has 3 ports.
> I don't see the difference between those two things.

The ports are at best 'pseudodevices'. Real devices have registers of
their own.

>> We're going to have to maintain a legacy
>> platform_device -> DT bindings hack somewhere anyway,
>> at least until the remaining other users of the driver
>> convert to D-T.
> I don't understand why you describe the method used in
> powerpc as a hack. It was the normal way to introduce
> DT support for platform devices back when it was implemented.

Just because its normal doesn't mean its not a hack :)

> It also had the advantage of not requiring any modifications
> to the generic driver, because it was shared between one
> architecture using DT (powerpc) and one that didn't (ARM).

It /did/ spawn a pretty hideous driver, though...

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-13 10:00                       ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-13 10:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

On 10/08/12 11:49, Arnd Bergmann wrote:
> On Thursday 09 August 2012, Ian Molton wrote:
>>>  The driver
>>> already knows all those offsets and they are always the same
>>> for all variants of mv643xx, right?
>> Yes, but its not clean. And no amount of refactoring is
>> really going to make a nice driver that also fits the ancient
>> (and badly thought out) OF bindings.
> In what way is it badly though out, or not clean? The use of
> underscores in the properties, and the way that the sram
> is configured is problematic, I agree. But The way that
> the three ports are addressed and how the PHY is found
> seems quite clever.

It forces one to load the MDIO driver first, because it maps ALL the
registers for both itself and all the ports, and the MDIO driver has no
way of knowing how many ethernet blocks are present (I have a device
here with two, and another with four). Thats anywhere from 1 to 12
ports, split across 1 to 4 address ranges, and theres a big gap in the
address range between controllers 0,1 and 2,3. *ALL* the devices on the
board are sharing ethernet block 0's MDIO bus. By pure luck it happens
to work, because the blocks 2,3 have an alias of the MDIO registers from
blocks 0,1.

Having the MDIO driver map the ethernet drivers memory is a terrible
solution, IMO. Ethernet drivers should map their own memory, and that
introduces the n-ports-per-block problem, because their address ranges
overlap.

I think the best solution is to make each ethernet block register 3 ports.

the PPC code can simply generate different fixups so that instead of
creating 3 devices, it creates one, with three ports.

>> If we have to break things, we can at least go for a nice
>> clean design, surely?
>>
>> The ports arent really child devices of the MAC. The MAC
>> just has 3 ports.
> I don't see the difference between those two things.

The ports are at best 'pseudodevices'. Real devices have registers of
their own.

>> We're going to have to maintain a legacy
>> platform_device -> DT bindings hack somewhere anyway,
>> at least until the remaining other users of the driver
>> convert to D-T.
> I don't understand why you describe the method used in
> powerpc as a hack. It was the normal way to introduce
> DT support for platform devices back when it was implemented.

Just because its normal doesn't mean its not a hack :)

> It also had the advantage of not requiring any modifications
> to the generic driver, because it was shared between one
> architecture using DT (powerpc) and one that didn't (ARM).

It /did/ spawn a pretty hideous driver, though...

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-13 10:00                       ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-13 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/08/12 11:49, Arnd Bergmann wrote:
> On Thursday 09 August 2012, Ian Molton wrote:
>>>  The driver
>>> already knows all those offsets and they are always the same
>>> for all variants of mv643xx, right?
>> Yes, but its not clean. And no amount of refactoring is
>> really going to make a nice driver that also fits the ancient
>> (and badly thought out) OF bindings.
> In what way is it badly though out, or not clean? The use of
> underscores in the properties, and the way that the sram
> is configured is problematic, I agree. But The way that
> the three ports are addressed and how the PHY is found
> seems quite clever.

It forces one to load the MDIO driver first, because it maps ALL the
registers for both itself and all the ports, and the MDIO driver has no
way of knowing how many ethernet blocks are present (I have a device
here with two, and another with four). Thats anywhere from 1 to 12
ports, split across 1 to 4 address ranges, and theres a big gap in the
address range between controllers 0,1 and 2,3. *ALL* the devices on the
board are sharing ethernet block 0's MDIO bus. By pure luck it happens
to work, because the blocks 2,3 have an alias of the MDIO registers from
blocks 0,1.

Having the MDIO driver map the ethernet drivers memory is a terrible
solution, IMO. Ethernet drivers should map their own memory, and that
introduces the n-ports-per-block problem, because their address ranges
overlap.

I think the best solution is to make each ethernet block register 3 ports.

the PPC code can simply generate different fixups so that instead of
creating 3 devices, it creates one, with three ports.

>> If we have to break things, we can at least go for a nice
>> clean design, surely?
>>
>> The ports arent really child devices of the MAC. The MAC
>> just has 3 ports.
> I don't see the difference between those two things.

The ports are at best 'pseudodevices'. Real devices have registers of
their own.

>> We're going to have to maintain a legacy
>> platform_device -> DT bindings hack somewhere anyway,
>> at least until the remaining other users of the driver
>> convert to D-T.
> I don't understand why you describe the method used in
> powerpc as a hack. It was the normal way to introduce
> DT support for platform devices back when it was implemented.

Just because its normal doesn't mean its not a hack :)

> It also had the advantage of not requiring any modifications
> to the generic driver, because it was shared between one
> architecture using DT (powerpc) and one that didn't (ARM).

It /did/ spawn a pretty hideous driver, though...

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-13 10:00                       ` Ian Molton
  (?)
@ 2012-08-16 16:30                         ` Ian Molton
  -1 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-16 16:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
	devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller

Ping :)

Can we get some consensus on the right approach here? I'm loathe to code
this if its going to be rejected.

I'd prefer the driver to be properly split so we dont have the MDIO
driver mapping the ethernet drivers address spaces, but if thats not
going to be merged, I'm not feeling like doing the work for nothing.

If the driver is to use the overlapping-address mapped-by-the-mdio
scheme, then so be it, but I could do with knowing.

Another point against the latter scheme is that the MDIO driver could
sensibly be used (the block is identical) on the ArmadaXP, which has 4
ethernet blocks rather than two, yet grouped in two pairs with a
discontiguous address range.

I'd like to get this moved along as soon as possible though.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-16 16:30                         ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-16 16:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

Ping :)

Can we get some consensus on the right approach here? I'm loathe to code
this if its going to be rejected.

I'd prefer the driver to be properly split so we dont have the MDIO
driver mapping the ethernet drivers address spaces, but if thats not
going to be merged, I'm not feeling like doing the work for nothing.

If the driver is to use the overlapping-address mapped-by-the-mdio
scheme, then so be it, but I could do with knowing.

Another point against the latter scheme is that the MDIO driver could
sensibly be used (the block is identical) on the ArmadaXP, which has 4
ethernet blocks rather than two, yet grouped in two pairs with a
discontiguous address range.

I'd like to get this moved along as soon as possible though.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-16 16:30                         ` Ian Molton
  0 siblings, 0 replies; 71+ messages in thread
From: Ian Molton @ 2012-08-16 16:30 UTC (permalink / raw)
  To: linux-arm-kernel

Ping :)

Can we get some consensus on the right approach here? I'm loathe to code
this if its going to be rejected.

I'd prefer the driver to be properly split so we dont have the MDIO
driver mapping the ethernet drivers address spaces, but if thats not
going to be merged, I'm not feeling like doing the work for nothing.

If the driver is to use the overlapping-address mapped-by-the-mdio
scheme, then so be it, but I could do with knowing.

Another point against the latter scheme is that the MDIO driver could
sensibly be used (the block is identical) on the ArmadaXP, which has 4
ethernet blocks rather than two, yet grouped in two pairs with a
discontiguous address range.

I'd like to get this moved along as soon as possible though.

-Ian

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-13 10:00                       ` Ian Molton
  (?)
@ 2012-08-17 12:13                         ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-17 12:13 UTC (permalink / raw)
  To: Ian Molton
  Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
	devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller

On Monday 13 August 2012, Ian Molton wrote:
> On 10/08/12 11:49, Arnd Bergmann wrote:
> > On Thursday 09 August 2012, Ian Molton wrote:
> >>>  The driver
> >>> already knows all those offsets and they are always the same
> >>> for all variants of mv643xx, right?
> >> Yes, but its not clean. And no amount of refactoring is
> >> really going to make a nice driver that also fits the ancient
> >> (and badly thought out) OF bindings.
> > In what way is it badly though out, or not clean? The use of
> > underscores in the properties, and the way that the sram
> > is configured is problematic, I agree. But The way that
> > the three ports are addressed and how the PHY is found
> > seems quite clever.
> 
> It forces one to load the MDIO driver first, because it maps ALL the
> registers for both itself and all the ports, and the MDIO driver has no
> way of knowing how many ethernet blocks are present (I have a device
> here with two, and another with four). Thats anywhere from 1 to 12
> ports, split across 1 to 4 address ranges, and theres a big gap in the
> address range between controllers 0,1 and 2,3. *ALL* the devices on the
> board are sharing ethernet block 0's MDIO bus. By pure luck it happens
> to work, because the blocks 2,3 have an alias of the MDIO registers from
> blocks 0,1.
> 
> Having the MDIO driver map the ethernet drivers memory is a terrible
> solution, IMO. Ethernet drivers should map their own memory, and that
> introduces the n-ports-per-block problem, because their address ranges
> overlap.
> 
> I think the best solution is to make each ethernet block register 3 ports.
> 
> the PPC code can simply generate different fixups so that instead of
> creating 3 devices, it creates one, with three ports.

Ok.

> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

I don't object to any device driver changes, but I do want to make
sure that the bindings are sensible and can coexist with the
ones that have been used for the past 5 years.

Maybe you can move the binding for the ethernet parts out of the
marvell.txt file into the place you want to use for the new
bindings and then extend it to cover both the old and the new style.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-17 12:13                         ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-17 12:13 UTC (permalink / raw)
  To: Ian Molton
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

On Monday 13 August 2012, Ian Molton wrote:
> On 10/08/12 11:49, Arnd Bergmann wrote:
> > On Thursday 09 August 2012, Ian Molton wrote:
> >>>  The driver
> >>> already knows all those offsets and they are always the same
> >>> for all variants of mv643xx, right?
> >> Yes, but its not clean. And no amount of refactoring is
> >> really going to make a nice driver that also fits the ancient
> >> (and badly thought out) OF bindings.
> > In what way is it badly though out, or not clean? The use of
> > underscores in the properties, and the way that the sram
> > is configured is problematic, I agree. But The way that
> > the three ports are addressed and how the PHY is found
> > seems quite clever.
> 
> It forces one to load the MDIO driver first, because it maps ALL the
> registers for both itself and all the ports, and the MDIO driver has no
> way of knowing how many ethernet blocks are present (I have a device
> here with two, and another with four). Thats anywhere from 1 to 12
> ports, split across 1 to 4 address ranges, and theres a big gap in the
> address range between controllers 0,1 and 2,3. *ALL* the devices on the
> board are sharing ethernet block 0's MDIO bus. By pure luck it happens
> to work, because the blocks 2,3 have an alias of the MDIO registers from
> blocks 0,1.
> 
> Having the MDIO driver map the ethernet drivers memory is a terrible
> solution, IMO. Ethernet drivers should map their own memory, and that
> introduces the n-ports-per-block problem, because their address ranges
> overlap.
> 
> I think the best solution is to make each ethernet block register 3 ports.
> 
> the PPC code can simply generate different fixups so that instead of
> creating 3 devices, it creates one, with three ports.

Ok.

> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

I don't object to any device driver changes, but I do want to make
sure that the bindings are sensible and can coexist with the
ones that have been used for the past 5 years.

Maybe you can move the binding for the ethernet parts out of the
marvell.txt file into the place you want to use for the new
bindings and then extend it to cover both the old and the new style.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-08-17 12:13                         ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-08-17 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 13 August 2012, Ian Molton wrote:
> On 10/08/12 11:49, Arnd Bergmann wrote:
> > On Thursday 09 August 2012, Ian Molton wrote:
> >>>  The driver
> >>> already knows all those offsets and they are always the same
> >>> for all variants of mv643xx, right?
> >> Yes, but its not clean. And no amount of refactoring is
> >> really going to make a nice driver that also fits the ancient
> >> (and badly thought out) OF bindings.
> > In what way is it badly though out, or not clean? The use of
> > underscores in the properties, and the way that the sram
> > is configured is problematic, I agree. But The way that
> > the three ports are addressed and how the PHY is found
> > seems quite clever.
> 
> It forces one to load the MDIO driver first, because it maps ALL the
> registers for both itself and all the ports, and the MDIO driver has no
> way of knowing how many ethernet blocks are present (I have a device
> here with two, and another with four). Thats anywhere from 1 to 12
> ports, split across 1 to 4 address ranges, and theres a big gap in the
> address range between controllers 0,1 and 2,3. *ALL* the devices on the
> board are sharing ethernet block 0's MDIO bus. By pure luck it happens
> to work, because the blocks 2,3 have an alias of the MDIO registers from
> blocks 0,1.
> 
> Having the MDIO driver map the ethernet drivers memory is a terrible
> solution, IMO. Ethernet drivers should map their own memory, and that
> introduces the n-ports-per-block problem, because their address ranges
> overlap.
> 
> I think the best solution is to make each ethernet block register 3 ports.
> 
> the PPC code can simply generate different fixups so that instead of
> creating 3 devices, it creates one, with three ports.

Ok.

> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

I don't object to any device driver changes, but I do want to make
sure that the bindings are sensible and can coexist with the
ones that have been used for the past 5 years.

Maybe you can move the binding for the ethernet parts out of the
marvell.txt file into the place you want to use for the new
bindings and then extend it to cover both the old and the new style.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-08-16 16:30                         ` Ian Molton
  (?)
@ 2012-09-10 14:22                             ` Arnd Bergmann
  -1 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-09-10 14:22 UTC (permalink / raw)
  To: Ian Molton
  Cc: thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	andrew-g2DYL2Zd6BY, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	ben.dooks-4yDnlxn2s6sWdaTGBSpHTA, dale-1viX+2+OPRFcxvNqPlePQg,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David Miller,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thursday 16 August 2012, Ian Molton wrote:
> Ping :)
> 
> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

Following up on the old discussion, I talked briefly about this
issue with BenH at the kernel summit. The outcome basically is that
it's a bit sad to have incompatible bindings, but it's not the end
of the world,and it's more important to do it right this time.

Just make sure that you use different values for the 'compatible'
strings and then do what you need to get the ARM hardware working.

Ideally, the new binding should be written in a way that powerpc
machines can use the same one, but the existing ones all use
an version of Open Firmware that is not going to get updated
and it's also not too likely that we are going to see new
powerpc machines based on this chip.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-09-10 14:22                             ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-09-10 14:22 UTC (permalink / raw)
  To: Ian Molton
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss, ben.dooks,
	linuxppc-dev, David Miller, linux-arm-kernel

On Thursday 16 August 2012, Ian Molton wrote:
> Ping :)
> 
> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

Following up on the old discussion, I talked briefly about this
issue with BenH at the kernel summit. The outcome basically is that
it's a bit sad to have incompatible bindings, but it's not the end
of the world,and it's more important to do it right this time.

Just make sure that you use different values for the 'compatible'
strings and then do what you need to get the ARM hardware working.

Ideally, the new binding should be written in a way that powerpc
machines can use the same one, but the existing ones all use
an version of Open Firmware that is not going to get updated
and it's also not too likely that we are going to see new
powerpc machines based on this chip.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-09-10 14:22                             ` Arnd Bergmann
  0 siblings, 0 replies; 71+ messages in thread
From: Arnd Bergmann @ 2012-09-10 14:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 16 August 2012, Ian Molton wrote:
> Ping :)
> 
> Can we get some consensus on the right approach here? I'm loathe to code
> this if its going to be rejected.
> 
> I'd prefer the driver to be properly split so we dont have the MDIO
> driver mapping the ethernet drivers address spaces, but if thats not
> going to be merged, I'm not feeling like doing the work for nothing.
> 
> If the driver is to use the overlapping-address mapped-by-the-mdio
> scheme, then so be it, but I could do with knowing.
> 
> Another point against the latter scheme is that the MDIO driver could
> sensibly be used (the block is identical) on the ArmadaXP, which has 4
> ethernet blocks rather than two, yet grouped in two pairs with a
> discontiguous address range.
> 
> I'd like to get this moved along as soon as possible though.

Following up on the old discussion, I talked briefly about this
issue with BenH at the kernel summit. The outcome basically is that
it's a bit sad to have incompatible bindings, but it's not the end
of the world,and it's more important to do it right this time.

Just make sure that you use different values for the 'compatible'
strings and then do what you need to get the ARM hardware working.

Ideally, the new binding should be written in a way that powerpc
machines can use the same one, but the existing ones all use
an version of Open Firmware that is not going to get updated
and it's also not too likely that we are going to see new
powerpc machines based on this chip.

	Arnd

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-09-10 14:22                             ` Arnd Bergmann
  (?)
@ 2012-09-11  6:03                                 ` Benjamin Herrenschmidt
  -1 siblings, 0 replies; 71+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-11  6:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	andrew-g2DYL2Zd6BY, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	ben.dooks-4yDnlxn2s6sWdaTGBSpHTA, dale-1viX+2+OPRFcxvNqPlePQg,
	Ian Molton, David Miller,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> Following up on the old discussion, I talked briefly about this
> issue with BenH at the kernel summit. The outcome basically is that
> it's a bit sad to have incompatible bindings, but it's not the end
> of the world,and it's more important to do it right this time.
> 
> Just make sure that you use different values for the 'compatible'
> strings and then do what you need to get the ARM hardware working.
> 
> Ideally, the new binding should be written in a way that powerpc
> machines can use the same one, but the existing ones all use
> an version of Open Firmware that is not going to get updated
> and it's also not too likely that we are going to see new
> powerpc machines based on this chip.

Right, mostly these machines where the Pegasos. Those came with a fairly
busted variant of Open Firmware which generated a pretty gross
device-tree.

For some reason, the manufacturer of those things was never willing to
fix anything in their firmware (despite the distributor providing
patches etc...), seemingly on the assumption that whatever they were
doing was perfect and operating system people like us didn't matter one
little bit :-)

So I don't care much about it. It would be nice to keep them working
since people in the community still have them but if it goes through
some "compat" code that detects old/broken device-trees and eventually
disappears when we finally drop support, then so be it.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-09-11  6:03                                 ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 71+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-11  6:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: thomas.petazzoni, andrew, netdev, devicetree-discuss,
	linuxppc-dev, ben.dooks, Ian Molton, David Miller,
	linux-arm-kernel

On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> Following up on the old discussion, I talked briefly about this
> issue with BenH at the kernel summit. The outcome basically is that
> it's a bit sad to have incompatible bindings, but it's not the end
> of the world,and it's more important to do it right this time.
> 
> Just make sure that you use different values for the 'compatible'
> strings and then do what you need to get the ARM hardware working.
> 
> Ideally, the new binding should be written in a way that powerpc
> machines can use the same one, but the existing ones all use
> an version of Open Firmware that is not going to get updated
> and it's also not too likely that we are going to see new
> powerpc machines based on this chip.

Right, mostly these machines where the Pegasos. Those came with a fairly
busted variant of Open Firmware which generated a pretty gross
device-tree.

For some reason, the manufacturer of those things was never willing to
fix anything in their firmware (despite the distributor providing
patches etc...), seemingly on the assumption that whatever they were
doing was perfect and operating system people like us didn't matter one
little bit :-)

So I don't care much about it. It would be nice to keep them working
since people in the community still have them but if it goes through
some "compat" code that detects old/broken device-trees and eventually
disappears when we finally drop support, then so be it.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-09-11  6:03                                 ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 71+ messages in thread
From: Benjamin Herrenschmidt @ 2012-09-11  6:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> Following up on the old discussion, I talked briefly about this
> issue with BenH at the kernel summit. The outcome basically is that
> it's a bit sad to have incompatible bindings, but it's not the end
> of the world,and it's more important to do it right this time.
> 
> Just make sure that you use different values for the 'compatible'
> strings and then do what you need to get the ARM hardware working.
> 
> Ideally, the new binding should be written in a way that powerpc
> machines can use the same one, but the existing ones all use
> an version of Open Firmware that is not going to get updated
> and it's also not too likely that we are going to see new
> powerpc machines based on this chip.

Right, mostly these machines where the Pegasos. Those came with a fairly
busted variant of Open Firmware which generated a pretty gross
device-tree.

For some reason, the manufacturer of those things was never willing to
fix anything in their firmware (despite the distributor providing
patches etc...), seemingly on the assumption that whatever they were
doing was perfect and operating system people like us didn't matter one
little bit :-)

So I don't care much about it. It would be nice to keep them working
since people in the community still have them but if it goes through
some "compat" code that detects old/broken device-trees and eventually
disappears when we finally drop support, then so be it.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
  2012-09-11  6:03                                 ` Benjamin Herrenschmidt
  (?)
@ 2012-10-21  1:52                                   ` Jason Cooper
  -1 siblings, 0 replies; 71+ messages in thread
From: Jason Cooper @ 2012-10-21  1:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	andrew-g2DYL2Zd6BY, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	ben.dooks-4yDnlxn2s6sWdaTGBSpHTA, Ian Molton,
	dale-1viX+2+OPRFcxvNqPlePQg, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	David Miller, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Pong.  ;-)

On Tue, Sep 11, 2012 at 04:03:31PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> > Following up on the old discussion, I talked briefly about this
> > issue with BenH at the kernel summit. The outcome basically is that
> > it's a bit sad to have incompatible bindings, but it's not the end
> > of the world,and it's more important to do it right this time.
> > 
> > Just make sure that you use different values for the 'compatible'
> > strings and then do what you need to get the ARM hardware working.
> > 
> > Ideally, the new binding should be written in a way that powerpc
> > machines can use the same one, but the existing ones all use
> > an version of Open Firmware that is not going to get updated
> > and it's also not too likely that we are going to see new
> > powerpc machines based on this chip.
> 
> Right, mostly these machines where the Pegasos. Those came with a fairly
> busted variant of Open Firmware which generated a pretty gross
> device-tree.
> 
> For some reason, the manufacturer of those things was never willing to
> fix anything in their firmware (despite the distributor providing
> patches etc...), seemingly on the assumption that whatever they were
> doing was perfect and operating system people like us didn't matter one
> little bit :-)
> 
> So I don't care much about it. It would be nice to keep them working
> since people in the community still have them but if it goes through
> some "compat" code that detects old/broken device-trees and eventually
> disappears when we finally drop support, then so be it.

Ian,

What is the status of this work?

thx,

Jason.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-10-21  1:52                                   ` Jason Cooper
  0 siblings, 0 replies; 71+ messages in thread
From: Jason Cooper @ 2012-10-21  1:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: thomas.petazzoni, andrew, Arnd Bergmann, netdev,
	devicetree-discuss, ben.dooks, Ian Molton, dale, linuxppc-dev,
	David Miller, linux-arm-kernel

Pong.  ;-)

On Tue, Sep 11, 2012 at 04:03:31PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> > Following up on the old discussion, I talked briefly about this
> > issue with BenH at the kernel summit. The outcome basically is that
> > it's a bit sad to have incompatible bindings, but it's not the end
> > of the world,and it's more important to do it right this time.
> > 
> > Just make sure that you use different values for the 'compatible'
> > strings and then do what you need to get the ARM hardware working.
> > 
> > Ideally, the new binding should be written in a way that powerpc
> > machines can use the same one, but the existing ones all use
> > an version of Open Firmware that is not going to get updated
> > and it's also not too likely that we are going to see new
> > powerpc machines based on this chip.
> 
> Right, mostly these machines where the Pegasos. Those came with a fairly
> busted variant of Open Firmware which generated a pretty gross
> device-tree.
> 
> For some reason, the manufacturer of those things was never willing to
> fix anything in their firmware (despite the distributor providing
> patches etc...), seemingly on the assumption that whatever they were
> doing was perfect and operating system people like us didn't matter one
> little bit :-)
> 
> So I don't care much about it. It would be nice to keep them working
> since people in the community still have them but if it goes through
> some "compat" code that detects old/broken device-trees and eventually
> disappears when we finally drop support, then so be it.

Ian,

What is the status of this work?

thx,

Jason.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
@ 2012-10-21  1:52                                   ` Jason Cooper
  0 siblings, 0 replies; 71+ messages in thread
From: Jason Cooper @ 2012-10-21  1:52 UTC (permalink / raw)
  To: linux-arm-kernel

Pong.  ;-)

On Tue, Sep 11, 2012 at 04:03:31PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-09-10 at 14:22 +0000, Arnd Bergmann wrote:
> > Following up on the old discussion, I talked briefly about this
> > issue with BenH at the kernel summit. The outcome basically is that
> > it's a bit sad to have incompatible bindings, but it's not the end
> > of the world,and it's more important to do it right this time.
> > 
> > Just make sure that you use different values for the 'compatible'
> > strings and then do what you need to get the ARM hardware working.
> > 
> > Ideally, the new binding should be written in a way that powerpc
> > machines can use the same one, but the existing ones all use
> > an version of Open Firmware that is not going to get updated
> > and it's also not too likely that we are going to see new
> > powerpc machines based on this chip.
> 
> Right, mostly these machines where the Pegasos. Those came with a fairly
> busted variant of Open Firmware which generated a pretty gross
> device-tree.
> 
> For some reason, the manufacturer of those things was never willing to
> fix anything in their firmware (despite the distributor providing
> patches etc...), seemingly on the assumption that whatever they were
> doing was perfect and operating system people like us didn't matter one
> little bit :-)
> 
> So I don't care much about it. It would be nice to keep them working
> since people in the community still have them but if it goes through
> some "compat" code that detects old/broken device-trees and eventually
> disappears when we finally drop support, then so be it.

Ian,

What is the status of this work?

thx,

Jason.

^ permalink raw reply	[flat|nested] 71+ messages in thread

end of thread, other threads:[~2012-10-21  2:46 UTC | newest]

Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-07 14:34 [PATCH v3 0/7] mv643xx.c: Add basic device tree support Ian Molton
2012-08-07 14:34 ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 1/7] Initial csb1724 board support (FDT) Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 2/7] mv643xx.c: Remove magic numbers Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 3/7] mv643xx.c: Add basic device tree support Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:56   ` Arnd Bergmann
2012-08-07 14:56     ` Arnd Bergmann
2012-08-07 15:56     ` Ian Molton
2012-08-07 15:56       ` Ian Molton
     [not found]       ` <50213ACB.6040301-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2012-08-07 20:25         ` Arnd Bergmann
2012-08-07 20:25           ` Arnd Bergmann
2012-08-07 20:25           ` Arnd Bergmann
2012-08-07 14:34 ` [PATCH v3 4/7] kirkwood: Add fixups for DT based mv643xx ethernet Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 5/7] csb1724: Enable device tree based mv643xx ethernet support Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 6/7] DT: Convert all kirkwood boards with mv643xx that use DT Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 14:34 ` [PATCH v3 7/7] NET: mv643xx: remove device name macro Ian Molton
2012-08-07 14:34   ` Ian Molton
2012-08-07 23:29 ` [PATCH v3 0/7] mv643xx.c: Add basic device tree support David Miller
2012-08-07 23:29   ` David Miller
2012-08-08  0:31   ` Matt Sealey
2012-08-08  0:31     ` Matt Sealey
2012-08-08  8:16   ` Arnd Bergmann
2012-08-08  8:16     ` Arnd Bergmann
2012-08-08  8:59     ` David Miller
2012-08-08  8:59       ` David Miller
2012-08-08  9:40     ` Ian Molton
2012-08-08  9:40       ` Ian Molton
2012-08-08  9:42       ` Ian Molton
2012-08-08  9:42         ` Ian Molton
2012-08-08 11:51       ` Ian Molton
2012-08-08 11:51         ` Ian Molton
2012-08-08 12:39         ` Arnd Bergmann
2012-08-08 12:39           ` Arnd Bergmann
2012-08-08 13:19           ` Ian Molton
2012-08-08 13:19             ` Ian Molton
2012-08-09 10:59             ` Ian Molton
2012-08-09 10:59               ` Ian Molton
2012-08-09 10:59               ` Ian Molton
2012-08-09 11:43               ` Arnd Bergmann
2012-08-09 11:43                 ` Arnd Bergmann
2012-08-09 11:43                 ` Arnd Bergmann
2012-08-09 15:21                 ` Ian Molton
2012-08-09 15:21                   ` Ian Molton
2012-08-09 15:21                   ` Ian Molton
2012-08-10 10:49                   ` Arnd Bergmann
2012-08-10 10:49                     ` Arnd Bergmann
2012-08-10 10:49                     ` Arnd Bergmann
2012-08-13 10:00                     ` Ian Molton
2012-08-13 10:00                       ` Ian Molton
2012-08-13 10:00                       ` Ian Molton
2012-08-16 16:30                       ` Ian Molton
2012-08-16 16:30                         ` Ian Molton
2012-08-16 16:30                         ` Ian Molton
     [not found]                         ` <502D201E.9030304-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2012-09-10 14:22                           ` Arnd Bergmann
2012-09-10 14:22                             ` Arnd Bergmann
2012-09-10 14:22                             ` Arnd Bergmann
     [not found]                             ` <201209101422.13875.arnd-r2nGTMty4D4@public.gmane.org>
2012-09-11  6:03                               ` Benjamin Herrenschmidt
2012-09-11  6:03                                 ` Benjamin Herrenschmidt
2012-09-11  6:03                                 ` Benjamin Herrenschmidt
2012-10-21  1:52                                 ` Jason Cooper
2012-10-21  1:52                                   ` Jason Cooper
2012-10-21  1:52                                   ` Jason Cooper
2012-08-17 12:13                       ` Arnd Bergmann
2012-08-17 12:13                         ` Arnd Bergmann
2012-08-17 12:13                         ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.