Update e1000 driver to use devres.

PATCH In review Brandon Philips 4 participants 8 messages Aug 15, 2007 – Aug 18, 2007

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Brandon Philips

author

2007-08-15 19:00 UTC #0 lore raw

PATCH 4/4 Update e1000 driver to use devres.

Conversion of e1000 probe() and remove() to devres.

Signed-off-by Brandon Philips <[email protected]>

---
 drivers/net/e1000/e1000.h      |    1 
 drivers/net/e1000/e1000_main.c |   92 ++++++++++++-----------------------------
 2 files changed, 28 insertions(+), 65 deletions(-)

drivers/net/e1000/e1000_main.c +27 ~~-65~~

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
@@ -860,15 +860,14 @@ e1000_probe(struct pci_dev *pdev,
 {
 	struct net_device *netdev;
 	struct e1000_adapter *adapter;
-	unsigned long mmio_start, mmio_len;
-	unsigned long flash_start, flash_len;
+	unsigned long mmio_len, flash_len;
 
 	static int cards_found = 0;
 	static int global_quad_port_a = 0; /* global ksp3 port a indication */
 	int i, err, pci_using_dac;
 	uint16_t eeprom_data = 0;
 	uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
-	if ((err = pci_enable_device(pdev)))
+	if ((err = pcim_enable_device(pdev)))
 		return err;
 
 	if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK)) &&

@@ -878,20 +877,19 @@ e1000_probe(struct pci_dev *pdev,
 		if ((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK)) &&
 		    (err = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK))) {
 			E1000_ERR("No usable DMA configuration, aborting\n");
-			goto err_dma;
+			return err;
 		}
 		pci_using_dac = 0;
 	}
 
 	if ((err = pci_request_regions(pdev, e1000_driver_name)))
-		goto err_pci_reg;
+		return err;
 
 	pci_set_master(pdev);
 
-	err = -ENOMEM;
-	netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+	netdev = devm_alloc_etherdev(&pdev->dev, sizeof(struct e1000_adapter));
 	if (!netdev)
-		goto err_alloc_etherdev;
+		return -ENOMEM;
 
 	SET_MODULE_OWNER(netdev);
 	SET_NETDEV_DEV(netdev, &pdev->dev);

@@ -903,13 +901,11 @@ e1000_probe(struct pci_dev *pdev,
 	adapter->hw.back = adapter;
 	adapter->msg_enable = (1 << debug) - 1;
 
-	mmio_start = pci_resource_start(pdev, BAR_0);
 	mmio_len = pci_resource_len(pdev, BAR_0);
 
-	err = -EIO;
-	adapter->hw.hw_addr = ioremap(mmio_start, mmio_len);
+	adapter->hw.hw_addr = pcim_iomap(pdev, BAR_0, mmio_len);
 	if (!adapter->hw.hw_addr)
-		goto err_ioremap;
+		return -EIO;
 
 	for (i = BAR_1; i <= BAR_5; i++) {
 		if (pci_resource_len(pdev, i) == 0)

@@ -943,8 +939,8 @@ e1000_probe(struct pci_dev *pdev,
 #endif
 	strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);
 
-	netdev->mem_start = mmio_start;
-	netdev->mem_end = mmio_start + mmio_len;
+	netdev->mem_start = pci_resource_start(pdev, BAR_0);
+	netdev->mem_end = netdev->mem_start + mmio_len;
 	netdev->base_addr = adapter->hw.io_base;
 
 	adapter->bd_number = cards_found;

@@ -952,16 +948,15 @@ e1000_probe(struct pci_dev *pdev,
 	/* setup the private structure */
 
 	if ((err = e1000_sw_init(adapter)))
-		goto err_sw_init;
+		return err;
 
 	err = -EIO;
 	/* Flash BAR mapping must happen after e1000_sw_init
 	 * because it depends on mac_type */
 	if ((adapter->hw.mac_type == e1000_ich8lan) &&
 	   (pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
-		flash_start = pci_resource_start(pdev, 1);
 		flash_len = pci_resource_len(pdev, 1);
-		adapter->hw.flash_address = ioremap(flash_start, flash_len);
+		adapter->hw.flash_address = pcim_iomap(pdev, 1, flash_len);
 		if (!adapter->hw.flash_address)
 			goto err_flashmap;
 	}

@@ -1163,29 +1158,11 @@ err_register:
 err_eeprom:
 	if (!e1000_check_phy_reset_block(&adapter->hw))
 		e1000_phy_hw_reset(&adapter->hw);
-
-	if (adapter->hw.flash_address)
-		iounmap(adapter->hw.flash_address);
 err_flashmap:
 #ifdef CONFIG_E1000_NAPI
 	for (i = 0; i < adapter->num_rx_queues; i++)
 		dev_put(&adapter->polling_netdev[i]);
 #endif
-
-	kfree(adapter->tx_ring);
-	kfree(adapter->rx_ring);
-#ifdef CONFIG_E1000_NAPI
-	kfree(adapter->polling_netdev);
-#endif
-err_sw_init:
-	iounmap(adapter->hw.hw_addr);
-err_ioremap:
-	free_netdev(netdev);
-err_alloc_etherdev:
-	pci_release_regions(pdev);
-err_pci_reg:
-err_dma:
-	pci_disable_device(pdev);
 	return err;
 }

@@ -1224,21 +1201,6 @@ e1000_remove(struct pci_dev *pdev)
 
 	if (!e1000_check_phy_reset_block(&adapter->hw))
 		e1000_phy_hw_reset(&adapter->hw);
-
-	kfree(adapter->tx_ring);
-	kfree(adapter->rx_ring);
-#ifdef CONFIG_E1000_NAPI
-	kfree(adapter->polling_netdev);
-#endif
-
-	iounmap(adapter->hw.hw_addr);
-	if (adapter->hw.flash_address)
-		iounmap(adapter->hw.flash_address);
-	pci_release_regions(pdev);
-
-	free_netdev(netdev);
-
-	pci_disable_device(pdev);
 }
 
 /**

@@ -1350,27 +1312,27 @@ e1000_sw_init(struct e1000_adapter *adap
 static int __devinit
 e1000_alloc_queues(struct e1000_adapter *adapter)
 {
-	adapter->tx_ring = kcalloc(adapter->num_tx_queues,
-	                           sizeof(struct e1000_tx_ring), GFP_KERNEL);
+	adapter->tx_ring = devm_kcalloc(&adapter->pdev->dev,
+					adapter->num_tx_queues,
+					sizeof(struct e1000_tx_ring),
+					GFP_KERNEL);
 	if (!adapter->tx_ring)
 		return -ENOMEM;
 
-	adapter->rx_ring = kcalloc(adapter->num_rx_queues,
-	                           sizeof(struct e1000_rx_ring), GFP_KERNEL);
-	if (!adapter->rx_ring) {
-		kfree(adapter->tx_ring);
+	adapter->rx_ring = devm_kcalloc(&adapter->pdev->dev,
+					adapter->num_rx_queues,
+					sizeof(struct e1000_rx_ring),
+					GFP_KERNEL);
+	if (!adapter->rx_ring)
 		return -ENOMEM;
-	}
 
 #ifdef CONFIG_E1000_NAPI
-	adapter->polling_netdev = kcalloc(adapter->num_rx_queues,
-	                                  sizeof(struct net_device),
-	                                  GFP_KERNEL);
-	if (!adapter->polling_netdev) {
-		kfree(adapter->tx_ring);
-		kfree(adapter->rx_ring);
+	adapter->polling_netdev = devm_kcalloc(&adapter->pdev->dev,
+					       adapter->num_rx_queues,
+					       sizeof(struct net_device),
+					       GFP_KERNEL);
+	if (!adapter->polling_netdev)
 		return -ENOMEM;
-	}
 #endif
 
 	return E1000_SUCCESS;

@@ -5174,7 +5136,7 @@ e1000_resume(struct pci_dev *pdev)
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	if ((err = pci_enable_device(pdev))) {
+	if ((err = pcim_enable_device(pdev))) {
 		printk(KERN_ERR "e1000: Cannot enable PCI device from suspend\n");
 		return err;
 	}

drivers/net/e1000/e1000.h +1 -0

Index: linux-2.6/drivers/net/e1000/e1000.h
===================================================================
@@ -41,6 +41,7 @@
 #include <linux/errno.h>
 #include <linux/ioport.h>
 #include <linux/pci.h>
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>

Signature · 5 lines

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Waskiewicz Jr, Peter P

2007-08-16 08:38 UTC #1 lore raw

Quoted 37 lines

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================

drivers/net/e1000/e1000_main.c +1 -2

@@ -860,15 +860,14 @@ e1000_probe(struct pci_dev *pdev,  {
 	struct net_device *netdev;
 	struct e1000_adapter *adapter;
-	unsigned long mmio_start, mmio_len;
-	unsigned long flash_start, flash_len;
+	unsigned long mmio_len, flash_len;
 
 	static int cards_found = 0;
 	static int global_quad_port_a = 0; /* global ksp3 port

a indication */

drivers/net/e1000/e1000_main.c +3 -3

 	int i, err, pci_using_dac;
 	uint16_t eeprom_data = 0;
 	uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
-	if ((err = pci_enable_device(pdev)))
+	if ((err = pcim_enable_device(pdev)))
 		return err;
 
 	if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK)) &&

@@ -878,20 +877,19 @@ e1000_probe(struct pci_dev *pdev,
 		if ((err = pci_set_dma_mask(pdev, DMA_32BIT_MASK)) &&
 		    (err = pci_set_consistent_dma_mask(pdev, 
 DMA_32BIT_MASK))) {
 			E1000_ERR("No usable DMA configuration, 
 aborting\n");
-			goto err_dma;
+			return err;
 		}
 		pci_using_dac = 0;
 	}
 
 	if ((err = pci_request_regions(pdev, e1000_driver_name)))
-		goto err_pci_reg;
+		return err;

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 895

Quoted 10 lines

drivers/net/e1000/e1000_main.c +3 -3

 
 	pci_set_master(pdev);
 
-	err = -ENOMEM;
-	netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+	netdev = devm_alloc_etherdev(&pdev->dev, sizeof(struct 
+e1000_adapter));
 	if (!netdev)
-		goto err_alloc_etherdev;
+		return -ENOMEM;

I'm a bit confused why you removed the goto's, and then removed all the
target unwinding code at the bottom of e1000_probe(). Those labels
clean up resources if something fails, like the err_sw_init label. I don't see anything in the devres code that jumps out at me that explains why we can do away with these cleanup routines. Thoughts?

Thanks,
-PJ Waskiewicz

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

Brandon Philips

author

2007-08-16 17:05 UTC #2 lore raw

Quoted 5 lines · Peter P

Quoted 5 lines

(quoted diff) +2 -2

-	err = -ENOMEM;
-	netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+	netdev = devm_alloc_etherdev(&pdev->dev, sizeof(struct 
+e1000_adapter));
 	if (!netdev)

Quoted 8 lines

Review
Quoted 2 lines
(quoted diff) +1 -1
-		goto err_alloc_etherdev;
+		return -ENOMEM;
I'm a bit confused why you removed the goto's, and then removed all the
target unwinding code at the bottom of e1000_probe(). Those labels
clean up resources if something fails, like the err_sw_init label. I don't see anything in the devres code that jumps out at me that explains why we can do away with these cleanup routines. Thoughts?

Have you read Documentation/driver-model/devres.txt? That has a good explanation. Here is a practical explanation on how it works too.

This is the output from a normal modprobe then rmmod of e1000 with devres debugging on.

# modprobe
DEVRES ADD f7fd6cc0 pcim_release (8 bytes)
DEVRES ADD f7a80fe0 devm_free_netdev (4 bytes) # netdev **p for free_netdev
DEVRES ADD f7dbe780 pcim_iomap_release (24 bytes) # adapter->hw.hw_addr
DEVRES ADD f7dbe9c0 devm_kzalloc_release (40 bytes) # adapter->tx_ring
DEVRES ADD f7dbe8c0 devm_kzalloc_release (44 bytes) # adapter->rx_ring

# rmmod
DEVRES REL f7dbe8c0 devm_kzalloc_release (44 bytes) # adapter->tx_ring
DEVRES REL f7dbe9c0 devm_kzalloc_release (40 bytes) # adapter->rx_ring
DEVRES REL f7dbe780 pcim_iomap_release (24 bytes) # adapter->hw.hw_addr
DEVRES REL f7a80fe0 devm_free_netdev (4 bytes) # called free_netdev
DEVRES REL f7fd6cc0 pcim_release (8 bytes)

Now if I insert a return -ENOMEM right after allocating tx_ring:

drivers/net/e1000/e1000_main.c +0 -0

@@ -1356,6 +1356,8 @@ e1000_alloc_queues(struct e1000_adapter *adapter)
 {
        adapter->tx_ring = kcalloc(adapter->num_tx_queues,
                                   sizeof(struct e1000_tx_ring),

GFP_KERNEL);
+

+       return -ENOMEM;
        if (!adapter->tx_ring)
                return -ENOMEM;

#insmod
DEVRES ADD f7a80e80 pcim_release (8 bytes) DEVRES ADD f7a80ca0 devm_free_netdev (4 bytes) DEVRES ADD eb7f0080 pcim_iomap_release (24 bytes) DEVRES ADD eb7f0000 devm_kzalloc_release (40 bytes)
e1000_sw_init: Unable to allocate memory for queues DEVRES REL eb7f0000 devm_kzalloc_release (40 bytes) DEVRES REL eb7f0080 pcim_iomap_release (24 bytes) DEVRES REL f7a80ca0 devm_free_netdev (4 bytes) DEVRES REL f7a80e80 pcim_release (8 bytes)
ACPI: PCI interrupt for device 0000:02:00.0 disabled
e1000: probe of 0000:02:00.0 failed with error -12

Since we are returning an error from probe the driver core calls devres_release_all(dev) which releases all of the resources in the right order. See really_probe() in drivers/base/dd.c.

SIDE NOTE
---------
I ran into a possible e1000 bug with the little -ENOMEM patch above both with and without the devres patches. The driver seems to leave the EEPROM in a bad state on error because I get this error after trying to insert the module again:

e1000_probe: The EEPROM Checksum Is Not Valid

A power cycle but not a reboot fixes it.

Thanks,

Brandon

Waskiewicz Jr, Peter P

2007-08-16 17:10 UTC #3 lore raw

Quoted 25 lines

Now if I insert a return -ENOMEM right after allocating tx_ring:
drivers/net/e1000/e1000_main.c +2 -0
@@ -1356,6 +1356,8 @@ e1000_alloc_queues(struct e1000_adapter 
 *adapter)  {
        adapter->tx_ring = kcalloc(adapter->num_tx_queues,
                                   sizeof(struct 
 e1000_tx_ring), GFP_KERNEL);
+
+       return -ENOMEM;
        if (!adapter->tx_ring)
                return -ENOMEM;
#insmod
DEVRES ADD f7a80e80 pcim_release (8 bytes) DEVRES ADD f7a80ca0 devm_free_netdev (4 bytes) DEVRES ADD eb7f0080 pcim_iomap_release (24 bytes) DEVRES ADD eb7f0000 devm_kzalloc_release (40 bytes)
e1000_sw_init: Unable to allocate memory for queues DEVRES REL eb7f0000 devm_kzalloc_release (40 bytes) DEVRES REL eb7f0080 pcim_iomap_release (24 bytes) DEVRES REL f7a80ca0 devm_free_netdev (4 bytes) DEVRES REL f7a80e80 pcim_release (8 bytes)
ACPI: PCI interrupt for device 0000:02:00.0 disabled
e1000: probe of 0000:02:00.0 failed with error -12

Quoted 3 lines

Since we are returning an error from probe the driver core calls devres_release_all(dev) which releases all of the resources in the right order. See really_probe() in drivers/base/dd.c.

This looks fine then to me. Thanks for the explanation.

-PJ

Tejun Heo

2007-08-16 11:44 UTC #4 lore raw

In reply to Brandon Philips

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 904

Quoted 2 lines · Brandon Philips

drivers/net/e1000/e1000_main.c +0 -1

-	mmio_start = pci_resource_start(pdev, BAR_0);
 	mmio_len = pci_resource_len(pdev, BAR_0);

You don't need mmio_len either.

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 906

Quoted 3 lines

drivers/net/e1000/e1000_main.c +1 -2

-	err = -EIO;
-	adapter->hw.hw_addr = ioremap(mmio_start, mmio_len);
+	adapter->hw.hw_addr = pcim_iomap(pdev, BAR_0, mmio_len);

Passing 0 as @max_len tells pci[m]_iomap() to use pci_resource_len() of the BAR.

Quoted 4 lines

drivers/net/e1000/e1000_main.c +0 -0

@@ -952,16 +948,15 @@ e1000_probe(struct pci_dev *pdev,
 	/* setup the private structure */
 
 	if ((err = e1000_sw_init(adapter)))

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 958

Quoted 10 lines

drivers/net/e1000/e1000_main.c +1 -2

-		goto err_sw_init;
+		return err;
 
 	err = -EIO;
 	/* Flash BAR mapping must happen after e1000_sw_init
 	 * because it depends on mac_type */
 	if ((adapter->hw.mac_type == e1000_ich8lan) &&
 	   (pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
-		flash_start = pci_resource_start(pdev, 1);
 		flash_len = pci_resource_len(pdev, 1);

Ditto.

Quoted 5 lines

drivers/net/e1000/e1000_main.c +1 -1

-		adapter->hw.flash_address = ioremap(flash_start, flash_len);
+		adapter->hw.flash_address = pcim_iomap(pdev, 1, flash_len);
 		if (!adapter->hw.flash_address)
 			goto err_flashmap;
 	}

Signature · 7 lines

tejun

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Kok, Auke

2007-08-16 17:36 UTC #5 lore raw

Quoted 30 lines · Tejun Heo

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 904
Quoted 2 lines · Brandon Philips
drivers/net/e1000/e1000_main.c +0 -1
-	mmio_start = pci_resource_start(pdev, BAR_0);
 	mmio_len = pci_resource_len(pdev, BAR_0);
You don't need mmio_len either.
drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 906
Quoted 3 lines
drivers/net/e1000/e1000_main.c +1 -2
-	err = -EIO;
-	adapter->hw.hw_addr = ioremap(mmio_start, mmio_len);
+	adapter->hw.hw_addr = pcim_iomap(pdev, BAR_0, mmio_len);
Passing 0 as @max_len tells pci[m]_iomap() to use pci_resource_len() of the BAR.
Quoted 4 lines
drivers/net/e1000/e1000_main.c +0 -0
@@ -952,16 +948,15 @@ e1000_probe(struct pci_dev *pdev,
 	/* setup the private structure */
 
 	if ((err = e1000_sw_init(adapter)))
drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 958
Quoted 10 lines
drivers/net/e1000/e1000_main.c +1 -2
-		goto err_sw_init;
+		return err;
 
 	err = -EIO;
 	/* Flash BAR mapping must happen after e1000_sw_init
 	 * because it depends on mac_type */
 	if ((adapter->hw.mac_type == e1000_ich8lan) &&
 	   (pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
-		flash_start = pci_resource_start(pdev, 1);
 		flash_len = pci_resource_len(pdev, 1);
Ditto.

Quoted 6 lines

drivers/net/e1000/e1000_main.c e1000_probe(struct pci_dev *pdev, · line 962
Quoted 5 lines
drivers/net/e1000/e1000_main.c +1 -1
-		adapter->hw.flash_address = ioremap(flash_start, flash_len);
+		adapter->hw.flash_address = pcim_iomap(pdev, 1, flash_len);
 		if (!adapter->hw.flash_address)
 			goto err_flashmap;
 	}

brandon,

seeing the multiple revisions I am scared that this will produce some fallout and e1000 already is quite fragile. I would suggest that you instead work against "e1000e" which is in a branch on jgarzik's netdev tree instead. This driver is new and it would be much more interesting to have devres used in here instead.

Since this driver is in -mm as well this would give your patches some testing before it goes upstream.

Let's leave e1000 alone for now if we can.

Auke

Brandon Philips

author

2007-08-17 20:25 UTC #6 lore raw

PATCH 1/1 e1000e: Update e1000e driver to use devres

Conversion of e1000e probe() and remove() to devres.

Depends on "[patch 1/4] Update net core to use devres."

Signed-off-by Brandon Philips <[email protected]>

---
 drivers/net/e1000e/netdev.c |   70 ++++++++++----------------------------------
 1 file changed, 17 insertions(+), 53 deletions(-)

drivers/net/e1000e/netdev.c +17 ~~-53~~

Index: linux-netdev/drivers/net/e1000e/netdev.c
===================================================================
@@ -2516,6 +2516,7 @@ void e1000e_reinit_locked(struct e1000_a
 static int __devinit e1000_sw_init(struct e1000_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	struct net_device *netdev = adapter->netdev;
 
 	adapter->rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN;

@@ -2523,11 +2524,13 @@ static int __devinit e1000_sw_init(struc
 	hw->mac.max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
 	hw->mac.min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
 
-	adapter->tx_ring = kzalloc(sizeof(struct e1000_ring), GFP_KERNEL);
+	adapter->tx_ring = devm_kzalloc(&pdev->dev,
+					sizeof(struct e1000_ring), GFP_KERNEL);
 	if (!adapter->tx_ring)
 		goto err;
 
-	adapter->rx_ring = kzalloc(sizeof(struct e1000_ring), GFP_KERNEL);
+	adapter->rx_ring = devm_kzalloc(&pdev->dev,
+					sizeof(struct e1000_ring), GFP_KERNEL);
 	if (!adapter->rx_ring)
 		goto err;

@@ -2544,8 +2547,6 @@ static int __devinit e1000_sw_init(struc
 
 err:
 	ndev_err(netdev, "Unable to allocate memory for queues\n");
-	kfree(adapter->rx_ring);
-	kfree(adapter->tx_ring);
 	return -ENOMEM;
 }

@@ -4016,15 +4017,13 @@ static int __devinit e1000_probe(struct 
 	struct e1000_adapter *adapter;
 	struct e1000_hw *hw;
 	const struct e1000_info *ei = e1000_info_tbl[ent->driver_data];
-	unsigned long mmio_start, mmio_len;
-	unsigned long flash_start, flash_len;
 
 	static int cards_found;
 	int i, err, pci_using_dac;
 	u16 eeprom_data = 0;
 	u16 eeprom_apme_mask = E1000_EEPROM_APME;
 
-	err = pci_enable_device(pdev);
+	err = pcim_enable_device(pdev);
 	if (err)
 		return err;

@@ -4042,21 +4041,20 @@ static int __devinit e1000_probe(struct 
 			if (err) {
 				dev_err(&pdev->dev, "No usable DMA "
 					"configuration, aborting\n");
-				goto err_dma;
+				return err;
 			}
 		}
 	}
 
 	err = pci_request_regions(pdev, e1000e_driver_name);
 	if (err)
-		goto err_pci_reg;
+		return err;
 
 	pci_set_master(pdev);
 
-	err = -ENOMEM;
-	netdev = alloc_etherdev(sizeof(struct e1000_adapter));
+	netdev = devm_alloc_etherdev(&pdev->dev, sizeof(struct e1000_adapter));
 	if (!netdev)
-		goto err_alloc_etherdev;
+		return -ENOMEM;
 
 	SET_MODULE_OWNER(netdev);
 	SET_NETDEV_DEV(netdev, &pdev->dev);

@@ -4073,21 +4071,16 @@ static int __devinit e1000_probe(struct 
 	adapter->hw.mac.type = ei->mac;
 	adapter->msg_enable = (1 << NETIF_MSG_DRV | NETIF_MSG_PROBE) - 1;
 
-	mmio_start = pci_resource_start(pdev, 0);
-	mmio_len = pci_resource_len(pdev, 0);
 
-	err = -EIO;
-	adapter->hw.hw_addr = ioremap(mmio_start, mmio_len);
+	adapter->hw.hw_addr = pcim_iomap(pdev, 0, 0);
 	if (!adapter->hw.hw_addr)
-		goto err_ioremap;
+		return -EIO;
 
 	if ((adapter->flags & FLAG_HAS_FLASH) &&
 	    (pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
-		flash_start = pci_resource_start(pdev, 1);
-		flash_len = pci_resource_len(pdev, 1);
-		adapter->hw.flash_address = ioremap(flash_start, flash_len);
+		adapter->hw.flash_address = pcim_iomap(pdev, 1, 0);
 		if (!adapter->hw.flash_address)
-			goto err_flashmap;
+			return -EIO;
 	}
 
 	/* construct the net_device struct */

@@ -4112,17 +4105,15 @@ static int __devinit e1000_probe(struct 
 #endif
 	strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);
 
-	netdev->mem_start = mmio_start;
-	netdev->mem_end = mmio_start + mmio_len;
+	netdev->mem_start = pci_resource_start(pdev, 0);
+	netdev->mem_end = netdev->mem_start + pci_resource_len(pdev, 0);
 
 	adapter->bd_number = cards_found++;
 
 	/* setup adapter struct */
 	err = e1000_sw_init(adapter);
 	if (err)
-		goto err_sw_init;
-
-	err = -EIO;
+		return err;
 
 	memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
 	memcpy(&hw->nvm.ops, ei->nvm_ops, sizeof(hw->nvm.ops));

@@ -4290,21 +4281,6 @@ err_eeprom:
 	if (!e1000_check_reset_block(&adapter->hw))
 		e1000_phy_hw_reset(&adapter->hw);
 
-	if (adapter->hw.flash_address)
-		iounmap(adapter->hw.flash_address);
-
-err_flashmap:
-	kfree(adapter->tx_ring);
-	kfree(adapter->rx_ring);
-err_sw_init:
-	iounmap(adapter->hw.hw_addr);
-err_ioremap:
-	free_netdev(netdev);
-err_alloc_etherdev:
-	pci_release_regions(pdev);
-err_pci_reg:
-err_dma:
-	pci_disable_device(pdev);
 	return err;
 }

@@ -4340,18 +4316,6 @@ static void __devexit e1000_remove(struc
 
 	if (!e1000_check_reset_block(&adapter->hw))
 		e1000_phy_hw_reset(&adapter->hw);
-
-	kfree(adapter->tx_ring);
-	kfree(adapter->rx_ring);
-
-	iounmap(adapter->hw.hw_addr);
-	if (adapter->hw.flash_address)
-		iounmap(adapter->hw.flash_address);
-	pci_release_regions(pdev);
-
-	free_netdev(netdev);
-
-	pci_disable_device(pdev);
 }
 
 /* PCI Error Recovery (ERS) */

Tejun Heo

Acked-by on v1 1/1

2007-08-18 00:52 UTC #7 lore raw

Re: [PATCH] e1000e: Update e1000e driver to use devres

Quoted 5 lines · Brandon Philips

Conversion of e1000e probe() and remove() to devres.
Depends on "[patch 1/4] Update net core to use devres."
Signed-off-by Brandon Philips <[email protected]>

Acked-by Tejun Heo <[email protected]>

Signature · 1 line

tejun