Re: [patch] PCI Cleanup

[Greg KH <gregkh@us.ibm.com>]

On Thu, Aug 15, 2002 at 10:58:17AM -0500, Kai Germaschewski wrote:
> On Wed, 14 Aug 2002, Grover, Andrew wrote:
> 
> > ACPI needs access to PCI config space, and it doesn't have a struct pci_dev
> > to pass to access functions. It doesn't look like your patch exposes an
> > interface that 1) doesn't require a pci_dev and 2) abstracts the PCI config
> > access method, does it?
> 
> I think drivers/hotplug/pci_hotplug_util.c implements something like you 
> need, pci_read_config_byte_nodev().
> 
> Of course that's currently only available for PCI hotplug, and for all I 
> can see the concept is somewhat messed up, but maybe that's an opportunity 
> to clean things up?

I would love to clean those functions up.

> Currently, pci_read_config_byte_nodev() will construct a fake struct 
> pci_dev, and then use the normal pci_read_config_byte(). I think it 
> makes more sense to actually do things the other way around.
> 
> For reading/writing config space, we need to know (dev, fn), and need the 
> access method (struct pci_ops), which is a property of the bridge plus 
> possibly some private data (the arch's sysdata). So the member
> 
> 	struct pci_ops *ops;
> 
> of struct pci_dev is actually not necessary, it will always be
> pdev->ops == pdev->bus->ops AFAICS.
> 
> So we could instead have
> 
> 	pci_bus_read_config_byte(struct pci_bus *bus, u8 dev, u8 fn, ...)
> 
> and for common use
> 
> 	static inline pci_read_config_byte(struct pci_dev, *pdev, ...)
> 	{
> 		return pci_bus_read_config_byte(pdev->bus,
> 						PCI_SLOT(pdev->devfn),
> 						PCI_FUNC(pdev->devfn));
> 	}
> 
> The PCI hotplug controllers / ACPI could then use the pci_bus_* variants, 
> when they don't have a struct pci_dev available. They would need at least 
> the root bridge's struct pci_bus, though.

Thats a good idea.  The hotplug controllers do have acess to the pci_bus
structure.  Andy, does ACPI have access to this when you are needing to
do these kinds of calls?

If there are no complaints, I think I'll go implement this, and move the
functions into the main pci code so that other parts of the kernel (like
ACPI) can use them.

Thanks for the idea!

greg k-h