mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCHv2 net-next 0/9] clean up with devm
@ 2024-10-01 18:29 Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource Rosen Penev
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

It happens to fix missing frees, especially with mdiobus functions.

v2: use reset_gpio variable to handle errors.

Rosen Penev (9):
  net: smsc911x: use devm_platform_ioremap_resource
  net: smsc911x: use devm_alloc_etherdev
  net: smsc911x: use devm for regulators
  net: smsc911x: use devm for mdiobus functions
  net: smsc911x: use devm for register_netdev
  net: smsc911x: remove debug stuff from _remove
  net: smsc911x: remove pointless NULL checks
  net: smsc911x: remove enable_resources function
  net: smsc911x: move reset_gpio member down

 drivers/net/ethernet/smsc/smsc911x.c | 231 ++++-----------------------
 1 file changed, 31 insertions(+), 200 deletions(-)

-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:22   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev Rosen Penev
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Allows removal of a bunch of code relating to grabbing resources.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 47 +++++-----------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 74f1ccc96459..3d4356df0070 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2319,7 +2319,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 {
 	struct net_device *dev;
 	struct smsc911x_data *pdata;
-	struct resource *res;
 
 	dev = platform_get_drvdata(pdev);
 	BUG_ON(!dev);
@@ -2334,15 +2333,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 	mdiobus_unregister(pdata->mii_bus);
 	mdiobus_free(pdata->mii_bus);
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-					   "smsc911x-memory");
-	if (!res)
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-	release_mem_region(res->start, resource_size(res));
-
-	iounmap(pdata->ioaddr);
-
 	(void)smsc911x_disable_resources(pdev);
 	smsc911x_free_resources(pdev);
 
@@ -2414,21 +2404,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct smsc911x_data *pdata;
 	struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);
-	struct resource *res;
-	int res_size, irq;
+	int irq;
 	int retval;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-					   "smsc911x-memory");
-	if (!res)
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		pr_warn("Could not allocate resource\n");
-		retval = -ENODEV;
-		goto out_0;
-	}
-	res_size = resource_size(res);
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq == -EPROBE_DEFER) {
 		retval = -EPROBE_DEFER;
@@ -2439,24 +2417,17 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_0;
 	}
 
-	if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) {
-		retval = -EBUSY;
-		goto out_0;
-	}
-
 	dev = alloc_etherdev(sizeof(struct smsc911x_data));
-	if (!dev) {
-		retval = -ENOMEM;
-		goto out_release_io_1;
-	}
+	if (!dev)
+		return -ENOMEM;
 
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
 	pdata = netdev_priv(dev);
 	dev->irq = irq;
-	pdata->ioaddr = ioremap(res->start, res_size);
-	if (!pdata->ioaddr) {
-		retval = -ENOMEM;
+	pdata->ioaddr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pdata->ioaddr)) {
+		retval = PTR_ERR(pdata->ioaddr);
 		goto out_ioremap_fail;
 	}
 
@@ -2467,7 +2438,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 
 	retval = smsc911x_request_resources(pdev);
 	if (retval)
-		goto out_request_resources_fail;
+		goto out_ioremap_fail;
 
 	retval = smsc911x_enable_resources(pdev);
 	if (retval)
@@ -2564,12 +2535,8 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	(void)smsc911x_disable_resources(pdev);
 out_enable_resources_fail:
 	smsc911x_free_resources(pdev);
-out_request_resources_fail:
-	iounmap(pdata->ioaddr);
 out_ioremap_fail:
 	free_netdev(dev);
-out_release_io_1:
-	release_mem_region(res->start, resource_size(res));
 out_0:
 	return retval;
 }
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:24   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators Rosen Penev
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Allows removal of various gotos and manual frees.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 3d4356df0070..3b3295b4e9e5 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2336,8 +2336,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 	(void)smsc911x_disable_resources(pdev);
 	smsc911x_free_resources(pdev);
 
-	free_netdev(dev);
-
 	pm_runtime_disable(&pdev->dev);
 }
 
@@ -2417,7 +2415,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_0;
 	}
 
-	dev = alloc_etherdev(sizeof(struct smsc911x_data));
+	dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct smsc911x_data));
 	if (!dev)
 		return -ENOMEM;
 
@@ -2426,10 +2424,8 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	pdata = netdev_priv(dev);
 	dev->irq = irq;
 	pdata->ioaddr = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(pdata->ioaddr)) {
-		retval = PTR_ERR(pdata->ioaddr);
-		goto out_ioremap_fail;
-	}
+	if (IS_ERR(pdata->ioaddr))
+		return PTR_ERR(pdata->ioaddr);
 
 	pdata->dev = dev;
 	pdata->msg_enable = ((1 << debug) - 1);
@@ -2438,7 +2434,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 
 	retval = smsc911x_request_resources(pdev);
 	if (retval)
-		goto out_ioremap_fail;
+		return retval;
 
 	retval = smsc911x_enable_resources(pdev);
 	if (retval)
@@ -2535,8 +2531,6 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	(void)smsc911x_disable_resources(pdev);
 out_enable_resources_fail:
 	smsc911x_free_resources(pdev);
-out_ioremap_fail:
-	free_netdev(dev);
 out_0:
 	return retval;
 }
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:27   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions Rosen Penev
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Allows to get rid of freeing functions.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 106 +++------------------------
 1 file changed, 12 insertions(+), 94 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 3b3295b4e9e5..68687df4eb3b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -132,14 +132,8 @@ struct smsc911x_data {
 	/* register access functions */
 	const struct smsc911x_ops *ops;
 
-	/* regulators */
-	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
-
 	/* Reset GPIO */
 	struct gpio_desc *reset_gpiod;
-
-	/* clock */
-	struct clk *clk;
 };
 
 /* Easy access to information */
@@ -369,41 +363,10 @@ smsc911x_rx_readfifo_shift(struct smsc911x_data *pdata, unsigned int *buf,
  */
 static int smsc911x_enable_resources(struct platform_device *pdev)
 {
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(ndev);
-	int ret = 0;
-
-	ret = regulator_bulk_enable(ARRAY_SIZE(pdata->supplies),
-			pdata->supplies);
-	if (ret)
-		netdev_err(ndev, "failed to enable regulators %d\n",
-				ret);
+	static const char *const supplies[] = { "vdd33a", "vddvario" };
 
-	if (!IS_ERR(pdata->clk)) {
-		ret = clk_prepare_enable(pdata->clk);
-		if (ret < 0)
-			netdev_err(ndev, "failed to enable clock %d\n", ret);
-	}
-
-	return ret;
-}
-
-/*
- * disable resources, currently just regulators.
- */
-static int smsc911x_disable_resources(struct platform_device *pdev)
-{
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(ndev);
-	int ret = 0;
-
-	ret = regulator_bulk_disable(ARRAY_SIZE(pdata->supplies),
-			pdata->supplies);
-
-	if (!IS_ERR(pdata->clk))
-		clk_disable_unprepare(pdata->clk);
-
-	return ret;
+	return devm_regulator_bulk_get_enable(&pdev->dev, ARRAY_SIZE(supplies),
+					      supplies);
 }
 
 /*
@@ -417,24 +380,7 @@ static int smsc911x_request_resources(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct smsc911x_data *pdata = netdev_priv(ndev);
-	int ret = 0;
-
-	/* Request regulators */
-	pdata->supplies[0].supply = "vdd33a";
-	pdata->supplies[1].supply = "vddvario";
-	ret = regulator_bulk_get(&pdev->dev,
-			ARRAY_SIZE(pdata->supplies),
-			pdata->supplies);
-	if (ret) {
-		/*
-		 * Retry on deferrals, else just report the error
-		 * and try to continue.
-		 */
-		if (ret == -EPROBE_DEFER)
-			return ret;
-		netdev_err(ndev, "couldn't get regulators %d\n",
-				ret);
-	}
+	struct clk *clk;
 
 	/* Request optional RESET GPIO */
 	pdata->reset_gpiod = devm_gpiod_get_optional(&pdev->dev,
@@ -442,32 +388,12 @@ static int smsc911x_request_resources(struct platform_device *pdev)
 						     GPIOD_OUT_LOW);
 
 	/* Request clock */
-	pdata->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(pdata->clk))
-		dev_dbg(&pdev->dev, "couldn't get clock %li\n",
-			PTR_ERR(pdata->clk));
-
-	return ret;
-}
-
-/*
- * Free resources, currently just regulators.
- *
- */
-static void smsc911x_free_resources(struct platform_device *pdev)
-{
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(ndev);
+	clk = devm_clk_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+				     "couldn't get clock");
 
-	/* Free regulators */
-	regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
-			pdata->supplies);
-
-	/* Free clock */
-	if (!IS_ERR(pdata->clk)) {
-		clk_put(pdata->clk);
-		pdata->clk = NULL;
-	}
+	return 0;
 }
 
 /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read
@@ -2333,9 +2259,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 	mdiobus_unregister(pdata->mii_bus);
 	mdiobus_free(pdata->mii_bus);
 
-	(void)smsc911x_disable_resources(pdev);
-	smsc911x_free_resources(pdev);
-
 	pm_runtime_disable(&pdev->dev);
 }
 
@@ -2438,12 +2361,11 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 
 	retval = smsc911x_enable_resources(pdev);
 	if (retval)
-		goto out_enable_resources_fail;
+		return retval;
 
 	if (pdata->ioaddr == NULL) {
 		SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
-		retval = -ENOMEM;
-		goto out_disable_resources;
+		return -ENOMEM;
 	}
 
 	retval = smsc911x_probe_config(&pdata->config, &pdev->dev);
@@ -2455,7 +2377,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 
 	if (retval) {
 		SMSC_WARN(pdata, probe, "Error smsc911x config not found");
-		goto out_disable_resources;
+		return retval;
 	}
 
 	/* assume standard, non-shifted, access to HW registers */
@@ -2527,10 +2449,6 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 out_init_fail:
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-out_disable_resources:
-	(void)smsc911x_disable_resources(pdev);
-out_enable_resources_fail:
-	smsc911x_free_resources(pdev);
 out_0:
 	return retval;
 }
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (2 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:28   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev Rosen Penev
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Frees are missing in _probe for these. OTOH simpler to use devm.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 68687df4eb3b..843e3606c2ea 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -991,13 +991,10 @@ static int smsc911x_mii_init(struct platform_device *pdev,
 {
 	struct smsc911x_data *pdata = netdev_priv(dev);
 	struct phy_device *phydev;
-	int err = -ENXIO;
 
-	pdata->mii_bus = mdiobus_alloc();
-	if (!pdata->mii_bus) {
-		err = -ENOMEM;
-		goto err_out_1;
-	}
+	pdata->mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	if (!pdata->mii_bus)
+		return -ENOMEM;
 
 	pdata->mii_bus->name = SMSC_MDIONAME;
 	snprintf(pdata->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
@@ -1028,9 +1025,9 @@ static int smsc911x_mii_init(struct platform_device *pdev,
 		pdata->mii_bus->phy_mask = ~(1 << 1);
 	}
 
-	if (mdiobus_register(pdata->mii_bus)) {
+	if (devm_mdiobus_register(&pdev->dev, pdata->mii_bus)) {
 		SMSC_WARN(pdata, probe, "Error registering mii bus");
-		goto err_out_free_bus_2;
+		return -ENXIO;
 	}
 
 	phydev = phy_find_first(pdata->mii_bus);
@@ -1038,11 +1035,6 @@ static int smsc911x_mii_init(struct platform_device *pdev,
 		phydev->mac_managed_pm = true;
 
 	return 0;
-
-err_out_free_bus_2:
-	mdiobus_free(pdata->mii_bus);
-err_out_1:
-	return err;
 }
 
 /* Gets the number of tx statuses in the fifo */
@@ -2256,9 +2248,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 
 	unregister_netdev(dev);
 
-	mdiobus_unregister(pdata->mii_bus);
-	mdiobus_free(pdata->mii_bus);
-
 	pm_runtime_disable(&pdev->dev);
 }
 
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (3 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:30   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove Rosen Penev
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

No need to call in _remove.

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

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 843e3606c2ea..4e0a277a5ee3 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2246,8 +2246,6 @@ static void smsc911x_drv_remove(struct platform_device *pdev)
 
 	SMSC_TRACE(pdata, ifdown, "Stopping driver");
 
-	unregister_netdev(dev);
-
 	pm_runtime_disable(&pdev->dev);
 }
 
@@ -2390,7 +2388,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_init_fail;
 	}
 
-	retval = register_netdev(dev);
+	retval = devm_register_netdev(&pdev->dev, dev);
 	if (retval) {
 		SMSC_WARN(pdata, probe, "Error %i registering device", retval);
 		goto out_init_fail;
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (4 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 22:30   ` Andrew Lunn
  2024-10-01 18:29 ` [PATCHv2 net-next 7/9] net: smsc911x: remove pointless NULL checks Rosen Penev
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Not needed. Now only contains a single call to pm_runtime_disable.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 4e0a277a5ee3..e757c5825620 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2235,17 +2235,6 @@ static int smsc911x_init(struct net_device *dev)
 
 static void smsc911x_drv_remove(struct platform_device *pdev)
 {
-	struct net_device *dev;
-	struct smsc911x_data *pdata;
-
-	dev = platform_get_drvdata(pdev);
-	BUG_ON(!dev);
-	pdata = netdev_priv(dev);
-	BUG_ON(!pdata);
-	BUG_ON(!pdata->ioaddr);
-
-	SMSC_TRACE(pdata, ifdown, "Stopping driver");
-
 	pm_runtime_disable(&pdev->dev);
 }
 
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 7/9] net: smsc911x: remove pointless NULL checks
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (5 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 8/9] net: smsc911x: remove enable_resources function Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 9/9] net: smsc911x: move reset_gpio member down Rosen Penev
  8 siblings, 0 replies; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

ioaddr can never be NULL. Probe aborts in such a case.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e757c5825620..c4c6480c0ffe 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2110,11 +2110,6 @@ static int smsc911x_init(struct net_device *dev)
 	spin_lock_init(&pdata->dev_lock);
 	spin_lock_init(&pdata->mac_lock);
 
-	if (pdata->ioaddr == NULL) {
-		SMSC_WARN(pdata, probe, "pdata->ioaddr: 0x00000000");
-		return -ENODEV;
-	}
-
 	/*
 	 * poll the READY bit in PMT_CTRL. Any other access to the device is
 	 * forbidden while this bit isn't set. Try for 100ms
@@ -2339,11 +2334,6 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	if (retval)
 		return retval;
 
-	if (pdata->ioaddr == NULL) {
-		SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
-		return -ENOMEM;
-	}
-
 	retval = smsc911x_probe_config(&pdata->config, &pdev->dev);
 	if (retval && config) {
 		/* copy config parameters across to pdata */
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 8/9] net: smsc911x: remove enable_resources function
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (6 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 7/9] net: smsc911x: remove pointless NULL checks Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  2024-10-01 18:29 ` [PATCHv2 net-next 9/9] net: smsc911x: move reset_gpio member down Rosen Penev
  8 siblings, 0 replies; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

With devm cleanups, only one function makes sense to have here.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index c4c6480c0ffe..77a5b766751c 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -359,25 +359,16 @@ smsc911x_rx_readfifo_shift(struct smsc911x_data *pdata, unsigned int *buf,
 }
 
 /*
- * enable regulator and clock resources.
- */
-static int smsc911x_enable_resources(struct platform_device *pdev)
-{
-	static const char *const supplies[] = { "vdd33a", "vddvario" };
-
-	return devm_regulator_bulk_get_enable(&pdev->dev, ARRAY_SIZE(supplies),
-					      supplies);
-}
-
-/*
- * Request resources, currently just regulators.
+ * Request and enable resources.
  *
  * The SMSC911x has two power pins: vddvario and vdd33a, in designs where
  * these are not always-on we need to request regulators to be turned on
  * before we can try to access the device registers.
  */
+
 static int smsc911x_request_resources(struct platform_device *pdev)
 {
+	static const char *const supplies[] = { "vdd33a", "vddvario" };
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct smsc911x_data *pdata = netdev_priv(ndev);
 	struct clk *clk;
@@ -393,7 +384,8 @@ static int smsc911x_request_resources(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
 				     "couldn't get clock");
 
-	return 0;
+	return devm_regulator_bulk_get_enable(&pdev->dev, ARRAY_SIZE(supplies),
+					      supplies);
 }
 
 /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read
@@ -2330,10 +2322,6 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	if (retval)
 		return retval;
 
-	retval = smsc911x_enable_resources(pdev);
-	if (retval)
-		return retval;
-
 	retval = smsc911x_probe_config(&pdata->config, &pdev->dev);
 	if (retval && config) {
 		/* copy config parameters across to pdata */
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCHv2 net-next 9/9] net: smsc911x: move reset_gpio member down
  2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
                   ` (7 preceding siblings ...)
  2024-10-01 18:29 ` [PATCHv2 net-next 8/9] net: smsc911x: remove enable_resources function Rosen Penev
@ 2024-10-01 18:29 ` Rosen Penev
  8 siblings, 0 replies; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 18:29 UTC (permalink / raw)
  To: netdev
  Cc: andrew, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

Not needed in struct. It seems to be unused actually.

Add error handling to avoid unused variable errors and to handle
-EPROBE_DEFER, if applicable.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 77a5b766751c..3b2388c1c939 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -131,9 +131,6 @@ struct smsc911x_data {
 
 	/* register access functions */
 	const struct smsc911x_ops *ops;
-
-	/* Reset GPIO */
-	struct gpio_desc *reset_gpiod;
 };
 
 /* Easy access to information */
@@ -369,14 +366,15 @@ smsc911x_rx_readfifo_shift(struct smsc911x_data *pdata, unsigned int *buf,
 static int smsc911x_request_resources(struct platform_device *pdev)
 {
 	static const char *const supplies[] = { "vdd33a", "vddvario" };
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(ndev);
+	struct gpio_desc *reset_gpiod;
 	struct clk *clk;
 
 	/* Request optional RESET GPIO */
-	pdata->reset_gpiod = devm_gpiod_get_optional(&pdev->dev,
-						     "reset",
-						     GPIOD_OUT_LOW);
+	reset_gpiod =
+		devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpiod))
+		return dev_err_probe(&pdev->dev, PTR_ERR(reset_gpiod),
+				     "GPIO device not found");
 
 	/* Request clock */
 	clk = devm_clk_get_optional(&pdev->dev, NULL);
-- 
2.46.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource
  2024-10-01 18:29 ` [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource Rosen Penev
@ 2024-10-01 22:22   ` Andrew Lunn
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:22 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

> @@ -2414,21 +2404,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
>  	struct net_device *dev;
>  	struct smsc911x_data *pdata;
>  	struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);
> -	struct resource *res;
> -	int res_size, irq;
> +	int irq;
>  	int retval;
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> -					   "smsc911x-memory");


> +	pdata->ioaddr = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pdata->ioaddr)) {
> +		retval = PTR_ERR(pdata->ioaddr);

I would expect some sort of comment in the commit messages which
explains why resource "smsc911x-memory" is always resource 0.


    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev
  2024-10-01 18:29 ` [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev Rosen Penev
@ 2024-10-01 22:24   ` Andrew Lunn
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:24 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 01, 2024 at 11:29:09AM -0700, Rosen Penev wrote:
> Allows removal of various gotos and manual frees.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators
  2024-10-01 18:29 ` [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators Rosen Penev
@ 2024-10-01 22:27   ` Andrew Lunn
  2024-10-01 23:44     ` Rosen Penev
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:27 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 01, 2024 at 11:29:10AM -0700, Rosen Penev wrote:
> Allows to get rid of freeing functions.

The commit message does not fit the code.


    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions
  2024-10-01 18:29 ` [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions Rosen Penev
@ 2024-10-01 22:28   ` Andrew Lunn
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:28 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 01, 2024 at 11:29:11AM -0700, Rosen Penev wrote:
> Frees are missing in _probe for these. OTOH simpler to use devm.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev
  2024-10-01 18:29 ` [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev Rosen Penev
@ 2024-10-01 22:30   ` Andrew Lunn
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:30 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 01, 2024 at 11:29:12AM -0700, Rosen Penev wrote:
> No need to call in _remove.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove
  2024-10-01 18:29 ` [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove Rosen Penev
@ 2024-10-01 22:30   ` Andrew Lunn
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-10-01 22:30 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 01, 2024 at 11:29:13AM -0700, Rosen Penev wrote:
> Not needed. Now only contains a single call to pm_runtime_disable.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators
  2024-10-01 22:27   ` Andrew Lunn
@ 2024-10-01 23:44     ` Rosen Penev
  0 siblings, 0 replies; 17+ messages in thread
From: Rosen Penev @ 2024-10-01 23:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, davem, edumazet, kuba, pabeni, linux-kernel, steve.glendinning

On Tue, Oct 1, 2024 at 3:27 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Oct 01, 2024 at 11:29:10AM -0700, Rosen Penev wrote:
> > Allows to get rid of freeing functions.
>
> The commit message does not fit the code.
This looks like a rebasing mistake. Will review.
>
>
>     Andrew
>
> ---
> pw-bot: cr

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-10-01 23:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-01 18:29 [PATCHv2 net-next 0/9] clean up with devm Rosen Penev
2024-10-01 18:29 ` [PATCHv2 net-next 1/9] net: smsc911x: use devm_platform_ioremap_resource Rosen Penev
2024-10-01 22:22   ` Andrew Lunn
2024-10-01 18:29 ` [PATCHv2 net-next 2/9] net: smsc911x: use devm_alloc_etherdev Rosen Penev
2024-10-01 22:24   ` Andrew Lunn
2024-10-01 18:29 ` [PATCHv2 net-next 3/9] net: smsc911x: use devm for regulators Rosen Penev
2024-10-01 22:27   ` Andrew Lunn
2024-10-01 23:44     ` Rosen Penev
2024-10-01 18:29 ` [PATCHv2 net-next 4/9] net: smsc911x: use devm for mdiobus functions Rosen Penev
2024-10-01 22:28   ` Andrew Lunn
2024-10-01 18:29 ` [PATCHv2 net-next 5/9] net: smsc911x: use devm for register_netdev Rosen Penev
2024-10-01 22:30   ` Andrew Lunn
2024-10-01 18:29 ` [PATCHv2 net-next 6/9] net: smsc911x: remove debug stuff from _remove Rosen Penev
2024-10-01 22:30   ` Andrew Lunn
2024-10-01 18:29 ` [PATCHv2 net-next 7/9] net: smsc911x: remove pointless NULL checks Rosen Penev
2024-10-01 18:29 ` [PATCHv2 net-next 8/9] net: smsc911x: remove enable_resources function Rosen Penev
2024-10-01 18:29 ` [PATCHv2 net-next 9/9] net: smsc911x: move reset_gpio member down Rosen Penev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®