[PATCH net-next 0/8] net: smsc911x: clean up with devm

[PATCH net-next 0/8] net: smsc911x: clean up with devm

[Rosen Penev]

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

Rosen Penev (8):
  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: smsc91xx: move down struct members
  net: smsc911x: remove pointless NULL checks

 drivers/net/ethernet/smsc/smsc911x.c | 216 ++++-----------------------
 1 file changed, 28 insertions(+), 188 deletions(-)

-- 
2.46.2

[PATCH net-next 1/8] net: smsc911x: use devm_platform_ioremap_resource

[Rosen Penev]

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

[PATCH net-next 2/8] net: smsc911x: use devm_alloc_etherdev

[Rosen Penev]

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

[PATCH net-next 3/8] net: smsc911x: use devm for regulators

[Rosen Penev]

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

[PATCH net-next 4/8] net: smsc911x: use devm for mdiobus functions

[Rosen Penev]

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

[PATCH net-next 5/8] net: smsc911x: use devm for register_netdev

[Rosen Penev]

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

[PATCH net-next 6/8] net: smsc911x: remove debug stuff from _remove

[Rosen Penev]

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

[PATCH net-next 7/8] net: smsc91xx: move down struct members

[Rosen Penev]

These are only used in these functions.

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

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e757c5825620..5eea873db853 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 */
@@ -378,14 +375,12 @@ static int smsc911x_enable_resources(struct platform_device *pdev)
  */
 static int smsc911x_request_resources(struct platform_device *pdev)
 {
-	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);
 
 	/* Request clock */
 	clk = devm_clk_get_optional(&pdev->dev, NULL);
-- 
2.46.2

Re: [PATCH net-next 7/8] net: smsc91xx: move down struct members

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

On Tue, Oct 1, 2024 at 4:13 AM Rosen Penev <rosenp@gmail.com> wrote:
>
> These are only used in these functions.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
LGTM,
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh A P

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net-next 7/8] net: smsc91xx: move down struct members

[kernel test robot]

Hi Rosen,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Rosen-Penev/net-smsc911x-use-devm_platform_ioremap_resource/20241001-064430
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240930224056.354349-8-rosenp%40gmail.com
patch subject: [PATCH net-next 7/8] net: smsc91xx: move down struct members
config: i386-randconfig-001-20241001 (https://download.01.org/0day-ci/archive/20241001/202410012317.iGlUJQY9-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241001/202410012317.iGlUJQY9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410012317.iGlUJQY9-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/smsc/smsc911x.c: In function 'smsc911x_request_resources':
>> drivers/net/ethernet/smsc/smsc911x.c:378:27: warning: variable 'reset_gpiod' set but not used [-Wunused-but-set-variable]
     378 |         struct gpio_desc *reset_gpiod;
         |                           ^~~~~~~~~~~


vim +/reset_gpiod +378 drivers/net/ethernet/smsc/smsc911x.c

   368	
   369	/*
   370	 * Request resources, currently just regulators.
   371	 *
   372	 * The SMSC911x has two power pins: vddvario and vdd33a, in designs where
   373	 * these are not always-on we need to request regulators to be turned on
   374	 * before we can try to access the device registers.
   375	 */
   376	static int smsc911x_request_resources(struct platform_device *pdev)
   377	{
 > 378		struct gpio_desc *reset_gpiod;
   379		struct clk *clk;
   380	
   381		/* Request optional RESET GPIO */
   382		reset_gpiod =
   383			devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
   384	
   385		/* Request clock */
   386		clk = devm_clk_get_optional(&pdev->dev, NULL);
   387		if (IS_ERR(clk))
   388			return dev_err_probe(&pdev->dev, PTR_ERR(clk),
   389					     "couldn't get clock");
   390	
   391		return 0;
   392	}
   393	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH net-next 8/8] net: smsc911x: remove pointless NULL checks

[Rosen Penev]

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

Signed-off-by: Rosen Penev <rosenp@gmail.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 5eea873db853..a74c3f9f7110 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2105,11 +2105,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
@@ -2334,11 +2329,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

Re: [PATCH net-next 8/8] net: smsc911x: remove pointless NULL checks

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

On Tue, Oct 1, 2024 at 4:13 AM Rosen Penev <rosenp@gmail.com> wrote:
>
> ioaddr can never be NULL. Probe aborts in such a case.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
LGTM,
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh A P

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net-next 0/8] net: smsc911x: clean up with devm

[Jakub Kicinski]

On Mon, 30 Sep 2024 15:40:48 -0700 Rosen Penev wrote:
> It happens to fix missing frees, especially with mdiobus functions.

Do you have the real hardware and have you tested this on it?

Please always include such information in the commit message.
Random conversions to devm are discouraged in networking.

Re: [PATCH net-next 0/8] net: smsc911x: clean up with devm

[Rosen Penev]

On Wed, Oct 2, 2024 at 5:32 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 30 Sep 2024 15:40:48 -0700 Rosen Penev wrote:
> > It happens to fix missing frees, especially with mdiobus functions.
>
> Do you have the real hardware and have you tested this on it?
>
> Please always include such information in the commit message.
> Random conversions to devm are discouraged in networking.
That's unfortunate.