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

[PATCH 0/6] 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 or they are set by the bootloader.

A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
board files.

Regards,

Markus


Markus Pargmann (6):
  DT doc: net: cpsw mac-address is optional
  net: cpsw: header, Add missing include
  net: cpsw: Add control-module macid driver
  net: cpsw: Use cpsw-ctrl-macid driver
  arm: dts: am33xx, Add device node for cpsw-ctrl-macid
  arm: dts: am335x beagle bone use processor macids

 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
 Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
 arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
 arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
 arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
 drivers/net/ethernet/ti/Kconfig                    |   8 ++
 drivers/net/ethernet/ti/Makefile                   |   1 +
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
 drivers/net/ethernet/ti/cpsw.h                     |   3 +
 10 files changed, 224 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.8.5.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] 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 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.8.5.1

[PATCH 2/6] 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 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.8.5.1

[PATCH 3/6] 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.

This driver is not used, unless it is defined in DT and referenced by a
cpsw slave with a phandle.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
 drivers/net/ethernet/ti/Kconfig                    |   8 ++
 drivers/net/ethernet/ti/Makefile                   |   1 +
 drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
 4 files changed, 178 insertions(+)
 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..abff2af
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
@@ -0,0 +1,31 @@
+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,cpsw-ctrl-macid	: 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 macid 0 or 1.
+
+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..24819ef 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -56,12 +56,20 @@ config TI_CPSW_PHY_SEL
 	  This driver supports configuring of the phy mode connected to
 	  the CPSW.
 
+config TI_CPSW_CTRL_MACID
+	boolean "TI CPSW internal MACID support"
+	depends on TI_CPSW
+	---help---
+	  This driver supports reading the hardcoded MACID from am33xx
+	  processors control module.
+
 config TI_CPSW
 	tristate "TI CPSW Switch Support"
 	depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
 	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..5a31c2b 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-ctrl-macid.c b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
new file mode 100644
index 0000000..e18c957
--- /dev/null
+++ b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
@@ -0,0 +1,138 @@
+/* CPSW Control Module MACID driver
+ *
+ * Copyright (C) 2013 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_get)(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 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, match);
+	priv = dev_get_drvdata(ctrl_dev);
+	of_node_put(args.np);
+	if (priv == NULL)
+		return -EPROBE_DEFER;
+
+	priv->cpsw_macid_get(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_get = 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.8.5.1

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

[Uwe Kleine-König]

On Wed, Dec 18, 2013 at 05:47:19PM +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.
> 
> This driver is not used, unless it is defined in DT and referenced by a
> cpsw slave with a phandle.
> 
> Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
>  drivers/net/ethernet/ti/Makefile                   |   1 +
>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
>  4 files changed, 178 insertions(+)
>  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..abff2af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> @@ -0,0 +1,31 @@
> +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,cpsw-ctrl-macid	: Should be <1>
#ti,mac-address-ctrl-cells?

> +
> +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 macid 0 or 1.
> +
> +Examples:
> +
> +	cpsw_ctrl_macid: cpsw-ctrl-macid@44e10630 {
> +		compatible = "ti,am3352-cpsw-ctrl-macid";
> +		#ti,mac-address-ctrl-cells = <1>;
> +		reg = <0x44e10630 0x16>;
s/0x16/0x10/

> +		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..24819ef 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -56,12 +56,20 @@ config TI_CPSW_PHY_SEL
>  	  This driver supports configuring of the phy mode connected to
>  	  the CPSW.
>  
> +config TI_CPSW_CTRL_MACID
> +	boolean "TI CPSW internal MACID support"
> +	depends on TI_CPSW
> +	---help---
> +	  This driver supports reading the hardcoded MACID from am33xx
> +	  processors control module.
> +
Would it be nicer to put this after the TI_CPSW definition. (Think
$(make config).)

>  config TI_CPSW
>  	tristate "TI CPSW Switch Support"
>  	depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
>  	select TI_DAVINCI_CPDMA
>  	select TI_DAVINCI_MDIO
>  	select TI_CPSW_PHY_SEL
> +	select TI_CPSW_CTRL_MACID
If TI_CPSW selects TI_CPSW_CTRL_MACID the latter doesn't need to depend
on the former. So this optin is user visible but never
user-(de)selectable. I'd say drop the Kconfig symbol and just add
cpsw-ctrl-macid.o to ti_cpsw-y in the Makefile (or really make it
optional).

>  	---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..5a31c2b 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-ctrl-macid.c b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> new file mode 100644
> index 0000000..e18c957
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> @@ -0,0 +1,138 @@
> +/* CPSW Control Module MACID driver
> + *
> + * Copyright (C) 2013 Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * 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_get)(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 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, match);
> +	priv = dev_get_drvdata(ctrl_dev);
> +	of_node_put(args.np);
> +	if (priv == NULL)
> +		return -EPROBE_DEFER;
> +
> +	priv->cpsw_macid_get(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_get = 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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.5.1
> 
> 

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

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

[Markus Pargmann]

Hi,

On Thu, Feb 13, 2014 at 08:44:31PM +0100, Uwe Kleine-König wrote:
> On Wed, Dec 18, 2013 at 05:47:19PM +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.
> > 
> > This driver is not used, unless it is defined in DT and referenced by a
> > cpsw slave with a phandle.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
> >  drivers/net/ethernet/ti/Kconfig                    |   8 ++
> >  drivers/net/ethernet/ti/Makefile                   |   1 +
> >  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
> >  4 files changed, 178 insertions(+)
> >  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..abff2af
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > @@ -0,0 +1,31 @@
> > +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,cpsw-ctrl-macid	: Should be <1>
> #ti,mac-address-ctrl-cells?

Sounds better, will fix.

> 
> > +
> > +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 macid 0 or 1.
> > +
> > +Examples:
> > +
> > +	cpsw_ctrl_macid: cpsw-ctrl-macid@44e10630 {
> > +		compatible = "ti,am3352-cpsw-ctrl-macid";
> > +		#ti,mac-address-ctrl-cells = <1>;
> > +		reg = <0x44e10630 0x16>;
> s/0x16/0x10/

Thanks, that's a bug, obviously we only have 4, not 5.5 registers.

> 
> > +		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..24819ef 100644
> > --- a/drivers/net/ethernet/ti/Kconfig
> > +++ b/drivers/net/ethernet/ti/Kconfig
> > @@ -56,12 +56,20 @@ config TI_CPSW_PHY_SEL
> >  	  This driver supports configuring of the phy mode connected to
> >  	  the CPSW.
> >  
> > +config TI_CPSW_CTRL_MACID
> > +	boolean "TI CPSW internal MACID support"
> > +	depends on TI_CPSW
> > +	---help---
> > +	  This driver supports reading the hardcoded MACID from am33xx
> > +	  processors control module.
> > +
> Would it be nicer to put this after the TI_CPSW definition. (Think
> $(make config).)

I inserted TI_CPSW_CTRL_MACID here because the other TI_CPSW specific
subdriver (TI_CPSW_PHY_SEL) was above TI_CPSW. But I could change this.

> 
> >  config TI_CPSW
> >  	tristate "TI CPSW Switch Support"
> >  	depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
> >  	select TI_DAVINCI_CPDMA
> >  	select TI_DAVINCI_MDIO
> >  	select TI_CPSW_PHY_SEL
> > +	select TI_CPSW_CTRL_MACID
> If TI_CPSW selects TI_CPSW_CTRL_MACID the latter doesn't need to depend
> on the former. So this optin is user visible but never
> user-(de)selectable. I'd say drop the Kconfig symbol and just add
> cpsw-ctrl-macid.o to ti_cpsw-y in the Makefile (or really make it
> optional).

As this is closely related to the cpsw driver, I think it's better to
make it non-optional and include it in the Makefile.

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

[PATCH 4/6] 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.

Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  5 +++++
 drivers/net/ethernet/ti/cpsw.c                 | 18 ++++++++++++++----
 drivers/net/ethernet/ti/cpsw.h                 |  2 ++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index c39f077..b95c38b 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -34,6 +34,11 @@ 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 compiledin, this can
+			  be set to a phandle with one argument, see
+			  cpsw-ctrl-macid.txt. If this method fails, cpsw falls
+			  back to mac-address or random 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.
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 5120d9c..382d793 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1804,9 +1804,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
 			 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);
+		ret = cpsw_ctrl_macid_read(slave_node, slave_data->mac_addr);
+		if (ret) {
+			if (ret == -EPROBE_DEFER)
+				return ret;
+
+			mac_addr = of_get_mac_address(slave_node);
+			if (mac_addr)
+				memcpy(slave_data->mac_addr, mac_addr,
+						ETH_ALEN);
+		}
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
 
@@ -1946,10 +1953,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.8.5.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] net: cpsw: Use cpsw-ctrl-macid driver

[Uwe Kleine-König]

Hello Markus,

On Wed, Dec 18, 2013 at 05:47:20PM +0100, Markus Pargmann wrote:
> Use ctrl-macid driver to obtain the macids stored in the processor. This
> is only done when defined in DT.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt |  5 +++++
>  drivers/net/ethernet/ti/cpsw.c                 | 18 ++++++++++++++----
>  drivers/net/ethernet/ti/cpsw.h                 |  2 ++
>  3 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index c39f077..b95c38b 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,11 @@ 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 compiledin, this can
> +			  be set to a phandle with one argument, see
> +			  cpsw-ctrl-macid.txt. If this method fails, cpsw falls
> +			  back to mac-address or random 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.
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 5120d9c..382d793 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1804,9 +1804,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>  		snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
>  			 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);
> +		ret = cpsw_ctrl_macid_read(slave_node, slave_data->mac_addr);
> +		if (ret) {
> +			if (ret == -EPROBE_DEFER)
> +				return ret;
> +
> +			mac_addr = of_get_mac_address(slave_node);
> +			if (mac_addr)
> +				memcpy(slave_data->mac_addr, mac_addr,
> +						ETH_ALEN);
> +		}
I'd do it the other way round: Use the contents from an explicit
"mac-address" or "local-mac-address" property (i.e. of_get_mac_address)
and if that doesn't return anything use the mac-address-ctrl as
fallback.

>  
>  		slave_data->phy_if = of_get_phy_mode(slave_node);
>  
> @@ -1946,10 +1953,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__ */
> -- 

-- 
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 4/6] net: cpsw: Use cpsw-ctrl-macid driver

[Markus Pargmann]

Hi Uwe,

On Thu, Feb 13, 2014 at 08:37:02PM +0100, Uwe Kleine-König wrote:
> Hello Markus,
> 
> On Wed, Dec 18, 2013 at 05:47:20PM +0100, Markus Pargmann wrote:
> > Use ctrl-macid driver to obtain the macids stored in the processor. This
> > is only done when defined in DT.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/net/cpsw.txt |  5 +++++
> >  drivers/net/ethernet/ti/cpsw.c                 | 18 ++++++++++++++----
> >  drivers/net/ethernet/ti/cpsw.h                 |  2 ++
> >  3 files changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> > index c39f077..b95c38b 100644
> > --- a/Documentation/devicetree/bindings/net/cpsw.txt
> > +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> > @@ -34,6 +34,11 @@ 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 compiledin, this can
> > +			  be set to a phandle with one argument, see
> > +			  cpsw-ctrl-macid.txt. If this method fails, cpsw falls
> > +			  back to mac-address or random 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.
> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> > index 5120d9c..382d793 100644
> > --- a/drivers/net/ethernet/ti/cpsw.c
> > +++ b/drivers/net/ethernet/ti/cpsw.c
> > @@ -1804,9 +1804,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
> >  		snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
> >  			 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);
> > +		ret = cpsw_ctrl_macid_read(slave_node, slave_data->mac_addr);
> > +		if (ret) {
> > +			if (ret == -EPROBE_DEFER)
> > +				return ret;
> > +
> > +			mac_addr = of_get_mac_address(slave_node);
> > +			if (mac_addr)
> > +				memcpy(slave_data->mac_addr, mac_addr,
> > +						ETH_ALEN);
> > +		}
> I'd do it the other way round: Use the contents from an explicit
> "mac-address" or "local-mac-address" property (i.e. of_get_mac_address)
> and if that doesn't return anything use the mac-address-ctrl as
> fallback.

Yes you are right. In this case this wouldn't even influence any boots
with u-boot which already set the correct mac-address property.

I will fix this.

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

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

[Markus Pargmann]

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

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index f6d8ffe..4e1bf08 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -676,6 +676,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 0x16>;
+				reg-names = "ctrl-macid";
+			};
 		};
 
 		ocmcram: ocmcram@40300000 {
-- 
1.8.5.1

[PATCH 6/6] arm: dts: am335x beagle bone use processor macids

[Markus Pargmann]

Use macids stored in the am335x chip.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 arch/arm/boot/dts/am335x-bone.dts      | 8 ++++++++
 arch/arm/boot/dts/am335x-boneblack.dts | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 94ee427..9b65a62 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -10,6 +10,14 @@
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
 
+&cpsw_emac0 {
+	ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
+};
+
+&cpsw_emac1 {
+	ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
+};
+
 &ldo3_reg {
 	regulator-min-microvolt = <1800000>;
 	regulator-max-microvolt = <3300000>;
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 6b71ad9..f6f0b40 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -10,6 +10,14 @@
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
 
+&cpsw_emac0 {
+	ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
+};
+
+&cpsw_emac1 {
+	ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
+};
+
 &ldo3_reg {
 	regulator-min-microvolt = <1800000>;
 	regulator-max-microvolt = <1800000>;
-- 
1.8.5.1

Re: [PATCH 6/6] arm: dts: am335x beagle bone use processor macids

[Uwe Kleine-König]

Hello,

On Wed, Dec 18, 2013 at 05:47:22PM +0100, Markus Pargmann wrote:
> Use macids stored in the am335x chip.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  arch/arm/boot/dts/am335x-bone.dts      | 8 ++++++++
>  arch/arm/boot/dts/am335x-boneblack.dts | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 94ee427..9b65a62 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -10,6 +10,14 @@
>  #include "am33xx.dtsi"
>  #include "am335x-bone-common.dtsi"
>  
> +&cpsw_emac0 {
> +	ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
> +};
> +
> +&cpsw_emac1 {
> +	ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
> +};
with the mac-address property overwriting the addresses found in the
mac-address-ctrl block as I suggested in reply to the respective patch,
I'd add this property not per machine but directly in am33xx.dtsi.

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 0/6] net: cpsw: Support for am335x chip MACIDs

[Mugunthan V N]

On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> 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 or they are set by the bootloader.
>
> A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
> board files.
>
> Regards,
>
> Markus
>
>
> Markus Pargmann (6):
>   DT doc: net: cpsw mac-address is optional
>   net: cpsw: header, Add missing include
>   net: cpsw: Add control-module macid driver
>   net: cpsw: Use cpsw-ctrl-macid driver
>   arm: dts: am33xx, Add device node for cpsw-ctrl-macid
>   arm: dts: am335x beagle bone use processor macids
>
>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
>  Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
>  arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
>  arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
>  arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
>  drivers/net/ethernet/ti/Makefile                   |   1 +
>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
>  drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
>  drivers/net/ethernet/ti/cpsw.h                     |   3 +
>  10 files changed, 224 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
>
Mac ID is to be filled by U-Boot and this kind of approach is already
rejected in linux-omap list.

If proper ethaddr/eth*addr is populated in U-boot environment variable
then mac-address dt property in ethernet* device nodes will be populated
before boot kernel in U-boot. So I don't think this patch series is
required.

Regards
Mugunthan V N

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

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 2182 bytes --]

On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> > 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 or they are set by the bootloader.
> >
> > A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
> > board files.
> >
> > Regards,
> >
> > Markus
> >
> >
> > Markus Pargmann (6):
> >   DT doc: net: cpsw mac-address is optional
> >   net: cpsw: header, Add missing include
> >   net: cpsw: Add control-module macid driver
> >   net: cpsw: Use cpsw-ctrl-macid driver
> >   arm: dts: am33xx, Add device node for cpsw-ctrl-macid
> >   arm: dts: am335x beagle bone use processor macids
> >
> >  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
> >  Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
> >  arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
> >  arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
> >  arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
> >  drivers/net/ethernet/ti/Kconfig                    |   8 ++
> >  drivers/net/ethernet/ti/Makefile                   |   1 +
> >  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
> >  drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
> >  drivers/net/ethernet/ti/cpsw.h                     |   3 +
> >  10 files changed, 224 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
> >
> Mac ID is to be filled by U-Boot and this kind of approach is already
> rejected in linux-omap list.
> 
> If proper ethaddr/eth*addr is populated in U-boot environment variable
> then mac-address dt property in ethernet* device nodes will be populated
> before boot kernel in U-boot. So I don't think this patch series is
> required.

but will u-boot read MACID from control module ?

-- 
balbi

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mugunthan V N]

On Wednesday 18 December 2013 10:38 PM, Felipe Balbi wrote:
> On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
>> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
>>> 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 or they are set by the bootloader.
>>>
>>> A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
>>> board files.
>>>
>>> Regards,
>>>
>>> Markus
>>>
>>>
>>> Markus Pargmann (6):
>>>   DT doc: net: cpsw mac-address is optional
>>>   net: cpsw: header, Add missing include
>>>   net: cpsw: Add control-module macid driver
>>>   net: cpsw: Use cpsw-ctrl-macid driver
>>>   arm: dts: am33xx, Add device node for cpsw-ctrl-macid
>>>   arm: dts: am335x beagle bone use processor macids
>>>
>>>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
>>>  Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
>>>  arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
>>>  arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
>>>  arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
>>>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
>>>  drivers/net/ethernet/ti/Makefile                   |   1 +
>>>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
>>>  drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
>>>  drivers/net/ethernet/ti/cpsw.h                     |   3 +
>>>  10 files changed, 224 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
>>>
>> Mac ID is to be filled by U-Boot and this kind of approach is already
>> rejected in linux-omap list.
>>
>> If proper ethaddr/eth*addr is populated in U-boot environment variable
>> then mac-address dt property in ethernet* device nodes will be populated
>> before boot kernel in U-boot. So I don't think this patch series is
>> required.
> but will u-boot read MACID from control module ?
>
Yes, U-Boot will read the MACID from control module and if a customer
wants to have his own MACID, U-boot ENV variable ethaddr/eth1addr must
be updated.

Regards
Mugunthan V N

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

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 2636 bytes --]

On Wed, Dec 18, 2013 at 10:40:58PM +0530, Mugunthan V N wrote:
> On Wednesday 18 December 2013 10:38 PM, Felipe Balbi wrote:
> > On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
> >> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> >>> 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 or they are set by the bootloader.
> >>>
> >>> A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
> >>> board files.
> >>>
> >>> Regards,
> >>>
> >>> Markus
> >>>
> >>>
> >>> Markus Pargmann (6):
> >>>   DT doc: net: cpsw mac-address is optional
> >>>   net: cpsw: header, Add missing include
> >>>   net: cpsw: Add control-module macid driver
> >>>   net: cpsw: Use cpsw-ctrl-macid driver
> >>>   arm: dts: am33xx, Add device node for cpsw-ctrl-macid
> >>>   arm: dts: am335x beagle bone use processor macids
> >>>
> >>>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
> >>>  Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
> >>>  arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
> >>>  arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
> >>>  arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
> >>>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
> >>>  drivers/net/ethernet/ti/Makefile                   |   1 +
> >>>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
> >>>  drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
> >>>  drivers/net/ethernet/ti/cpsw.h                     |   3 +
> >>>  10 files changed, 224 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
> >>>
> >> Mac ID is to be filled by U-Boot and this kind of approach is already
> >> rejected in linux-omap list.
> >>
> >> If proper ethaddr/eth*addr is populated in U-boot environment variable
> >> then mac-address dt property in ethernet* device nodes will be populated
> >> before boot kernel in U-boot. So I don't think this patch series is
> >> required.
> > but will u-boot read MACID from control module ?
> >
> Yes, U-Boot will read the MACID from control module and if a customer
> wants to have his own MACID, U-boot ENV variable ethaddr/eth1addr must
> be updated.

cool, then I agree this series shouldn't be applied ;-)

-- 
balbi

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Uwe Kleine-König]

Hello,

On Wed, Dec 18, 2013 at 11:13:01AM -0600, Felipe Balbi wrote:
> On Wed, Dec 18, 2013 at 10:40:58PM +0530, Mugunthan V N wrote:
> > On Wednesday 18 December 2013 10:38 PM, Felipe Balbi wrote:
> > > On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
> > >> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> > >> Mac ID is to be filled by U-Boot and this kind of approach is already
> > >> rejected in linux-omap list.
> > >>
> > >> If proper ethaddr/eth*addr is populated in U-boot environment variable
> > >> then mac-address dt property in ethernet* device nodes will be populated
> > >> before boot kernel in U-boot. So I don't think this patch series is
> > >> required.
> > > but will u-boot read MACID from control module ?
> > >
> > Yes, U-Boot will read the MACID from control module and if a customer
> > wants to have his own MACID, U-boot ENV variable ethaddr/eth1addr must
> > be updated.
> 
> cool, then I agree this series shouldn't be applied ;-)
But even then I'd suggest to take at least patches 1 and 2.

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 0/6] net: cpsw: Support for am335x chip MACIDs

[Markus Pargmann]

Hi,

On Wed, Dec 18, 2013 at 10:40:58PM +0530, Mugunthan V N wrote:
> On Wednesday 18 December 2013 10:38 PM, Felipe Balbi wrote:
> > On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
> >> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> >>> 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 or they are set by the bootloader.
> >>>
> >>> A device node is added in am33xx dtsi and used by the cpsw slaves in the bone
> >>> board files.
> >>>
> >>> Regards,
> >>>
> >>> Markus
> >>>
> >>>
> >>> Markus Pargmann (6):
> >>>   DT doc: net: cpsw mac-address is optional
> >>>   net: cpsw: header, Add missing include
> >>>   net: cpsw: Add control-module macid driver
> >>>   net: cpsw: Use cpsw-ctrl-macid driver
> >>>   arm: dts: am33xx, Add device node for cpsw-ctrl-macid
> >>>   arm: dts: am335x beagle bone use processor macids
> >>>
> >>>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
> >>>  Documentation/devicetree/bindings/net/cpsw.txt     |   7 +-
> >>>  arch/arm/boot/dts/am335x-bone.dts                  |   8 ++
> >>>  arch/arm/boot/dts/am335x-boneblack.dts             |   8 ++
> >>>  arch/arm/boot/dts/am33xx.dtsi                      |   7 ++
> >>>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
> >>>  drivers/net/ethernet/ti/Makefile                   |   1 +
> >>>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
> >>>  drivers/net/ethernet/ti/cpsw.c                     |  18 ++-
> >>>  drivers/net/ethernet/ti/cpsw.h                     |   3 +
> >>>  10 files changed, 224 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
> >>>
> >> Mac ID is to be filled by U-Boot and this kind of approach is already
> >> rejected in linux-omap list.
> >>
> >> If proper ethaddr/eth*addr is populated in U-boot environment variable
> >> then mac-address dt property in ethernet* device nodes will be populated
> >> before boot kernel in U-boot. So I don't think this patch series is
> >> required.
> > but will u-boot read MACID from control module ?
> >
> Yes, U-Boot will read the MACID from control module and if a customer
> wants to have his own MACID, U-boot ENV variable ethaddr/eth1addr must
> be updated.

I think we should not rely on any bootloader to setup the macids
correctly.

U-Boot is not the only bootloader. There are others which may not
support cpsw or don't support devicetree or don't load the cpsw driver
automatically when no ethernet connection is used. Most installed
bootloaders use their local storage to load the kernel, so how can we be
certain that the bootloader added the correct MACIDs to the devicetree?
Why can't the kernel be bootloader independent?


A cpsw slave defined in am33xx.dtsi:

cpsw_emac0: slave@4a100200 {
	/* Filled in by U-Boot */
	mac-address = [ 00 00 00 00 00 00 ];
};

This is not a proper hardware description, only a clear statement that
the kernel depends on U-Boot or any other bootloader that does always
set the mac-address the same way U-Boot does. But that DT is not usable
in anything else than Linux and U-Boot.

The TI reference manual clearly lists the MACID registers in the control
module so we can use that to describe the source of the MACIDs in DT,
independent of U-Boot. Any bootloader can use such a devictree and parse
the correct location in the control module.

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 |