[PATCH v2 0/5] net: cpsw: Support for am335x chip MACIDs

[PATCH v2 0/5] net: cpsw: Support for am335x chip MACIDs

[Markus Pargmann]

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

-- 
1.9.0

[PATCH v2 1/5] net: cpsw: document mac-address being 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 05d660e..c39f077 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -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.
-- 
1.9.0

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

[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.

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 574f49d..1b71067 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];
-- 
1.9.0

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

[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.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../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

diff --git a/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
new file mode 100644
index 0000000..4eb39f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
@@ -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@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@4a100200 {
+		ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
+	};
+
+	cpsw_emac1: slave@4a100300 {
+		ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
+	};
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 53150c2..a8796d6 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -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.
 
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 9cfaab8..fb24937 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -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
diff --git a/drivers/net/ethernet/ti/cpsw-ctrl-macid.c b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
new file mode 100644
index 0000000..3362bed
--- /dev/null
+++ b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
@@ -0,0 +1,138 @@
+/* CPSW Control Module MACID driver
+ *
+ * Copyright (C) 2014 Markus Pargmann <mpa@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include "cpsw.h"
+
+#define AM33XX_CTRL_MAC_LO_REG(id) (0x8 * id)
+#define AM33XX_CTRL_MAC_HI_REG(id) (0x8 * id + 0x4)
+
+struct cpsw_ctrl_macid {
+	struct device *dev;
+	u8 __iomem *ctrl_macid;
+	void (*cpsw_macid_provide)(struct cpsw_ctrl_macid *priv, int slave,
+			u8 *mac_addr);
+};
+
+static void cpsw_ctrl_get_macid(struct cpsw_ctrl_macid *priv, int slave,
+		u8 *mac_addr)
+{
+	u32 macid_lo;
+	u32 macid_hi;
+
+	macid_lo = readl(priv->ctrl_macid + AM33XX_CTRL_MAC_LO_REG(slave));
+	macid_hi = readl(priv->ctrl_macid + AM33XX_CTRL_MAC_HI_REG(slave));
+
+	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;
+}
+
+static struct platform_driver cpsw_ctrl_macid_driver;
+
+static int cpsw_ctrl_macid_match(struct device *dev, void *data)
+{
+	struct device_node *node = (struct device_node *)data;
+
+	return dev->of_node == node &&
+		dev->driver == &cpsw_ctrl_macid_driver.driver;
+}
+
+int cpsw_ctrl_macid_read(struct device_node *np, u8 *mac_addr)
+{
+	struct device *ctrl_dev;
+	struct cpsw_ctrl_macid *priv;
+	struct of_phandle_args args;
+	int ret;
+
+	ret = of_parse_phandle_with_args(np, "ti,mac-address-ctrl",
+			"#ti,mac-address-ctrl-cells", 0, &args);
+	if (ret)
+		return ret;
+
+	if (args.args_count != 1 || args.args[0] < 0 || args.args[0] > 1) {
+		pr_err("Failed to parse ti,mac-address-module phandle because of invalid arguments\n");
+		return -EINVAL;
+	}
+
+	ctrl_dev = bus_find_device(&platform_bus_type, NULL, args.np,
+			cpsw_ctrl_macid_match);
+	priv = dev_get_drvdata(ctrl_dev);
+	of_node_put(args.np);
+	if (priv == NULL)
+		return -EPROBE_DEFER;
+
+	priv->cpsw_macid_provide(priv, args.args[0], mac_addr);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpsw_ctrl_macid_read);
+
+static const struct of_device_id cpsw_ctrl_macid_of_ids[] = {
+	{
+		.compatible	= "ti,am3352-cpsw-ctrl-macid",
+		.data		= cpsw_ctrl_get_macid,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, cpsw_ctrl_macid_of_ids);
+
+static int cpsw_ctrl_macid_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	const struct of_device_id *of_id;
+	struct cpsw_ctrl_macid *priv;
+
+	of_id = of_match_node(cpsw_ctrl_macid_of_ids, pdev->dev.of_node);
+	if (!of_id)
+		return -EINVAL;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "unable to alloc memory for cpsw-ctrl-macid\n");
+		return -ENOMEM;
+	}
+
+	priv->cpsw_macid_provide = of_id->data;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl-macid");
+	priv->ctrl_macid = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->ctrl_macid))
+		return PTR_ERR(priv->ctrl_macid);
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	dev_info(&pdev->dev, "TI CPSW ctrl macid loaded\n");
+	return 0;
+}
+
+static struct platform_driver cpsw_ctrl_macid_driver = {
+	.probe		= cpsw_ctrl_macid_probe,
+	.driver		= {
+		.name	= "cpsw-ctrl-macid",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(cpsw_ctrl_macid_of_ids),
+	},
+};
+
+module_platform_driver(cpsw_ctrl_macid_driver);
+MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.0

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

[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

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 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(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index c39f077..b997b96 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -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.
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 651087b..05f4948 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -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;
 
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 1b71067..222eebe 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -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__ */
-- 
1.9.0

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

[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.

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

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 6d95d3d..5aff257 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -690,11 +690,13 @@
 			cpsw_emac0: slave@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@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@44e10650 {
@@ -702,6 +704,13 @@
 				reg= <0x44e10650 0x4>;
 				reg-names = "gmii-sel";
 			};
+
+			cpsw_ctrl_macid: cpsw-ctrl-macid@44e10630 {
+				compatible = "ti,am3352-cpsw-ctrl-macid";
+				#ti,mac-address-ctrl-cells = <1>;
+				reg = <0x44e10630 0x10>;
+				reg-names = "ctrl-macid";
+			};
 		};
 
 		ocmcram: ocmcram@40300000 {
-- 
1.9.0

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

[Uwe Kleine-König]

Hello Markus,

On Sat, Mar 15, 2014 at 02:07:42PM +0100, Markus Pargmann wrote:
> 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.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  .../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
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> new file mode 100644
> index 0000000..4eb39f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> @@ -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.

> + - 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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Tony Lindgren]

Hi,

* Markus Pargmann <mpa@pengutronix.de> [140315 06:12]:
> 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

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

[Markus Pargmann]

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

Hi,

On Mon, Mar 17, 2014 at 10:11:36AM -0700, Tony Lindgren wrote:
> Hi,
> 
> * Markus Pargmann <mpa@pengutronix.de> [140315 06:12]:
> > 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.

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

Regards,

Markus

> 
> Regards,
> 
> Tony
> 

-- 
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 v2 3/5] net: cpsw: Add control-module macid driver

[Markus Pargmann]

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

Hi Uwe,

On Mon, Mar 17, 2014 at 10:05:08AM +0100, Uwe Kleine-König wrote:
> Hello Markus,
> 
> On Sat, Mar 15, 2014 at 02:07:42PM +0100, Markus Pargmann wrote:
> > 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.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  .../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
> > 
> > diff --git a/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > new file mode 100644
> > index 0000000..4eb39f6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > @@ -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.

> 
> > + - 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
-- 
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 --]