[PATCH 1/1] net/macb: add DT support

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: Jamie Iles <redacted>
Cc: Nicolas Ferre <redacted>
---
 Documentation/devicetree/bindings/net/macb.txt |   22 ++++++++
 drivers/net/ethernet/cadence/macb.c            |   65 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2b727ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence EMACB
+
+Implemeted on Atmel AT91 & AVR32 SoC
+
+Required properties:
+- compatible : Should be "atmel,macb" for Atmel
+- reg : Address and length of the register set for the device
+- interrupts : Should contain macb interrupt
+- phy-mode : String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii",
+
+Optional properties:
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+	macb0: macb@fffc4000 {
+		compatible = "atmel,macb";
+		reg = <oxfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "mii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a437b46..2c345bc 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -20,6 +20,9 @@
 #include <linux/etherdevice.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 
 #include <mach/board.h>

@@ -81,6 +84,20 @@ static void __init macb_get_hwaddr(struct macb *bp)
 	addr[4] = top & 0xff;
 	addr[5] = (top >> 8) & 0xff;
 
+#ifdef CONFIG_OF
+	/*
+	 * 2) from device tree data
+	 */
+	if (!is_valid_ether_addr(addr)) {
+		struct device_node *np = bp->pdev->dev.of_node;
+		if (np) {
+			const char *mac = of_get_mac_address(np);
+			if (mac)
+				memcpy(addr, mac, sizeof(addr));
+		}
+	}
+#endif
+
 	if (is_valid_ether_addr(addr)) {
 		memcpy(bp->dev->dev_addr, addr, sizeof(addr));
 	} else {

@@ -191,7 +208,6 @@ static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
-	struct eth_platform_data *pdata;
 	int ret;
 
 	phydev = phy_find_first(bp->mii_bus);

@@ -200,14 +216,11 @@ static int macb_mii_probe(struct net_device *dev)
 		return -1;
 	}
 
-	pdata = bp->pdev->dev.platform_data;
 	/* TODO : add pin_irq */
 
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, 0,
-				 pdata && pdata->is_rmii ?
-				 PHY_INTERFACE_MODE_RMII :
-				 PHY_INTERFACE_MODE_MII);
+				 bp->phy_interface);
 	if (ret) {
 		printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
 		return ret;

@@ -1117,6 +1130,30 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id macb_dt_ids[] = {
+	{ .compatible = "atmel,macb" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, macb_dt_ids);
+
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np)
+		return of_get_phy_mode(np);
+
+	return -ENODEV;
+}
+#else
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __init macb_probe(struct platform_device *pdev)
 {
 	struct eth_platform_data *pdata;

@@ -1210,20 +1247,31 @@ static int __init macb_probe(struct platform_device *pdev)
 	macb_writel(bp, NCFGR, config);
 
 	macb_get_hwaddr(bp);
-	pdata = pdev->dev.platform_data;
 
-	if (pdata && pdata->is_rmii)
+	err = macb_get_phy_mode_dt(pdev);
+	if (err < 0) {
+		pdata = pdev->dev.platform_data;
+		if (pdata && pdata->is_rmii)
+			bp->phy_interface = PHY_INTERFACE_MODE_RMII;
+		else
+			bp->phy_interface = PHY_INTERFACE_MODE_MII;
+	} else {
+		bp->phy_interface = err;
+	}
+
+	if (bp->phy_interface == PHY_INTERFACE_MODE_RMII) {
 #if defined(CONFIG_ARCH_AT91)
 		macb_writel(bp, USRIO, (MACB_BIT(RMII) | MACB_BIT(CLKEN)) );
 #else
 		macb_writel(bp, USRIO, 0);
 #endif
-	else
+	} else {
 #if defined(CONFIG_ARCH_AT91)
 		macb_writel(bp, USRIO, MACB_BIT(CLKEN));
 #else
 		macb_writel(bp, USRIO, MACB_BIT(MII));
 #endif
+	}
 
 	bp->tx_pending = DEF_TX_RING_PENDING;
 

@@ -1344,6 +1392,7 @@ static struct platform_driver macb_driver = {
 	.driver		= {
 		.name		= "macb",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(macb_dt_ids),
 	},
 };
 

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d3212f6..8342744 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h

@@ -389,6 +389,8 @@ struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	phy_interface_t		phy_interface;
 };
 
 #endif /* _MACB_H */

-- 
1.7.7

Hi Jean-Christophe,

On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Jamie Iles <redacted>
Cc: Nicolas Ferre <redacted>

This looks OK to me in principle.  I can't easily test this at the 
moment, but as I don't have a DT platform that has the clk framework up 
and running.  A couple of nits/questions inline, but thanks for doing 
this!

Jamie

I think something along the lines of "Binding for the Cadence MACB 
Ethernet controller" rather than listing specific parts might be 
clearer.

+
+Required properties:
+- compatible : Should be "atmel,macb" for Atmel
+- reg : Address and length of the register set for the device
+- interrupts : Should contain macb interrupt
+- phy-mode : String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii",
+
+Optional properties:
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+	macb0: macb@fffc4000 {

Rob pointed out to me a little while ago that the preferred naming from 
the ePAPR document would be:

	macb0: ethernet@fffc4000

so it might be worth being consistent here.

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns 
is the correct stock ticker symbol for Cadence.

I'm a bit conflicted here.  I think we should always use the MAC address 
from the device tree if it is present even if the current MAC address is 
valid.

cdns,macb again.

On 15:58 Fri 18 Nov     , Jamie Iles wrote:

Hi Jean-Christophe,

On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Jamie Iles <redacted>
Cc: Nicolas Ferre <redacted>

This looks OK to me in principle.  I can't easily test this at the 
moment, but as I don't have a DT platform that has the clk framework up 
and running.  A couple of nits/questions inline, but thanks for doing 
this!

Jamie

quoted

---
 Documentation/devicetree/bindings/net/macb.txt |   22 ++++++++
 drivers/net/ethernet/cadence/macb.c            |   65 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2b727ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence EMACB
+
+Implemeted on Atmel AT91 & AVR32 SoC

I think something along the lines of "Binding for the Cadence MACB 
Ethernet controller" rather than listing specific parts might be 
clearer.

I prefer as we will have implementation detail in the binding

quoted

+
+Required properties:
+- compatible : Should be "atmel,macb" for Atmel
+- reg : Address and length of the register set for the device
+- interrupts : Should contain macb interrupt
+- phy-mode : String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii",
+
+Optional properties:
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+	macb0: macb@fffc4000 {

Rob pointed out to me a little while ago that the preferred naming from 
the ePAPR document would be:

	macb0: ethernet@fffc4000

so it might be worth being consistent here.

ok

quoted

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns 
is the correct stock ticker symbol for Cadence.

here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb

quoted

+		reg = <oxfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "mii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a437b46..2c345bc 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -20,6 +20,9 @@
 #include <linux/etherdevice.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 
 #include <mach/board.h>

@@ -81,6 +84,20 @@ static void __init macb_get_hwaddr(struct macb *bp)
 	addr[4] = top & 0xff;
 	addr[5] = (top >> 8) & 0xff;
 
+#ifdef CONFIG_OF
+	/*
+	 * 2) from device tree data
+	 */
+	if (!is_valid_ether_addr(addr)) {
+		struct device_node *np = bp->pdev->dev.of_node;
+		if (np) {
+			const char *mac = of_get_mac_address(np);
+			if (mac)
+				memcpy(addr, mac, sizeof(addr));
+		}
+	}
+#endif

I'm a bit conflicted here.  I think we should always use the MAC address 
from the device tree if it is present even if the current MAC address is 
valid.

if the mac is already programmed in the register we just keep it
I prefer this way if the bootloader set it we keep it

Best Regards,
J.

On Sun, Nov 20, 2011 at 05:47:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

On 15:58 Fri 18 Nov     , Jamie Iles wrote:

quoted

Hi Jean-Christophe,

On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Jamie Iles <redacted>
Cc: Nicolas Ferre <redacted>

This looks OK to me in principle.  I can't easily test this at the 
moment, but as I don't have a DT platform that has the clk framework up 
and running.  A couple of nits/questions inline, but thanks for doing 
this!

Jamie

quoted

---
 Documentation/devicetree/bindings/net/macb.txt |   22 ++++++++
 drivers/net/ethernet/cadence/macb.c            |   65 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2b727ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence EMACB
+
+Implemeted on Atmel AT91 & AVR32 SoC

I think something along the lines of "Binding for the Cadence MACB 
Ethernet controller" rather than listing specific parts might be 
clearer.

I prefer as we will have implementation detail in the binding

I can't see any Atmel specific implementation detail here though so lets 
keep it generic for now.  There isn't a benefit to keeping a list of 
SoC's that the device is implemented in here as it'll only become out of 
date.  We need to make it easy for other vendors to reuse the binding + 
driver.

quoted

quoted

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns 
is the correct stock ticker symbol for Cadence.

here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb

Well if we really can't detect the difference from the revision register 
then we should have "cdns,macb" *and* "atmel,at91-macb" at least then 
where platforms could claim compatibility as:

	compatible = "atmel,at91-macb", "cdns,macb";

If we consider that another vendor integrates the Cadence IP, then it 
makes much more sense to claim compatibility with a Cadence string 
rather than an Atmel one...

quoted

quoted

+		reg = <oxfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "mii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a437b46..2c345bc 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -20,6 +20,9 @@
 #include <linux/etherdevice.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 
 #include <mach/board.h>

@@ -81,6 +84,20 @@ static void __init macb_get_hwaddr(struct macb *bp)
 	addr[4] = top & 0xff;
 	addr[5] = (top >> 8) & 0xff;
 
+#ifdef CONFIG_OF
+	/*
+	 * 2) from device tree data
+	 */
+	if (!is_valid_ether_addr(addr)) {
+		struct device_node *np = bp->pdev->dev.of_node;
+		if (np) {
+			const char *mac = of_get_mac_address(np);
+			if (mac)
+				memcpy(addr, mac, sizeof(addr));
+		}
+	}
+#endif

I'm a bit conflicted here.  I think we should always use the MAC address 
from the device tree if it is present even if the current MAC address is 
valid.

if the mac is already programmed in the register we just keep it
I prefer this way if the bootloader set it we keep it

But I don't think that makes sense - if there is a MAC address in the 
DT, which is an optional property then the DT author must want to set 
the MAC address from the DT.  We should really prefer an explicit 
assignment over an implicit one.

Jamie

On 11/20/2011 06:11 PM, Jamie Iles :

On Sun, Nov 20, 2011 at 05:47:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

On 15:58 Fri 18 Nov     , Jamie Iles wrote:

quoted

Hi Jean-Christophe,

On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
Cc: Jamie Iles <redacted>
Cc: Nicolas Ferre <redacted>

This looks OK to me in principle.  I can't easily test this at the 
moment, but as I don't have a DT platform that has the clk framework up 
and running.  A couple of nits/questions inline, but thanks for doing 
this!

Jamie

quoted

---
 Documentation/devicetree/bindings/net/macb.txt |   22 ++++++++
 drivers/net/ethernet/cadence/macb.c            |   65 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2b727ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence EMACB
+
+Implemeted on Atmel AT91 & AVR32 SoC

I think something along the lines of "Binding for the Cadence MACB 
Ethernet controller" rather than listing specific parts might be 
clearer.

I prefer as we will have implementation detail in the binding

I can't see any Atmel specific implementation detail here though so lets 
keep it generic for now.  There isn't a benefit to keeping a list of 
SoC's that the device is implemented in here as it'll only become out of 
date.  We need to make it easy for other vendors to reuse the binding + 
driver.

Yes, now that Jamie has made the driver generic, we should not advertise
for specific SoC...

quoted

quoted

quoted

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns 
is the correct stock ticker symbol for Cadence.

here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb

No, before comma means "manufacturer".

Well if we really can't detect the difference from the revision register 
then we should have "cdns,macb" *and* "atmel,at91-macb" at least then 
where platforms could claim compatibility as:

	compatible = "atmel,at91-macb", "cdns,macb";

If we consider that another vendor integrates the Cadence IP, then it 
makes much more sense to claim compatibility with a Cadence string 
rather than an Atmel one...

Yes, it seems that you manage to use the revision register to identify
the IP. So here again, maybe the generic compatible string is enough...

quoted

quoted

quoted

+		reg = <oxfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "mii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a437b46..2c345bc 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -20,6 +20,9 @@
 #include <linux/etherdevice.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 
 #include <mach/board.h>

@@ -81,6 +84,20 @@ static void __init macb_get_hwaddr(struct macb *bp)
 	addr[4] = top & 0xff;
 	addr[5] = (top >> 8) & 0xff;
 
+#ifdef CONFIG_OF
+	/*
+	 * 2) from device tree data
+	 */
+	if (!is_valid_ether_addr(addr)) {
+		struct device_node *np = bp->pdev->dev.of_node;
+		if (np) {
+			const char *mac = of_get_mac_address(np);
+			if (mac)
+				memcpy(addr, mac, sizeof(addr));
+		}
+	}
+#endif

I'm a bit conflicted here.  I think we should always use the MAC address 
from the device tree if it is present even if the current MAC address is 
valid.

if the mac is already programmed in the register we just keep it
I prefer this way if the bootloader set it we keep it

But I don't think that makes sense - if there is a MAC address in the 
DT, which is an optional property then the DT author must want to set 
the MAC address from the DT.  We should really prefer an explicit 
assignment over an implicit one.

Yes, that seems sensible.

Best regards,
-- 
Nicolas Ferre

On 11/18/2011 03:29 PM, Jean-Christophe PLAGNIOL-VILLARD :

Ditto with Jamie's comments.

+
+Required properties:
+- compatible : Should be "atmel,macb" for Atmel
+- reg : Address and length of the register set for the device
+- interrupts : Should contain macb interrupt
+- phy-mode : String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii",
+
+Optional properties:
+- local-mac-address : 6 bytes, mac address
+
+Examples:
+
+	macb0: macb@fffc4000 {
+		compatible = "atmel,macb";

Ditto.

Maybe here also, check is_valid_ether_addr(mac): it seems safer.

-- 
Nicolas Ferre

On 11/20/2011 06:11 PM, Jamie Iles :

On Sun, Nov 20, 2011 at 05:47:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

On 15:58 Fri 18 Nov     , Jamie Iles wrote:

quoted

Hi Jean-Christophe,

On Fri, Nov 18, 2011 at 03:29:25PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

allow the DT to pass the mac address and the phy mode

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<redacted>
Cc: Jamie Iles<redacted>
Cc: Nicolas Ferre<redacted>

This looks OK to me in principle.  I can't easily test this at the
moment, but as I don't have a DT platform that has the clk framework up
and running.  A couple of nits/questions inline, but thanks for doing
this!

Jamie

quoted

---
  Documentation/devicetree/bindings/net/macb.txt |   22 ++++++++
  drivers/net/ethernet/cadence/macb.c            |   65 +++++++++++++++++++++---
  drivers/net/ethernet/cadence/macb.h            |    2 +
  3 files changed, 81 insertions(+), 8 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2b727ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence EMACB
+
+Implemeted on Atmel AT91&  AVR32 SoC

I think something along the lines of "Binding for the Cadence MACB
Ethernet controller" rather than listing specific parts might be
clearer.

I prefer as we will have implementation detail in the binding

I can't see any Atmel specific implementation detail here though so lets
keep it generic for now.  There isn't a benefit to keeping a list of
SoC's that the device is implemented in here as it'll only become out of
date.  We need to make it easy for other vendors to reuse the binding +
driver.

quoted

quoted

quoted

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns
is the correct stock ticker symbol for Cadence.

here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb

Well if we really can't detect the difference from the revision register
then we should have "cdns,macb" *and* "atmel,at91-macb" at least then
where platforms could claim compatibility as:

	compatible = "atmel,at91-macb", "cdns,macb";

re-thinking about this I propose that we go for the following compatible 
string for macb:

- compatible: Should be "cdns,<chip>-macb"

And as the first SoC that have embedded an emacb that is compatible with 
current 10/100 AT91 usage is AVR32 at32ap7000... We may end up with 
"cdns,at32ap7000-macb" compatible string. The first ones with different 
synthesis parameters where at91sam9260/3 so I may also add:
"cdns,at91sam9260-macb".
Then you will have to add the first SoC that uses the gigabit version of 
the macb...
What do you think about that?

BTW, "cdns" seems not included in the vendor-prefixes.txt file yet...

Bye,
-- 
Nicolas Ferre

On Fri, Dec 02, 2011 at 04:30:36PM +0100, Nicolas Ferre wrote:

On 11/20/2011 06:11 PM, Jamie Iles :

quoted

On Sun, Nov 20, 2011 at 05:47:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

On 15:58 Fri 18 Nov     , Jamie Iles wrote:

[...]

quoted

quoted

quoted

quoted

+		compatible = "atmel,macb";

This should be "cdns,macb" as it isn't Atmel specific.  I believe cdns
is the correct stock ticker symbol for Cadence.

here I put "atmel,macb" on purpose to specify the difference of the IP between
the soc, in fact it should have been atmel-at91,macb

Well if we really can't detect the difference from the revision register
then we should have "cdns,macb" *and* "atmel,at91-macb" at least then
where platforms could claim compatibility as:

compatible = "atmel,at91-macb", "cdns,macb";

re-thinking about this I propose that we go for the following
compatible string for macb:

- compatible: Should be "cdns,<chip>-macb"

And as the first SoC that have embedded an emacb that is compatible
with current 10/100 AT91 usage is AVR32 at32ap7000... We may end up
with "cdns,at32ap7000-macb" compatible string. The first ones with
different synthesis parameters where at91sam9260/3 so I may also
add:
"cdns,at91sam9260-macb".
Then you will have to add the first SoC that uses the gigabit
version of the macb...
What do you think about that?

Sure, that works for me, though I guess this is a much more general 
thing than this one binding, but that does make sense to me.  I think 
that keeping a general "cdns,macb" _too_ still makes sense though as 
lots of it may well be detectable and it will probably be difficult for 
one SoC vendor to know whether their IP instantiation really is the same 
as another vendors...  Either way I don't have a strong opinion on that.

BTW, "cdns" seems not included in the vendor-prefixes.txt file yet...

No, that one is missing.  If you want to add it then feel free, if not 
I'll add it to my list of patches to do!

Jamie

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <redacted>
Cc: Jamie Iles <redacted>
---
 Documentation/devicetree/bindings/net/macb.txt |   22 +++++++
 drivers/net/ethernet/cadence/macb.c            |   71 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 87 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..7f0b90a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence MACB Ethernet controller
+
+Required properties:
+- compatible: Should be "cdns,[<chip>-]macb"
+  Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
+  Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain macb interrupt
+- phy-mode: String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii", "gmii", "rgmii".
+
+Optional properties:
+- local-mac-address: 6 bytes, mac address
+
+Examples:
+
+	macb0: ethernet@fffc4000 {
+		compatible = "cdns,at32ap7000-macb";
+		reg = <0xfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "rmii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 64d6146..103c6e6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -23,6 +23,8 @@
 #include <linux/platform_data/macb.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 
 #include "macb.h"
 

@@ -191,7 +193,6 @@ static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
-	struct macb_platform_data *pdata;
 	int ret;
 
 	phydev = phy_find_first(bp->mii_bus);

@@ -200,14 +201,11 @@ static int macb_mii_probe(struct net_device *dev)
 		return -1;
 	}
 
-	pdata = bp->pdev->dev.platform_data;
 	/* TODO : add pin_irq */
 
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, 0,
-				 pdata && pdata->is_rmii ?
-				 PHY_INTERFACE_MODE_RMII :
-				 PHY_INTERFACE_MODE_MII);
+				 bp->phy_interface);
 	if (ret) {
 		netdev_err(dev, "Could not attach to PHY\n");
 		return ret;

@@ -1244,6 +1242,50 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id macb_dt_ids[] = {
+	{ .compatible = "cdns,at32ap7000-macb" },
+	{ .compatible = "cdns,at91sam9260-macb" },
+	{ .compatible = "cdns,macb" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, macb_dt_ids);
+
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np)
+		return of_get_phy_mode(np);
+
+	return -ENODEV;
+}
+
+static int __devinit macb_get_hwaddr_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	if (np) {
+		const char *mac = of_get_mac_address(np);
+		if (mac) {
+			memcpy(pdev->dev->dev_addr, mac, ETH_ALEN);
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+#else
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+static int __devinit macb_get_hwaddr_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __init macb_probe(struct platform_device *pdev)
 {
 	struct macb_platform_data *pdata;

@@ -1318,10 +1360,22 @@ static int __init macb_probe(struct platform_device *pdev)
 	config |= macb_dbw(bp);
 	macb_writel(bp, NCFGR, config);
 
-	macb_get_hwaddr(bp);
-	pdata = pdev->dev.platform_data;
+	err = macb_get_hwaddr_dt(pdev);
+	if (err < 0)
+		macb_get_hwaddr(bp);
+
+	err = macb_get_phy_mode_dt(pdev);
+	if (err < 0) {
+		pdata = pdev->dev.platform_data;
+		if (pdata && pdata->is_rmii)
+			bp->phy_interface = PHY_INTERFACE_MODE_RMII;
+		else
+			bp->phy_interface = PHY_INTERFACE_MODE_MII;
+	} else {
+		bp->phy_interface = err;
+	}
 
-	if (pdata && pdata->is_rmii)
+	if (bp->phy_interface == PHY_INTERFACE_MODE_RMII)
 #if defined(CONFIG_ARCH_AT91)
 		macb_or_gem_writel(bp, USRIO, (MACB_BIT(RMII) |
 					       MACB_BIT(CLKEN)));

@@ -1444,6 +1498,7 @@ static struct platform_driver macb_driver = {
 	.driver		= {
 		.name		= "macb",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(macb_dt_ids),
 	},
 };
 

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 1931078..335e288 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h

@@ -532,6 +532,8 @@ struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	phy_interface_t		phy_interface;
 };
 
 static inline bool macb_is_gem(struct macb *bp)

-- 
1.7.5.4

Hi Nicolas,

On Fri, Dec 02, 2011 at 06:14:10PM +0100, Nicolas Ferre wrote:

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <redacted>
Cc: Jamie Iles <redacted>

Looks nice to me.  There's a patch below to add the GEM stuff to the 
binding too if you want to role that in.

Acked-by: Jamie Iles <redacted>

8<----

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 7f0b90a..e09e3fb 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -1,9 +1,11 @@
-* Cadence MACB Ethernet controller
+* Cadence MACB/GEM Ethernet controller
 
 Required properties:
-- compatible: Should be "cdns,[<chip>-]macb"
+- compatible: Should be "cdns,[<chip>-]{macb,gem}"
   Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
+  Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
+  the Cadence GEM, or the generic form "cdns,gem".
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
 - phy-mode: String, operation mode of the PHY interface.

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 103c6e6..89060e6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb" },
 	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,pc302-gem" },
+	{ .compatible = "cdns,gem" },
 	{ /* sentinel */ }
 };

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <redacted>
Cc: Jamie Iles <redacted>
---
v2: modify macb_get_hwaddr_dt() parameter

 Documentation/devicetree/bindings/net/macb.txt |   22 +++++++
 drivers/net/ethernet/cadence/macb.c            |   71 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 87 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..7f0b90a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,22 @@
+* Cadence MACB Ethernet controller
+
+Required properties:
+- compatible: Should be "cdns,[<chip>-]macb"
+  Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
+  Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain macb interrupt
+- phy-mode: String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii", "gmii", "rgmii".
+
+Optional properties:
+- local-mac-address: 6 bytes, mac address
+
+Examples:
+
+	macb0: ethernet@fffc4000 {
+		compatible = "cdns,at32ap7000-macb";
+		reg = <0xfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "rmii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 64d6146..5e75855 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -23,6 +23,8 @@
 #include <linux/platform_data/macb.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 
 #include "macb.h"
 

@@ -191,7 +193,6 @@ static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
-	struct macb_platform_data *pdata;
 	int ret;
 
 	phydev = phy_find_first(bp->mii_bus);

@@ -200,14 +201,11 @@ static int macb_mii_probe(struct net_device *dev)
 		return -1;
 	}
 
-	pdata = bp->pdev->dev.platform_data;
 	/* TODO : add pin_irq */
 
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, 0,
-				 pdata && pdata->is_rmii ?
-				 PHY_INTERFACE_MODE_RMII :
-				 PHY_INTERFACE_MODE_MII);
+				 bp->phy_interface);
 	if (ret) {
 		netdev_err(dev, "Could not attach to PHY\n");
 		return ret;

@@ -1244,6 +1242,50 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id macb_dt_ids[] = {
+	{ .compatible = "cdns,at32ap7000-macb" },
+	{ .compatible = "cdns,at91sam9260-macb" },
+	{ .compatible = "cdns,macb" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, macb_dt_ids);
+
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np)
+		return of_get_phy_mode(np);
+
+	return -ENODEV;
+}
+
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	struct device_node *np = bp->pdev->dev.of_node;
+	if (np) {
+		const char *mac = of_get_mac_address(np);
+		if (mac) {
+			memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+#else
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __init macb_probe(struct platform_device *pdev)
 {
 	struct macb_platform_data *pdata;

@@ -1318,10 +1360,22 @@ static int __init macb_probe(struct platform_device *pdev)
 	config |= macb_dbw(bp);
 	macb_writel(bp, NCFGR, config);
 
-	macb_get_hwaddr(bp);
-	pdata = pdev->dev.platform_data;
+	err = macb_get_hwaddr_dt(bp);
+	if (err < 0)
+		macb_get_hwaddr(bp);
+
+	err = macb_get_phy_mode_dt(pdev);
+	if (err < 0) {
+		pdata = pdev->dev.platform_data;
+		if (pdata && pdata->is_rmii)
+			bp->phy_interface = PHY_INTERFACE_MODE_RMII;
+		else
+			bp->phy_interface = PHY_INTERFACE_MODE_MII;
+	} else {
+		bp->phy_interface = err;
+	}
 
-	if (pdata && pdata->is_rmii)
+	if (bp->phy_interface == PHY_INTERFACE_MODE_RMII)
 #if defined(CONFIG_ARCH_AT91)
 		macb_or_gem_writel(bp, USRIO, (MACB_BIT(RMII) |
 					       MACB_BIT(CLKEN)));

@@ -1444,6 +1498,7 @@ static struct platform_driver macb_driver = {
 	.driver		= {
 		.name		= "macb",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(macb_dt_ids),
 	},
 };
 

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 1931078..335e288 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h

@@ -532,6 +532,8 @@ struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	phy_interface_t		phy_interface;
 };
 
 static inline bool macb_is_gem(struct macb *bp)

-- 
1.7.5.4

Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
enable it in at91sam9m10g45ek board device tree file.

Signed-off-by: Nicolas Ferre <redacted>
---
 arch/arm/boot/dts/at91sam9g45.dtsi     |    7 +++++++
 arch/arm/boot/dts/at91sam9m10g45ek.dts |    6 ++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index e89b1d7..67f94d3 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi

@@ -101,6 +101,13 @@
 				atmel,use-dma-tx;
 				status = "disabled";
 			};
+
+			macb0: ethernet@fffbc000 {
+				compatible = "cdns,at32ap7000-macb", "cdns,macb";
+				reg = <0xfffbc000 0x100>;
+				interrupts = <25 4>;
+				status = "disabled";
+			};
 		};
 	};
 };

diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index 85b34f5..17377a2 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts

@@ -30,6 +30,12 @@
 			usart1: serial@fff90000 {
 				status = "okay";
 			};
+
+			macb0: ethernet@fffbc000 {
+				local-mac-address = [3a 0e 03 04 05 06];
+				phy-mode = "rmii";
+				status = "okay";
+			};
 		};
 	};
 };

-- 
1.7.5.4

On 12/02/2011 06:28 PM, Jamie Iles :

Hi Nicolas,

On Fri, Dec 02, 2011 at 06:14:10PM +0100, Nicolas Ferre wrote:

quoted

From: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre<redacted>
Cc: Jamie Iles<redacted>

Looks nice to me.  There's a patch below to add the GEM stuff to the
binding too if you want to role that in.

Yes, I will push it in a "v3" (I was busy correcting a bug and did not 
see your answer before sending v2...).

BTW, I think we may also modify the MII/RMII selection code for adding 
gigabit selection... but maybe you already have the patches?

Best regards,
-- 
Nicolas Ferre

From: Nicolas Ferre <redacted>
Date: Fri,  2 Dec 2011 18:43:02 +0100

From: Jean-Christophe PLAGNIOL-VILLARD <redacted>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>
[nicolas.ferre@atmel.com: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <redacted>
Cc: Jamie Iles <redacted>
---
v2: modify macb_get_hwaddr_dt() parameter

You'll have to respin these two macb patches, as they don't apply properly
to the net-next tree which is where they should be targetted.

Thanks.

On 18:50 Fri 02 Dec     , Nicolas Ferre wrote:

why?

please drop this is not supposed to be in the dts but updated for each board

Best Regards,
J.

On 12/02/2011 06:58 PM, David Miller :

From: Nicolas Ferre<redacted>
Date: Fri,  2 Dec 2011 18:43:02 +0100

quoted

From: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre<redacted>
Cc: Jamie Iles<redacted>
---
v2: modify macb_get_hwaddr_dt() parameter

You'll have to respin these two macb patches, as they don't apply properly
to the net-next tree which is where they should be targetted.

David,

In fact, the patches are designed to be added on top of Jamie's rework 
of the macb driver that is now in Arnd's "arm-soc" git tree (for-next 
branch). You discussed with him here:

http://www.spinics.net/lists/arm-kernel/msg118717.html

Should I go this path also or wait for this material to be merged and 
update on net-next at this time (after 3.3 merge window...)?

Best regards,
-- 
Nicolas Ferre

On 12/03/2011 06:56 AM, Jean-Christophe PLAGNIOL-VILLARD :

On 18:50 Fri 02 Dec     , Nicolas Ferre wrote:

quoted

Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
enable it in at91sam9m10g45ek board device tree file.

Signed-off-by: Nicolas Ferre<redacted>
---
  arch/arm/boot/dts/at91sam9g45.dtsi     |    7 +++++++
  arch/arm/boot/dts/at91sam9m10g45ek.dts |    6 ++++++
  2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index e89b1d7..67f94d3 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi

@@ -101,6 +101,13 @@
  				atmel,use-dma-tx;
  				status = "disabled";
  			};
+
+			macb0: ethernet@fffbc000 {
+				compatible = "cdns,at32ap7000-macb", "cdns,macb";
+				reg =<0xfffbc000 0x100>;
+				interrupts =<25 4>;

why?

It is the new AIC specification that was reworked in this patch:
https://lkml.org/lkml/2011/12/1/238

quoted

+				status = "disabled";
+			};
  		};
  	};
  };

diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index 85b34f5..17377a2 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts

@@ -30,6 +30,12 @@
  			usart1: serial@fff90000 {
  				status = "okay";
  			};
+
+			macb0: ethernet@fffbc000 {
+				local-mac-address = [3a 0e 03 04 05 06];

please drop this is not supposed to be in the dts but updated for each board

Yes, sure: I kept in because it was allowing me to test. But for sure, 
this should go away...

Bye,
-- 
Nicolas Ferre

On Fri, Dec 02, 2011 at 06:53:58PM +0100, Nicolas Ferre wrote:

On 12/02/2011 06:28 PM, Jamie Iles :

[...]

quoted

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 103c6e6..89060e6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb" },
 	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,pc302-gem" },
+	{ .compatible = "cdns,gem" },
 	{ /* sentinel */ }
 };

BTW, I think we may also modify the MII/RMII selection code for
adding gigabit selection... but maybe you already have the patches?

No, I don't have access to a version of the GEM with gigabit support.  
The platforms I was working on used it as a 10/100 controller, but had 
to use the GEM to get hardware timestamping support.

Jamie

On 12/05/2011 12:48 PM, Jamie Iles :

On Fri, Dec 02, 2011 at 06:53:58PM +0100, Nicolas Ferre wrote:

quoted

On 12/02/2011 06:28 PM, Jamie Iles :

[...]

quoted

quoted

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 103c6e6..89060e6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
  	{ .compatible = "cdns,at32ap7000-macb" },
  	{ .compatible = "cdns,at91sam9260-macb" },
  	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,pc302-gem" },
+	{ .compatible = "cdns,gem" },
  	{ /* sentinel */ }
  };

BTW, I think we may also modify the MII/RMII selection code for
adding gigabit selection... but maybe you already have the patches?

No, I don't have access to a version of the GEM with gigabit support.
The platforms I was working on used it as a 10/100 controller, but had
to use the GEM to get hardware timestamping support.

Ok, Thanks for your feedback. I send a v3 patch now.

Best regards,
-- 
Nicolas Ferre

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <redacted>
[jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org: add "gem" compatibility strings and doc]
Acked-by: Jamie Iles<redacted>
---
v3: add "gem" compatibility strings
v2: modify macb_get_hwaddr_dt() parameter

 Documentation/devicetree/bindings/net/macb.txt |   24 ++++++++
 drivers/net/ethernet/cadence/macb.c            |   73 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 91 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..2e24f05
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -0,0 +1,24 @@
+* Cadence MACB/GEM Ethernet controller
+
+Required properties:
+- compatible: Should be "cdns,[<chip>-]{macb|gem}"
+  Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
+  Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
+  Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
+  the Cadence GEM, or the generic form: "cdns,gem".
+- reg: Address and length of the register set for the device
+- interrupts: Should contain macb interrupt
+- phy-mode: String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii", "gmii", "rgmii".
+
+Optional properties:
+- local-mac-address: 6 bytes, mac address
+
+Examples:
+
+	macb0: ethernet@fffc4000 {
+		compatible = "cdns,at32ap7000-macb";
+		reg = <0xfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "rmii";
+	};

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 64d6146..baf1a0d 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c

@@ -23,6 +23,8 @@
 #include <linux/platform_data/macb.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 
 #include "macb.h"
 

@@ -191,7 +193,6 @@ static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
-	struct macb_platform_data *pdata;
 	int ret;
 
 	phydev = phy_find_first(bp->mii_bus);

@@ -200,14 +201,11 @@ static int macb_mii_probe(struct net_device *dev)
 		return -1;
 	}
 
-	pdata = bp->pdev->dev.platform_data;
 	/* TODO : add pin_irq */
 
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, 0,
-				 pdata && pdata->is_rmii ?
-				 PHY_INTERFACE_MODE_RMII :
-				 PHY_INTERFACE_MODE_MII);
+				 bp->phy_interface);
 	if (ret) {
 		netdev_err(dev, "Could not attach to PHY\n");
 		return ret;

@@ -1244,6 +1242,52 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id macb_dt_ids[] = {
+	{ .compatible = "cdns,at32ap7000-macb" },
+	{ .compatible = "cdns,at91sam9260-macb" },
+	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,pc302-gem" },
+	{ .compatible = "cdns,gem" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, macb_dt_ids);
+
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np)
+		return of_get_phy_mode(np);
+
+	return -ENODEV;
+}
+
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	struct device_node *np = bp->pdev->dev.of_node;
+	if (np) {
+		const char *mac = of_get_mac_address(np);
+		if (mac) {
+			memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+#else
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __init macb_probe(struct platform_device *pdev)
 {
 	struct macb_platform_data *pdata;

@@ -1318,10 +1362,22 @@ static int __init macb_probe(struct platform_device *pdev)
 	config |= macb_dbw(bp);
 	macb_writel(bp, NCFGR, config);
 
-	macb_get_hwaddr(bp);
-	pdata = pdev->dev.platform_data;
+	err = macb_get_hwaddr_dt(bp);
+	if (err < 0)
+		macb_get_hwaddr(bp);
+
+	err = macb_get_phy_mode_dt(pdev);
+	if (err < 0) {
+		pdata = pdev->dev.platform_data;
+		if (pdata && pdata->is_rmii)
+			bp->phy_interface = PHY_INTERFACE_MODE_RMII;
+		else
+			bp->phy_interface = PHY_INTERFACE_MODE_MII;
+	} else {
+		bp->phy_interface = err;
+	}
 
-	if (pdata && pdata->is_rmii)
+	if (bp->phy_interface == PHY_INTERFACE_MODE_RMII)
 #if defined(CONFIG_ARCH_AT91)
 		macb_or_gem_writel(bp, USRIO, (MACB_BIT(RMII) |
 					       MACB_BIT(CLKEN)));

@@ -1444,6 +1500,7 @@ static struct platform_driver macb_driver = {
 	.driver		= {
 		.name		= "macb",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(macb_dt_ids),
 	},
 };
 

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 1931078..335e288 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h

@@ -532,6 +532,8 @@ struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	phy_interface_t		phy_interface;
 };
 
 static inline bool macb_is_gem(struct macb *bp)

-- 
1.7.5.4

Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
enable it in at91sam9m10g45ek board device tree file.

Signed-off-by: Nicolas Ferre <redacted>
---
v3: - form a thread with "macb DT support" patch
    - remove "local-mac-address" from 9m10g45ek.dts file but use it in
      documentation example.

 Documentation/devicetree/bindings/net/macb.txt |    1 +
 arch/arm/boot/dts/at91sam9g45.dtsi             |    7 +++++++
 arch/arm/boot/dts/at91sam9m10g45ek.dts         |    5 +++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 2e24f05..44afa0e 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt

@@ -21,4 +21,5 @@ Examples:
 		reg = <0xfffc4000 0x4000>;
 		interrupts = <21>;
 		phy-mode = "rmii";
+		local-mac-address = [3a 0e 03 04 05 06];
 	};

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index e89b1d7..67f94d3 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi

@@ -101,6 +101,13 @@
 				atmel,use-dma-tx;
 				status = "disabled";
 			};
+
+			macb0: ethernet@fffbc000 {
+				compatible = "cdns,at32ap7000-macb", "cdns,macb";
+				reg = <0xfffbc000 0x100>;
+				interrupts = <25 4>;
+				status = "disabled";
+			};
 		};
 	};
 };

diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index 85b34f5..a387e77 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts

@@ -30,6 +30,11 @@
 			usart1: serial@fff90000 {
 				status = "okay";
 			};
+
+			macb0: ethernet@fffbc000 {
+				phy-mode = "rmii";
+				status = "okay";
+			};
 		};
 	};
 };

-- 
1.7.5.4

On 12:59 Mon 05 Dec     , Nicolas Ferre wrote:

Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
enable it in at91sam9m10g45ek board device tree file.

Signed-off-by: Nicolas Ferre <redacted>

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <redacted>

Best Regards,
J.

On 12/05/2011 12:59 PM, Nicolas Ferre :

From: Jean-Christophe PLAGNIOL-VILLARD<redacted>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<redacted>
[nicolas.ferre@atmel.com: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre<redacted>
[jamie@jamieiles.com: add "gem" compatibility strings and doc]
Acked-by: Jamie Iles<redacted>

David, can I have your acknowledgment on this patch so that I can send 
it through arm-soc git tree? This way it will reside on top of previous 
work by Jamie that is already in arm-soc git (for-next branch).

Thanks, best regards,