[PATCH 0/2] stmac: add glue for NXP LPC18xx family

[PATCH 0/2] stmac: add glue for NXP LPC18xx family

[Joachim Eastwood]

This small patch set adds support for using the stmmac on the NXP
LPC18xx/43xx Cortex-M3/4 MCU family and add binding documentation.

Base support for the family can be found here:
http://marc.info/?l=linux-arm-kernel&m=143016894704253&w=2

DTS changes will be posted once base support, clk support and
reset are upstream. All this has been posted on the relevant
lists and my hope is that it will make it into Linux 4.1.

Below is the boot log from the stmmac driver:
[    2.831037] stmmac - user ID: 0x11, Synopsys ID: 0x36
[    2.837085]  Ring mode enabled
[    2.840559]  DMA HW capability register supported
[    2.845457]  Enhanced/Alternate descriptors
[    2.850866]  Enabled extended descriptors
[    2.855351]  RX Checksum Offload Engine supported (type 2)
[    2.861847]  TX Checksum insertion supported
[    2.867106]  Wake-Up On Lan supported
[    2.873938]  Enable RX Mitigation via HW Watchdog Timer
[    2.923122] libphy: stmmac: probed
[    2.927750] eth0: PHY ID 0007c0f1 at 1 IRQ POLL (stmmac-0:01) active
[   94.735120]  No MAC Management Counters available
[   97.767758] stmmaceth 40010000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off

Joachim Eastwood (2):
  stmac: add dwmac glue for NXP 18xx/43xx family
  doc: dt: add documentation for nxp,lpc1850-dwmac

 .../devicetree/bindings/net/nxp,lpc1850-dwmac.txt  | 20 ++++++
 drivers/net/ethernet/stmicro/stmmac/Makefile       |  3 +-
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c    | 76 ++++++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  1 +
 5 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/nxp,lpc1850-dwmac.txt
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c

-- 
1.8.0

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Joachim Eastwood]

Add support for using the dwmac on NXP LPC18xx and
LPC43xx devices.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/Makefile       |  3 +-
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c    | 76 ++++++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  1 +
 4 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 73c2715a27f3..5c165d8d7004 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -6,7 +6,8 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o	\
 
 obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
 stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o dwmac-sunxi.o	\
-		       dwmac-sti.o dwmac-socfpga.o dwmac-rk.o
+		       dwmac-sti.o dwmac-socfpga.o dwmac-rk.o		\
+		       dwmac-lpc18xx.o
 
 obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
 stmmac-pci-objs:= stmmac_pci.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c
new file mode 100644
index 000000000000..f59c9006dbdd
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c
@@ -0,0 +1,76 @@
+/*
+ * STMAC glue for NXP LPC18xx/LPC43xx Ethernet
+ *
+ * Copyright (C) 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * Based on dwmac-sunxi.c, Copyright (C) 2013 Chen-Yu Tsai
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/regmap.h>
+#include <linux/stmmac.h>
+
+/* Register defines for CREG syscon */
+#define LPC18XX_CREG_CREG6			0x12c
+# define LPC18XX_CREG_CREG6_ETHMODE_MASK	0x7
+# define LPC18XX_CREG_CREG6_ETHMODE_MII		0x0
+# define LPC18XX_CREG_CREG6_ETHMODE_RMII	0x4
+
+struct lpc18xx_dwmac_priv_data {
+	struct regmap *reg;
+	int interface;
+};
+
+static void *lpc18xx_dwmac_setup(struct platform_device *pdev)
+{
+	struct lpc18xx_dwmac_priv_data *dwmac;
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return ERR_PTR(-ENOMEM);
+
+	dwmac->interface = of_get_phy_mode(pdev->dev.of_node);
+	if (dwmac->interface < 0)
+		return ERR_PTR(dwmac->interface);
+
+	dwmac->reg = syscon_regmap_lookup_by_compatible("nxp,lpc1850-creg");
+	if (IS_ERR(dwmac->reg)) {
+		dev_err(&pdev->dev, "Syscon lookup failed\n");
+		return dwmac->reg;
+	}
+
+	return dwmac;
+}
+
+static int lpc18xx_dwmac_init(struct platform_device *pdev, void *priv)
+{
+	struct lpc18xx_dwmac_priv_data *dwmac = priv;
+	u8 ethmode;
+
+	if (dwmac->interface == PHY_INTERFACE_MODE_MII) {
+		ethmode = LPC18XX_CREG_CREG6_ETHMODE_MII;
+	} else if (dwmac->interface == PHY_INTERFACE_MODE_RMII) {
+		ethmode = LPC18XX_CREG_CREG6_ETHMODE_RMII;
+	} else {
+		dev_err(&pdev->dev, "Only MII and RMII mode supported\n");
+		return -EINVAL;
+	}
+
+	regmap_update_bits(dwmac->reg, LPC18XX_CREG_CREG6,
+			   LPC18XX_CREG_CREG6_ETHMODE_MASK, ethmode);
+
+	return 0;
+}
+
+const struct stmmac_of_data lpc18xx_dwmac_data = {
+	.has_gmac = 1,
+	.setup = lpc18xx_dwmac_setup,
+	.init = lpc18xx_dwmac_init,
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 705bbdf93940..1e7f70e1ff63 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -36,6 +36,7 @@ static const struct of_device_id stmmac_dt_ids[] = {
 	{ .compatible = "rockchip,rk3288-gmac", .data = &rk3288_gmac_data},
 	{ .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
 	{ .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
+	{ .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data},
 	{ .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
 	{ .compatible = "st,stih416-dwmac", .data = &stih4xx_dwmac_data},
 	{ .compatible = "st,stid127-dwmac", .data = &stid127_dwmac_data},
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
index 093eb99e5ffd..59fe8fb46a48 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
@@ -19,6 +19,7 @@
 #ifndef __STMMAC_PLATFORM_H__
 #define __STMMAC_PLATFORM_H__
 
+extern const struct stmmac_of_data lpc18xx_dwmac_data;
 extern const struct stmmac_of_data meson6_dwmac_data;
 extern const struct stmmac_of_data sun7i_gmac_data;
 extern const struct stmmac_of_data stih4xx_dwmac_data;
-- 
1.8.0

[PATCH 2/2] doc: dt: add documentation for nxp,lpc1850-dwmac

[Joachim Eastwood]

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 .../devicetree/bindings/net/nxp,lpc1850-dwmac.txt    | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nxp,lpc1850-dwmac.txt

diff --git a/Documentation/devicetree/bindings/net/nxp,lpc1850-dwmac.txt b/Documentation/devicetree/bindings/net/nxp,lpc1850-dwmac.txt
new file mode 100644
index 000000000000..7edba1264f6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nxp,lpc1850-dwmac.txt
@@ -0,0 +1,20 @@
+* NXP LPC1850 GMAC ethernet controller
+
+This device is a platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+Required properties:
+ - compatible:  Should contain "nxp,lpc1850-dwmac"
+
+Examples:
+
+mac: ethernet at 40010000 {
+	compatible = "nxp,lpc1850-dwmac", "snps,dwmac-3.611", "snps,dwmac";
+	reg = <0x40010000 0x2000>;
+	interrupts = <5>;
+	interrupt-names = "macirq";
+	clocks = <&ccu1 CLK_CPU_ETHERNET>;
+	clock-names = "stmmaceth";
+	resets = <&rgu 22>;
+	reset-names = "stmmaceth";
+}
-- 
1.8.0

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Arnd Bergmann]

On Saturday 02 May 2015 18:40:41 Joachim Eastwood wrote:
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -36,6 +36,7 @@ static const struct of_device_id stmmac_dt_ids[] = {
>         { .compatible = "rockchip,rk3288-gmac", .data = &rk3288_gmac_data},
>         { .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
>         { .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
> +       { .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data},
>         { .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
>         { .compatible = "st,stih416-dwmac", .data = &stih4xx_dwmac_data},
>         { .compatible = "st,stid127-dwmac", .data = &stid127_dwmac_data},
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
> 

Any chance you could turn this around and do the probing in the normal
order, with a platform driver that registers to your compatible string
and calls into a common base module?

Unfortunately, something went wrong when we first started adding platform
specific hacks to the driver. I tried to fix it up back then, and IIRC
it was agreed that it should be changed but my patches for some reason
missed out on getting merged and the mistake propagated afterwards.

It should be fairly straightforward to split the probe function
into two and export a function that takes a device and a stmmac_of_data
pointer as arguments, and declare a module_platform_driver in your
code.

	Arnd

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Joachim Eastwood]

Hi Arnd,

On 2 May 2015 at 21:31, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 02 May 2015 18:40:41 Joachim Eastwood wrote:
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -36,6 +36,7 @@ static const struct of_device_id stmmac_dt_ids[] = {
>>         { .compatible = "rockchip,rk3288-gmac", .data = &rk3288_gmac_data},
>>         { .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
>>         { .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
>> +       { .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data},
>>         { .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
>>         { .compatible = "st,stih416-dwmac", .data = &stih4xx_dwmac_data},
>>         { .compatible = "st,stid127-dwmac", .data = &stid127_dwmac_data},
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>>
>
> Any chance you could turn this around and do the probing in the normal
> order, with a platform driver that registers to your compatible string
> and calls into a common base module?

I can see what I can come up with.

> Unfortunately, something went wrong when we first started adding platform
> specific hacks to the driver. I tried to fix it up back then, and IIRC
> it was agreed that it should be changed but my patches for some reason
> missed out on getting merged and the mistake propagated afterwards.

Do you happen to have a link to these patches so I could take a look?

> It should be fairly straightforward to split the probe function
> into two and export a function that takes a device and a stmmac_of_data
> pointer as arguments, and declare a module_platform_driver in your
> code.

regards,
Joachim Eastwood

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Joachim Eastwood]

>On Saturday 02 May 2015 18:40:41 Joachim Eastwood wrote:
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -36,6 +36,7 @@ static const struct of_device_id stmmac_dt_ids[] = {
>>         { .compatible = "rockchip,rk3288-gmac", .data = &rk3288_gmac_data},
>>         { .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
>>         { .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
>> +       { .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data},
>>         { .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
>>         { .compatible = "st,stih416-dwmac", .data = &stih4xx_dwmac_data},
>>         { .compatible = "st,stid127-dwmac", .data = &stid127_dwmac_data},
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>>
>
>Any chance you could turn this around and do the probing in the normal
>order, with a platform driver that registers to your compatible string
>and calls into a common base module?
>
>Unfortunately, something went wrong when we first started adding platform
>specific hacks to the driver. I tried to fix it up back then, and IIRC
>it was agreed that it should be changed but my patches for some reason
>missed out on getting merged and the mistake propagated afterwards.
>
>It should be fairly straightforward to split the probe function
>into two and export a function that takes a device and a stmmac_of_data
>pointer as arguments, and declare a module_platform_driver in your
>code.

How about something like the patch below. All it does is to play a
little trick with the of_match_device in the dt config function and
export the probe/remove/pm stuff for the driver.

The dwmac-lpc18xx would then become something like this:

static const struct stmmac_of_data lpc18xx_dwmac_data = {
	.has_gmac = 1,
	.setup = lpc18xx_dwmac_setup,
	.init = lpc18xx_dwmac_init,
};

static const struct of_device_id lpc18xx_dwmac_match[] = {
	{ .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data },
	{ }
};
MODULE_DEVICE_TABLE(of, lpc18xx_dwmac_match);

static struct platform_driver lpc18xx_dwmac_driver = {
	.probe  = stmmac_pltfr_probe,
	.remove = stmmac_pltfr_remove,
	.driver = {
		.name           = "lpc18xx-dwmac",
		.pm             = &stmmac_pltfr_pm_ops,
		.of_match_table = lpc18xx_dwmac_match,
	},
};
module_platform_driver(lpc18xx_dwmac_driver);

All though this seem to work, stmmac_platform.c could benefit from
some refactoring. But this patch and then fixing the other DT users
could be a first step.

regards,
Joachim Eastwood

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 14 +++++++++-----
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h |  4 ++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ccfa4fa02f6a..a7d8ae081b24 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -129,11 +129,12 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
 	struct device_node *np = pdev->dev.of_node;
 	struct stmmac_dma_cfg *dma_cfg;
 	const struct of_device_id *device;
+	struct device *dev = &pdev->dev;
 
 	if (!np)
 		return -ENODEV;
 
-	device = of_match_device(stmmac_dt_ids, &pdev->dev);
+	device = of_match_device(dev->driver->of_match_table, dev);
 	if (!device)
 		return -ENODEV;
 
@@ -264,7 +265,7 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
  * the necessary platform resources, invoke custom helper (if required) and
  * invoke the main probe function.
  */
-static int stmmac_pltfr_probe(struct platform_device *pdev)
+int stmmac_pltfr_probe(struct platform_device *pdev)
 {
 	int ret = 0;
 	struct resource *res;
@@ -370,6 +371,7 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(stmmac_pltfr_probe);
 
 /**
  * stmmac_pltfr_remove
@@ -377,7 +379,7 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
  * Description: this function calls the main to free the net resources
  * and calls the platforms hook and release the resources (e.g. mem).
  */
-static int stmmac_pltfr_remove(struct platform_device *pdev)
+int stmmac_pltfr_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
@@ -391,6 +393,7 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(stmmac_pltfr_remove);
 
 #ifdef CONFIG_PM_SLEEP
 /**
@@ -434,8 +437,9 @@ static int stmmac_pltfr_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-static SIMPLE_DEV_PM_OPS(stmmac_pltfr_pm_ops,
-			 stmmac_pltfr_suspend, stmmac_pltfr_resume);
+SIMPLE_DEV_PM_OPS(stmmac_pltfr_pm_ops, stmmac_pltfr_suspend,
+				       stmmac_pltfr_resume);
+EXPORT_SYMBOL_GPL(stmmac_pltfr_pm_ops);
 
 static struct platform_driver stmmac_pltfr_driver = {
 	.probe = stmmac_pltfr_probe,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
index 59fe8fb46a48..5be0b101bffd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
@@ -19,6 +19,10 @@
 #ifndef __STMMAC_PLATFORM_H__
 #define __STMMAC_PLATFORM_H__
 
+int stmmac_pltfr_probe(struct platform_device *pdev);
+int stmmac_pltfr_remove(struct platform_device *pdev);
+extern const struct dev_pm_ops stmmac_pltfr_pm_ops;
+
 extern const struct stmmac_of_data lpc18xx_dwmac_data;
 extern const struct stmmac_of_data meson6_dwmac_data;
 extern const struct stmmac_of_data sun7i_gmac_data;
-- 
1.8.0

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Arnd Bergmann]

On Monday 04 May 2015 00:14:11 Joachim Eastwood wrote:
=> >It should be fairly straightforward to split the probe function
> >into two and export a function that takes a device and a stmmac_of_data
> >pointer as arguments, and declare a module_platform_driver in your
> >code.
> 
> How about something like the patch below. All it does is to play a
> little trick with the of_match_device in the dt config function and
> export the probe/remove/pm stuff for the driver.
> 
> The dwmac-lpc18xx would then become something like this:
> 
> static const struct stmmac_of_data lpc18xx_dwmac_data = {
> 	.has_gmac = 1,
> 	.setup = lpc18xx_dwmac_setup,
> 	.init = lpc18xx_dwmac_init,
> };
> 
> static const struct of_device_id lpc18xx_dwmac_match[] = {
> 	{ .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data },
> 	{ }
> };
> MODULE_DEVICE_TABLE(of, lpc18xx_dwmac_match);
> 
> static struct platform_driver lpc18xx_dwmac_driver = {
> 	.probe  = stmmac_pltfr_probe,
> 	.remove = stmmac_pltfr_remove,
> 	.driver = {
> 		.name           = "lpc18xx-dwmac",
> 		.pm             = &stmmac_pltfr_pm_ops,
> 		.of_match_table = lpc18xx_dwmac_match,
> 	},
> };
> module_platform_driver(lpc18xx_dwmac_driver);
> 
> All though this seem to work, stmmac_platform.c could benefit from
> some refactoring. But this patch and then fixing the other DT users
> could be a first step.

Sounds good, yes.

> -	device = of_match_device(stmmac_dt_ids, &pdev->dev);
> +	device = of_match_device(dev->driver->of_match_table, dev);
>  	if (!device)
>  		return -ENODEV;

Ah, that is a nice trick to avoid passing the various match tables
from a lot of duplicated probe functions.

I wonder if we could generalize that for use by any driver and
introduce a common helper

void *of_platform_match_data(struct device *dev)
{
	struct of_device_id *id;

	if (!dev || !dev->of_node)
		return NULL;

	id = of_match_device(dev->driver->of_match_table, dev);
	if (!id)
		return NULL;

	return id->data;
}
EXPORT_SYMBOL_GPL(of_platform_match_data);

I think that could save a few lines from many drivers, and it had not
occurred to me that we can take this shortcut.

The rest of your patch also looks great to me.

	Arnd

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

[Joachim Eastwood]

On 4 May 2015 at 20:46, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 04 May 2015 00:14:11 Joachim Eastwood wrote:
> => >It should be fairly straightforward to split the probe function
>> >into two and export a function that takes a device and a stmmac_of_data
>> >pointer as arguments, and declare a module_platform_driver in your
>> >code.
>>
>> How about something like the patch below. All it does is to play a
>> little trick with the of_match_device in the dt config function and
>> export the probe/remove/pm stuff for the driver.
>>
>> The dwmac-lpc18xx would then become something like this:
>>
>> static const struct stmmac_of_data lpc18xx_dwmac_data = {
>>       .has_gmac = 1,
>>       .setup = lpc18xx_dwmac_setup,
>>       .init = lpc18xx_dwmac_init,
>> };
>>
>> static const struct of_device_id lpc18xx_dwmac_match[] = {
>>       { .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data },
>>       { }
>> };
>> MODULE_DEVICE_TABLE(of, lpc18xx_dwmac_match);
>>
>> static struct platform_driver lpc18xx_dwmac_driver = {
>>       .probe  = stmmac_pltfr_probe,
>>       .remove = stmmac_pltfr_remove,
>>       .driver = {
>>               .name           = "lpc18xx-dwmac",
>>               .pm             = &stmmac_pltfr_pm_ops,
>>               .of_match_table = lpc18xx_dwmac_match,
>>       },
>> };
>> module_platform_driver(lpc18xx_dwmac_driver);
>>
>> All though this seem to work, stmmac_platform.c could benefit from
>> some refactoring. But this patch and then fixing the other DT users
>> could be a first step.
>
> Sounds good, yes.
>
>> -     device = of_match_device(stmmac_dt_ids, &pdev->dev);
>> +     device = of_match_device(dev->driver->of_match_table, dev);
>>       if (!device)
>>               return -ENODEV;
>
> Ah, that is a nice trick to avoid passing the various match tables
> from a lot of duplicated probe functions.
>
> I wonder if we could generalize that for use by any driver and
> introduce a common helper

I realised that and posted kinda of a RFC to the devicetree list yesterday.
http://marc.info/?l=devicetree&m=143068795621692&w=2

> void *of_platform_match_data(struct device *dev)
> {
>         struct of_device_id *id;
>
>         if (!dev || !dev->of_node)
>                 return NULL;
>
>         id = of_match_device(dev->driver->of_match_table, dev);
>         if (!id)
>                 return NULL;
>
>         return id->data;
> }
> EXPORT_SYMBOL_GPL(of_platform_match_data);

Almost the same as your proposal except for the dev pointer checking
and the name.

> I think that could save a few lines from many drivers, and it had not
> occurred to me that we can take this shortcut.
>
> The rest of your patch also looks great to me.

Great, I'll put together a new series of patches.

regards,
Joachim