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

Kenji Kaneshige
2006-06-08T13:39:22Z

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