2024-01-09 04:08:55

by Florian Fainelli

[permalink] [raw]
Subject: Re: [RFC 0/9] PCI: introduce the concept of power sequencing of PCIe devices

Hello,

On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> During last year's Linux Plumbers we had several discussions centered
> around the need to power-on PCI devices before they can be detected on
> the bus.
>
> The consensus during the conference was that we need to introduce a
> class of "PCI slot drivers" that would handle the power-sequencing.
>
> After some additional brain-storming with Manivannan and the realization
> that the DT maintainers won't like adding any "fake" nodes not
> representing actual devices, we decided to reuse the existing
> infrastructure provided by the PCIe port drivers.
>
> The general idea is to instantiate platform devices for child nodes of
> the PCIe port DT node. For those nodes for which a power-sequencing
> driver exists, we bind it and let it probe. The driver then triggers a
> rescan of the PCI bus with the aim of detecting the now powered-on
> device. The device will consume the same DT node as the platform,
> power-sequencing device. We use device links to make the latter become
> the parent of the former.
>
> The main advantage of this approach is not modifying the existing DT in
> any way and especially not adding any "fake" platform devices.

There is prior work in that area which was applied, but eventually reverted:

https://www.spinics.net/lists/linux-pci/msg119136.html

and finally re-applied albeit in a different shape:

https://lore.kernel.org/all/[email protected]/

so we might want to think about how to have pcie-brcmstb.c converted
over your proposed approach. AFAIR there is also pcie-rockchip.c which
has some rudimentary support for voltage regulators of PCIe end-points.

What does not yet appear in this RFC is support for suspend/resume,
especially for power states where both the RC and the EP might be losing
power. There also needs to be some thoughts given to wake-up enabled
PCIe devices like Wi-Fi which might need to remain powered on to service
Wake-on-WLAN frames if nothing else.

I sense a potential for a lot of custom power sequencing drivers being
added and ultimately leading to the decision to create a "generic" one
which is entirely driven by Device Tree properties...

Thanks for doing this!
--
Florian


2024-01-09 07:08:55

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [RFC 0/9] PCI: introduce the concept of power sequencing of PCIe devices

On Tue, Jan 9, 2024 at 12:09 PM Florian Fainelli <[email protected]> wrote:
>
> Hello,
>
> On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > During last year's Linux Plumbers we had several discussions centered
> > around the need to power-on PCI devices before they can be detected on
> > the bus.
> >
> > The consensus during the conference was that we need to introduce a
> > class of "PCI slot drivers" that would handle the power-sequencing.
> >
> > After some additional brain-storming with Manivannan and the realization
> > that the DT maintainers won't like adding any "fake" nodes not
> > representing actual devices, we decided to reuse the existing
> > infrastructure provided by the PCIe port drivers.
> >
> > The general idea is to instantiate platform devices for child nodes of
> > the PCIe port DT node. For those nodes for which a power-sequencing
> > driver exists, we bind it and let it probe. The driver then triggers a
> > rescan of the PCI bus with the aim of detecting the now powered-on
> > device. The device will consume the same DT node as the platform,
> > power-sequencing device. We use device links to make the latter become
> > the parent of the former.
> >
> > The main advantage of this approach is not modifying the existing DT in
> > any way and especially not adding any "fake" platform devices.
>
> There is prior work in that area which was applied, but eventually reverted:
>
> https://www.spinics.net/lists/linux-pci/msg119136.html
>
> and finally re-applied albeit in a different shape:
>
> https://lore.kernel.org/all/[email protected]/
>
> so we might want to think about how to have pcie-brcmstb.c converted
> over your proposed approach. AFAIR there is also pcie-rockchip.c which
> has some rudimentary support for voltage regulators of PCIe end-points.

I think the current in-tree approaches mostly target either PCIe slots,
whether full size or mini-PCIe or M.2, or soldered-on components that
either only have a single power rail, have internal regulators, or have
surrounding circuitry that would be incorporated on a PCIe card.

These all have standardized power rails (+12V, +3.3V, +3.3V aux, etc.).

> What does not yet appear in this RFC is support for suspend/resume,
> especially for power states where both the RC and the EP might be losing
> power. There also needs to be some thoughts given to wake-up enabled
> PCIe devices like Wi-Fi which might need to remain powered on to service
> Wake-on-WLAN frames if nothing else.
>
> I sense a potential for a lot of custom power sequencing drivers being
> added and ultimately leading to the decision to create a "generic" one
> which is entirely driven by Device Tree properties...

We can have one "generic" slot power sequencing driver, which just
enables all the power rails together. I would very much like to see that.

I believe the power sequencing in this series is currently targeting more
tightly coupled designs that use power rails directly from the PMIC, and
thus require more explicit power sequencing.

ChenYu


> Thanks for doing this!
> --
> Florian
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2024-01-09 07:43:07

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [RFC 0/9] PCI: introduce the concept of power sequencing of PCIe devices

On Tue, Jan 09, 2024 at 03:08:32PM +0800, Chen-Yu Tsai wrote:
> On Tue, Jan 9, 2024 at 12:09 PM Florian Fainelli <[email protected]> wrote:
> >
> > Hello,
> >
> > On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > During last year's Linux Plumbers we had several discussions centered
> > > around the need to power-on PCI devices before they can be detected on
> > > the bus.
> > >
> > > The consensus during the conference was that we need to introduce a
> > > class of "PCI slot drivers" that would handle the power-sequencing.
> > >
> > > After some additional brain-storming with Manivannan and the realization
> > > that the DT maintainers won't like adding any "fake" nodes not
> > > representing actual devices, we decided to reuse the existing
> > > infrastructure provided by the PCIe port drivers.
> > >
> > > The general idea is to instantiate platform devices for child nodes of
> > > the PCIe port DT node. For those nodes for which a power-sequencing
> > > driver exists, we bind it and let it probe. The driver then triggers a
> > > rescan of the PCI bus with the aim of detecting the now powered-on
> > > device. The device will consume the same DT node as the platform,
> > > power-sequencing device. We use device links to make the latter become
> > > the parent of the former.
> > >
> > > The main advantage of this approach is not modifying the existing DT in
> > > any way and especially not adding any "fake" platform devices.
> >
> > There is prior work in that area which was applied, but eventually reverted:
> >
> > https://www.spinics.net/lists/linux-pci/msg119136.html
> >
> > and finally re-applied albeit in a different shape:
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > so we might want to think about how to have pcie-brcmstb.c converted
> > over your proposed approach. AFAIR there is also pcie-rockchip.c which
> > has some rudimentary support for voltage regulators of PCIe end-points.
>
> I think the current in-tree approaches mostly target either PCIe slots,
> whether full size or mini-PCIe or M.2, or soldered-on components that
> either only have a single power rail, have internal regulators, or have
> surrounding circuitry that would be incorporated on a PCIe card.
>
> These all have standardized power rails (+12V, +3.3V, +3.3V aux, etc.).
>

Right. But ideally, they should also be converted to use this power sequencing
driver at some point in the future.

> > What does not yet appear in this RFC is support for suspend/resume,
> > especially for power states where both the RC and the EP might be losing
> > power. There also needs to be some thoughts given to wake-up enabled
> > PCIe devices like Wi-Fi which might need to remain powered on to service
> > Wake-on-WLAN frames if nothing else.
> >

I don't think it is necessary to add PM support in this series itself. Even
though PM support is always nice to have or even necessary for controllers
pulling the power plug during suspend, it makes sense to merge this basic
implementation and add features on top.

It should be noted that both the controller drivers and the power sequencing
drivers should be in sync during suspend i.e., if the controller driver decides
to put the device into D3Cold and turning off the controller power supplies,
then this driver should also power off the device. But if the controller driver
decides to keep the device in low power states (ASPM), then this driver
_should_not_ turn off the device power.

Right now, there is no global policy for controller drivers and each one does a
different job. IMO this should also be fixed, but that also requires an
agreement from PM folks.

(Well there is one more entity in this loop, PCIe device drivers... sigh)

> > I sense a potential for a lot of custom power sequencing drivers being
> > added and ultimately leading to the decision to create a "generic" one
> > which is entirely driven by Device Tree properties...
>
> We can have one "generic" slot power sequencing driver, which just
> enables all the power rails together. I would very much like to see that.
>

Yeah. And that "generic" driver could be used for simple usecases mentioned
above.

> I believe the power sequencing in this series is currently targeting more
> tightly coupled designs that use power rails directly from the PMIC, and
> thus require more explicit power sequencing.
>

Precisely!

- Mani

> ChenYu
>
>
> > Thanks for doing this!
> > --
> > Florian
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

--
மணிவண்ணன் சதாசிவம்

2024-01-09 09:25:03

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC 0/9] PCI: introduce the concept of power sequencing of PCIe devices

Florian Fainelli <[email protected]> writes:

> Hello,
>
> On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <[email protected]>
>> During last year's Linux Plumbers we had several discussions
>> centered
>> around the need to power-on PCI devices before they can be detected on
>> the bus.
>> The consensus during the conference was that we need to introduce a
>> class of "PCI slot drivers" that would handle the power-sequencing.
>> After some additional brain-storming with Manivannan and the
>> realization
>> that the DT maintainers won't like adding any "fake" nodes not
>> representing actual devices, we decided to reuse the existing
>> infrastructure provided by the PCIe port drivers.
>> The general idea is to instantiate platform devices for child nodes
>> of
>> the PCIe port DT node. For those nodes for which a power-sequencing
>> driver exists, we bind it and let it probe. The driver then triggers a
>> rescan of the PCI bus with the aim of detecting the now powered-on
>> device. The device will consume the same DT node as the platform,
>> power-sequencing device. We use device links to make the latter become
>> the parent of the former.
>> The main advantage of this approach is not modifying the existing DT
>> in
>> any way and especially not adding any "fake" platform devices.
>
> There is prior work in that area which was applied, but eventually reverted:
>
> https://www.spinics.net/lists/linux-pci/msg119136.html
>
> and finally re-applied albeit in a different shape:
>
> https://lore.kernel.org/all/[email protected]/
>
> so we might want to think about how to have pcie-brcmstb.c converted
> over your proposed approach. AFAIR there is also pcie-rockchip.c which
> has some rudimentary support for voltage regulators of PCIe
> end-points.
>
> What does not yet appear in this RFC is support for suspend/resume,
> especially for power states where both the RC and the EP might be
> losing power. There also needs to be some thoughts given to wake-up
> enabled PCIe devices like Wi-Fi which might need to remain powered on
> to service Wake-on-WLAN frames if nothing else.

Good point, suspend and resume is very important for ath11k and ath12k.
And although I'm not expecting any issues with hibernation please also
consider that. Currently ath11k hibernation is broken because of MHI and
it has been pain trying to fix that. So best to make sure these cases
work from the beginning.

> I sense a potential for a lot of custom power sequencing drivers being
> added and ultimately leading to the decision to create a "generic" one
> which is entirely driven by Device Tree properties...
>
> Thanks for doing this!

A big thank you from me too! It's great to finally see this solved so
that ath11k and ath12k can be used in wider range of devices.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2024-01-09 09:29:49

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [RFC 0/9] PCI: introduce the concept of power sequencing of PCIe devices

Hi ChenYu,

CC wsa + renesas-soc

On Tue, Jan 9, 2024 at 8:08 AM Chen-Yu Tsai <[email protected]> wrote:
> On Tue, Jan 9, 2024 at 12:09 PM Florian Fainelli <[email protected]> wrote:
> > On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > During last year's Linux Plumbers we had several discussions centered
> > > around the need to power-on PCI devices before they can be detected on
> > > the bus.
> > >
> > > The consensus during the conference was that we need to introduce a
> > > class of "PCI slot drivers" that would handle the power-sequencing.
> > >
> > > After some additional brain-storming with Manivannan and the realization
> > > that the DT maintainers won't like adding any "fake" nodes not
> > > representing actual devices, we decided to reuse the existing
> > > infrastructure provided by the PCIe port drivers.
> > >
> > > The general idea is to instantiate platform devices for child nodes of
> > > the PCIe port DT node. For those nodes for which a power-sequencing
> > > driver exists, we bind it and let it probe. The driver then triggers a
> > > rescan of the PCI bus with the aim of detecting the now powered-on
> > > device. The device will consume the same DT node as the platform,
> > > power-sequencing device. We use device links to make the latter become
> > > the parent of the former.
> > >
> > > The main advantage of this approach is not modifying the existing DT in
> > > any way and especially not adding any "fake" platform devices.
> >
> > There is prior work in that area which was applied, but eventually reverted:
> >
> > https://www.spinics.net/lists/linux-pci/msg119136.html
> >
> > and finally re-applied albeit in a different shape:
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > so we might want to think about how to have pcie-brcmstb.c converted
> > over your proposed approach. AFAIR there is also pcie-rockchip.c which
> > has some rudimentary support for voltage regulators of PCIe end-points.
>
> I think the current in-tree approaches mostly target either PCIe slots,
> whether full size or mini-PCIe or M.2, or soldered-on components that
> either only have a single power rail, have internal regulators, or have
> surrounding circuitry that would be incorporated on a PCIe card.
>
> These all have standardized power rails (+12V, +3.3V, +3.3V aux, etc.).

Indeed. E.g. R-Car PCIe just got support for that in commit
6797e4da2dd1e2c8 ("PCI: rcar-host: Add support for optional regulators")
in pci/next.

> > What does not yet appear in this RFC is support for suspend/resume,
> > especially for power states where both the RC and the EP might be losing
> > power. There also needs to be some thoughts given to wake-up enabled
> > PCIe devices like Wi-Fi which might need to remain powered on to service
> > Wake-on-WLAN frames if nothing else.
> >
> > I sense a potential for a lot of custom power sequencing drivers being
> > added and ultimately leading to the decision to create a "generic" one
> > which is entirely driven by Device Tree properties...
>
> We can have one "generic" slot power sequencing driver, which just
> enables all the power rails together. I would very much like to see that.
>
> I believe the power sequencing in this series is currently targeting more
> tightly coupled designs that use power rails directly from the PMIC, and
> thus require more explicit power sequencing.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds