Re: [PATCH] drivers: pci: convert generic host controller to DT host bridge creation API

[Liviu Dudau <liviu@dudau.co.uk>]

On Wed, Aug 20, 2014 at 01:27:57PM +0200, Arnd Bergmann wrote:
> On Tuesday 12 August 2014, Liviu Dudau wrote:
> > +       return of_create_pci_host_bridge(dev, 0, 0xff, &gen_pci_ops,
> > +                                       gen_pci_setup, pci);
> 
> I had not noticed it earlier, but the setup callback is actually a feature
> of the arm32 PCI code that I had hoped to avoid when moving to the
> generic API. Can we do this as a more regular sequence of
> 
> 
> 	ret = of_create_pci_host_bridge(dev, 0, 0xff, &gen_pci_ops, pci);
> 	if (ret)
> 		return ret;
> 
> 	ret = gen_pci_setup(pci);
> 	if (ret)
> 		pci_destroy_host_bridge(dev, pci);
> 	return ret;
> 
> ?
> 
> 	Arnd

Hi Arnd,

That has been the general approach of my patchset up to v9. But, as Bjorn has
mentioned in his v8 review and I have put in my cover letter, the regular
aproach means that architectures that use pci_scan_root_bus() will have to
drop their one liner and replace it with the more verbose of_create_pci_host_bridge()
followed by pci_scan_child_bus() and pci_bus_add_devices() (basically, the content
of pci_scan_root_bus()). For those architectures it will lead to a net increase of
lines of code.

The patch for pci-host-generic.c is the first to use the callback setup function, but
not the only one. My PCI host bridge driver for Juno has the same need, and I'm betting
all other host bridge controllers will use it as it will be the only opportunity to
finish the controller setup before we start scanning the child busses. I'm trying to
balance ease of read vs ease of use here and it is the best version I've come up with
so far.

Best regards,
Liviu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...