Re: [PATCH 0/4] PCI legacy I/O port free driver (take 3)

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

Jeff Garzik wrote:
> Kenji Kaneshige wrote:
> 
>>Hi,
>>
>>Here is an updated set of patches for PCI legacy I/O port free drivers
>>which incorporates feedbacks. Summary of changes from the previous
>>version are:
>>
>>    - Removed the device_flags field from struct pci_device_id, which
>>      was introduced in the previous version of patch
>>
>>    - Changed e1000 driver to use the driver_data field in struct
>>      pci_device_id to see if the device needs I/O port regions.
>>
>>    - Added proper messages instead of WARN_ON() at the error.
>>
>>    - Updated the Documentation/pci.txt
>>
>>I'm attaching the following four patches:
>>
>>    [patch 1/4] Add no_ioport flag into pci_dev
>>    [patch 2/4] Update Documentation/pci.txt
>>    [patch 3/4] Make Intel e1000 driver legacy I/O port free
>>    [patch 4/4] Make Emulex lpfc driver legacy I/O port free
>>
>>I'm attaching the brief description below about what the problem I'm
>>trying to solve is.
>>
>>Thanks,
>>Kenji Kaneshige
>>
>>
>>Brief Description
>>~~~~~~~~~~~~~~~~~
>>I encountered a problem that some PCI devices don't work on my system
>>which have huge number of PCI devices.
>>
>>It is mandatory for all PCI device drivers to enable the device by
>>calling pci_enable_device() which enables all regions probed from the
>>device's BARs. If pci_enable_device() failes to enable any regions
>>probed from BARs, it returns as error. On the large servers, I/O port
>>resource could not be assigned to all PCI devices because it is
>>limited (64KB on Intel Architecture[1]) and it would be fragmented
>>(I/O base register of PCI-to-PCI bridge will usually be aligned to a
>>4KB boundary[2]). In this case, the devices which have no I/O port
>>resource assigned don't work because pci_enable_device() for those
>>devices failes. This is what happened on my machine.
> 
> 
> This series still leaves a lot to be desired, and creates unnecessary
> driver churn.  The better solution is:
> 
> 1) pci_enable_device() enables what it can
> 

I guess your idea is changing pci_enable_device() not to return as error
even if it fails to enable some regions. Is it correct? If yes, we need
to change all architecture dependent code (e.g. pcibios_enable_device())
to do that, and it would need much bigger change.


> 2) Drivers, as they already do, will fail if they cannot map the desired
> memory or IO resources that are needed.
> 
> Thus, the PCI layer needs only to do #1, and existing driver code
> handles the rest of the situation as one currently expects.
> 

Many driver uses pci_request_regions() instead of calling pci_request_region()
for each region. That is, we need to consider the same problem at
pci_request_regions() time. Converting drivers to use pci_request_region()
instead of pci_request_regions() requires many changes and it would be
troublesome for driver writers.

Thanks,
Kenji Kaneshige