Re: [PATCH v5 1/7] pci: Introduce pci_register_io_range() helper function.

Arnd Bergmann
2014-03-10T15:58:21Z

On Monday 10 March 2014 14:45:19 Liviu Dudau wrote:
>
> So, if I understand you correctly, you would prefer to fail here and hence stop the
> parsing for the x86, rather than pretending everything is OK and going through the
> motions?

Yes, on x86 it is clearly a bug if we end up calling this function.

> That was not my original thinking when I've introduced this function here. The main
> purpose of the function is to help the correct translation of IO addresses in
> pci_address_to_pio(). As Jason has explained very nicely, we have 3 types of
> architectures here that we try to support:
> - the ones that have separate IO address space (x86)
> - the ones that have 1:1 mapping between physical IO addresses and logical ports

Still not convinced this second one actually exists.

> - the architectures that memory map the IO addresses in virtual address space
> and then translate the logical addresses into virtual based on a given offset.
>
> For the first two types we don't want to do anything special. Architectures that
> fall in the last category will have to provide their own version of this function,
> with the arm64 version being generic enough to be used as de facto?

The page flags might be different across architectures: arm32 currently uses
MT_DEVICE, which only exists on arm32 and derived architectures (unicore32,
arm64).

> But I can see your point of view as well. I just don't know if that is good enough
> for powerpc and microblaze. With the way things are in my patch, they should be able
> to switch to of_create_pci_host_bridge() easily*, with your suggestion they will
> have to provide their implementation for pci_register_io_range().

I think there would still be a lot to do have powerpc switch over to
of_create_pci_host_bridge(), the more likely thing to happen is to have
that architecture implement its own copy that calls the same internal helpers
and does some more things that we may not want on other architectures.

Microblaze can probably be changed to use of_create_pci_host_bridge()
and need no custom code at all, it should need only a subset of what
we need for arm64.

> We really need to get another architecture converted. If there are no other takers I
> will make a stab once the current push towards upstreaming AArch64 hardware support
> slows down.

Moving over microblaze I think would be a good start. It has rather
specific requirements since there is only one host driver, but then again
that PCI host implementation might be shared with zynq (or synthesizable
there). I also wonder whether it's actually related to the X-gene PCI,
since I know some of the ppc4xx use Xilinx PCI and X-gene is also
based on those.

Arnd