| Message ID | 1394888864-11007-4-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 -- 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
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 >
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
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");
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