mbox series

[RFC,0/9] PCI: introduce the concept of power sequencing of PCIe devices

Message ID 20240104130123.37115-1-brgl@bgdev.pl
Headers show
Series PCI: introduce the concept of power sequencing of PCIe devices | expand

Message

Bartosz Golaszewski Jan. 4, 2024, 1:01 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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.

Bartosz Golaszewski (9):
  arm64: dts: qcom: sm8250: describe the PCIe port
  arm64: dts: qcom: qrb5165-rb5: describe the WLAN module of QCA6390
  PCI/portdrv: create platform devices for child OF nodes
  PCI: hold the rescan mutex when scanning for the first time
  PCI/pwrseq: add pwrseq core code
  dt-bindings: vendor-prefixes: add a PCI prefix for Qualcomm Atheros
  dt-bindings: wireless: ath11k: describe QCA6390
  PCI/pwrseq: add a pwrseq driver for QCA6390
  arm64: defconfig: enable the PCIe power sequencing for QCA6390

 .../net/wireless/qcom,ath11k-pci.yaml         |  14 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   1 +
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts      |  24 +++
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |  10 +
 arch/arm64/configs/defconfig                  |   2 +
 drivers/pci/pcie/Kconfig                      |   2 +
 drivers/pci/pcie/Makefile                     |   2 +
 drivers/pci/pcie/portdrv.c                    |   3 +-
 drivers/pci/pcie/pwrseq/Kconfig               |  19 ++
 drivers/pci/pcie/pwrseq/Makefile              |   4 +
 drivers/pci/pcie/pwrseq/pcie-pwrseq-qca6390.c | 197 ++++++++++++++++++
 drivers/pci/pcie/pwrseq/pwrseq.c              |  83 ++++++++
 drivers/pci/probe.c                           |   2 +
 include/linux/pcie-pwrseq.h                   |  24 +++
 14 files changed, 386 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pwrseq/Kconfig
 create mode 100644 drivers/pci/pcie/pwrseq/Makefile
 create mode 100644 drivers/pci/pcie/pwrseq/pcie-pwrseq-qca6390.c
 create mode 100644 drivers/pci/pcie/pwrseq/pwrseq.c
 create mode 100644 include/linux/pcie-pwrseq.h

Comments

Neil Armstrong Jan. 8, 2024, 3:24 p.m. UTC | #1
Hi,

On 04/01/2024 14:01, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> 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.

I've successfully tested this serie for the WCN7850 Wifi/BT combo onboard chip
present on the SM8550-QRD and SM8650-QRD boards and it works just fine.

Here's a branch with the wcn7850 vreg table added to the pwrseq driver,
and the DT changes:
https://git.codelinaro.org/neil.armstrong/linux/-/commits/topic/sm8x50/wcn7850-wifi-pwrseq/?ref_type=heads

Thanks,
Neil

> 
> Bartosz Golaszewski (9):
>    arm64: dts: qcom: sm8250: describe the PCIe port
>    arm64: dts: qcom: qrb5165-rb5: describe the WLAN module of QCA6390
>    PCI/portdrv: create platform devices for child OF nodes
>    PCI: hold the rescan mutex when scanning for the first time
>    PCI/pwrseq: add pwrseq core code
>    dt-bindings: vendor-prefixes: add a PCI prefix for Qualcomm Atheros
>    dt-bindings: wireless: ath11k: describe QCA6390
>    PCI/pwrseq: add a pwrseq driver for QCA6390
>    arm64: defconfig: enable the PCIe power sequencing for QCA6390
> 
>   .../net/wireless/qcom,ath11k-pci.yaml         |  14 ++
>   .../devicetree/bindings/vendor-prefixes.yaml  |   1 +
>   arch/arm64/boot/dts/qcom/qrb5165-rb5.dts      |  24 +++
>   arch/arm64/boot/dts/qcom/sm8250.dtsi          |  10 +
>   arch/arm64/configs/defconfig                  |   2 +
>   drivers/pci/pcie/Kconfig                      |   2 +
>   drivers/pci/pcie/Makefile                     |   2 +
>   drivers/pci/pcie/portdrv.c                    |   3 +-
>   drivers/pci/pcie/pwrseq/Kconfig               |  19 ++
>   drivers/pci/pcie/pwrseq/Makefile              |   4 +
>   drivers/pci/pcie/pwrseq/pcie-pwrseq-qca6390.c | 197 ++++++++++++++++++
>   drivers/pci/pcie/pwrseq/pwrseq.c              |  83 ++++++++
>   drivers/pci/probe.c                           |   2 +
>   include/linux/pcie-pwrseq.h                   |  24 +++
>   14 files changed, 386 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/pci/pcie/pwrseq/Kconfig
>   create mode 100644 drivers/pci/pcie/pwrseq/Makefile
>   create mode 100644 drivers/pci/pcie/pwrseq/pcie-pwrseq-qca6390.c
>   create mode 100644 drivers/pci/pcie/pwrseq/pwrseq.c
>   create mode 100644 include/linux/pcie-pwrseq.h
>
Bartosz Golaszewski Jan. 8, 2024, 4:10 p.m. UTC | #2
On Mon, Jan 8, 2024 at 4:24 PM Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Hi,
>
> On 04/01/2024 14:01, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > 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.
>
> I've successfully tested this serie for the WCN7850 Wifi/BT combo onboard chip
> present on the SM8550-QRD and SM8650-QRD boards and it works just fine.
>
> Here's a branch with the wcn7850 vreg table added to the pwrseq driver,
> and the DT changes:
> https://git.codelinaro.org/neil.armstrong/linux/-/commits/topic/sm8x50/wcn7850-wifi-pwrseq/?ref_type=heads

Thanks, I'll integrate them into v2.

Bart
Geert Uytterhoeven Jan. 9, 2024, 9:29 a.m. UTC | #3
Hi ChenYu,

CC wsa + renesas-soc

On Tue, Jan 9, 2024 at 8:08 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> On Tue, Jan 9, 2024 at 12:09 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > On 1/4/2024 5:01 AM, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > 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/20220716222454.29914-1-jim2101024@gmail.com/
> >
> > 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