Skip to contentSkip to conversation

net: cpsw: Support for am335x chip MACIDs

PATCH Superseded Markus Pargmann 3 participants 10 messages Mar 15, 2014 – Mar 18, 2014

Subsystems (3)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover net: cpsw: Support for am335x chip MACIDs

Hi,

This series introduces a driver to read and use the MACIDs stored in the am335x control module. These are read-only registers for a unique MACID. At the moment the MACIDs are generated randomly when the mac-address property is not a valid mac address.

In v2 I changed the precedence of mac-address and this driver. This driver is only used when no mac-address was set by the bootloader. This way we can avoid using random MAC addresses. There are other minor style and documentation fixes in v2.

Best regards,

Markus

Markus Pargmann (5):
net: cpsw: document mac-address being optional
net: cpsw: make cpsw.h self-contained
net: cpsw: Add control-module macid driver
net: cpsw: Use cpsw-ctrl-macid driver
arm: dts: am33xx, Add device node for cpsw-ctrl-macid

 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  32 +++++
 Documentation/devicetree/bindings/net/cpsw.txt     |   8 +-
 arch/arm/boot/dts/am33xx.dtsi                      |   9 ++
 drivers/net/ethernet/ti/Kconfig                    |   1 +
 drivers/net/ethernet/ti/Makefile                   |   2 +-
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.c                     |  16 ++-
 drivers/net/ethernet/ti/cpsw.h                     |   3 +
 8 files changed, 204 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
 create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
Signature · 1 line
1.9.0
#1 lore raw

PATCH v2 1/5 net: cpsw: document mac-address being optional

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

---
 Documentation/devicetree/bindings/net/cpsw.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/net/cpsw.txt +1 -1
index 05d660e..c39f077 100644
@@ -30,10 +30,10 @@ Required properties:
 - phy_id		: Specifies slave phy id
 - phy-mode		: The interface between the SoC and the PHY (a string
 			  that of_get_phy_mode() can understand)
-- mac-address		: Specifies slave MAC address
 
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
+- mac-address		: Specifies slave MAC address
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
Signature · 1 line
1.9.0
#2 lore raw

PATCH v2 2/5 net: cpsw: make cpsw.h self-contained

In reply to Markus Pargmann

cpsw.h uses the symbol MII_BUS_ID_SIZE which is defined in <linux/phy.h>. Add the respective #include to not depend on users to include it themselves.

---
 drivers/net/ethernet/ti/cpsw.h | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/ti/cpsw.h +1 -0
index 574f49d..1b71067 100644
@@ -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];
Signature · 1 line
1.9.0
#3 lore raw

PATCH v2 3/5 net: cpsw: Add control-module macid driver

In reply to Markus Pargmann

This driver extracts the hardware macid from the control module of am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw to get the macid from within the processor.

---
 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  32 +++++
 drivers/net/ethernet/ti/Kconfig                    |   1 +
 drivers/net/ethernet/ti/Makefile                   |   2 +-
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 4 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
 create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +32 -0
new file mode 100644
index 0000000..4eb39f6
@@ -0,0 +1,32 @@
+TI CPSW ctrl macid Devicetree bindings
+--------------------------------------
+
+Required properties:
+ - compatible		: Should be "ti,am3352-cpsw-ctrl-macid"
+ - reg			: physical base address and size of the cpsw
+			  registers map
+ - reg-names		: names of the register map given in "reg" node
+ - #ti,mac-address-ctrl-cells	: Should be <1>
+
+When used from cpsw, "ti,mac-address-ctrl" should be a phandle to this device
+node with one argument, 0 or 1 to select the first or second macid
+respectively.
+
+Examples:
+
+	cpsw_ctrl_macid: cpsw-ctrl-macid at 44e10630 {
+		compatible = "ti,am3352-cpsw-ctrl-macid";
+		#ti,mac-address-ctrl-cells = <1>;
+		reg = <0x44e10630 0x16>;
+		reg-names = "ctrl-macid";
+	};
+
+Used in cpsw slave nodes like this:
+
+	cpsw_emac0: slave at 4a100200 {
+		ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
+	};
+
+	cpsw_emac1: slave at 4a100300 {
+		ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
+	};
drivers/net/ethernet/ti/Kconfig +1 -0
index 53150c2..a8796d6 100644
@@ -62,6 +62,7 @@ config TI_CPSW
 	select TI_DAVINCI_CPDMA
 	select TI_DAVINCI_MDIO
 	select TI_CPSW_PHY_SEL
+	select TI_CPSW_CTRL_MACID
 	---help---
 	  This driver supports TI's CPSW Ethernet Switch.
 
drivers/net/ethernet/ti/Makefile +1 -1
index 9cfaab8..fb24937 100644
@@ -8,5 +8,5 @@ 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) += ti_cpsw.o
+obj-$(CONFIG_TI_CPSW) += ti_cpsw.o cpsw-ctrl-macid.o
 ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o
drivers/net/ethernet/ti/cpsw-ctrl-macid.c new file +138 -0

138 lines not on this page · raw message

Signature · 1 line
1.9.0
#4 lore raw

PATCH v2 3/5 net: cpsw: Add control-module macid driver

Hello Markus,

Quoted 20 lines · Markus Pargmann

This driver extracts the hardware macid from the control module of am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw to get the macid from within the processor.

---
 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  32 +++++
 drivers/net/ethernet/ti/Kconfig                    |   1 +
 drivers/net/ethernet/ti/Makefile                   |   2 +-
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 4 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
 create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +0 -0
new file mode 100644
index 0000000..4eb39f6

Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt line 5

Quoted 5 lines
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +5 -0
@@ -0,0 +1,32 @@
+TI CPSW ctrl macid Devicetree bindings
+--------------------------------------
+
+Required properties:
+ - compatible		: Should be "ti,am3352-cpsw-ctrl-macid"

this is called am3352-..., still you add it (in patch 5) to am33xx.dtsi and in the commit log you wrote about am335x. Looks abstruse.

Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt line 9

Quoted 4 lines
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +4 -0
+ - reg			: physical base address and size of the cpsw
+			  registers map
+ - reg-names		: names of the register map given in "reg" node
+ - #ti,mac-address-ctrl-cells	: Should be <1>

Would be sensible to drop this property, or at least let it default to 1 if missing?

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#5 lore raw

PATCH v2 3/5 net: cpsw: Add control-module macid driver

Hi Uwe,

Quoted 17 lines · Uwe Kleine-K?nig

Hello Markus,

Quoted 14 lines · Markus Pargmann

This driver extracts the hardware macid from the control module of am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw to get the macid from within the processor.

---
 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  32 +++++
 drivers/net/ethernet/ti/Kconfig                    |   1 +
 drivers/net/ethernet/ti/Makefile                   |   2 +-
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 4 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
 create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
Quoted 13 lines

Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +5 -0
new file mode 100644
index 0000000..4eb39f6
@@ -0,0 +1,32 @@
+TI CPSW ctrl macid Devicetree bindings
+--------------------------------------
+
+Required properties:
+ - compatible		: Should be "ti,am3352-cpsw-ctrl-macid"

this is called am3352-..., still you add it (in patch 5) to am33xx.dtsi and in the commit log you wrote about am335x. Looks abstruse.

This is of course for the whole am335x series. But as the cpsw phy_sel driver already uses "ti,am3352-cpsw-phy-sel" as compatible, I didn't want to create more confusion about the bindings and stick with a similar compatible pattern.

Quoted 7 lines

Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt line 9

Quoted 5 lines
Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt new file +4 -0
+ - reg			: physical base address and size of the cpsw
+			  registers map
+ - reg-names		: names of the register map given in "reg" node
+ - #ti,mac-address-ctrl-cells	: Should be <1>

Would be sensible to drop this property, or at least let it default to 1 if missing?

I would actually prefer to have this property here. But I will implement a default value for this in the driver.

Thanks,

Markus

Signature · 11 lines
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 |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140318/adb39b09/attachment.sig>
#6 lore raw

PATCH v2 3/5 net: cpsw: Add control-module macid driver

In reply to Markus Pargmann

Hi,

Quoted 3 lines · Markus Pargmann

This driver extracts the hardware macid from the control module of am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw to get the macid from within the processor.

Few things have improved recently :) This can be now implemented in a much cleaner way using regmap against the already defined syscon node.

For an example, see how the MMC PBIAS regulator is using regmap in Linux next:

11469e0bb1 (regulator: add pbias regulator support) cd042fe5c1 (ARM: dts: add pbias dt node)

That avoids the problem of the tinkering with SoC specific registers that belong to another device.

So please update this series for regmap, let's not add more mapping of system control module registers to the drivers.

Regards,

Tony

#7 lore raw

PATCH v2 3/5 net: cpsw: Add control-module macid driver

Hi,

Quoted 14 lines · Tony Lindgren

Hi,

Quoted 3 lines · Markus Pargmann

This driver extracts the hardware macid from the control module of am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw to get the macid from within the processor.

Few things have improved recently :) This can be now implemented in a much cleaner way using regmap against the already defined syscon node.

For an example, see how the MMC PBIAS regulator is using regmap in Linux next:

Quoted 8 lines

11469e0bb1 (regulator: add pbias regulator support) cd042fe5c1 (ARM: dts: add pbias dt node)

That avoids the problem of the tinkering with SoC specific registers that belong to another device.

So please update this series for regmap, let's not add more mapping of system control module registers to the drivers.

Thanks, I will have a look into this and update the series.

Regards,

Markus

Quoted 5 lines

Regards,

Tony

Signature · 11 lines
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 |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140318/fa76d622/attachment.sig>
#8 lore raw

PATCH v2 4/5 net: cpsw: Use cpsw-ctrl-macid driver

In reply to Markus Pargmann

Use ctrl-macid driver to obtain the macids stored in the processor. This is only done when defined in DT.

The internal macid is not used if mac-address is given explicitly. So it does not change the behavior if the bootloader provides a mac address through the mac-address property

---
 Documentation/devicetree/bindings/net/cpsw.txt |  6 ++++++
 drivers/net/ethernet/ti/cpsw.c                 | 16 +++++++++++++---
 drivers/net/ethernet/ti/cpsw.h                 |  2 ++
 3 files changed, 21 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/net/cpsw.txt +6 -0
index c39f077..b997b96 100644
@@ -34,6 +34,12 @@ Required properties:
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
 - mac-address		: Specifies slave MAC address
+- ti,mac-address-ctrl	: When cpsw-ctrl-macid support is compiled-in, this can
+			  be set to a phandle with one argument, see
+			  cpsw-ctrl-macid.txt. If this method fails, cpsw falls
+			  back to a random mac-address. An explicit mac-address
+			  property takes precedence.
+
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
drivers/net/ethernet/ti/cpsw.c +13 -3
index 651087b..05f4948 100644
@@ -1892,8 +1892,15 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		}
 
 		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_ctrl_macid_read(slave_node,
+					slave_data->mac_addr);
+			if (ret == -EPROBE_DEFER)
+				return ret;
+		}
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
 		if (slave_data->phy_if < 0) {
@@ -2038,10 +2045,13 @@ static int cpsw_probe(struct platform_device *pdev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(&pdev->dev);
 
-	if (cpsw_probe_dt(&priv->data, pdev)) {
+	ret = cpsw_probe_dt(&priv->data, pdev);
+	if (ret == -EINVAL) {
 		pr_err("cpsw: platform data missing\n");
 		ret = -ENODEV;
 		goto clean_runtime_disable_ret;
+	} else if (ret) {
+		goto clean_runtime_disable_ret;
 	}
 	data = &priv->data;
 
drivers/net/ethernet/ti/cpsw.h +2 -0
index 1b71067..222eebe 100644
@@ -42,4 +42,6 @@ struct cpsw_platform_data {
 
 void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave);
 
+int cpsw_ctrl_macid_read(struct device_node *np, u8 *mac_addr);
+
 #endif /* __CPSW_H__ */
Signature · 1 line
1.9.0
#9 lore raw

PATCH v2 5/5 arm: dts: am33xx, Add device node for cpsw-ctrl-macid

In reply to Markus Pargmann

Add cpsw-ctrl-macid to the am33xx dtsi file. It does not change the behaviour of boards with a provided mac-address, so it is safe to add it for all boards with this CPU.

---
 arch/arm/boot/dts/am33xx.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am33xx.dtsi +9 -0
index 6d95d3d..5aff257 100644
@@ -690,11 +690,13 @@
 			cpsw_emac0: slave at 4a100200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
 			};
 
 			cpsw_emac1: slave at 4a100300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
 			};
 
 			phy_sel: cpsw-phy-sel at 44e10650 {
@@ -702,6 +704,13 @@
 				reg= <0x44e10650 0x4>;
 				reg-names = "gmii-sel";
 			};
+
+			cpsw_ctrl_macid: cpsw-ctrl-macid at 44e10630 {
+				compatible = "ti,am3352-cpsw-ctrl-macid";
+				#ti,mac-address-ctrl-cells = <1>;
+				reg = <0x44e10630 0x10>;
+				reg-names = "ctrl-macid";
+			};
 		};
 
 		ocmcram: ocmcram at 40300000 {
Signature · 1 line
1.9.0