| Message ID | 20240104130123.37115-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | PCI: introduce the concept of power sequencing of PCIe devices | expand |
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 >
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
Hello, 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. 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!
On Tue, Jan 9, 2024 at 12:09 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > Hello, > > 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.). > 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 > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 <f.fainelli@gmail.com> wrote: > > > > Hello, > > > > 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.). > 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 > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Florian Fainelli <f.fainelli@gmail.com> writes: > Hello, > > 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. > > 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.
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
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