Re: [PATCH v2 1/2] PCI: Add new method for registering PCI hosts

[Liviu Dudau <liviu.dudau@arm.com>]

On Fri, Jul 01, 2016 at 04:24:50PM +0200, Arnd Bergmann wrote:
> On Friday, July 1, 2016 3:14:47 PM CEST Liviu Dudau wrote:
> > 
> > I'm confused. Why are we butchering the old pci_alloc_host_bridge() function
> > to then go back and add re-add it in a different form?
> 
> Good point, we could just keep the existing implementation and make it
> an exported function without the bus argument.
> 
> > The old pci_alloc_host_bridge() was doing mostly what the above
> > function does minus the bridge->private line. And the &bridge[1] construct
> > could very well be pointing to the next struct pci_host_bridge aligned address,
> > which means parts of priv area are ignored.
> 
> I don't know what you mean with that. Would you rather write this as?
> 
> 	bridge->private = bridge + 1;
> 
> or leave it out and add a helper function
> 
> void *pci_host_bridge_private(struct pci_host_bridge *bridge)
> {
> 	return &bridge[1];
> }

none of the above

> 
> or do you mean we should have extra alignment in there so the
> private pointer has a minimum alignment higher than the
> alignment of struct pci_host_bridge?

but this ^. bridge pointer arithmetic means +1 is not necessarily +sizeof(struct pci_host_bridge)
bytes. AFAIK that can be rounded to the nearest natural alignment for pointers on that
architecture.

> 
> I'm absolutely fine with any of those suggestions, whichever
> makes the nicest API.

Does anyone need to subclass the pci_host_bridge structure? And is appending data right
after the structure useful?

Best regards,
Liviu

> 
> 	Arnd
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯