[PATCH 0/3] e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

[PATCH 0/3] e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

[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

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 <izumi.taku@jp.fujitsu.com>

[PATCH 1/3] e1000: make ioport free

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


Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@jp.fujitsu.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)

Index: linux-2.6.25.7/drivers/net/e1000/e1000.h
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000/e1000.h
+++ 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 {
Index: linux-2.6.25.7/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000/e1000_main.c
+++ linux-2.6.25.7/drivers/net/e1000/e1000_main.c
@@ -909,6 +909,44 @@ static void e1000_dump_eeprom(struct e10
 }

 /**
+ * e1000_is_need_ioport - determine if an adapter needs ioport resources or not
+ * @pdev: PCI device information struct
+ *
+ * Returns true if an adapter needs ioport resources
+ **/
+
+static int
+e1000_is_need_ioport(struct pci_dev *pdev)
+{
+	switch (pdev->device) {
+	case E1000_DEV_ID_82540EM:
+	case E1000_DEV_ID_82540EM_LOM:
+	case E1000_DEV_ID_82540EP:
+	case E1000_DEV_ID_82540EP_LOM:
+	case E1000_DEV_ID_82540EP_LP:
+	case E1000_DEV_ID_82541EI:
+	case E1000_DEV_ID_82541EI_MOBILE:
+	case E1000_DEV_ID_82541ER_LOM:
+	case E1000_DEV_ID_82541ER:
+	case E1000_DEV_ID_82541GI:
+	case E1000_DEV_ID_82541GI_LF:
+	case E1000_DEV_ID_82541GI_MOBILE:
+	case E1000_DEV_ID_82544EI_COPPER:
+	case E1000_DEV_ID_82544EI_FIBER:
+	case E1000_DEV_ID_82544GC_COPPER:
+	case E1000_DEV_ID_82544GC_LOM:
+	case E1000_DEV_ID_82545EM_COPPER:
+	case E1000_DEV_ID_82545EM_FIBER:
+	case E1000_DEV_ID_82546EB_COPPER:
+	case E1000_DEV_ID_82546EB_FIBER:
+	case E1000_DEV_ID_82546EB_QUAD_COPPER:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/**
  * e1000_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in e1000_pci_tbl
@@ -932,9 +970,19 @@ e1000_probe(struct pci_dev *pdev,
 	int i, err, pci_using_dac;
 	uint16_t eeprom_data = 0;
 	uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
+	int bars, need_ioport;
 	DECLARE_MAC_BUF(mac);

-	if ((err = pci_enable_device(pdev)))
+	/* do not allocate ioport bars when not needed */
+	need_ioport = e1000_is_need_ioport(pdev);
+	if (need_ioport) {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
+		err = pci_enable_device(pdev);
+	} else {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM);
+		err = pci_enable_device_mem(pdev);
+	}
+	if (err)
 		return err;

 	if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK)) &&
@@ -949,7 +997,8 @@ e1000_probe(struct pci_dev *pdev,
 		pci_using_dac = 0;
 	}

-	if ((err = pci_request_regions(pdev, e1000_driver_name)))
+	err = pci_request_selected_regions(pdev, bars, e1000_driver_name);
+	if (err)
 		goto err_pci_reg;

 	pci_set_master(pdev);
@@ -967,6 +1016,8 @@ e1000_probe(struct pci_dev *pdev,
 	adapter->pdev = pdev;
 	adapter->hw.back = adapter;
 	adapter->msg_enable = (1 << debug) - 1;
+	adapter->bars = bars;
+	adapter->need_ioport = need_ioport;

 	err = -EIO;
 	adapter->hw.hw_addr = ioremap(pci_resource_start(pdev, BAR_0),
@@ -974,12 +1025,15 @@ e1000_probe(struct pci_dev *pdev,
 	if (!adapter->hw.hw_addr)
 		goto err_ioremap;

-	for (i = BAR_1; i <= BAR_5; i++) {
-		if (pci_resource_len(pdev, i) == 0)
-			continue;
-		if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
-			adapter->hw.io_base = pci_resource_start(pdev, i);
-			break;
+	if (adapter->need_ioport) {
+		for (i = BAR_1; i <= BAR_5; i++) {
+			if (pci_resource_len(pdev, i) == 0)
+				continue;
+			if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
+				adapter->hw.io_base =
+					pci_resource_start(pdev, i);
+				break;
+			}
 		}
 	}

@@ -1251,7 +1305,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);
@@ -1304,7 +1358,7 @@ e1000_remove(struct pci_dev *pdev)
 	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);

@@ -5242,7 +5296,12 @@ e1000_resume(struct pci_dev *pdev)

 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	if ((err = pci_enable_device(pdev))) {
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
 		printk(KERN_ERR "e1000: Cannot enable PCI device from suspend\n");
 		return err;
 	}
@@ -5337,8 +5396,13 @@ static pci_ers_result_t e1000_io_slot_re
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev->priv;
+	int err;

-	if (pci_enable_device(pdev)) {
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
 		printk(KERN_ERR "e1000: Cannot re-enable PCI device after reset.\n");
 		return PCI_ERS_RESULT_DISCONNECT;
 	}

[PATCH 2/3] e1000e: make ioport free

[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 <izumi.taku@jp.fujitsu.com>
---
 drivers/net/e1000e/e1000.h  |    4 +++
 drivers/net/e1000e/netdev.c |   48 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 6 deletions(-)

Index: linux-2.6.25.7/drivers/net/e1000e/e1000.h
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000e/e1000.h
+++ 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 {
Index: linux-2.6.25.7/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000e/netdev.c
+++ linux-2.6.25.7/drivers/net/e1000e/netdev.c
@@ -3523,7 +3523,11 @@ 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);
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
 	if (err) {
 		dev_err(&pdev->dev,
 			"Cannot enable PCI device from suspend\n");
@@ -3622,9 +3626,14 @@ 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)) {
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		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;
@@ -3700,6 +3709,21 @@ static void e1000_print_device_info(stru
 }

 /**
+ * e1000e_is_need_ioport - determine if an adapter needs ioport resources or not
+ * @pdev: PCI device information struct
+ *
+ * Returns true if an adapters needs ioport resources
+ **/
+static int e1000e_is_need_ioport(struct pci_dev *pdev)
+{
+	switch (pdev->device) {
+	/* Currently there are no adapters that need ioport resources */
+	default:
+		return false;
+	}
+}
+
+/**
  * e1000_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in e1000_pci_tbl
@@ -3724,9 +3748,19 @@ 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, need_ioport;

 	e1000e_disable_l1aspm(pdev);
-	err = pci_enable_device(pdev);
+
+	/* do not allocate ioport bars when not needed */
+	need_ioport = e1000e_is_need_ioport(pdev);
+	if (need_ioport) {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
+		err = pci_enable_device(pdev);
+	} else {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM);
+		err = pci_enable_device_mem(pdev);
+	}
 	if (err)
 		return err;

@@ -3749,7 +3783,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 +3807,8 @@ 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;
+	adapter->need_ioport = need_ioport;

 	mmio_start = pci_resource_start(pdev, 0);
 	mmio_len = pci_resource_len(pdev, 0);
@@ -4001,7 +4037,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 +4081,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);

[PATCH 3/3] igb: make ioport free

[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 <izumi.taku@jp.fujitsu.com>
---
 drivers/net/igb/igb.h      |    4 +++
 drivers/net/igb/igb_main.c |   47 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 6 deletions(-)

Index: linux-2.6.25.7/drivers/net/igb/igb.h
===================================================================
--- linux-2.6.25.7.orig/drivers/net/igb/igb.h
+++ 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 {
Index: linux-2.6.25.7/drivers/net/igb/igb_main.c
===================================================================
--- linux-2.6.25.7.orig/drivers/net/igb/igb_main.c
+++ linux-2.6.25.7/drivers/net/igb/igb_main.c
@@ -820,6 +820,21 @@ void igb_reset(struct igb_adapter *adapt
 }

 /**
+ * igb_is_need_ioport - determine if an adapter needs ioport resources or not
+ * @pdev: PCI device information struct
+ *
+ * Returns true if an adapter needs ioport resources
+ **/
+static int igb_is_need_ioport(struct pci_dev *pdev)
+{
+	switch (pdev->device) {
+	/* Currently there are no adapters that need ioport resources */
+	default:
+		return false;
+	}
+}
+
+/**
  * igb_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in igb_pci_tbl
@@ -843,8 +858,17 @@ static int __devinit igb_probe(struct pc
 	u16 eeprom_data = 0;
 	u16 eeprom_apme_mask = IGB_EEPROM_APME;
 	u32 part_num;
+	int bars, need_ioport;

-	err = pci_enable_device(pdev);
+	/* do not allocate ioport bars when not needed */
+	need_ioport = igb_is_need_ioport(pdev);
+	if (need_ioport) {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
+		err = pci_enable_device(pdev);
+	} else {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM);
+		err = pci_enable_device_mem(pdev);
+	}
 	if (err)
 		return err;

@@ -866,7 +890,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 +910,8 @@ 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;
+	adapter->need_ioport = need_ioport;

 	mmio_start = pci_resource_start(pdev, 0);
 	mmio_len = pci_resource_len(pdev, 0);
@@ -1121,7 +1147,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 +1194,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 +3995,11 @@ static int igb_resume(struct pci_dev *pd

 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	err = pci_enable_device(pdev);
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
 	if (err) {
 		dev_err(&pdev->dev,
 			"igb: Cannot enable PCI device from suspend\n");
@@ -4072,8 +4102,13 @@ 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)) {
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		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;

Re: [PATCH 0/3] e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

[Kok, Auke]

Taku Izumi wrote:
> 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

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

Re: [PATCH 0/3] e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

[Taku Izumi]

Dear Auke

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

> 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 <izumi.taku@jp.fujitsu.com>

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

[Taku Izumi]

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 <izumi.taku@jp.fujitsu.com>
---
 drivers/net/e1000e/e1000.h  |    3 +++
 drivers/net/e1000e/netdev.c |   20 ++++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

Index: linux-2.6.25.7/drivers/net/e1000e/e1000.h
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000e/e1000.h
+++ 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 {
Index: linux-2.6.25.7/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.25.7.orig/drivers/net/e1000e/netdev.c
+++ 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);

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

[Taku Izumi]

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 <izumi.taku@jp.fujitsu.com>
---
 drivers/net/igb/igb.h      |    3 +++
 drivers/net/igb/igb_main.c |   19 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

Index: linux-2.6.25.7/drivers/net/igb/igb.h
===================================================================
--- linux-2.6.25.7.orig/drivers/net/igb/igb.h
+++ 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 {
Index: linux-2.6.25.7/drivers/net/igb/igb_main.c
===================================================================
--- linux-2.6.25.7.orig/drivers/net/igb/igb_main.c
+++ 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;

Re: [PATCH 0/3] e1000,e1000e,igb: make ioport free for adapters that need NO ioport resources

[Jeff Kirsher]

On Mon, Jun 23, 2008 at 1:28 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> Dear Auke
>
>>> * [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.
>
>> 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 <jeffrey.t.kirsher@intel.com>

-- 
Cheers,
Jeff

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

[Jeff Garzik]

Taku Izumi wrote:
> 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 <kusumi.tomohiro@jp.fujitsu.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  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

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

[Jeff Garzik]

Taku Izumi wrote:
> 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 <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/net/e1000e/e1000.h  |    4 +++
>  drivers/net/e1000e/netdev.c |   48 ++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 46 insertions(+), 6 deletions(-)

applied 2-3

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

[Taku Izumi]

Dear Jeff

Jeff Garzik wrote:
> Taku Izumi wrote:
>> 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 <kusumi.tomohiro@jp.fujitsu.com>
>> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
>> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>> ---
>>  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
>

I'll re-create one.

Taku Izumi <izumi.taku@jp.fujitsu.com>

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

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


Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@jp.fujitsu.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/net/e1000/e1000.h      |    4 +
 drivers/net/e1000/e1000_main.c |   88 +++++++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)

Index: netdev-2.6/drivers/net/e1000/e1000.h
===================================================================
--- netdev-2.6.orig/drivers/net/e1000/e1000.h
+++ 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 {
Index: netdev-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- netdev-2.6.orig/drivers/net/e1000/e1000_main.c
+++ netdev-2.6/drivers/net/e1000/e1000_main.c
@@ -872,6 +872,44 @@ static void e1000_dump_eeprom(struct e10
 }

 /**
+ * e1000_is_need_ioport - determine if an adapter needs ioport resources or not
+ * @pdev: PCI device information struct
+ *
+ * Returns true if an adapter needs ioport resources
+ **/
+
+static int
+e1000_is_need_ioport(struct pci_dev *pdev)
+{
+	switch (pdev->device) {
+	case E1000_DEV_ID_82540EM:
+	case E1000_DEV_ID_82540EM_LOM:
+	case E1000_DEV_ID_82540EP:
+	case E1000_DEV_ID_82540EP_LOM:
+	case E1000_DEV_ID_82540EP_LP:
+	case E1000_DEV_ID_82541EI:
+	case E1000_DEV_ID_82541EI_MOBILE:
+	case E1000_DEV_ID_82541ER_LOM:
+	case E1000_DEV_ID_82541ER:
+	case E1000_DEV_ID_82541GI:
+	case E1000_DEV_ID_82541GI_LF:
+	case E1000_DEV_ID_82541GI_MOBILE:
+	case E1000_DEV_ID_82544EI_COPPER:
+	case E1000_DEV_ID_82544EI_FIBER:
+	case E1000_DEV_ID_82544GC_COPPER:
+	case E1000_DEV_ID_82544GC_LOM:
+	case E1000_DEV_ID_82545EM_COPPER:
+	case E1000_DEV_ID_82545EM_FIBER:
+	case E1000_DEV_ID_82546EB_COPPER:
+	case E1000_DEV_ID_82546EB_FIBER:
+	case E1000_DEV_ID_82546EB_QUAD_COPPER:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/**
  * e1000_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in e1000_pci_tbl
@@ -895,9 +933,19 @@ e1000_probe(struct pci_dev *pdev,
 	int i, err, pci_using_dac;
 	u16 eeprom_data = 0;
 	u16 eeprom_apme_mask = E1000_EEPROM_APME;
+	int bars, need_ioport;
 	DECLARE_MAC_BUF(mac);

-	if ((err = pci_enable_device(pdev)))
+	/* do not allocate ioport bars when not needed */
+	need_ioport = e1000_is_need_ioport(pdev);
+	if (need_ioport) {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
+		err = pci_enable_device(pdev);
+	} else {
+		bars = pci_select_bars(pdev, IORESOURCE_MEM);
+		err = pci_enable_device_mem(pdev);
+	}
+	if (err)
 		return err;

 	if (!(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK)) &&
@@ -912,7 +960,8 @@ e1000_probe(struct pci_dev *pdev,
 		pci_using_dac = 0;
 	}

-	if ((err = pci_request_regions(pdev, e1000_driver_name)))
+	err = pci_request_selected_regions(pdev, bars, e1000_driver_name);
+	if (err)
 		goto err_pci_reg;

 	pci_set_master(pdev);
@@ -930,6 +979,8 @@ e1000_probe(struct pci_dev *pdev,
 	adapter->pdev = pdev;
 	adapter->hw.back = adapter;
 	adapter->msg_enable = (1 << debug) - 1;
+	adapter->bars = bars;
+	adapter->need_ioport = need_ioport;

 	err = -EIO;
 	adapter->hw.hw_addr = ioremap(pci_resource_start(pdev, BAR_0),
@@ -937,12 +988,15 @@ e1000_probe(struct pci_dev *pdev,
 	if (!adapter->hw.hw_addr)
 		goto err_ioremap;

-	for (i = BAR_1; i <= BAR_5; i++) {
-		if (pci_resource_len(pdev, i) == 0)
-			continue;
-		if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
-			adapter->hw.io_base = pci_resource_start(pdev, i);
-			break;
+	if (adapter->need_ioport) {
+		for (i = BAR_1; i <= BAR_5; i++) {
+			if (pci_resource_len(pdev, i) == 0)
+				continue;
+			if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
+				adapter->hw.io_base =
+					pci_resource_start(pdev, i);
+				break;
+			}
 		}
 	}

@@ -1214,7 +1268,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);
@@ -1267,7 +1321,7 @@ e1000_remove(struct pci_dev *pdev)
 	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);

@@ -5198,7 +5252,12 @@ e1000_resume(struct pci_dev *pdev)

 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	if ((err = pci_enable_device(pdev))) {
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
 		printk(KERN_ERR "e1000: Cannot enable PCI device from suspend\n");
 		return err;
 	}
@@ -5292,8 +5351,13 @@ static pci_ers_result_t e1000_io_slot_re
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev->priv;
+	int err;

-	if (pci_enable_device(pdev)) {
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
 		printk(KERN_ERR "e1000: Cannot re-enable PCI device after reset.\n");
 		return PCI_ERS_RESULT_DISCONNECT;
 	}

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

[Jeff Kirsher]

On Fri, Jun 27, 2008 at 6:05 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> 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 <kusumi.tomohiro@jp.fujitsu.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  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.

-- 
Cheers,
Jeff