Re: [PATCH v8 3/9] pci: Introduce pci_register_io_range() helper function.

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

On Wed, Jul 09, 2014 at 07:32:37AM +0100, Arnd Bergmann wrote:
> On Wednesday 09 July 2014, Liviu Dudau wrote:
> > > Maybe that assumption is guaranteed by OF, but it doesn't hold for ACPI;
> > > ACPI can describe several I/O port apertures for a single bridge, each
> > > associated with a different CPU physical memory region.
> > 
> > That is actually a good catch, I've completely missed the fact that
> > io_range->pci_addr could be non-zero.
> 
> Hmm, that's what I thought in my initial review, but you convinced me
> that it's actually correct later on, and I still believe it is. Maybe
> now you got confused by your own code?

Man, it has been too long. Yes, I am now confused by my own code, which
is not a good sign.

> 
> Please have another look, I think your code in pci_host_bridge_of_get_ranges
> sufficiently handles the registration to the PCI code with the correct
> io_offset. The only thing that we might want to add is to record the
> PCI address along with the bridge->io_base: For the host driver to
> set up the mapping window correctly, you either need both of them, or
> you assume they are already set up.

Hmm, having another look at pci_host_bridge_of_get_range() I'm not convinced
that we need another storage for pci_addr. The resource gets added to the
list of resources used by the bridge offsetted by range.pci_addr, so when
re-creating the PCI bus address the value should come in play.

I will double check but I think the code is correct as it is. Sorry for the
early confusion.

Best regards,
Liviu

> 
> 	Arnd
> 

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