[PATCH v3 0/7] net: cpsw: Support for am335x chip MACIDs

[PATCH v3 0/7] net: cpsw: Support for am335x chip MACIDs

[Markus Pargmann]

Hi,

This series adds support to the cpsw driver to read the MACIDs of the am335x
chip and use them as fallback. These addresses are only used if there are no
mac addresses in the devicetree, for example set by a bootloader.

In v3 I removed the previously seperate driver and included the changes into
the main cpsw driver. I now check for a am33xx machine to not make this
operation when running on davinci.

Patches 1-4 are some minor fixes for the cpsw driver.
Patch 5 adds the readout support.
Patches 6 and 7 are for am33xx.dtsi to actually enable this feature.

Best regards,

Markus Pargmann


Markus Pargmann (7):
  DT doc: net: cpsw mac-address is optional
  net: cpsw: Add missing return value
  net: cpsw: header, Add missing include
  net: cpsw: Replace pr_err by dev_err
  net: cpsw: Add am33xx MACID readout
  am33xx: define syscon control module device node
  arm: dts: am33xx, Add syscon phandle to cpsw node

 Documentation/devicetree/bindings/net/cpsw.txt |  6 +++-
 arch/arm/boot/dts/am33xx.dtsi                  |  6 ++++
 drivers/net/ethernet/ti/Kconfig                |  2 ++
 drivers/net/ethernet/ti/Makefile               |  1 +
 drivers/net/ethernet/ti/cpsw.c                 | 49 ++++++++++++++++++++++++--
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 6 files changed, 61 insertions(+), 4 deletions(-)

-- 
2.0.1

[PATCH v3 1/7] DT doc: net: cpsw mac-address is optional

[Markus Pargmann]

mac-address is an optional property. If no mac-address is set, a random
mac-address will be generated.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/net/cpsw.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index ae2b8b7f9c38..107caf174a0e 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -29,10 +29,10 @@ Slave Properties:
 Required properties:
 - phy_id		: Specifies slave phy id
 - phy-mode		: See ethernet.txt file in the same directory
-- mac-address		: See ethernet.txt file in the same directory
 
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
+- mac-address		: See ethernet.txt file in the same directory
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
-- 
2.0.1

[PATCH v3 2/7] net: cpsw: Add missing return value

[Markus Pargmann]

ret is set 0 at this point, so jumping to that error label would result
in a return value of 0. Set ret to -ENOMEM to return a proper error
value.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ff380dac6629..43b2777f8e04 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2043,6 +2043,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	priv->irq_enabled = true;
 	if (!priv->cpts) {
 		dev_err(&pdev->dev, "error allocating cpts\n");
+		ret = -ENOMEM;
 		goto clean_ndev_ret;
 	}
 
-- 
2.0.1

[PATCH v3 3/7] net: cpsw: header, Add missing include

[Markus Pargmann]

"MII_BUS_ID_SIZE" is defined in linux/phy.h which is not included in the
cpsw.h file.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/net/ethernet/ti/cpsw.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 574f49da693f..1b710674630c 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -15,6 +15,7 @@
 #define __CPSW_H__
 
 #include <linux/if_ether.h>
+#include <linux/phy.h>
 
 struct cpsw_slave_data {
 	char		phy_id[MII_BUS_ID_SIZE];
-- 
2.0.1

[PATCH v3 4/7] net: cpsw: Replace pr_err by dev_err

[Markus Pargmann]

Use dev_err instead of pr_err.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 43b2777f8e04..b52df53441b0 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1901,7 +1901,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		mdio = of_find_device_by_node(mdio_node);
 		of_node_put(mdio_node);
 		if (!mdio) {
-			pr_err("Missing mdio platform device\n");
+			dev_err(&pdev->dev, "Missing mdio platform device\n");
 			return -EINVAL;
 		}
 		snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
-- 
2.0.1

[PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Markus Pargmann]

This patch adds a function to get the MACIDs from the am33xx SoC
control module registers which hold unique vendor MACIDs. This is only
used if of_get_mac_address() fails to get a valid mac address.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  4 +++
 drivers/net/ethernet/ti/Kconfig                |  2 ++
 drivers/net/ethernet/ti/Makefile               |  1 +
 drivers/net/ethernet/ti/cpsw.c                 | 46 ++++++++++++++++++++++++--
 4 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 107caf174a0e..33fe8462edf4 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -24,6 +24,8 @@ Optional properties:
 - ti,hwmods		: Must be "cpgmac0"
 - no_bd_ram		: Must be 0 or 1
 - dual_emac		: Specifies Switch to act as Dual EMAC
+- syscon		: Phandle to the system control device node, which is
+			  the control module device of the am33x
 
 Slave Properties:
 Required properties:
@@ -57,6 +59,7 @@ Examples:
 		active_slave = <0>;
 		cpts_clock_mult = <0x80000000>;
 		cpts_clock_shift = <29>;
+		syscon = <&cm>;
 		cpsw_emac0: slave@0 {
 			phy_id = <&davinci_mdio>, <0>;
 			phy-mode = "rgmii-txid";
@@ -85,6 +88,7 @@ Examples:
 		active_slave = <0>;
 		cpts_clock_mult = <0x80000000>;
 		cpts_clock_shift = <29>;
+		syscon = <&cm>;
 		cpsw_emac0: slave@0 {
 			phy_id = <&davinci_mdio>, <0>;
 			phy-mode = "rgmii-txid";
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 53150c25a96b..afaf0196ffd2 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -62,6 +62,8 @@ config TI_CPSW
 	select TI_DAVINCI_CPDMA
 	select TI_DAVINCI_MDIO
 	select TI_CPSW_PHY_SEL
+	select MFD_SYSCON
+	select REGMAP
 	---help---
 	  This driver supports TI's CPSW Ethernet Switch.
 
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 9cfaab8152be..5a31c2b322ee 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o
 obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o
 obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o
 obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o
+obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o
 obj-$(CONFIG_TI_CPSW) += ti_cpsw.o
 ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index b52df53441b0..aa13f68a178c 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -33,6 +33,8 @@
 #include <linux/of_net.h>
 #include <linux/of_device.h>
 #include <linux/if_vlan.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <linux/pinctrl/consumer.h>
 
@@ -1796,6 +1798,39 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
 	slave->port_vlan = data->dual_emac_res_vlan;
 }
 
+#define AM33XX_CTRL_MAC_LO_REG(id) (0x630 + 0x8 * id)
+#define AM33XX_CTRL_MAC_HI_REG(id) (0x630 + 0x8 * id + 0x4)
+
+static int cpsw_am33xx_cm_get_macid(struct device *dev, int slave,
+		u8 *mac_addr)
+{
+	u32 macid_lo;
+	u32 macid_hi;
+	struct regmap *syscon;
+
+	if (!of_machine_is_compatible("ti,am33xx"))
+		return 0;
+
+	syscon = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
+	if (IS_ERR(syscon)) {
+		if (PTR_ERR(syscon) == -ENODEV)
+			return 0;
+		return PTR_ERR(syscon);
+	}
+
+	regmap_read(syscon, AM33XX_CTRL_MAC_LO_REG(slave), &macid_lo);
+	regmap_read(syscon, AM33XX_CTRL_MAC_HI_REG(slave), &macid_hi);
+
+	mac_addr[5] = (macid_lo >> 8) & 0xff;
+	mac_addr[4] = macid_lo & 0xff;
+	mac_addr[3] = (macid_hi >> 24) & 0xff;
+	mac_addr[2] = (macid_hi >> 16) & 0xff;
+	mac_addr[1] = (macid_hi >> 8) & 0xff;
+	mac_addr[0] = macid_hi & 0xff;
+
+	return 0;
+}
+
 static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			 struct platform_device *pdev)
 {
@@ -1908,8 +1943,15 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			 PHY_ID_FMT, mdio->name, phyid);
 
 		mac_addr = of_get_mac_address(slave_node);
-		if (mac_addr)
-			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
+		if (mac_addr) {
+			memcpy(slave_data->mac_addr, mac_addr,
+					ETH_ALEN);
+		} else {
+			ret = cpsw_am33xx_cm_get_macid(&pdev->dev, i,
+					slave_data->mac_addr);
+			if (ret)
+				return ret;
+		}
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
 		if (slave_data->phy_if < 0) {
-- 
2.0.1

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]


> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> index 9cfaab8152be..5a31c2b322ee 100644
> --- a/drivers/net/ethernet/ti/Makefile
> +++ b/drivers/net/ethernet/ti/Makefile
> @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o
>  obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o
>  obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o
>  obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o
> +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o
>  obj-$(CONFIG_TI_CPSW) += ti_cpsw.o
>  ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o

Leftover from your last series?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

Hi,

On Sat, Aug 16, 2014 at 11:46:48AM -0500, Wolfram Sang wrote:
> 
> > diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> > index 9cfaab8152be..5a31c2b322ee 100644
> > --- a/drivers/net/ethernet/ti/Makefile
> > +++ b/drivers/net/ethernet/ti/Makefile
> > @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o
> >  obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o
> >  obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o
> >  obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o
> > +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o
> >  obj-$(CONFIG_TI_CPSW) += ti_cpsw.o
> >  ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o
> 
> Leftover from your last series?

Yes thanks, will remove it for the next version.

Best regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]


> +	mac_addr[5] = (macid_lo >> 8) & 0xff;
> +	mac_addr[4] = macid_lo & 0xff;
> +	mac_addr[3] = (macid_hi >> 24) & 0xff;
> +	mac_addr[2] = (macid_hi >> 16) & 0xff;
> +	mac_addr[1] = (macid_hi >> 8) & 0xff;
> +	mac_addr[0] = macid_hi & 0xff;

That looks twisted, but I assume that you tested it is correct.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Sat, Aug 16, 2014 at 11:53:18AM -0500, Wolfram Sang wrote:
> 
> > +	mac_addr[5] = (macid_lo >> 8) & 0xff;
> > +	mac_addr[4] = macid_lo & 0xff;
> > +	mac_addr[3] = (macid_hi >> 24) & 0xff;
> > +	mac_addr[2] = (macid_hi >> 16) & 0xff;
> > +	mac_addr[1] = (macid_hi >> 8) & 0xff;
> > +	mac_addr[0] = macid_hi & 0xff;
> 
> That looks twisted, but I assume that you tested it is correct.

The registers are twisted that way.

Best regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Mugunthan V N]

On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote:
> +	mac_addr[5] = (macid_lo >> 8) & 0xff;
> +	mac_addr[4] = macid_lo & 0xff;
> +	mac_addr[3] = (macid_hi >> 24) & 0xff;
> +	mac_addr[2] = (macid_hi >> 16) & 0xff;
> +	mac_addr[1] = (macid_hi >> 8) & 0xff;
> +	mac_addr[0] = macid_hi & 0xff;
> +
This will fail incase of DRA74x and DRA72x platforms, please check for
u-boot src for parsing logic as TRM is not out yet. Below is the actual
code for DRA7 platforms for MAC address parsing

mac_addr[0] = (mac_hi & 0xFF0000) >> 16;
mac_addr[1] = (mac_hi & 0xFF00) >> 8;
mac_addr[2] = mac_hi & 0xFF;
mac_addr[3] = (mac_lo & 0xFF0000) >> 16;
mac_addr[4] = (mac_lo & 0xFF00) >> 8;
mac_addr[5] = mac_lo & 0xFF;

Regards
Mugunthan V N

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Steven Rostedt]

On Mon, 18 Aug 2014 23:54:26 +0530
Mugunthan V N <mugunthanvnm@ti.com> wrote:

> On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote:
> > +	mac_addr[5] = (macid_lo >> 8) & 0xff;
> > +	mac_addr[4] = macid_lo & 0xff;
> > +	mac_addr[3] = (macid_hi >> 24) & 0xff;
> > +	mac_addr[2] = (macid_hi >> 16) & 0xff;
> > +	mac_addr[1] = (macid_hi >> 8) & 0xff;
> > +	mac_addr[0] = macid_hi & 0xff;
> > +
> This will fail incase of DRA74x and DRA72x platforms, please check for
> u-boot src for parsing logic as TRM is not out yet. Below is the actual
> code for DRA7 platforms for MAC address parsing
> 
> mac_addr[0] = (mac_hi & 0xFF0000) >> 16;
> mac_addr[1] = (mac_hi & 0xFF00) >> 8;
> mac_addr[2] = mac_hi & 0xFF;
> mac_addr[3] = (mac_lo & 0xFF0000) >> 16;
> mac_addr[4] = (mac_lo & 0xFF00) >> 8;
> mac_addr[5] = mac_lo & 0xFF;
> 

But this fails with my beaglebone white.

I tested Markus's patches and it came up with the same ethaddr that
U-Boot had.

>From U-Boot:

ethaddr=d4:94:a1:8b:ec:78

With Markus's changes:

 eth0      Link encap:Ethernet  HWaddr D4:94:A1:8B:EC:78

But when I changed the code to match what you wrote, I got this:

 eth0      Link encap:Ethernet  HWaddr CE:5A:8B:0E:44:45

but it also gave me:

cpsw 4a100000.ethernet: Random MACID = ce:5a:8b:0e:44:45

which means it failed the valid mac test.

Here's how I implemented your change:

#if 1
        mac_addr[0] = (macid_hi & 0xFF0000) >> 16;
        mac_addr[1] = (macid_hi & 0xFF00) >> 8;
        mac_addr[2] = macid_hi & 0xFF;
        mac_addr[3] = (macid_lo & 0xFF0000) >> 16;
        mac_addr[4] = (macid_lo & 0xFF00) >> 8;
        mac_addr[5] = macid_lo & 0xFF;

#else   
        mac_addr[5] = (macid_lo >> 8) & 0xff;
        mac_addr[4] = macid_lo & 0xff;
        mac_addr[3] = (macid_hi >> 24) & 0xff;
        mac_addr[2] = (macid_hi >> 16) & 0xff;
        mac_addr[1] = (macid_hi >> 8) & 0xff;
        mac_addr[0] = macid_hi & 0xff;
#endif  

Just to be consistent, I updated the code as this too:

        mac_addr[0] = (macid_hi >> 16) & 0xFF;
        mac_addr[1] = (macid_hi >> 8) & 0xFF;
        mac_addr[2] = macid_hi & 0xFF;
        mac_addr[3] = (macid_lo >> 16) & 0xFF;
        mac_addr[4] = (macid_lo >> 8) & 0xFF;
        mac_addr[5] = macid_lo & 0xFF;

With the same affect.

Thus, for this patchset, as is:

Tested-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Mugunthan V N]

On Tuesday 19 August 2014 01:11 AM, Steven Rostedt wrote:
> On Mon, 18 Aug 2014 23:54:26 +0530
> Mugunthan V N <mugunthanvnm@ti.com> wrote:
>
>> On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote:
>>> +	mac_addr[5] = (macid_lo >> 8) & 0xff;
>>> +	mac_addr[4] = macid_lo & 0xff;
>>> +	mac_addr[3] = (macid_hi >> 24) & 0xff;
>>> +	mac_addr[2] = (macid_hi >> 16) & 0xff;
>>> +	mac_addr[1] = (macid_hi >> 8) & 0xff;
>>> +	mac_addr[0] = macid_hi & 0xff;
>>> +
>> This will fail incase of DRA74x and DRA72x platforms, please check for
>> u-boot src for parsing logic as TRM is not out yet. Below is the actual
>> code for DRA7 platforms for MAC address parsing
>>
>> mac_addr[0] = (mac_hi & 0xFF0000) >> 16;
>> mac_addr[1] = (mac_hi & 0xFF00) >> 8;
>> mac_addr[2] = mac_hi & 0xFF;
>> mac_addr[3] = (mac_lo & 0xFF0000) >> 16;
>> mac_addr[4] = (mac_lo & 0xFF00) >> 8;
>> mac_addr[5] = mac_lo & 0xFF;
>>
> But this fails with my beaglebone white.
>
> I tested Markus's patches and it came up with the same ethaddr that
> U-Boot had.
>
> From U-Boot:
>
> ethaddr=d4:94:a1:8b:ec:78
>
> With Markus's changes:
>
>  eth0      Link encap:Ethernet  HWaddr D4:94:A1:8B:EC:78
>
> But when I changed the code to match what you wrote, I got this:
>
>  eth0      Link encap:Ethernet  HWaddr CE:5A:8B:0E:44:45
>
> but it also gave me:
>
> cpsw 4a100000.ethernet: Random MACID = ce:5a:8b:0e:44:45
>
> which means it failed the valid mac test.
>
> Here's how I implemented your change:
>
> #if 1
>         mac_addr[0] = (macid_hi & 0xFF0000) >> 16;
>         mac_addr[1] = (macid_hi & 0xFF00) >> 8;
>         mac_addr[2] = macid_hi & 0xFF;
>         mac_addr[3] = (macid_lo & 0xFF0000) >> 16;
>         mac_addr[4] = (macid_lo & 0xFF00) >> 8;
>         mac_addr[5] = macid_lo & 0xFF;
>
> #else   
>         mac_addr[5] = (macid_lo >> 8) & 0xff;
>         mac_addr[4] = macid_lo & 0xff;
>         mac_addr[3] = (macid_hi >> 24) & 0xff;
>         mac_addr[2] = (macid_hi >> 16) & 0xff;
>         mac_addr[1] = (macid_hi >> 8) & 0xff;
>         mac_addr[0] = macid_hi & 0xff;
> #endif  
>
> Just to be consistent, I updated the code as this too:
>
>         mac_addr[0] = (macid_hi >> 16) & 0xFF;
>         mac_addr[1] = (macid_hi >> 8) & 0xFF;
>         mac_addr[2] = macid_hi & 0xFF;
>         mac_addr[3] = (macid_lo >> 16) & 0xFF;
>         mac_addr[4] = (macid_lo >> 8) & 0xFF;
>         mac_addr[5] = macid_lo & 0xFF;
>
> With the same affect.
>
> Thus, for this patchset, as is:
>
> Tested-by: Steven Rostedt <rostedt@goodmis.org>

This will fail for DRA7xx not in AM33xx

Regards
Mugunthan V N

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Steven Rostedt]

On Tue, 19 Aug 2014 01:28:09 +0530
Mugunthan V N <mugunthanvnm@ti.com> wrote:


> This will fail for DRA7xx not in AM33xx

OK, is there a way to test the difference?

-- Steve

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Javier Martinez Canillas]

Hello Mugunthan,

On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>
>> Thus, for this patchset, as is:
>>
>> Tested-by: Steven Rostedt <rostedt@goodmis.org>
>
> This will fail for DRA7xx not in AM33xx
>

cpsw_am33xx_cm_get_macid() checks for
of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is
not an am33xx. cpsw_probe_dt() only propagates the return value if is
not 0 so this patch does not change the semantics for other SoCs
besides am33xx.

If the driver already fails for DRA7xx that certainly is not this
patch's fault. Of course it would be nice to add support for DRA7xx as
well but I think that could be a follow-up patch and shouldn't be a
blocker to merge this change if is useful for users.

> Regards
> Mugunthan V N
> --

Best regards,
Javier

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]

Hi,

On Tue, Aug 19, 2014 at 12:50:59AM +0200, Javier Martinez Canillas wrote:
> Hello Mugunthan,
> 
> On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> >>
> >> Thus, for this patchset, as is:
> >>
> >> Tested-by: Steven Rostedt <rostedt@goodmis.org>
> >
> > This will fail for DRA7xx not in AM33xx
> >
> 
> cpsw_am33xx_cm_get_macid() checks for
> of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is
> not an am33xx. cpsw_probe_dt() only propagates the return value if is
> not 0 so this patch does not change the semantics for other SoCs
> besides am33xx.

Yes, this patch is only about the am33xx. I don't have the DRA7xx
hardware so I am not able to test on that hardware. Mugunthan, perhaps
you can supply some followup patches for DRA7xx.

Best regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 5/7] net: cpsw: Add am33xx MACID readout

[Mugunthan V N]

On Tuesday 19 August 2014 02:20 PM, Markus Pargmann wrote:
> Hi,
>
> On Tue, Aug 19, 2014 at 12:50:59AM +0200, Javier Martinez Canillas wrote:
>> Hello Mugunthan,
>>
>> On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> Thus, for this patchset, as is:
>>>>
>>>> Tested-by: Steven Rostedt <rostedt@goodmis.org>
>>> This will fail for DRA7xx not in AM33xx
>>>
>> cpsw_am33xx_cm_get_macid() checks for
>> of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is
>> not an am33xx. cpsw_probe_dt() only propagates the return value if is
>> not 0 so this patch does not change the semantics for other SoCs
>> besides am33xx.
> Yes, this patch is only about the am33xx. I don't have the DRA7xx
> hardware so I am not able to test on that hardware. Mugunthan, perhaps
> you can supply some followup patches for DRA7xx.
>
>

I will check on this thursday and update.

Regards
Mugunthan V N

[PATCH v3 6/7] am33xx: define syscon control module device node

[Markus Pargmann]

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4a4e02d0ce9e..cb1113bcc290 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -132,6 +132,11 @@
 			};
 		};
 
+		cm: syscon@44e10000 {
+			compatible = "ti,am33xx-controlmodule", "syscon";
+			reg = <0x44e10000 0x800>;
+		};
+
 		intc: interrupt-controller@48200000 {
 			compatible = "ti,omap2-intc";
 			interrupt-controller;
-- 
2.0.1

[PATCH v3 7/7] arm: dts: am33xx, Add syscon phandle to cpsw node

[Markus Pargmann]

There are 2 MACIDs stored in the control module of the am33xx. These are
read by the cpsw driver if no valid MACID was found in the devicetree.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 arch/arm/boot/dts/am33xx.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index cb1113bcc290..5e9bfb3b9f1d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -692,6 +692,7 @@
 			 */
 			interrupts = <40 41 42 43>;
 			ranges;
+			syscon = <&cm>;
 			status = "disabled";
 
 			davinci_mdio: mdio@4a101000 {
-- 
2.0.1

Re: [PATCH v3 0/7] net: cpsw: Support for am335x chip MACIDs

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

On Sat, Aug 16, 2014 at 05:18:28PM +0200, Markus Pargmann wrote:
> Hi,
> 
> This series adds support to the cpsw driver to read the MACIDs of the am335x
> chip and use them as fallback. These addresses are only used if there are no
> mac addresses in the devicetree, for example set by a bootloader.
> 
> In v3 I removed the previously seperate driver and included the changes into
> the main cpsw driver. I now check for a am33xx machine to not make this
> operation when running on davinci.
> 
> Patches 1-4 are some minor fixes for the cpsw driver.
> Patch 5 adds the readout support.
> Patches 6 and 7 are for am33xx.dtsi to actually enable this feature.

Minor comments, but in general, looks good to me:

Reviewed-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>

Tests on real HW need to wait another 9 days...


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]