[PATCH net-next v2 0/3] First bunch of Tulip cleanups

[PATCH net-next v2 0/3] First bunch of Tulip cleanups

[Moritz Fischer]

This series is the first bunch of minor cleanups for the de2104x driver
to make it look and behave more like a modern driver.

These changes replace some of the non-devres versions with devres
versions of functions to simplify the error paths.

Next up after this will be the ioremap part.

Changes from v1:
- Fix issue with the pci_enable_device patch.

Moritz Fischer (3):
  net: dec: tulip: de2104x: Replace alloc_etherdev by
    devm_alloc_etherdev
  net: dec: tulip: de2104x: Replace pci_enable_device with devres
    version
  net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup()

 drivers/net/ethernet/dec/tulip/de2104x.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

-- 
2.28.0

[PATCH net-next v2 1/3] net: dec: tulip: de2104x: Replace alloc_etherdev by devm_alloc_etherdev

[Moritz Fischer]

Replace devm_alloc_etherdev() with its devres version.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/net/ethernet/dec/tulip/de2104x.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 42b798a3fad4..9bcfc82b71d1 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -1986,7 +1986,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 #endif
 
 	/* allocate a new ethernet device structure, and fill in defaults */
-	dev = alloc_etherdev(sizeof(struct de_private));
+	dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct de_private));
 	if (!dev)
 		return -ENOMEM;
 
@@ -2011,7 +2011,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* wake up device, assign resources */
 	rc = pci_enable_device(pdev);
 	if (rc)
-		goto err_out_free;
+		return rc;
 
 	/* reserve PCI resources to ensure driver atomicity */
 	rc = pci_request_regions(pdev, DRV_NAME);
@@ -2098,8 +2098,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	pci_release_regions(pdev);
 err_out_disable:
 	pci_disable_device(pdev);
-err_out_free:
-	free_netdev(dev);
 	return rc;
 }
 
@@ -2114,7 +2112,6 @@ static void de_remove_one(struct pci_dev *pdev)
 	iounmap(de->regs);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
-	free_netdev(dev);
 }
 
 #ifdef CONFIG_PM
-- 
2.28.0

[PATCH net-next v2 2/3] net: dec: tulip: de2104x: Replace pci_enable_device with devres version

[Moritz Fischer]

Replace pci_enable_device() with its devres counterpart
pcim_enable_device().

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Note: Please check my logic on this, it would seem to me
calling pci_disable_device() on devices enabled with
pcim_enable_device() *should* be fine.

Changes from v1:
- Fixed missing replace for resume function
---
 drivers/net/ethernet/dec/tulip/de2104x.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 9bcfc82b71d1..698d79bc4784 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -2009,14 +2009,14 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netif_carrier_off(dev);
 
 	/* wake up device, assign resources */
-	rc = pci_enable_device(pdev);
+	rc = pcim_enable_device(pdev);
 	if (rc)
 		return rc;
 
 	/* reserve PCI resources to ensure driver atomicity */
 	rc = pci_request_regions(pdev, DRV_NAME);
 	if (rc)
-		goto err_out_disable;
+		return rc;
 
 	/* check for invalid IRQ value */
 	if (pdev->irq < 2) {
@@ -2096,8 +2096,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	iounmap(regs);
 err_out_res:
 	pci_release_regions(pdev);
-err_out_disable:
-	pci_disable_device(pdev);
 	return rc;
 }
 
@@ -2111,7 +2109,6 @@ static void de_remove_one(struct pci_dev *pdev)
 	kfree(de->ee_data);
 	iounmap(de->regs);
 	pci_release_regions(pdev);
-	pci_disable_device(pdev);
 }
 
 #ifdef CONFIG_PM
@@ -2164,7 +2161,7 @@ static int de_resume (struct pci_dev *pdev)
 		goto out;
 	if (!netif_running(dev))
 		goto out_attach;
-	if ((retval = pci_enable_device(pdev))) {
+	if ((retval = pcim_enable_device(pdev))) {
 		netdev_err(dev, "pci_enable_device failed in resume\n");
 		goto out;
 	}
-- 
2.28.0

[PATCH net-next v2 3/3] net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup()

[Moritz Fischer]

Replace an instance of kmemdup() with the devres counted version
instead.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/net/ethernet/dec/tulip/de2104x.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 698d79bc4784..a3a002c8e9ac 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -1940,7 +1940,8 @@ static void de21041_get_srom_info(struct de_private *de)
 			de->media[i].csr15 = t21041_csr15[i];
 	}
 
-	de->ee_data = kmemdup(&ee_data[0], DE_EEPROM_SIZE, GFP_KERNEL);
+	de->ee_data = devm_kmemdup(&de->pdev->dev, &ee_data[0], DE_EEPROM_SIZE,
+				   GFP_KERNEL);
 
 	return;
 
@@ -2092,7 +2093,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 err_out_iomap:
-	kfree(de->ee_data);
 	iounmap(regs);
 err_out_res:
 	pci_release_regions(pdev);
@@ -2106,7 +2106,6 @@ static void de_remove_one(struct pci_dev *pdev)
 
 	BUG_ON(!dev);
 	unregister_netdev(dev);
-	kfree(de->ee_data);
 	iounmap(de->regs);
 	pci_release_regions(pdev);
 }
-- 
2.28.0

Re: [PATCH net-next v2 0/3] First bunch of Tulip cleanups

[David Miller]

From: Moritz Fischer <mdf@kernel.org>
Date: Mon, 14 Sep 2020 21:24:49 -0700

> This series is the first bunch of minor cleanups for the de2104x driver
> to make it look and behave more like a modern driver.
> 
> These changes replace some of the non-devres versions with devres
> versions of functions to simplify the error paths.
> 
> Next up after this will be the ioremap part.

I really don't consider a "conversion" over to devres for older drivers
a suitable cleanup.

There are no resource handling bugs you are fixing, the driver uses
the APIs it uses correctly, and the coding style is reasonable.

Therefore, I'm sorry I'm not applying these changes.