[patch 4/4] Update e1000 driver to use devres.

[patch 4/4] Update e1000 driver to use devres.

[Brandon Philips]

[-- Attachment #1: e1000-devres.patch --]
[-- Type: text/plain, Size: 6880 bytes --]

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

Signed-off-by: Brandon Philips <bphilips@suse.de>
---
 drivers/net/e1000/e1000.h      |    1 
 drivers/net/e1000/e1000_main.c |   92 ++++++++++++-----------------------------
 2 files changed, 28 insertions(+), 65 deletions(-)

Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c
+++ 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;
 	}
Index: linux-2.6/drivers/net/e1000/e1000.h
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000.h
+++ 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>

-- 

-------------------------------------------------------------------------
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/

Re: [patch 4/4] Update e1000 driver to use devres.

[Waskiewicz Jr, Peter P]

> Index: linux-2.6/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/e1000/e1000_main.c
> +++ 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;

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/

Re: [patch 4/4] Update e1000 driver to use devres.

[Brandon Philips]

On 01:38 Thu 16 Aug 2007, Waskiewicz Jr, Peter P wrote:
> > -	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?

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:
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -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

-------------------------------------------------------------------------
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/

RE: [patch 4/4] Update e1000 driver to use devres.

[Waskiewicz Jr, Peter P]

> Now if I insert a return -ENOMEM right after allocating tx_ring:
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -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.

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

-PJ

Re: [patch 4/4] Update e1000 driver to use devres.

[Tejun Heo]

Brandon Philips wrote:
> -	mmio_start = pci_resource_start(pdev, BAR_0);
>  	mmio_len = pci_resource_len(pdev, BAR_0);

You don't need mmio_len either.

> -	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.

> @@ -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);

Ditto.

> -		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;
>  	}

-- 
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/

Re: [patch 4/4] Update e1000 driver to use devres.

[Kok, Auke]

Tejun Heo wrote:
> Brandon Philips wrote:
>> -	mmio_start = pci_resource_start(pdev, BAR_0);
>>  	mmio_len = pci_resource_len(pdev, BAR_0);
> 
> You don't need mmio_len either.
> 
>> -	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.
> 
>> @@ -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);
> 
> Ditto.
> 
>> -		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

-------------------------------------------------------------------------
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/

[PATCH] e1000e: Update e1000e driver to use devres

[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 <bphilips@suse.de>

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

Index: linux-netdev/drivers/net/e1000e/netdev.c
===================================================================
--- linux-netdev.orig/drivers/net/e1000e/netdev.c
+++ 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) */

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

[Tejun Heo]

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

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun