Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

[Bjorn Helgaas <bhelgaas@google.com>]

On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> BenH found:
> | 928bea964827d7824b548c1f8e06eccbbc4d0d7d
> | PCI: Delay enabling bridges until they're needed
> 
> break PCI on powerpc.  The reason is that the PCIe port driver will
> call pci_enable_device() on the bridge, so device enabled (but skip
> pci_set_master because pcie_port_auto and no acpi on powerpc ).
> 
> Because of that, pci_enable_bridge() later on (called as a result of the
> child device driver doing pci_enable_device) will see the bridge as
> already enabled and will not call pci_set_master() on it.
> 
> Fixed by add checking in pci_enable_bridge, and call pci_set_master
> if driver skip that.
> That will make the code more robot and wade off problem for missing
> pci_set_master in drivers.
> 
> Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/pci/pci.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
>  
>  	pci_enable_bridge(dev->bus->self);
>  
> -	if (pci_is_enabled(dev))
> +	if (pci_is_enabled(dev)) {
> +		if (!dev->is_busmaster) {
> +			dev_warn(&dev->dev, "driver skip pci_set_master, fix it!\n");

I know this is already in Linus' tree, but if we're going to enable
bus mastering here, what's the point of the warning?  If somebody
fixes the driver by adding a pci_set_master() call there, does that
improve something?

Bjorn

> +			pci_set_master(dev);
> +		}
>  		return;
> +	}
> +
>  	retval = pci_enable_device(dev);
>  	if (retval)
>  		dev_err(&dev->dev, "Error enabling bridge (%d), continuing\n",