[PATCH net-next 03/11] net: fsl_pq_mdio: use platform_get_resource

[PATCH net-next 03/11] net: fsl_pq_mdio: use platform_get_resource

[Rosen Penev]

Replace of_address_to_resource with platform_get_resource. No need to
use the of_node when the pdev is sufficient.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index d7f9d99fe782..f14607555f33 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -414,7 +414,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	struct device_node *tbi;
 	struct mii_bus *new_bus;
 	struct device_node *np;
-	struct resource res;
+	struct resource *res;
 	int err;
 
 	data = device_get_match_data(dev);
@@ -433,15 +433,15 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	new_bus->write = &fsl_pq_mdio_write;
 	new_bus->reset = &fsl_pq_mdio_reset;
 
-	np = dev->of_node;
-	err = of_address_to_resource(np, 0, &res);
-	if (err < 0) {
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
 		dev_err(dev, "could not obtain address information\n");
-		return err;
+		return -ENOMEM;
 	}
 
+	np = dev->of_node;
 	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%pOFn@%llx", np,
-		 (unsigned long long)res.start);
+		 (unsigned long long)res->start);
 
 	priv->map = of_iomap(np, 0);
 	if (!priv->map)
@@ -453,7 +453,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	 * contains the offset of the MII registers inside the mapped register
 	 * space.
 	 */
-	if (data->mii_offset > resource_size(&res)) {
+	if (data->mii_offset > resource_size(res)) {
 		dev_err(dev, "invalid register map\n");
 		err = -EINVAL;
 		goto error;
@@ -480,13 +480,12 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 				err = -EBUSY;
 				goto error;
 			}
-			set_tbipa(*prop, pdev,
-				  data->get_tbipa, priv->map, &res);
+			set_tbipa(*prop, pdev, data->get_tbipa, priv->map, res);
 		}
 	}
 
 	if (data->ucc_configure)
-		data->ucc_configure(res.start, res.end);
+		data->ucc_configure(res->start, res->end);
 
 	err = of_mdiobus_register(new_bus, np);
 	if (err) {
-- 
2.47.0

[PATCH net-next 00/11] gianfar and mdio: modernize

[Rosen Penev]

Probe cleanups for gianfar and fsl_pq_mdio drivers

All were tested on a WatchGuard T10 device.

Rosen Penev (11):
  net: fsl_pq_mdio: use dev variable in _probe
  net: fsl_pq_mdio: use devm for mdiobus_alloc_size
  net: fsl_pq_mdio: use platform_get_resource
  net: fsl_pq_mdio: use devm for of_iomap
  net: fsl_pq_mdio: return directly in probe
  net: gianfar: use devm_alloc_etherdev_mqs
  net: gianfar: use devm for register_netdev
  net: gianfar: assign ofdev to priv struct
  net: gianfar: remove free_gfar_dev
  net: gianfar: alloc queues with devm
  net: gianfar: iomap with devm

 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 92 +++++++------------
 drivers/net/ethernet/freescale/gianfar.c     | 93 ++++----------------
 2 files changed, 50 insertions(+), 135 deletions(-)

-- 
2.50.0

[PATCH net-next 01/11] net: fsl_pq_mdio: use dev variable in _probe

[Rosen Penev]

Moving &pdev->dev to its own variable makes the code slightly more
readable.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 26 ++++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 56d2f79fb7e3..108e760c7a5f 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -409,16 +409,17 @@ static void set_tbipa(const u32 tbipa_val, struct platform_device *pdev,
 static int fsl_pq_mdio_probe(struct platform_device *pdev)
 {
 	const struct fsl_pq_mdio_data *data;
-	struct device_node *np = pdev->dev.of_node;
-	struct resource res;
-	struct device_node *tbi;
+	struct device *dev = &pdev->dev;
 	struct fsl_pq_mdio_priv *priv;
+	struct device_node *tbi;
 	struct mii_bus *new_bus;
+	struct device_node *np;
+	struct resource res;
 	int err;
 
-	data = device_get_match_data(&pdev->dev);
+	data = device_get_match_data(dev);
 	if (!data) {
-		dev_err(&pdev->dev, "Failed to match device\n");
+		dev_err(dev, "Failed to match device\n");
 		return -ENODEV;
 	}
 
@@ -432,9 +433,10 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	new_bus->write = &fsl_pq_mdio_write;
 	new_bus->reset = &fsl_pq_mdio_reset;
 
+	np = dev->of_node;
 	err = of_address_to_resource(np, 0, &res);
 	if (err < 0) {
-		dev_err(&pdev->dev, "could not obtain address information\n");
+		dev_err(dev, "could not obtain address information\n");
 		goto error;
 	}
 
@@ -454,20 +456,19 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	 * space.
 	 */
 	if (data->mii_offset > resource_size(&res)) {
-		dev_err(&pdev->dev, "invalid register map\n");
+		dev_err(dev, "invalid register map\n");
 		err = -EINVAL;
 		goto error;
 	}
 	priv->regs = priv->map + data->mii_offset;
 
-	new_bus->parent = &pdev->dev;
+	new_bus->parent = dev;
 	platform_set_drvdata(pdev, new_bus);
 
 	if (data->get_tbipa) {
 		for_each_child_of_node(np, tbi) {
 			if (of_node_is_type(tbi, "tbi-phy")) {
-				dev_dbg(&pdev->dev, "found TBI PHY node %pOFP\n",
-					tbi);
+				dev_dbg(dev, "found TBI PHY node %pOFP\n", tbi);
 				break;
 			}
 		}
@@ -475,7 +476,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 		if (tbi) {
 			const u32 *prop = of_get_property(tbi, "reg", NULL);
 			if (!prop) {
-				dev_err(&pdev->dev,
+				dev_err(dev,
 					"missing 'reg' property in node %pOF\n",
 					tbi);
 				err = -EBUSY;
@@ -491,8 +492,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 
 	err = of_mdiobus_register(new_bus, np);
 	if (err) {
-		dev_err(&pdev->dev, "cannot register %s as MDIO bus\n",
-			new_bus->name);
+		dev_err(dev, "cannot register %s as MDIO bus\n", new_bus->name);
 		goto error;
 	}
 
-- 
2.50.0

[PATCH net-next 02/11] net: fsl_pq_mdio: use devm for mdiobus_alloc_size

[Rosen Penev]

Using devm avoids having to manually free. In the case of this driver,
it's simple enough that it's ideal for devm.

There also seems to be a mistake here. Using kfree instead of
mdiobus_free.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 108e760c7a5f..d7f9d99fe782 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -423,7 +423,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	new_bus = mdiobus_alloc_size(sizeof(*priv));
+	new_bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
 	if (!new_bus)
 		return -ENOMEM;
 
@@ -437,17 +437,15 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	err = of_address_to_resource(np, 0, &res);
 	if (err < 0) {
 		dev_err(dev, "could not obtain address information\n");
-		goto error;
+		return err;
 	}
 
 	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%pOFn@%llx", np,
 		 (unsigned long long)res.start);
 
 	priv->map = of_iomap(np, 0);
-	if (!priv->map) {
-		err = -ENOMEM;
-		goto error;
-	}
+	if (!priv->map)
+		return -ENOMEM;
 
 	/*
 	 * Some device tree nodes represent only the MII registers, and
@@ -502,8 +500,6 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	if (priv->map)
 		iounmap(priv->map);
 
-	kfree(new_bus);
-
 	return err;
 }
 
@@ -517,7 +513,6 @@ static void fsl_pq_mdio_remove(struct platform_device *pdev)
 	mdiobus_unregister(bus);
 
 	iounmap(priv->map);
-	mdiobus_free(bus);
 }
 
 static struct platform_driver fsl_pq_mdio_driver = {
-- 
2.50.0

[PATCH net-next 03/11] net: fsl_pq_mdio: use platform_get_resource

[Rosen Penev]

Replace of_address_to_resource with platform_get_resource. No need to
use the of_node when the pdev is sufficient.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index d7f9d99fe782..f14607555f33 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -414,7 +414,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	struct device_node *tbi;
 	struct mii_bus *new_bus;
 	struct device_node *np;
-	struct resource res;
+	struct resource *res;
 	int err;
 
 	data = device_get_match_data(dev);
@@ -433,15 +433,15 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	new_bus->write = &fsl_pq_mdio_write;
 	new_bus->reset = &fsl_pq_mdio_reset;
 
-	np = dev->of_node;
-	err = of_address_to_resource(np, 0, &res);
-	if (err < 0) {
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
 		dev_err(dev, "could not obtain address information\n");
-		return err;
+		return -ENOMEM;
 	}
 
+	np = dev->of_node;
 	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%pOFn@%llx", np,
-		 (unsigned long long)res.start);
+		 (unsigned long long)res->start);
 
 	priv->map = of_iomap(np, 0);
 	if (!priv->map)
@@ -453,7 +453,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	 * contains the offset of the MII registers inside the mapped register
 	 * space.
 	 */
-	if (data->mii_offset > resource_size(&res)) {
+	if (data->mii_offset > resource_size(res)) {
 		dev_err(dev, "invalid register map\n");
 		err = -EINVAL;
 		goto error;
@@ -480,13 +480,12 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 				err = -EBUSY;
 				goto error;
 			}
-			set_tbipa(*prop, pdev,
-				  data->get_tbipa, priv->map, &res);
+			set_tbipa(*prop, pdev, data->get_tbipa, priv->map, res);
 		}
 	}
 
 	if (data->ucc_configure)
-		data->ucc_configure(res.start, res.end);
+		data->ucc_configure(res->start, res->end);
 
 	err = of_mdiobus_register(new_bus, np);
 	if (err) {
-- 
2.50.0

[PATCH net-next 04/11] net: fsl_pq_mdio: use devm for of_iomap

[Rosen Penev]

Using devm for of_iomap avoids having to manually iounmap in error paths
for this simple driver.

Add a note for why not devm_platform helper.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 24 ++++++++------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index f14607555f33..640929a4562d 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -443,7 +443,12 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%pOFn@%llx", np,
 		 (unsigned long long)res->start);
 
-	priv->map = of_iomap(np, 0);
+	/*
+	 * While tempting, this cannot be converted to
+	 * devm_platform_get_and_ioremap_resource as some platforms overlap the
+	 * memory regions with the ethernet node.
+	 */
+	priv->map = devm_of_iomap(dev, np, 0, NULL);
 	if (!priv->map)
 		return -ENOMEM;
 
@@ -455,8 +460,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	 */
 	if (data->mii_offset > resource_size(res)) {
 		dev_err(dev, "invalid register map\n");
-		err = -EINVAL;
-		goto error;
+		return -EINVAL;
 	}
 	priv->regs = priv->map + data->mii_offset;
 
@@ -477,8 +481,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 				dev_err(dev,
 					"missing 'reg' property in node %pOF\n",
 					tbi);
-				err = -EBUSY;
-				goto error;
+				return -EBUSY;
 			}
 			set_tbipa(*prop, pdev, data->get_tbipa, priv->map, res);
 		}
@@ -490,16 +493,10 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	err = of_mdiobus_register(new_bus, np);
 	if (err) {
 		dev_err(dev, "cannot register %s as MDIO bus\n", new_bus->name);
-		goto error;
+		return err;
 	}
 
 	return 0;
-
-error:
-	if (priv->map)
-		iounmap(priv->map);
-
-	return err;
 }
 
 
@@ -507,11 +504,8 @@ static void fsl_pq_mdio_remove(struct platform_device *pdev)
 {
 	struct device *device = &pdev->dev;
 	struct mii_bus *bus = dev_get_drvdata(device);
-	struct fsl_pq_mdio_priv *priv = bus->priv;
 
 	mdiobus_unregister(bus);
-
-	iounmap(priv->map);
 }
 
 static struct platform_driver fsl_pq_mdio_driver = {
-- 
2.50.0

[PATCH net-next 05/11] net: fsl_pq_mdio: return directly in probe

[Rosen Penev]

Instead of generating two errors in probe, just return directly to
generate one.

mdiobus_register was switched away from the of_ variant as no children
are being used.

No more need for a _remove function.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 640929a4562d..12b6c11d9cf9 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -415,7 +415,6 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	struct mii_bus *new_bus;
 	struct device_node *np;
 	struct resource *res;
-	int err;
 
 	data = device_get_match_data(dev);
 	if (!data) {
@@ -465,7 +464,6 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	priv->regs = priv->map + data->mii_offset;
 
 	new_bus->parent = dev;
-	platform_set_drvdata(pdev, new_bus);
 
 	if (data->get_tbipa) {
 		for_each_child_of_node(np, tbi) {
@@ -490,22 +488,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	if (data->ucc_configure)
 		data->ucc_configure(res->start, res->end);
 
-	err = of_mdiobus_register(new_bus, np);
-	if (err) {
-		dev_err(dev, "cannot register %s as MDIO bus\n", new_bus->name);
-		return err;
-	}
-
-	return 0;
-}
-
-
-static void fsl_pq_mdio_remove(struct platform_device *pdev)
-{
-	struct device *device = &pdev->dev;
-	struct mii_bus *bus = dev_get_drvdata(device);
-
-	mdiobus_unregister(bus);
+	return devm_mdiobus_register(dev, new_bus);
 }
 
 static struct platform_driver fsl_pq_mdio_driver = {
@@ -514,7 +497,6 @@ static struct platform_driver fsl_pq_mdio_driver = {
 		.of_match_table = fsl_pq_mdio_match,
 	},
 	.probe = fsl_pq_mdio_probe,
-	.remove = fsl_pq_mdio_remove,
 };
 
 module_platform_driver(fsl_pq_mdio_driver);
-- 
2.50.0

[PATCH net-next 06/11] net: gianfar: use devm_alloc_etherdev_mqs

[Rosen Penev]

There seems to be a mistake here. There's a num_rx_qs variable that is not
being passed to the allocation function. The mq variant just calls mqs
with the last parameter of the former duplicated to the last parameter
of the latter. That's fine if they match. Not sure they do.

Also avoids manual free_netdev

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 7c0f049f0938..05dedb6c9848 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -476,8 +476,6 @@ static void free_gfar_dev(struct gfar_private *priv)
 			kfree(priv->gfargrp[i].irqinfo[j]);
 			priv->gfargrp[i].irqinfo[j] = NULL;
 		}
-
-	free_netdev(priv->ndev);
 }
 
 static void disable_napi(struct gfar_private *priv)
@@ -672,7 +670,8 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 		return -EINVAL;
 	}
 
-	*pdev = alloc_etherdev_mq(sizeof(*priv), num_tx_qs);
+	*pdev = devm_alloc_etherdev_mqs(&ofdev->dev, sizeof(*priv), num_tx_qs,
+					num_rx_qs);
 	dev = *pdev;
 	if (NULL == dev)
 		return -ENOMEM;
-- 
2.50.0

[PATCH net-next 07/11] net: gianfar: use devm for register_netdev

[Rosen Penev]

Avoid manual unregister of netdev.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 05dedb6c9848..53839dfc5e7a 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -3265,7 +3265,7 @@ static int gfar_probe(struct platform_device *ofdev)
 	/* Carrier starts down, phylib will bring it up */
 	netif_carrier_off(dev);
 
-	err = register_netdev(dev);
+	err = devm_register_netdev(&ofdev->dev, dev);
 
 	if (err) {
 		pr_err("%s: Cannot register net device, aborting\n", dev->name);
@@ -3334,8 +3334,6 @@ static void gfar_remove(struct platform_device *ofdev)
 	of_node_put(priv->phy_node);
 	of_node_put(priv->tbi_node);
 
-	unregister_netdev(priv->ndev);
-
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
 
-- 
2.50.0

[PATCH net-next 08/11] net: gianfar: assign ofdev to priv struct

[Rosen Penev]

This is done in probe but not of_init. This will be used for further
devm conversions.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 53839dfc5e7a..2e9971ae475e 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -678,6 +678,8 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 
 	priv = netdev_priv(dev);
 	priv->ndev = dev;
+	priv->ofdev = ofdev;
+	priv->dev = &ofdev->dev;
 
 	priv->mode = mode;
 
-- 
2.50.0

[PATCH net-next 09/11] net: gianfar: remove free_gfar_dev

[Rosen Penev]

Use devm for kzalloc. Allows to remove free_gfar_dev as devm handles
freeing it now.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 2e9971ae475e..a93244415274 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -467,17 +467,6 @@ static void unmap_group_regs(struct gfar_private *priv)
 			iounmap(priv->gfargrp[i].regs);
 }
 
-static void free_gfar_dev(struct gfar_private *priv)
-{
-	int i, j;
-
-	for (i = 0; i < priv->num_grps; i++)
-		for (j = 0; j < GFAR_NUM_IRQS; j++) {
-			kfree(priv->gfargrp[i].irqinfo[j]);
-			priv->gfargrp[i].irqinfo[j] = NULL;
-		}
-}
-
 static void disable_napi(struct gfar_private *priv)
 {
 	int i;
@@ -505,8 +494,8 @@ static int gfar_parse_group(struct device_node *np,
 	int i;
 
 	for (i = 0; i < GFAR_NUM_IRQS; i++) {
-		grp->irqinfo[i] = kzalloc(sizeof(struct gfar_irqinfo),
-					  GFP_KERNEL);
+		grp->irqinfo[i] = devm_kzalloc(
+			priv->dev, sizeof(struct gfar_irqinfo), GFP_KERNEL);
 		if (!grp->irqinfo[i])
 			return -ENOMEM;
 	}
@@ -811,7 +800,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	gfar_free_rx_queues(priv);
 tx_alloc_failed:
 	gfar_free_tx_queues(priv);
-	free_gfar_dev(priv);
 	return err;
 }
 
@@ -3324,7 +3312,6 @@ static int gfar_probe(struct platform_device *ofdev)
 	gfar_free_tx_queues(priv);
 	of_node_put(priv->phy_node);
 	of_node_put(priv->tbi_node);
-	free_gfar_dev(priv);
 	return err;
 }
 
@@ -3342,7 +3329,6 @@ static void gfar_remove(struct platform_device *ofdev)
 	unmap_group_regs(priv);
 	gfar_free_rx_queues(priv);
 	gfar_free_tx_queues(priv);
-	free_gfar_dev(priv);
 }
 
 #ifdef CONFIG_PM
-- 
2.50.0

[PATCH net-next 10/11] net: gianfar: alloc queues with devm

[Rosen Penev]

Remove the freeing functions as they no longer serve a purpose. devm
handles this automatically.

There seems to be a mistake here where free_tx_queue is called on
failure. Just let devm deal with it.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 38 +++++-------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index a93244415274..bc1d7c4bd1a7 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -413,8 +413,8 @@ static int gfar_alloc_tx_queues(struct gfar_private *priv)
 	int i;
 
 	for (i = 0; i < priv->num_tx_queues; i++) {
-		priv->tx_queue[i] = kzalloc(sizeof(struct gfar_priv_tx_q),
-					    GFP_KERNEL);
+		priv->tx_queue[i] = devm_kzalloc(
+			priv->dev, sizeof(struct gfar_priv_tx_q), GFP_KERNEL);
 		if (!priv->tx_queue[i])
 			return -ENOMEM;
 
@@ -431,8 +431,8 @@ static int gfar_alloc_rx_queues(struct gfar_private *priv)
 	int i;
 
 	for (i = 0; i < priv->num_rx_queues; i++) {
-		priv->rx_queue[i] = kzalloc(sizeof(struct gfar_priv_rx_q),
-					    GFP_KERNEL);
+		priv->rx_queue[i] = devm_kzalloc(
+			priv->dev, sizeof(struct gfar_priv_rx_q), GFP_KERNEL);
 		if (!priv->rx_queue[i])
 			return -ENOMEM;
 
@@ -442,22 +442,6 @@ static int gfar_alloc_rx_queues(struct gfar_private *priv)
 	return 0;
 }
 
-static void gfar_free_tx_queues(struct gfar_private *priv)
-{
-	int i;
-
-	for (i = 0; i < priv->num_tx_queues; i++)
-		kfree(priv->tx_queue[i]);
-}
-
-static void gfar_free_rx_queues(struct gfar_private *priv)
-{
-	int i;
-
-	for (i = 0; i < priv->num_rx_queues; i++)
-		kfree(priv->rx_queue[i]);
-}
-
 static void unmap_group_regs(struct gfar_private *priv)
 {
 	int i;
@@ -678,16 +662,16 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 
 	err = gfar_alloc_tx_queues(priv);
 	if (err)
-		goto tx_alloc_failed;
+		return err;
 
 	err = gfar_alloc_rx_queues(priv);
 	if (err)
-		goto rx_alloc_failed;
+		return err;
 
 	err = of_property_read_string(np, "model", &model);
 	if (err) {
 		pr_err("Device model property missing, aborting\n");
-		goto rx_alloc_failed;
+		return err;
 	}
 
 	/* Init Rx queue filer rule set linked list */
@@ -796,10 +780,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 
 err_grp_init:
 	unmap_group_regs(priv);
-rx_alloc_failed:
-	gfar_free_rx_queues(priv);
-tx_alloc_failed:
-	gfar_free_tx_queues(priv);
 	return err;
 }
 
@@ -3308,8 +3288,6 @@ static int gfar_probe(struct platform_device *ofdev)
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
 	unmap_group_regs(priv);
-	gfar_free_rx_queues(priv);
-	gfar_free_tx_queues(priv);
 	of_node_put(priv->phy_node);
 	of_node_put(priv->tbi_node);
 	return err;
@@ -3327,8 +3305,6 @@ static void gfar_remove(struct platform_device *ofdev)
 		of_phy_deregister_fixed_link(np);
 
 	unmap_group_regs(priv);
-	gfar_free_rx_queues(priv);
-	gfar_free_tx_queues(priv);
 }
 
 #ifdef CONFIG_PM
-- 
2.50.0

[PATCH net-next 11/11] net: gianfar: iomap with devm

[Rosen Penev]

Remove unmap_group_regs as it no longer served a purpose. devm can
handle this automatically.

Remove gotos as they are no longer needed.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 26 +++++-------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index bc1d7c4bd1a7..1932c6d8bc66 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -442,15 +442,6 @@ static int gfar_alloc_rx_queues(struct gfar_private *priv)
 	return 0;
 }
 
-static void unmap_group_regs(struct gfar_private *priv)
-{
-	int i;
-
-	for (i = 0; i < MAXGROUPS; i++)
-		if (priv->gfargrp[i].regs)
-			iounmap(priv->gfargrp[i].regs);
-}
-
 static void disable_napi(struct gfar_private *priv)
 {
 	int i;
@@ -484,7 +475,7 @@ static int gfar_parse_group(struct device_node *np,
 			return -ENOMEM;
 	}
 
-	grp->regs = of_iomap(np, 0);
+	grp->regs = devm_of_iomap(priv->dev, np, 0, NULL);
 	if (!grp->regs)
 		return -ENOMEM;
 
@@ -691,13 +682,13 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 			err = gfar_parse_group(child, priv, model);
 			if (err) {
 				of_node_put(child);
-				goto err_grp_init;
+				return err;
 			}
 		}
 	} else { /* SQ_SG_MODE */
 		err = gfar_parse_group(np, priv, model);
 		if (err)
-			goto err_grp_init;
+			return err;
 	}
 
 	if (of_property_read_bool(np, "bd-stash")) {
@@ -720,7 +711,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 
 	err = of_get_ethdev_address(np, dev);
 	if (err == -EPROBE_DEFER)
-		goto err_grp_init;
+		return err;
 	if (err) {
 		eth_hw_addr_random(dev);
 		dev_info(&ofdev->dev, "Using random MAC address: %pM\n", dev->dev_addr);
@@ -768,7 +759,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	if (!priv->phy_node && of_phy_is_fixed_link(np)) {
 		err = of_phy_register_fixed_link(np);
 		if (err)
-			goto err_grp_init;
+			return err;
 
 		priv->phy_node = of_node_get(np);
 	}
@@ -777,10 +768,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 	priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
 
 	return 0;
-
-err_grp_init:
-	unmap_group_regs(priv);
-	return err;
 }
 
 static u32 cluster_entry_per_class(struct gfar_private *priv, u32 rqfar,
@@ -3287,7 +3274,6 @@ static int gfar_probe(struct platform_device *ofdev)
 register_fail:
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
-	unmap_group_regs(priv);
 	of_node_put(priv->phy_node);
 	of_node_put(priv->tbi_node);
 	return err;
@@ -3303,8 +3289,6 @@ static void gfar_remove(struct platform_device *ofdev)
 
 	if (of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
-
-	unmap_group_regs(priv);
 }
 
 #ifdef CONFIG_PM
-- 
2.50.0

Re: [PATCH net-next 00/11] gianfar and mdio: modernize

[Jakub Kicinski]

On Thu, 10 Jul 2025 13:40:21 -0700 Rosen Penev wrote:
> Probe cleanups for gianfar and fsl_pq_mdio drivers
> 
> All were tested on a WatchGuard T10 device.

devm_ conversions for old HW are definitely not worth reviewer
bandwidth.

Quoting documentation:

  Clean-up patches
  ~~~~~~~~~~~~~~~~
  
  Netdev discourages patches which perform simple clean-ups, which are not in
  the context of other work. For example:
  
  * Addressing ``checkpatch.pl`` warnings
  * Addressing :ref:`Local variable ordering<rcs>` issues
  * Conversions to device-managed APIs (``devm_`` helpers)
  
  This is because it is felt that the churn that such changes produce comes
  at a greater cost than the value of such clean-ups.
  
  Conversely, spelling and grammar fixes are not discouraged.
  
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#clean-up-patches
-- 
pw-bot: reject