Re: [PATCH v2] powerpc/pci: Improve device hotplug initialization

[Guenter Roeck <linux@roeck-us.net>]

On Mon, Jun 10, 2013 at 10:18:08AM -0700, Guenter Roeck wrote:
> Commit 37f02195b (powerpc/pci: fix PCI-e devices rescan issue on powerpc
> platform) fixes a problem with interrupt and DMA initialization on hot
> plugged devices. With this commit, interrupt and DMA initialization for
> hot plugged devices is handled in the pci device enable function.
> 
> This approach has a couple of drawbacks. First, it creates two code paths
> for device initialization, one for hot plugged devices and another for devices
> known during the initial PCI scan. Second, the initialization code for hot
> plugged devices is only called when the device is enabled, ie typically
> in the probe function. Also, the platform specific setup code is called each
> time pci_enable_device() is called, not only once during device discovery,
> meaning it is actually called multiple times, once for devices discovered
> during the initial scan and again each time a driver is re-loaded.
> 
> The visible result is that interrupt pins are only assigned to hot plugged
> devices when the device driver is loaded. Effectively this changes the PCI
> probe API, since pci_dev->irq and the device's dma configuration will now
> only be valid after pci_enable() was called at least once. A more subtle
> change is that platform specific PCI device setup is moved from device
> discovery into the driver's probe function, more specifically into the
> pci_enable_device() call.
> 
> To fix the inconsistencies, add new function pcibios_add_device.
> Call pcibios_setup_device from pcibios_setup_bus_devices if device setup
> is not complete, and from pcibios_add_device if bus setup is complete.
> 
> With this change, device setup code is moved back into device initialization,
> and called exactly once for both static and hot plugged devices.
> 
> Cc: Yuanquan Chen <Yuanquan.Chen@freescale.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Ensure that PCI bus fixup code has been executed before calling
>     device setup code.
> 
Hi Ben,

any comments/feedback on this approach ?

It is much less invasive than before and should address your concerns.

Thanks,
Guenter

>  arch/powerpc/kernel/pci-common.c |   17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 7f2273c..6909b13 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -992,7 +992,7 @@ void pcibios_setup_bus_self(struct pci_bus *bus)
>  		ppc_md.pci_dma_bus_setup(bus);
>  }
>  
> -void pcibios_setup_device(struct pci_dev *dev)
> +static void pcibios_setup_device(struct pci_dev *dev)
>  {
>  	/* Fixup NUMA node as it may not be setup yet by the generic
>  	 * code and is needed by the DMA init
> @@ -1013,6 +1013,17 @@ void pcibios_setup_device(struct pci_dev *dev)
>  		ppc_md.pci_irq_fixup(dev);
>  }
>  
> +int pcibios_add_device(struct pci_dev *dev)
> +{
> +	/*
> +	 * We can only call pcibios_setup_device() after bus setup is complete,
> +	 * since some of the platform specific DMA setup code depends on it.
> +	 */
> +	if (dev->bus->is_added)
> +		pcibios_setup_device(dev);
> +	return 0;
> +}
> +
>  void pcibios_setup_bus_devices(struct pci_bus *bus)
>  {
>  	struct pci_dev *dev;
> @@ -1467,10 +1478,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>  		if (ppc_md.pcibios_enable_device_hook(dev))
>  			return -EINVAL;
>  
> -	/* avoid pcie irq fix up impact on cardbus */
> -	if (dev->hdr_type != PCI_HEADER_TYPE_CARDBUS)
> -		pcibios_setup_device(dev);
> -
>  	return pci_enable_resources(dev, mask);
>  }
>  
> -- 
> 1.7.9.7
> 
>