cns3xxx PCIe domain support

cns3xxx PCIe domain support

[Arnd Bergmann]

I'm working on a patch series to simplify the ARM pci-common
implementation and came across cns3xxx, which unlike the other
non-DT platforms does not partition the bus space of PCI domain
zero but instead uses the default bus range 0...255 for each
host bridge without assigning domain numbers.

Before linux-3.20, commitm c88d54ba0c3d ("CNS3xxx: Remove
artificial dependency on pci_sys_data domain."), it used to
assign a domain number, but since then, the pci-common code
just supports a single domain.

My question is: does this actually work correctly without
leading to duplicate domain/bus/device/function tuples?
Is there anything I'm missing in the code, or could it be
that the machines that Koen and Krzysztof were running
on never used more than one of the host bridges?

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Lorenzo Pieralisi]

On Sun, Jan 03, 2021 at 11:47:45PM +0100, Arnd Bergmann wrote:
> I'm working on a patch series to simplify the ARM pci-common
> implementation and came across cns3xxx, which unlike the other
> non-DT platforms does not partition the bus space of PCI domain
> zero but instead uses the default bus range 0...255 for each
> host bridge without assigning domain numbers.
> 
> Before linux-3.20, commitm c88d54ba0c3d ("CNS3xxx: Remove
> artificial dependency on pci_sys_data domain."), it used to
> assign a domain number, but since then, the pci-common code
> just supports a single domain.
> 
> My question is: does this actually work correctly without
> leading to duplicate domain/bus/device/function tuples?
> Is there anything I'm missing in the code, or could it be
> that the machines that Koen and Krzysztof were running
> on never used more than one of the host bridges?

Hi Arnd,

Happy New Year !! I don't think there is anything you are missing, I
think that's an issue. A possible solution would consist in selecting
PCI_DOMAINS_GENERIC (the OF code should fall back to using a counter for
PCI domains - which should do the trick), I'd rather avoid adding back
an arch/arm hook to set-up the domain number

Please let me know how we can handle this, thanks a lot for spotting
it.

Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Arnd Bergmann]

On Mon, Jan 4, 2021 at 12:34 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Sun, Jan 03, 2021 at 11:47:45PM +0100, Arnd Bergmann wrote:
> > I'm working on a patch series to simplify the ARM pci-common
> > implementation and came across cns3xxx, which unlike the other
> > non-DT platforms does not partition the bus space of PCI domain
> > zero but instead uses the default bus range 0...255 for each
> > host bridge without assigning domain numbers.
> >
> > Before linux-3.20, commitm c88d54ba0c3d ("CNS3xxx: Remove
> > artificial dependency on pci_sys_data domain."), it used to
> > assign a domain number, but since then, the pci-common code
> > just supports a single domain.
> >
> > My question is: does this actually work correctly without
> > leading to duplicate domain/bus/device/function tuples?
> > Is there anything I'm missing in the code, or could it be
> > that the machines that Koen and Krzysztof were running
> > on never used more than one of the host bridges?
>
> Hi Arnd,
>
> Happy New Year !! I don't think there is anything you are missing, I
> think that's an issue. A possible solution would consist in selecting
> PCI_DOMAINS_GENERIC (the OF code should fall back to using a counter for
> PCI domains - which should do the trick), I'd rather avoid adding back
> an arch/arm hook to set-up the domain number

Actually on this platform, CONFIG_PCI_DOMAINS_GENERIC is
already enabled, however it still uses board files instead of
devicetree, so the automatic assignment can not work.

> Please let me know how we can handle this, thanks a lot for spotting
> it.

As a treewide cleanup, I was hoping we could just make the domain
a field in the pci_host_bridge structure that can get set by the host
bridge driver, and then remove the various other ways of setting
the data, and then only have a trivial pci_domain_nr() implementation
everywhere.

For the cns3xxx case, I wonder if anyone actually cares. If
there are still users, the treewide change would make it trivial
to set it up right, while backporting would be harder. I noticed
that openwrt removed cns3xxx support in August with the
explanation that the platform is not used much anymore,
and I suspect that any users outside of openwrt stopped updating
their kernels long ago.

     Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Koen Vandeputte]

On 04.01.21 14:48, Arnd Bergmann wrote:
> ...
> As a treewide cleanup, I was hoping we could just make the domain
> a field in the pci_host_bridge structure that can get set by the host
> bridge driver, and then remove the various other ways of setting
> the data, and then only have a trivial pci_domain_nr() implementation
> everywhere.
>
> For the cns3xxx case, I wonder if anyone actually cares. If
> there are still users, the treewide change would make it trivial
> to set it up right, while backporting would be harder. I noticed
> that openwrt removed cns3xxx support in August with the
> explanation that the platform is not used much anymore,
> and I suspect that any users outside of openwrt stopped updating
> their kernels long ago.
>
>       Arnd

Hi Arnd,

Yes, it was removed and so far I've received 0 complaints/questions 
about it.
Even in the years before removal, it hardly ever generated any fuzz.
(I guess I was nearly the only one judging by mail history ..)

The board I used for working on this target also became EOL upstream, as 
the SoC used on it also EOL'd at Cavium.

I tried 2 times to still add support for it using kernel 5.4 last 
summer, but supporting some
pheripherals mentioned in the boardfile resulted in devicetree horror.
(It's me lacking DT knowledge here)

All these issues, combined with EOL status and an extremely low audience 
resulted in final removal.

Regards,

Koen


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Lorenzo Pieralisi]

On Mon, Jan 04, 2021 at 02:48:20PM +0100, Arnd Bergmann wrote:
> On Mon, Jan 4, 2021 at 12:34 PM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Sun, Jan 03, 2021 at 11:47:45PM +0100, Arnd Bergmann wrote:
> > > I'm working on a patch series to simplify the ARM pci-common
> > > implementation and came across cns3xxx, which unlike the other
> > > non-DT platforms does not partition the bus space of PCI domain
> > > zero but instead uses the default bus range 0...255 for each
> > > host bridge without assigning domain numbers.
> > >
> > > Before linux-3.20, commitm c88d54ba0c3d ("CNS3xxx: Remove
> > > artificial dependency on pci_sys_data domain."), it used to
> > > assign a domain number, but since then, the pci-common code
> > > just supports a single domain.
> > >
> > > My question is: does this actually work correctly without
> > > leading to duplicate domain/bus/device/function tuples?
> > > Is there anything I'm missing in the code, or could it be
> > > that the machines that Koen and Krzysztof were running
> > > on never used more than one of the host bridges?
> >
> > Hi Arnd,
> >
> > Happy New Year !! I don't think there is anything you are missing, I
> > think that's an issue. A possible solution would consist in selecting
> > PCI_DOMAINS_GENERIC (the OF code should fall back to using a counter for
> > PCI domains - which should do the trick), I'd rather avoid adding back
> > an arch/arm hook to set-up the domain number
> 
> Actually on this platform, CONFIG_PCI_DOMAINS_GENERIC is
> already enabled, however it still uses board files instead of
> devicetree, so the automatic assignment can not work.

It should work, pci_bus_find_domain_nr() falls back to a counter in
of_pci_bus_find_domain_nr() if no DT is present.

Are you in a position to test it ?

> > Please let me know how we can handle this, thanks a lot for spotting
> > it.
> 
> As a treewide cleanup, I was hoping we could just make the domain
> a field in the pci_host_bridge structure that can get set by the host
> bridge driver, and then remove the various other ways of setting
> the data, and then only have a trivial pci_domain_nr() implementation
> everywhere.

I think that was the main driver behind CONFIG_PCI_DOMAINS_GENERIC,
I don't remember why we stopped the effort to the
current CONFIG_PCI_DOMAINS_GENERIC implementation but if you manage
to extend it it is obviously nice.

> For the cns3xxx case, I wonder if anyone actually cares. If
> there are still users, the treewide change would make it trivial
> to set it up right, while backporting would be harder. I noticed
> that openwrt removed cns3xxx support in August with the
> explanation that the platform is not used much anymore,
> and I suspect that any users outside of openwrt stopped updating
> their kernels long ago.

Can you check please whether CONFIG_PCI_DOMAINS_GENERIC still assigns
separate domain numbers even with no DT support at all ?

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Arnd Bergmann]

On Mon, Jan 4, 2021 at 4:36 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Mon, Jan 04, 2021 at 02:48:20PM +0100, Arnd Bergmann wrote:
> > On Mon, Jan 4, 2021 at 12:34 PM Lorenzo Pieralisi
> > > Happy New Year !! I don't think there is anything you are missing, I
> > > think that's an issue. A possible solution would consist in selecting
> > > PCI_DOMAINS_GENERIC (the OF code should fall back to using a counter for
> > > PCI domains - which should do the trick), I'd rather avoid adding back
> > > an arch/arm hook to set-up the domain number
> >
> > Actually on this platform, CONFIG_PCI_DOMAINS_GENERIC is
> > already enabled, however it still uses board files instead of
> > devicetree, so the automatic assignment can not work.
>
> It should work, pci_bus_find_domain_nr() falls back to a counter in
> of_pci_bus_find_domain_nr() if no DT is present.

Ah, I see. I completely missed that part, as I assumed that
of_pci_bus_find_domain_nr() did not attempt to  do the right
thing on non-DT platforms. This part of the code is probably correct
then, as I have no reason to believe there is something wrong with
it, even if the rest of the platform has other problems.

It does raise a different issue though, which is that on
orion/mv78xx0/dove, the same code should trigger now that
they use CONFIG_ARCH_MULTIPLATFORM but no DT, and the
logic to assign distinct bus numbers for each host bridge is
rather pointless.

I don't think it's broken, but it does appear to be an inadvertent
change in behavior, as the platforms would have used only PCI
domain zero before the multiplatform conversion.

> Are you in a position to test it ?

No.

> > For the cns3xxx case, I wonder if anyone actually cares. If
> > there are still users, the treewide change would make it trivial
> > to set it up right, while backporting would be harder. I noticed
> > that openwrt removed cns3xxx support in August with the
> > explanation that the platform is not used much anymore,
> > and I suspect that any users outside of openwrt stopped updating
> > their kernels long ago.
>
> Can you check please whether CONFIG_PCI_DOMAINS_GENERIC still assigns
> separate domain numbers even with no DT support at all ?

From inspection, I'd say the code still works as designed.

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Krzysztof Hałasa]

Arnd Bergmann <arnd@kernel.org> writes:

> For the cns3xxx case, I wonder if anyone actually cares. If
> there are still users, the treewide change would make it trivial
> to set it up right, while backporting would be harder. I noticed
> that openwrt removed cns3xxx support in August with the
> explanation that the platform is not used much anymore,
> and I suspect that any users outside of openwrt stopped updating
> their kernels long ago.

I'm still using CNS3xxx-based Gateworks' boards (Laguna), with some
custom patch set, but the last kernels are over 2 years old. I have some
plan to update, but the probability it will happen very soon is rather
low. I guess I will test and, if needed, fix it when the time comes.

I'm not using them with OpenWrt, though.
They are basically a platform for (the old, parallel, not express)
mini-PCI cards and similar stuff. Nothing connected to the Internet etc.
-- 
Krzysztof Halasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cns3xxx PCIe domain support

[Lorenzo Pieralisi]

On Tue, Jan 05, 2021 at 12:54:36AM +0100, Arnd Bergmann wrote:
> On Mon, Jan 4, 2021 at 4:36 PM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Mon, Jan 04, 2021 at 02:48:20PM +0100, Arnd Bergmann wrote:
> > > On Mon, Jan 4, 2021 at 12:34 PM Lorenzo Pieralisi
> > > > Happy New Year !! I don't think there is anything you are missing, I
> > > > think that's an issue. A possible solution would consist in selecting
> > > > PCI_DOMAINS_GENERIC (the OF code should fall back to using a counter for
> > > > PCI domains - which should do the trick), I'd rather avoid adding back
> > > > an arch/arm hook to set-up the domain number
> > >
> > > Actually on this platform, CONFIG_PCI_DOMAINS_GENERIC is
> > > already enabled, however it still uses board files instead of
> > > devicetree, so the automatic assignment can not work.
> >
> > It should work, pci_bus_find_domain_nr() falls back to a counter in
> > of_pci_bus_find_domain_nr() if no DT is present.
> 
> Ah, I see. I completely missed that part, as I assumed that
> of_pci_bus_find_domain_nr() did not attempt to  do the right
> thing on non-DT platforms. This part of the code is probably correct
> then, as I have no reason to believe there is something wrong with
> it, even if the rest of the platform has other problems.
> 
> It does raise a different issue though, which is that on
> orion/mv78xx0/dove, the same code should trigger now that
> they use CONFIG_ARCH_MULTIPLATFORM but no DT, and the
> logic to assign distinct bus numbers for each host bridge is
> rather pointless.

Yes and potentially buggy (what happens if any host controller exhausts
the bus range (0x0-0xff) is a question mark looking at that code -
sure, it never happens in practice especially on those systems).

Lorenzo

> I don't think it's broken, but it does appear to be an inadvertent
> change in behavior, as the platforms would have used only PCI
> domain zero before the multiplatform conversion.
> 
> > Are you in a position to test it ?
> 
> No.
> 
> > > For the cns3xxx case, I wonder if anyone actually cares. If
> > > there are still users, the treewide change would make it trivial
> > > to set it up right, while backporting would be harder. I noticed
> > > that openwrt removed cns3xxx support in August with the
> > > explanation that the platform is not used much anymore,
> > > and I suspect that any users outside of openwrt stopped updating
> > > their kernels long ago.
> >
> > Can you check please whether CONFIG_PCI_DOMAINS_GENERIC still assigns
> > separate domain numbers even with no DT support at all ?
> 
> From inspection, I'd say the code still works as designed.
> 
>       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel