Re: [PATCH 3/4] Make Intel e1000 driver legacy I/O port free

[Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>]

Hi Jeff,

Thank you for reviewing.

Jeff Garzik wrote:
> Kenji Kaneshige wrote:
> 

(snip.)

>>+	INTEL_E1000_ETHERNET_DEVICE(0x1099, E1000_NO_IOPORT),
>>+	INTEL_E1000_ETHERNET_DEVICE(0x109A, E1000_NO_IOPORT),
>>+	INTEL_E1000_ETHERNET_DEVICE(0x10B5, E1000_NO_IOPORT),
>>+	INTEL_E1000_ETHERNET_DEVICE(0x10B9, 0),
>> 	/* required last entry */
>> 	{0,}
>> };
> 
> 
> Why change all the entries?  I would just change the ones with flags...
> 

I'm sorry. I don't understand what you mean. Could you tell me
how should I change?


>>@@ -621,7 +621,14 @@
>> 	int i, err, pci_using_dac;
>> 	uint16_t eeprom_data;
>> 	uint16_t eeprom_apme_mask = E1000_EEPROM_APME;
>>-	if ((err = pci_enable_device(pdev)))
>>+	int bars;
>>+
>>+	if (ent->driver_data & E1000_NO_IOPORT)
>>+		bars = pci_select_bars(pdev, IORESOURCE_MEM);
>>+	else
>>+		bars = pci_select_bars(pdev, IORESOURCE_MEM | IORESOURCE_IO);
>>+
>>+	if ((err = pci_enable_device_bars(pdev, bars)))
>> 		return err;
> 
> 
> NAK, this is an obvious regression.
> 
> pci_enable_device() also powers up the device, and enables irq delivery
> (on e.g. cardbus), and is allowed to do other platform-specific device
> bring-up tasks.
> 

No, those tasks are done through pci_enable_device_bars() called from
pci_enable_device() actually. In addition, I made small changes to
pci_enable_device() and pci_enable_device_bars() in another patch ([PATCH 1/4]).
Now pci_enable_device_bars() just call pci_enable_device_bars() like below:

int
pci_enable_device(struct pci_dev *dev)
{
        int err = pci_enable_device_bars(dev, (1 << PCI_NUM_RESOURCES) - 1);
        if (err)
                return err;
        return 0;
}

Please see the following URL about this another patch.

http://www.uwsg.iu.edu/hypermail/linux/kernel/0606.0/1726.html

Thanks,
Kenji Kaneshige