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

From: Liviu Dudau
Date: Wed Jul 09 2014 - 05:13:33 EST


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! /
---------------
Â\_(ã)_/Â

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/