Skip to contentSkip to conversation

e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

PATCH In review Taku Izumi 4 participants 14 messages Jun 20, 2008 – Jun 27, 2008

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Taku Izumi

author
#0 lore raw

cover e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

Hi all,

Only a few Intel Gigabit Adapters need ioport resources for workaround, but most do not need them. Most adapters work properly without them. In the case where ioport resources are not assigned to adapters, this can happen on the large system, drivers' probe function fails like the following, and adapters can not be used as a result.

e1000e 0002:22:00.0: device not available because of BAR 2 [0:1f] collisions
e1000e: probe of 0002:22:00.0 failed with error -22

These patches corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. These are based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

* [PATCH 1/3] e1000: make ioport free
* [PATCH 2/3] e1000e: make ioport free
* [PATCH 3/3] igb: make ioport free

Best regards,
Taku Izumi <[email protected]>

author
#1 lore raw

PATCH 1/3 e1000: make ioport free

This patch makes e1000 driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. This is based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

Signed-off-by Tomohiro Kusumi <[email protected]>
Signed-off-by Auke Kok <[email protected]>
Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)
drivers/net/e1000/e1000.h +4 -0
Index: linux-2.6.25.7/drivers/net/e1000/e1000.h
===================================================================
@@ -343,6 +343,10 @@ struct e1000_adapter {
 	boolean_t quad_port_a;
 	unsigned long flags;
 	uint32_t eeprom_wol;
+
+	/* for ioport free */
+	int	bars;
+	int	need_ioport;
 };
 
 enum e1000_state_t {
drivers/net/e1000/e1000_main.c +76 -12

145 lines not on this page · raw message

#2 lore raw

Re: [PATCH 1/3] e1000: make ioport free

Quoted 6 lines · Taku Izumi

This patch makes e1000 driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. This is based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

Quoted 7 lines
Signed-off-by Tomohiro Kusumi <[email protected]>
Signed-off-by Auke Kok <[email protected]>
Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)

ACK but failed to apply

author
#3 lore raw

Re: [PATCH 1/3] e1000: make ioport free

Dear Jeff

Quoted 15 lines · Jeff Garzik
Quoted 6 lines · Taku Izumi

This patch makes e1000 driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. This is based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

Quoted 7 lines
Signed-off-by Tomohiro Kusumi <[email protected]>
Signed-off-by Auke Kok <[email protected]>
Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)
Quoted 2 lines

ACK but failed to apply

I'll re-create one.

Taku Izumi <[email protected]>

author
#4 lore raw

Re: [PATCH 1/3] e1000: make ioport free

This patch makes e1000 driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. This is based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

Signed-off-by Tomohiro Kusumi <[email protected]>
Signed-off-by Auke Kok <[email protected]>
Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)
drivers/net/e1000/e1000.h +4 -0
Index: netdev-2.6/drivers/net/e1000/e1000.h
===================================================================
@@ -342,6 +342,10 @@ struct e1000_adapter {
 	bool quad_port_a;
 	unsigned long flags;
 	u32 eeprom_wol;
+
+	/* for ioport free */
+	int bars;
+	int need_ioport;
 };
 
 enum e1000_state_t {
drivers/net/e1000/e1000_main.c +76 -12

145 lines not on this page · raw message

#5 lore raw

Re: [PATCH 1/3] e1000: make ioport free

Quoted 6 lines · Taku Izumi

This patch makes e1000 driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. This is based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

Quoted 7 lines
Signed-off-by Tomohiro Kusumi <[email protected]>
Signed-off-by Auke Kok <[email protected]>
Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)

I will add this into my e1000 queue and push it out in my next patchset for e1000.

Signature · 2 lines
Cheers,
Jeff
author
#6 lore raw

PATCH 2/3 e1000e: make ioport free

In reply to Taku Izumi

This patch makes e1000e driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed.

Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000e/e1000.h  |    4 +++
 drivers/net/e1000e/netdev.c |   48 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 6 deletions(-)
drivers/net/e1000e/e1000.h +4 -0
Index: linux-2.6.25.7/drivers/net/e1000e/e1000.h
===================================================================
@@ -266,6 +266,10 @@ struct e1000_adapter {
 	unsigned long led_status;
 
 	unsigned int flags;
+
+	/* for ioport free */
+	int bars;
+	int need_ioport;
 };
 
 struct e1000_info {
drivers/net/e1000e/netdev.c +42 -6

100 lines not on this page · raw message

author
#7 lore raw

Re: [PATCH 2/3] e1000e: make ioport free

This patch makes e1000e driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources.

Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000e/e1000.h  |    3 +++
 drivers/net/e1000e/netdev.c |   20 ++++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)
drivers/net/e1000e/e1000.h +3 -0
Index: linux-2.6.25.7/drivers/net/e1000e/e1000.h
===================================================================
@@ -266,6 +266,9 @@ struct e1000_adapter {
 	unsigned long led_status;
 
 	unsigned int flags;
+
+	/* for ioport free */
+	int bars;
 };
 
 struct e1000_info {
drivers/net/e1000e/netdev.c +14 -6
Index: linux-2.6.25.7/drivers/net/e1000e/netdev.c
===================================================================
@@ -3523,7 +3523,8 @@ static int e1000_resume(struct pci_dev *
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	e1000e_disable_l1aspm(pdev);
-	err = pci_enable_device(pdev);
+
+	err = pci_enable_device_mem(pdev);
 	if (err) {
 		dev_err(&pdev->dev,
 			"Cannot enable PCI device from suspend\n");

@@ -3622,9 +3623,11 @@ static pci_ers_result_t e1000_io_slot_re
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	int err;
 
 	e1000e_disable_l1aspm(pdev);
-	if (pci_enable_device(pdev)) {
+	err = pci_enable_device_mem(pdev);
+	if (err) {
 		dev_err(&pdev->dev,
 			"Cannot re-enable PCI device after reset.\n");
 		return PCI_ERS_RESULT_DISCONNECT;

@@ -3724,9 +3727,13 @@ static int __devinit e1000_probe(struct
 	int i, err, pci_using_dac;
 	u16 eeprom_data = 0;
 	u16 eeprom_apme_mask = E1000_EEPROM_APME;
+	int bars;
 
 	e1000e_disable_l1aspm(pdev);
-	err = pci_enable_device(pdev);
+
+	/* do not allocate ioport bars when not needed */
+	bars = pci_select_bars(pdev, IORESOURCE_MEM);
+	err = pci_enable_device_mem(pdev);
 	if (err)
 		return err;
 

@@ -3749,7 +3756,7 @@ static int __devinit e1000_probe(struct
 		}
 	}
 
-	err = pci_request_regions(pdev, e1000e_driver_name);
+	err = pci_request_selected_regions(pdev, bars, e1000e_driver_name);
 	if (err)
 		goto err_pci_reg;
 

@@ -3773,6 +3780,7 @@ static int __devinit e1000_probe(struct
 	adapter->hw.adapter = adapter;
 	adapter->hw.mac.type = ei->mac;
 	adapter->msg_enable = (1 << NETIF_MSG_DRV | NETIF_MSG_PROBE) - 1;
+	adapter->bars = bars;
 
 	mmio_start = pci_resource_start(pdev, 0);
 	mmio_len = pci_resource_len(pdev, 0);

@@ -4001,7 +4009,7 @@ err_sw_init:
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_release_regions(pdev);
+	pci_release_selected_regions(pdev, bars);
 err_pci_reg:
 err_dma:
 	pci_disable_device(pdev);

@@ -4045,7 +4053,7 @@ static void __devexit e1000_remove(struc
 	iounmap(adapter->hw.hw_addr);
 	if (adapter->hw.flash_address)
 		iounmap(adapter->hw.flash_address);
-	pci_release_regions(pdev);
+	pci_release_selected_regions(pdev, adapter->bars);
 
 	free_netdev(netdev);
#8 lore raw

Re: [PATCH 2/3] e1000e: make ioport free

In reply to Taku Izumi
Quoted 9 lines · Taku Izumi

This patch makes e1000e driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed.

Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/e1000e/e1000.h  |    4 +++
 drivers/net/e1000e/netdev.c |   48 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 6 deletions(-)

applied 2-3

author
#9 lore raw

PATCH 3/3 igb: make ioport free

In reply to Taku Izumi

This patch makes igb driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources as long as they are not really needed.

Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/igb/igb.h      |    4 +++
 drivers/net/igb/igb_main.c |   47 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 6 deletions(-)
drivers/net/igb/igb.h +4 -0
Index: linux-2.6.25.7/drivers/net/igb/igb.h
===================================================================
@@ -271,6 +271,10 @@ struct igb_adapter {
 	unsigned int msi_enabled;
 
 	u32 eeprom_wol;
+
+	/* for ioport free */
+	int bars;
+	int need_ioport;
 };
 
 enum e1000_state_t {
drivers/net/igb/igb_main.c +41 -6

97 lines not on this page · raw message

author
#10 lore raw

Re: [PATCH 3/3] igb: make ioport free

This patch makes igb driver ioport-free.
This corrects behavior in probe function so as not to request ioport resources.

Signed-off-by Taku Izumi <[email protected]>
---
 drivers/net/igb/igb.h      |    3 +++
 drivers/net/igb/igb_main.c |   19 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)
drivers/net/igb/igb.h +3 -0
Index: linux-2.6.25.7/drivers/net/igb/igb.h
===================================================================
@@ -271,6 +271,9 @@ struct igb_adapter {
 	unsigned int msi_enabled;
 
 	u32 eeprom_wol;
+
+	/* for ioport free */
+	int bars;
 };
 
 enum e1000_state_t {
drivers/net/igb/igb_main.c +13 -6
Index: linux-2.6.25.7/drivers/net/igb/igb_main.c
===================================================================
@@ -843,8 +843,11 @@ static int __devinit igb_probe(struct pc
 	u16 eeprom_data = 0;
 	u16 eeprom_apme_mask = IGB_EEPROM_APME;
 	u32 part_num;
+	int bars;
 
-	err = pci_enable_device(pdev);
+	/* do not allocate ioport bars when not needed */
+	bars = pci_select_bars(pdev, IORESOURCE_MEM);
+	err = pci_enable_device_mem(pdev);
 	if (err)
 		return err;
 

@@ -866,7 +869,7 @@ static int __devinit igb_probe(struct pc
 		}
 	}
 
-	err = pci_request_regions(pdev, igb_driver_name);
+	err = pci_request_selected_regions(pdev, bars, igb_driver_name);
 	if (err)
 		goto err_pci_reg;
 

@@ -886,6 +889,7 @@ static int __devinit igb_probe(struct pc
 	hw = &adapter->hw;
 	hw->back = adapter;
 	adapter->msg_enable = NETIF_MSG_DRV | NETIF_MSG_PROBE;
+	adapter->bars = bars;
 
 	mmio_start = pci_resource_start(pdev, 0);
 	mmio_len = pci_resource_len(pdev, 0);

@@ -1121,7 +1125,7 @@ err_hw_init:
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
-	pci_release_regions(pdev);
+	pci_release_selected_regions(pdev, bars);
 err_pci_reg:
 err_dma:
 	pci_disable_device(pdev);

@@ -1168,7 +1172,7 @@ static void __devexit igb_remove(struct
 	iounmap(adapter->hw.hw_addr);
 	if (adapter->hw.flash_address)
 		iounmap(adapter->hw.flash_address);
-	pci_release_regions(pdev);
+	pci_release_selected_regions(pdev, adapter->bars);
 
 	free_netdev(netdev);
 

@@ -3969,7 +3973,8 @@ static int igb_resume(struct pci_dev *pd
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	err = pci_enable_device(pdev);
+
+	err = pci_enable_device_mem(pdev);
 	if (err) {
 		dev_err(&pdev->dev,
 			"igb: Cannot enable PCI device from suspend\n");

@@ -4072,8 +4077,10 @@ static pci_ers_result_t igb_io_slot_rese
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	int err;
 
-	if (pci_enable_device(pdev)) {
+	err = pci_enable_device_mem(pdev);
+	if (err) {
 		dev_err(&pdev->dev,
 			"Cannot re-enable PCI device after reset.\n");
 		return PCI_ERS_RESULT_DISCONNECT;
#11 lore raw
In reply to Taku Izumi
Quoted 11 lines · Taku Izumi

Hi all,

Only a few Intel Gigabit Adapters need ioport resources for workaround, but most do not need them. Most adapters work properly without them. In the case where ioport resources are not assigned to adapters, this can happen on the large system, drivers' probe function fails like the following, and adapters can not be used as a result.

e1000e 0002:22:00.0: device not available because of BAR 2 [0:1f] collisions
e1000e: probe of 0002:22:00.0 failed with error -22

Quoted 7 lines

These patches corrects behavior in probe function so as not to request ioport resources as long as they are not really needed. These are based on the ioport-free patch of e1000 driver from Auke Kok and Tomohiro Kusumi.

* [PATCH 1/3] e1000: make ioport free
* [PATCH 2/3] e1000e: make ioport free
* [PATCH 3/3] igb: make ioport free

I think patch 2 and 3 are way too large since igb and e1000e can be totally ioport free at all times. There is no need to keep compatibility code for ioport in those drivers as it's unlikely that this will ever be needed.

So, perhaps you can remove the ioport code from those 2 drivers (e1000e/igb) completely and resubmit to Jeff Kirsher?

Cheers,

Auke

author
#12 lore raw

Dear Auke

Quoted 7 lines
Quoted 3 lines

* [PATCH 1/3] e1000: make ioport free
* [PATCH 2/3] e1000e: make ioport free
* [PATCH 3/3] igb: make ioport free

I think patch 2 and 3 are way too large since igb and e1000e can be totally ioport free at all times. There is no need to keep compatibility code for ioport in those drivers as it's unlikely that this will ever be needed.

I left an old implementation in case adapters that need ioport resources appear, but it's true that these codes are a little redundant for igb and e1000e drivers.

Quoted 2 lines

So, perhaps you can remove the ioport code from those 2 drivers (e1000e/igb) completely and resubmit to Jeff Kirsher?

OK, I'll send them later.

Best regards,
Taku Izumi <[email protected]>

#13 lore raw
Quoted 17 lines · Taku Izumi

Dear Auke

Quoted 9 lines
Quoted 3 lines

* [PATCH 1/3] e1000: make ioport free
* [PATCH 2/3] e1000e: make ioport free
* [PATCH 3/3] igb: make ioport free

I think patch 2 and 3 are way too large since igb and e1000e can be totally ioport
free at all times. There is no need to keep compatibility code for ioport in those
drivers as it's unlikely that this will ever be needed.

I left an old implementation in case adapters that need ioport resources appear,
but it's true that these codes are a little redundant for igb and e1000e drivers.

Quoted 6 lines
Quoted 3 lines

So, perhaps you can remove the ioport code from those 2 drivers (e1000e/igb)
completely and resubmit to Jeff Kirsher?

OK, I'll send them later.

The updated patchset looks fine. ACK.

Signed-off-by Jeff Kirsher <[email protected]>
Signature · 2 lines
Cheers,
Jeff