| Message ID | 20240216203215.40870-1-brgl@bgdev.pl (mailing list archive) |
|---|---|
| Headers | show |
| Series | power: sequencing: implement the subsystem and add first users | expand |
On Fri, 16 Feb 2024 at 22:33, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > First, I'd like to apologize for the somewhat chaotic previous iterations > of this series and improper versioning which was rightfully pointed out > to me. I figured that the scope changed so much that it didn't make sense > to consider previous submissions part of the same series as the original > RFC but others thought otherwise so this one becomes v5 and I'll keep the > versioning going forward. > > This is the summary of the work so far: > > v1: Original RFC: > > https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ > > v2: First real patch series (should have been PATCH v2) adding what I > referred to back then as PCI power sequencing: > > https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > > v3: RFC for the DT representation of the PMU supplying the WLAN and BT > modules inside the QCA6391 package (was largely separate from the > series but probably should have been called PATCH or RFC v3): > > https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > > v4: Second attempt at the full series with changed scope (introduction of > the pwrseq subsystem, should have been RFC v4) > > https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > > === > > With that out of the way, I'd like to get down to explaining the two > problems I'm trying to solve. > > Problem statement #1: Dynamic bus chicken-and-egg problem. > > Certain on-board PCI devices need to be powered up before they are can be > detected but their PCI drivers won't get bound until the device is > powered-up so enabling the relevant resources in the PCI device driver > itself is impossible. > > Problem statement #2: Sharing inter-dependent resources between devices. > > Certain devices that use separate drivers (often on different busses) > share resources (regulators, clocks, etc.). Typically these resources > are reference-counted but in some cases there are additional interactions > between them to consider, for example specific power-up sequence timings. > > === > > The reason for tackling both of these problems in a single series is the > fact the the platform I'm working on - Qualcomm RB5 - deals with both and > both need to be addressed in order to enable WLAN and Bluetooth support > upstream. > > The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that > takes inputs from the host and exposes LDO outputs consumed by the BT and > WLAN modules which can be powered-up and down independently. However > a delay of 100ms must be respected between enabling the BT- and > WLAN-enable GPIOs[*]. > > === > > This series is logically split into several sections. I'll go > patch-by-patch and explain each step. > > Patch 1/18: > > This is a commit taken from the list by Jonathan Cameron that adds > a __free() helper for OF nodes. Not strictly related to the series but > until said commit ends in next, I need to carry it with this series. > > Patch 2/18: > > This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 > and sm8550 reference platforms use it in the WCN7850 module. > > Patches 3/18-6/18: > > These contain all relevant DT bindings changes. We add new documents for > the QCA6390 PMU and ATH12K devices as well as extend the bindings for the > Qualcomm Bluetooth and ATH11K modules with regulators used by them in > QCA6390. > > Patches 7/18-9/18: > > These contain changes to device-tree sources for the three platforms we > work with in this series. As the WCN7850 module doesn't require any > specific timings introducing dependencies between the Bluetooth and WLAN > modules, while the QCA6390 does, we take two different approaches to how > me model them in DT. > > For WCN7850 we hide the existence of the PMU as modeling it is simply not > necessary. The BT and WLAN devices on the device-tree are represented as > consuming the inputs (relevant to the functionality of each) of the PMU > directly. We are describing the hardware. From the hardware point of view, there is a PMU. I think at some point we would really like to describe all Qualcomm/Atheros WiFI+BT units using this PMU approach, including the older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > For QCA6390 on RB5 we add the PMU node as a platform device. It consumes > regulators and GPIOs from the host and exposed regulators consumer in turn > by the BT and WLAN modules. This represents the internal structure of the > package. > > Patches 10/18-14/18: > > These contain the bulk of the PCI changes for this series. We introduce > a simple framework for powering up PCI devices before detecting them on > the bus and the first user of this library in the form of the WCN7850 PCI > power control driver. > > The general approach is as follows: PCI devices that need special > treatment before they can be powered up, scanned and bound to their PCI > drivers must be described on the device-tree as child nodes of the PCI > port node. These devices will be instantiated on the platform bus. They > will in fact be generic platform devices with the compatible of the form > used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We > add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl > drivers. These drivers are platform drivers that will now be matched > against the devices instantiated from port children just like any other > platform pairs. > > Both the power control platform device *AND* the associated PCI device > reuse the same OF node and have access to the same properties. The goal > of the platform driver is to request and bring up any required resources > and let the pwrctl framework know that it's now OK to rescan the bus and > detect the devices. When the device is bound, we are notified about it > by the PCI bus notifier event and can establish a device link between the > power control device and the PCI device so that any future extension for > power-management will already be able to work with the correct hierachy. > > The reusing of the OF node is the reason for the small changes to the PCI > OF core: as the bootloader can possibly leave the relevant regulators on > before booting linux, the PCI device can be detected before its platform > abstraction is probed. In this case, we find that device first and mark > its OF node as reused. The pwrctl framework handles the opposite case > (when the PCI device is detected only after the platform driver > successfully enabled it). > > Patches 15/18-16/18: > > These add a relatively simple power sequencing subsystem and the first > driver using it: the pwrseq module for the QCA6390 PMU. > > For the record: Bjorn suggested a different solution: a regulator driver > that would - based on which regulators are enabled by a consumer - enable > relevant resources (drive the enable GPIOs) while respecting the > HW-specific delays. This would however require significant and yet > unprecised changed to the regulator subsystem as well as be an abuse of > the regulator provider API akin to using the reset framework for power > sequencing as proposed before. > > Instead I'm proposing to add a subsystem that allows different devices to > use a shared power sequence split into consumer-specific as well as > common "units". > > A power sequence provider driver registers a set of units with pwrseq > core. Each unit can be enabled and disabled and contains an optional list > of other units which must be enabled before it itself can be. A unit > represents a discreet chunk of the power sequence. > > It also registers a list of targets: a target is an abstraction wrapping > a unit which allows consumers to tell pwrseq which unit they want to > reach. Real-life example is the driver we're adding here: there's a set > of common regulators, two PCIe-specific ones and two enable GPIOs: one > for Bluetooth and one for WLAN. > > The Bluetooth driver requests a descriptor to the power sequencer and > names the target it wants to reach: > > pwrseq = devm_pwrseq_get(dev, "bluetooth"); Is this target tied to the device or not? If not, this might become a limitation, if somebody installs two WiFi/BT modules to a single device. > The pwrseq core then knows that when the driver calls: > > pwrseq_power_on(pwrseq); > > It must enable the "bluetooth-enable" unit but it depends on the > "regulators-common" unit so this one is enabled first. The provider > driver is also in charge of assuring an appropriate delay between > enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are > handled by the "wlan-enable" unit and so are not enabled until the WLAN > driver requests the "wlan" target to be powered on. > > Another thing worth discussing is the way we associate the consumer with > the relevant power sequencer. DT maintainers have expressed a discontent > with the existing mmc pwrseq bindings and have NAKed an earlier > initiative to introduce global pwrseq bindings to the kernel[1]. > > In this approach, we model the existing regulators and GPIOs in DT but > the pwrseq subsystem requires each provider to provide a .match() > callback. Whenever a consumer requests a power sequencer handle, we > iterate over the list of pwrseq drivers and call .match() for each. It's > up to the driver to verify in a platform-specific way whether it deals > with its consumer and let the core pwrseq code know. This looks really nice, it will allow us to migrate the BT driver to always use pwrseq instead of regulators without touching the DT. > > The advantage of this over reusing the regulator or reset subsystem is > that it's more generalized and can handle resources of all kinds as well > as deal with any kind of power-on sequences: for instance, Qualcomm has > a PCI switch they want a driver for but this switch requires enabling > some resources first (PCI pwrctl) and then configuring the device over > I2C (which can be handled by the pwrseq provider). > > Patch 17/18: > > This patch makes the Qualcomm Bluetooth driver get and use the power > sequencer for QCA6390. > > Patch 18/18: > > While tiny, this patch is possibly the highlight of the entire series. > It uses the two abstraction layers we introduced before to create an > elegant power sequencing PCI power control driver and supports the ath11k > module on QCA6390. > > With this series we can now enable BT and WLAN on several new Qualcomm > boards upstream. > > I tested the series on RB5 while Neil tested it on sm8650-qrd and > sm8550-qrd. > > Best Regards, > Bartosz Golaszewski > > It's hard to list the changes between versions here as the scope changed > significantly between each iteration and some versions were not even full > series but rather RFCs for parts of the solution. For this reason, I'll > only start listing changes starting from v6. > > [*] This is what the docs say. In practice it seems that this delay can be > ignored. However the subsequent model - QCA6490 - *does* require users to > respect it, so the problem remains valid. > > [1] https://lore.kernel.org/netdev/20210829131305.534417-1-dmitry.baryshkov@linaro.org/ > > Bartosz Golaszewski (15): > arm64: defconfig: enable ath12k as a module > dt-bindings: regulator: describe the PMU module of the QCA6390 package > dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390 > dt-bindings: new: wireless: qcom,ath11k: describe the ath11k on > QCA6390 > dt-bindings: new: wireless: describe the ath12k PCI module > arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 > PCI: hold the rescan mutex when scanning for the first time > PCI/pwrctl: reuse the OF node for power controlled devices > PCI/pwrctl: create platform devices for child OF nodes of the port > node > PCI/pwrctl: add PCI power control core code > PCI/pwrctl: add a power control driver for WCN7850 > power: sequencing: implement the pwrseq core > power: pwrseq: add a driver for the QCA6390 PMU module > Bluetooth: qca: use the power sequencer for QCA6390 > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > Jonathan Cameron (1): > of: Add cleanup.h based auto release via __free(device_node) markings. > > Neil Armstrong (2): > arm64: dts: qcom: sm8550-qrd: add the Wifi node > arm64: dts: qcom: sm8650-qrd: add the Wifi node > > .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + > .../net/wireless/qcom,ath11k-pci.yaml | 28 + > .../net/wireless/qcom,ath12k-pci.yaml | 103 ++ > .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 +++ > MAINTAINERS | 8 + > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 123 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 37 + > arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 29 + > arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + > arch/arm64/configs/defconfig | 1 + > drivers/bluetooth/hci_qca.c | 31 + > drivers/pci/Kconfig | 1 + > drivers/pci/Makefile | 1 + > drivers/pci/bus.c | 9 +- > drivers/pci/of.c | 14 +- > drivers/pci/probe.c | 2 + > drivers/pci/pwrctl/Kconfig | 25 + > drivers/pci/pwrctl/Makefile | 7 + > drivers/pci/pwrctl/core.c | 136 +++ > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 84 ++ > drivers/pci/pwrctl/pci-pwrctl-wcn7850.c | 202 ++++ > drivers/pci/remove.c | 2 + > drivers/power/Kconfig | 1 + > drivers/power/Makefile | 1 + > drivers/power/sequencing/Kconfig | 28 + > drivers/power/sequencing/Makefile | 6 + > drivers/power/sequencing/core.c | 1065 +++++++++++++++++ > drivers/power/sequencing/pwrseq-qca6390.c | 353 ++++++ > include/linux/of.h | 2 + > include/linux/pci-pwrctl.h | 51 + > include/linux/pwrseq/consumer.h | 56 + > include/linux/pwrseq/provider.h | 75 ++ > 34 files changed, 2678 insertions(+), 16 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-pci.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml > create mode 100644 drivers/pci/pwrctl/Kconfig > create mode 100644 drivers/pci/pwrctl/Makefile > create mode 100644 drivers/pci/pwrctl/core.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-wcn7850.c > create mode 100644 drivers/power/sequencing/Kconfig > create mode 100644 drivers/power/sequencing/Makefile > create mode 100644 drivers/power/sequencing/core.c > create mode 100644 drivers/power/sequencing/pwrseq-qca6390.c > create mode 100644 include/linux/pci-pwrctl.h > create mode 100644 include/linux/pwrseq/consumer.h > create mode 100644 include/linux/pwrseq/provider.h > > -- > 2.40.1 >
On 18/02/2024 13:53, Dmitry Baryshkov wrote: > On Fri, 16 Feb 2024 at 22:33, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> >> First, I'd like to apologize for the somewhat chaotic previous iterations >> of this series and improper versioning which was rightfully pointed out >> to me. I figured that the scope changed so much that it didn't make sense >> to consider previous submissions part of the same series as the original >> RFC but others thought otherwise so this one becomes v5 and I'll keep the >> versioning going forward. >> >> This is the summary of the work so far: >> >> v1: Original RFC: >> >> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ >> >> v2: First real patch series (should have been PATCH v2) adding what I >> referred to back then as PCI power sequencing: >> >> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ >> >> v3: RFC for the DT representation of the PMU supplying the WLAN and BT >> modules inside the QCA6391 package (was largely separate from the >> series but probably should have been called PATCH or RFC v3): >> >> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ >> >> v4: Second attempt at the full series with changed scope (introduction of >> the pwrseq subsystem, should have been RFC v4) >> >> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ >> >> === >> >> With that out of the way, I'd like to get down to explaining the two >> problems I'm trying to solve. >> >> Problem statement #1: Dynamic bus chicken-and-egg problem. >> >> Certain on-board PCI devices need to be powered up before they are can be >> detected but their PCI drivers won't get bound until the device is >> powered-up so enabling the relevant resources in the PCI device driver >> itself is impossible. >> >> Problem statement #2: Sharing inter-dependent resources between devices. >> >> Certain devices that use separate drivers (often on different busses) >> share resources (regulators, clocks, etc.). Typically these resources >> are reference-counted but in some cases there are additional interactions >> between them to consider, for example specific power-up sequence timings. >> >> === >> >> The reason for tackling both of these problems in a single series is the >> fact the the platform I'm working on - Qualcomm RB5 - deals with both and >> both need to be addressed in order to enable WLAN and Bluetooth support >> upstream. >> >> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that >> takes inputs from the host and exposes LDO outputs consumed by the BT and >> WLAN modules which can be powered-up and down independently. However >> a delay of 100ms must be respected between enabling the BT- and >> WLAN-enable GPIOs[*]. >> >> === >> >> This series is logically split into several sections. I'll go >> patch-by-patch and explain each step. >> >> Patch 1/18: >> >> This is a commit taken from the list by Jonathan Cameron that adds >> a __free() helper for OF nodes. Not strictly related to the series but >> until said commit ends in next, I need to carry it with this series. >> >> Patch 2/18: >> >> This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 >> and sm8550 reference platforms use it in the WCN7850 module. >> >> Patches 3/18-6/18: >> >> These contain all relevant DT bindings changes. We add new documents for >> the QCA6390 PMU and ATH12K devices as well as extend the bindings for the >> Qualcomm Bluetooth and ATH11K modules with regulators used by them in >> QCA6390. >> >> Patches 7/18-9/18: >> >> These contain changes to device-tree sources for the three platforms we >> work with in this series. As the WCN7850 module doesn't require any >> specific timings introducing dependencies between the Bluetooth and WLAN >> modules, while the QCA6390 does, we take two different approaches to how >> me model them in DT. >> >> For WCN7850 we hide the existence of the PMU as modeling it is simply not >> necessary. The BT and WLAN devices on the device-tree are represented as >> consuming the inputs (relevant to the functionality of each) of the PMU >> directly. > > We are describing the hardware. From the hardware point of view, there > is a PMU. I think at some point we would really like to describe all > Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). While I agree with older WiFi+BT units, I don't think it's needed for WCN7850 since BT+WiFi are now designed to be fully independent and PMU is transparent. > >> For QCA6390 on RB5 we add the PMU node as a platform device. It consumes >> regulators and GPIOs from the host and exposed regulators consumer in turn >> by the BT and WLAN modules. This represents the internal structure of the >> package. >> >> Patches 10/18-14/18: >> >> These contain the bulk of the PCI changes for this series. We introduce >> a simple framework for powering up PCI devices before detecting them on >> the bus and the first user of this library in the form of the WCN7850 PCI >> power control driver. >> >> The general approach is as follows: PCI devices that need special >> treatment before they can be powered up, scanned and bound to their PCI >> drivers must be described on the device-tree as child nodes of the PCI >> port node. These devices will be instantiated on the platform bus. They >> will in fact be generic platform devices with the compatible of the form >> used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We >> add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl >> drivers. These drivers are platform drivers that will now be matched >> against the devices instantiated from port children just like any other >> platform pairs. >> >> Both the power control platform device *AND* the associated PCI device >> reuse the same OF node and have access to the same properties. The goal >> of the platform driver is to request and bring up any required resources >> and let the pwrctl framework know that it's now OK to rescan the bus and >> detect the devices. When the device is bound, we are notified about it >> by the PCI bus notifier event and can establish a device link between the >> power control device and the PCI device so that any future extension for >> power-management will already be able to work with the correct hierachy. >> >> The reusing of the OF node is the reason for the small changes to the PCI >> OF core: as the bootloader can possibly leave the relevant regulators on >> before booting linux, the PCI device can be detected before its platform >> abstraction is probed. In this case, we find that device first and mark >> its OF node as reused. The pwrctl framework handles the opposite case >> (when the PCI device is detected only after the platform driver >> successfully enabled it). >> >> Patches 15/18-16/18: >> >> These add a relatively simple power sequencing subsystem and the first >> driver using it: the pwrseq module for the QCA6390 PMU. >> >> For the record: Bjorn suggested a different solution: a regulator driver >> that would - based on which regulators are enabled by a consumer - enable >> relevant resources (drive the enable GPIOs) while respecting the >> HW-specific delays. This would however require significant and yet >> unprecised changed to the regulator subsystem as well as be an abuse of >> the regulator provider API akin to using the reset framework for power >> sequencing as proposed before. >> >> Instead I'm proposing to add a subsystem that allows different devices to >> use a shared power sequence split into consumer-specific as well as >> common "units". >> >> A power sequence provider driver registers a set of units with pwrseq >> core. Each unit can be enabled and disabled and contains an optional list >> of other units which must be enabled before it itself can be. A unit >> represents a discreet chunk of the power sequence. >> >> It also registers a list of targets: a target is an abstraction wrapping >> a unit which allows consumers to tell pwrseq which unit they want to >> reach. Real-life example is the driver we're adding here: there's a set >> of common regulators, two PCIe-specific ones and two enable GPIOs: one >> for Bluetooth and one for WLAN. >> >> The Bluetooth driver requests a descriptor to the power sequencer and >> names the target it wants to reach: >> >> pwrseq = devm_pwrseq_get(dev, "bluetooth"); > > Is this target tied to the device or not? If not, this might become a > limitation, if somebody installs two WiFi/BT modules to a single > device. > >> The pwrseq core then knows that when the driver calls: >> >> pwrseq_power_on(pwrseq); >> >> It must enable the "bluetooth-enable" unit but it depends on the >> "regulators-common" unit so this one is enabled first. The provider >> driver is also in charge of assuring an appropriate delay between >> enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are >> handled by the "wlan-enable" unit and so are not enabled until the WLAN >> driver requests the "wlan" target to be powered on. >> >> Another thing worth discussing is the way we associate the consumer with >> the relevant power sequencer. DT maintainers have expressed a discontent >> with the existing mmc pwrseq bindings and have NAKed an earlier >> initiative to introduce global pwrseq bindings to the kernel[1]. >> >> In this approach, we model the existing regulators and GPIOs in DT but >> the pwrseq subsystem requires each provider to provide a .match() >> callback. Whenever a consumer requests a power sequencer handle, we >> iterate over the list of pwrseq drivers and call .match() for each. It's >> up to the driver to verify in a platform-specific way whether it deals >> with its consumer and let the core pwrseq code know. > > This looks really nice, it will allow us to migrate the BT driver to > always use pwrseq instead of regulators without touching the DT. > >> >> The advantage of this over reusing the regulator or reset subsystem is >> that it's more generalized and can handle resources of all kinds as well >> as deal with any kind of power-on sequences: for instance, Qualcomm has >> a PCI switch they want a driver for but this switch requires enabling >> some resources first (PCI pwrctl) and then configuring the device over >> I2C (which can be handled by the pwrseq provider). >> >> Patch 17/18: >> >> This patch makes the Qualcomm Bluetooth driver get and use the power >> sequencer for QCA6390. >> >> Patch 18/18: >> >> While tiny, this patch is possibly the highlight of the entire series. >> It uses the two abstraction layers we introduced before to create an >> elegant power sequencing PCI power control driver and supports the ath11k >> module on QCA6390. >> >> With this series we can now enable BT and WLAN on several new Qualcomm >> boards upstream. >> >> I tested the series on RB5 while Neil tested it on sm8650-qrd and >> sm8550-qrd. >> >> Best Regards, >> Bartosz Golaszewski >> >> It's hard to list the changes between versions here as the scope changed >> significantly between each iteration and some versions were not even full >> series but rather RFCs for parts of the solution. For this reason, I'll >> only start listing changes starting from v6. >> >> [*] This is what the docs say. In practice it seems that this delay can be >> ignored. However the subsequent model - QCA6490 - *does* require users to >> respect it, so the problem remains valid. >> >> [1] https://lore.kernel.org/netdev/20210829131305.534417-1-dmitry.baryshkov@linaro.org/ >> >> Bartosz Golaszewski (15): >> arm64: defconfig: enable ath12k as a module >> dt-bindings: regulator: describe the PMU module of the QCA6390 package >> dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390 >> dt-bindings: new: wireless: qcom,ath11k: describe the ath11k on >> QCA6390 >> dt-bindings: new: wireless: describe the ath12k PCI module >> arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 >> PCI: hold the rescan mutex when scanning for the first time >> PCI/pwrctl: reuse the OF node for power controlled devices >> PCI/pwrctl: create platform devices for child OF nodes of the port >> node >> PCI/pwrctl: add PCI power control core code >> PCI/pwrctl: add a power control driver for WCN7850 >> power: sequencing: implement the pwrseq core >> power: pwrseq: add a driver for the QCA6390 PMU module >> Bluetooth: qca: use the power sequencer for QCA6390 >> PCI/pwrctl: add a PCI power control driver for power sequenced devices >> >> Jonathan Cameron (1): >> of: Add cleanup.h based auto release via __free(device_node) markings. >> >> Neil Armstrong (2): >> arm64: dts: qcom: sm8550-qrd: add the Wifi node >> arm64: dts: qcom: sm8650-qrd: add the Wifi node >> >> .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + >> .../net/wireless/qcom,ath11k-pci.yaml | 28 + >> .../net/wireless/qcom,ath12k-pci.yaml | 103 ++ >> .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 +++ >> MAINTAINERS | 8 + >> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 123 +- >> arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + >> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 37 + >> arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + >> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 29 + >> arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + >> arch/arm64/configs/defconfig | 1 + >> drivers/bluetooth/hci_qca.c | 31 + >> drivers/pci/Kconfig | 1 + >> drivers/pci/Makefile | 1 + >> drivers/pci/bus.c | 9 +- >> drivers/pci/of.c | 14 +- >> drivers/pci/probe.c | 2 + >> drivers/pci/pwrctl/Kconfig | 25 + >> drivers/pci/pwrctl/Makefile | 7 + >> drivers/pci/pwrctl/core.c | 136 +++ >> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 84 ++ >> drivers/pci/pwrctl/pci-pwrctl-wcn7850.c | 202 ++++ >> drivers/pci/remove.c | 2 + >> drivers/power/Kconfig | 1 + >> drivers/power/Makefile | 1 + >> drivers/power/sequencing/Kconfig | 28 + >> drivers/power/sequencing/Makefile | 6 + >> drivers/power/sequencing/core.c | 1065 +++++++++++++++++ >> drivers/power/sequencing/pwrseq-qca6390.c | 353 ++++++ >> include/linux/of.h | 2 + >> include/linux/pci-pwrctl.h | 51 + >> include/linux/pwrseq/consumer.h | 56 + >> include/linux/pwrseq/provider.h | 75 ++ >> 34 files changed, 2678 insertions(+), 16 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-pci.yaml >> create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml >> create mode 100644 drivers/pci/pwrctl/Kconfig >> create mode 100644 drivers/pci/pwrctl/Makefile >> create mode 100644 drivers/pci/pwrctl/core.c >> create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c >> create mode 100644 drivers/pci/pwrctl/pci-pwrctl-wcn7850.c >> create mode 100644 drivers/power/sequencing/Kconfig >> create mode 100644 drivers/power/sequencing/Makefile >> create mode 100644 drivers/power/sequencing/core.c >> create mode 100644 drivers/power/sequencing/pwrseq-qca6390.c >> create mode 100644 include/linux/pci-pwrctl.h >> create mode 100644 include/linux/pwrseq/consumer.h >> create mode 100644 include/linux/pwrseq/provider.h >> >> -- >> 2.40.1 >> > >
On 16/02/2024 21:31, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > First, I'd like to apologize for the somewhat chaotic previous iterations > of this series and improper versioning which was rightfully pointed out > to me. I figured that the scope changed so much that it didn't make sense > to consider previous submissions part of the same series as the original > RFC but others thought otherwise so this one becomes v5 and I'll keep the > versioning going forward. > > This is the summary of the work so far: > > v1: Original RFC: > > https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ > > v2: First real patch series (should have been PATCH v2) adding what I > referred to back then as PCI power sequencing: > > https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > > v3: RFC for the DT representation of the PMU supplying the WLAN and BT > modules inside the QCA6391 package (was largely separate from the > series but probably should have been called PATCH or RFC v3): > > https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > > v4: Second attempt at the full series with changed scope (introduction of > the pwrseq subsystem, should have been RFC v4) > > https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > > === > > With that out of the way, I'd like to get down to explaining the two > problems I'm trying to solve. > > Problem statement #1: Dynamic bus chicken-and-egg problem. > > Certain on-board PCI devices need to be powered up before they are can be > detected but their PCI drivers won't get bound until the device is > powered-up so enabling the relevant resources in the PCI device driver > itself is impossible. > > Problem statement #2: Sharing inter-dependent resources between devices. > > Certain devices that use separate drivers (often on different busses) > share resources (regulators, clocks, etc.). Typically these resources > are reference-counted but in some cases there are additional interactions > between them to consider, for example specific power-up sequence timings. > > === > > The reason for tackling both of these problems in a single series is the > fact the the platform I'm working on - Qualcomm RB5 - deals with both and > both need to be addressed in order to enable WLAN and Bluetooth support > upstream. > > The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that > takes inputs from the host and exposes LDO outputs consumed by the BT and > WLAN modules which can be powered-up and down independently. However > a delay of 100ms must be respected between enabling the BT- and > WLAN-enable GPIOs[*]. > > === > > This series is logically split into several sections. I'll go > patch-by-patch and explain each step. > > Patch 1/18: > > This is a commit taken from the list by Jonathan Cameron that adds > a __free() helper for OF nodes. Not strictly related to the series but > until said commit ends in next, I need to carry it with this series. > > Patch 2/18: > > This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 > and sm8550 reference platforms use it in the WCN7850 module. > > Patches 3/18-6/18: > > These contain all relevant DT bindings changes. We add new documents for > the QCA6390 PMU and ATH12K devices as well as extend the bindings for the > Qualcomm Bluetooth and ATH11K modules with regulators used by them in > QCA6390. > > Patches 7/18-9/18: > > These contain changes to device-tree sources for the three platforms we > work with in this series. As the WCN7850 module doesn't require any > specific timings introducing dependencies between the Bluetooth and WLAN > modules, while the QCA6390 does, we take two different approaches to how > me model them in DT. > > For WCN7850 we hide the existence of the PMU as modeling it is simply not > necessary. The BT and WLAN devices on the device-tree are represented as > consuming the inputs (relevant to the functionality of each) of the PMU > directly. > > For QCA6390 on RB5 we add the PMU node as a platform device. It consumes > regulators and GPIOs from the host and exposed regulators consumer in turn > by the BT and WLAN modules. This represents the internal structure of the > package. > > Patches 10/18-14/18: > > These contain the bulk of the PCI changes for this series. We introduce > a simple framework for powering up PCI devices before detecting them on > the bus and the first user of this library in the form of the WCN7850 PCI > power control driver. > > The general approach is as follows: PCI devices that need special > treatment before they can be powered up, scanned and bound to their PCI > drivers must be described on the device-tree as child nodes of the PCI > port node. These devices will be instantiated on the platform bus. They > will in fact be generic platform devices with the compatible of the form > used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We > add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl > drivers. These drivers are platform drivers that will now be matched > against the devices instantiated from port children just like any other > platform pairs. > > Both the power control platform device *AND* the associated PCI device > reuse the same OF node and have access to the same properties. The goal > of the platform driver is to request and bring up any required resources > and let the pwrctl framework know that it's now OK to rescan the bus and > detect the devices. When the device is bound, we are notified about it > by the PCI bus notifier event and can establish a device link between the > power control device and the PCI device so that any future extension for > power-management will already be able to work with the correct hierachy. > > The reusing of the OF node is the reason for the small changes to the PCI > OF core: as the bootloader can possibly leave the relevant regulators on > before booting linux, the PCI device can be detected before its platform > abstraction is probed. In this case, we find that device first and mark > its OF node as reused. The pwrctl framework handles the opposite case > (when the PCI device is detected only after the platform driver > successfully enabled it). > > Patches 15/18-16/18: > > These add a relatively simple power sequencing subsystem and the first > driver using it: the pwrseq module for the QCA6390 PMU. > > For the record: Bjorn suggested a different solution: a regulator driver > that would - based on which regulators are enabled by a consumer - enable > relevant resources (drive the enable GPIOs) while respecting the > HW-specific delays. This would however require significant and yet > unprecised changed to the regulator subsystem as well as be an abuse of > the regulator provider API akin to using the reset framework for power > sequencing as proposed before. > > Instead I'm proposing to add a subsystem that allows different devices to > use a shared power sequence split into consumer-specific as well as > common "units". > > A power sequence provider driver registers a set of units with pwrseq > core. Each unit can be enabled and disabled and contains an optional list > of other units which must be enabled before it itself can be. A unit > represents a discreet chunk of the power sequence. > > It also registers a list of targets: a target is an abstraction wrapping > a unit which allows consumers to tell pwrseq which unit they want to > reach. Real-life example is the driver we're adding here: there's a set > of common regulators, two PCIe-specific ones and two enable GPIOs: one > for Bluetooth and one for WLAN. > > The Bluetooth driver requests a descriptor to the power sequencer and > names the target it wants to reach: > > pwrseq = devm_pwrseq_get(dev, "bluetooth"); > > The pwrseq core then knows that when the driver calls: > > pwrseq_power_on(pwrseq); > > It must enable the "bluetooth-enable" unit but it depends on the > "regulators-common" unit so this one is enabled first. The provider > driver is also in charge of assuring an appropriate delay between > enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are > handled by the "wlan-enable" unit and so are not enabled until the WLAN > driver requests the "wlan" target to be powered on. > > Another thing worth discussing is the way we associate the consumer with > the relevant power sequencer. DT maintainers have expressed a discontent > with the existing mmc pwrseq bindings and have NAKed an earlier > initiative to introduce global pwrseq bindings to the kernel[1]. > > In this approach, we model the existing regulators and GPIOs in DT but > the pwrseq subsystem requires each provider to provide a .match() > callback. Whenever a consumer requests a power sequencer handle, we > iterate over the list of pwrseq drivers and call .match() for each. It's > up to the driver to verify in a platform-specific way whether it deals > with its consumer and let the core pwrseq code know. > > The advantage of this over reusing the regulator or reset subsystem is > that it's more generalized and can handle resources of all kinds as well > as deal with any kind of power-on sequences: for instance, Qualcomm has > a PCI switch they want a driver for but this switch requires enabling > some resources first (PCI pwrctl) and then configuring the device over > I2C (which can be handled by the pwrseq provider). > > Patch 17/18: > > This patch makes the Qualcomm Bluetooth driver get and use the power > sequencer for QCA6390. > > Patch 18/18: > > While tiny, this patch is possibly the highlight of the entire series. > It uses the two abstraction layers we introduced before to create an > elegant power sequencing PCI power control driver and supports the ath11k > module on QCA6390. > > With this series we can now enable BT and WLAN on several new Qualcomm > boards upstream. > > I tested the series on RB5 while Neil tested it on sm8650-qrd and > sm8550-qrd. > > Best Regards, > Bartosz Golaszewski > > It's hard to list the changes between versions here as the scope changed > significantly between each iteration and some versions were not even full > series but rather RFCs for parts of the solution. For this reason, I'll > only start listing changes starting from v6. > > [*] This is what the docs say. In practice it seems that this delay can be > ignored. However the subsequent model - QCA6490 - *does* require users to > respect it, so the problem remains valid. > > [1] https://lore.kernel.org/netdev/20210829131305.534417-1-dmitry.baryshkov@linaro.org/ > > Bartosz Golaszewski (15): > arm64: defconfig: enable ath12k as a module > dt-bindings: regulator: describe the PMU module of the QCA6390 package > dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390 > dt-bindings: new: wireless: qcom,ath11k: describe the ath11k on > QCA6390 > dt-bindings: new: wireless: describe the ath12k PCI module > arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 > PCI: hold the rescan mutex when scanning for the first time > PCI/pwrctl: reuse the OF node for power controlled devices > PCI/pwrctl: create platform devices for child OF nodes of the port > node > PCI/pwrctl: add PCI power control core code > PCI/pwrctl: add a power control driver for WCN7850 > power: sequencing: implement the pwrseq core > power: pwrseq: add a driver for the QCA6390 PMU module > Bluetooth: qca: use the power sequencer for QCA6390 > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > Jonathan Cameron (1): > of: Add cleanup.h based auto release via __free(device_node) markings. > > Neil Armstrong (2): > arm64: dts: qcom: sm8550-qrd: add the Wifi node > arm64: dts: qcom: sm8650-qrd: add the Wifi node > > .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + > .../net/wireless/qcom,ath11k-pci.yaml | 28 + > .../net/wireless/qcom,ath12k-pci.yaml | 103 ++ > .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 +++ > MAINTAINERS | 8 + > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 123 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 37 + > arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 29 + > arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + > arch/arm64/configs/defconfig | 1 + > drivers/bluetooth/hci_qca.c | 31 + > drivers/pci/Kconfig | 1 + > drivers/pci/Makefile | 1 + > drivers/pci/bus.c | 9 +- > drivers/pci/of.c | 14 +- > drivers/pci/probe.c | 2 + > drivers/pci/pwrctl/Kconfig | 25 + > drivers/pci/pwrctl/Makefile | 7 + > drivers/pci/pwrctl/core.c | 136 +++ > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 84 ++ > drivers/pci/pwrctl/pci-pwrctl-wcn7850.c | 202 ++++ > drivers/pci/remove.c | 2 + > drivers/power/Kconfig | 1 + > drivers/power/Makefile | 1 + > drivers/power/sequencing/Kconfig | 28 + > drivers/power/sequencing/Makefile | 6 + > drivers/power/sequencing/core.c | 1065 +++++++++++++++++ > drivers/power/sequencing/pwrseq-qca6390.c | 353 ++++++ > include/linux/of.h | 2 + > include/linux/pci-pwrctl.h | 51 + > include/linux/pwrseq/consumer.h | 56 + > include/linux/pwrseq/provider.h | 75 ++ > 34 files changed, 2678 insertions(+), 16 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-pci.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml > create mode 100644 drivers/pci/pwrctl/Kconfig > create mode 100644 drivers/pci/pwrctl/Makefile > create mode 100644 drivers/pci/pwrctl/core.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-wcn7850.c > create mode 100644 drivers/power/sequencing/Kconfig > create mode 100644 drivers/power/sequencing/Makefile > create mode 100644 drivers/power/sequencing/core.c > create mode 100644 drivers/power/sequencing/pwrseq-qca6390.c > create mode 100644 include/linux/pci-pwrctl.h > create mode 100644 include/linux/pwrseq/consumer.h > create mode 100644 include/linux/pwrseq/provider.h > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550-QRD Thanks, Neil
On Mon, 19 Feb 2024 at 10:14, Neil Armstrong <neil.armstrong@linaro.org> wrote: > > On 18/02/2024 13:53, Dmitry Baryshkov wrote: > > On Fri, 16 Feb 2024 at 22:33, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >> > >> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >> > >> First, I'd like to apologize for the somewhat chaotic previous iterations > >> of this series and improper versioning which was rightfully pointed out > >> to me. I figured that the scope changed so much that it didn't make sense > >> to consider previous submissions part of the same series as the original > >> RFC but others thought otherwise so this one becomes v5 and I'll keep the > >> versioning going forward. > >> > >> This is the summary of the work so far: > >> > >> v1: Original RFC: > >> > >> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ > >> > >> v2: First real patch series (should have been PATCH v2) adding what I > >> referred to back then as PCI power sequencing: > >> > >> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > >> > >> v3: RFC for the DT representation of the PMU supplying the WLAN and BT > >> modules inside the QCA6391 package (was largely separate from the > >> series but probably should have been called PATCH or RFC v3): > >> > >> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > >> > >> v4: Second attempt at the full series with changed scope (introduction of > >> the pwrseq subsystem, should have been RFC v4) > >> > >> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > >> > >> === > >> > >> With that out of the way, I'd like to get down to explaining the two > >> problems I'm trying to solve. > >> > >> Problem statement #1: Dynamic bus chicken-and-egg problem. > >> > >> Certain on-board PCI devices need to be powered up before they are can be > >> detected but their PCI drivers won't get bound until the device is > >> powered-up so enabling the relevant resources in the PCI device driver > >> itself is impossible. > >> > >> Problem statement #2: Sharing inter-dependent resources between devices. > >> > >> Certain devices that use separate drivers (often on different busses) > >> share resources (regulators, clocks, etc.). Typically these resources > >> are reference-counted but in some cases there are additional interactions > >> between them to consider, for example specific power-up sequence timings. > >> > >> === > >> > >> The reason for tackling both of these problems in a single series is the > >> fact the the platform I'm working on - Qualcomm RB5 - deals with both and > >> both need to be addressed in order to enable WLAN and Bluetooth support > >> upstream. > >> > >> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that > >> takes inputs from the host and exposes LDO outputs consumed by the BT and > >> WLAN modules which can be powered-up and down independently. However > >> a delay of 100ms must be respected between enabling the BT- and > >> WLAN-enable GPIOs[*]. > >> > >> === > >> > >> This series is logically split into several sections. I'll go > >> patch-by-patch and explain each step. > >> > >> Patch 1/18: > >> > >> This is a commit taken from the list by Jonathan Cameron that adds > >> a __free() helper for OF nodes. Not strictly related to the series but > >> until said commit ends in next, I need to carry it with this series. > >> > >> Patch 2/18: > >> > >> This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 > >> and sm8550 reference platforms use it in the WCN7850 module. > >> > >> Patches 3/18-6/18: > >> > >> These contain all relevant DT bindings changes. We add new documents for > >> the QCA6390 PMU and ATH12K devices as well as extend the bindings for the > >> Qualcomm Bluetooth and ATH11K modules with regulators used by them in > >> QCA6390. > >> > >> Patches 7/18-9/18: > >> > >> These contain changes to device-tree sources for the three platforms we > >> work with in this series. As the WCN7850 module doesn't require any > >> specific timings introducing dependencies between the Bluetooth and WLAN > >> modules, while the QCA6390 does, we take two different approaches to how > >> me model them in DT. > >> > >> For WCN7850 we hide the existence of the PMU as modeling it is simply not > >> necessary. The BT and WLAN devices on the device-tree are represented as > >> consuming the inputs (relevant to the functionality of each) of the PMU > >> directly. > > > > We are describing the hardware. From the hardware point of view, there > > is a PMU. I think at some point we would really like to describe all > > Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > While I agree with older WiFi+BT units, I don't think it's needed for > WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > transparent. I don't see any significant difference between WCN6750/WCN6855 and WCN7850 from the PMU / power up point of view. Could you please point me to the difference?
On 19/02/2024 10:22, Dmitry Baryshkov wrote: > On Mon, 19 Feb 2024 at 10:14, Neil Armstrong <neil.armstrong@linaro.org> wrote: >> >> On 18/02/2024 13:53, Dmitry Baryshkov wrote: >>> On Fri, 16 Feb 2024 at 22:33, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>> >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> First, I'd like to apologize for the somewhat chaotic previous iterations >>>> of this series and improper versioning which was rightfully pointed out >>>> to me. I figured that the scope changed so much that it didn't make sense >>>> to consider previous submissions part of the same series as the original >>>> RFC but others thought otherwise so this one becomes v5 and I'll keep the >>>> versioning going forward. >>>> >>>> This is the summary of the work so far: >>>> >>>> v1: Original RFC: >>>> >>>> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ >>>> >>>> v2: First real patch series (should have been PATCH v2) adding what I >>>> referred to back then as PCI power sequencing: >>>> >>>> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ >>>> >>>> v3: RFC for the DT representation of the PMU supplying the WLAN and BT >>>> modules inside the QCA6391 package (was largely separate from the >>>> series but probably should have been called PATCH or RFC v3): >>>> >>>> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ >>>> >>>> v4: Second attempt at the full series with changed scope (introduction of >>>> the pwrseq subsystem, should have been RFC v4) >>>> >>>> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ >>>> >>>> === >>>> >>>> With that out of the way, I'd like to get down to explaining the two >>>> problems I'm trying to solve. >>>> >>>> Problem statement #1: Dynamic bus chicken-and-egg problem. >>>> >>>> Certain on-board PCI devices need to be powered up before they are can be >>>> detected but their PCI drivers won't get bound until the device is >>>> powered-up so enabling the relevant resources in the PCI device driver >>>> itself is impossible. >>>> >>>> Problem statement #2: Sharing inter-dependent resources between devices. >>>> >>>> Certain devices that use separate drivers (often on different busses) >>>> share resources (regulators, clocks, etc.). Typically these resources >>>> are reference-counted but in some cases there are additional interactions >>>> between them to consider, for example specific power-up sequence timings. >>>> >>>> === >>>> >>>> The reason for tackling both of these problems in a single series is the >>>> fact the the platform I'm working on - Qualcomm RB5 - deals with both and >>>> both need to be addressed in order to enable WLAN and Bluetooth support >>>> upstream. >>>> >>>> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that >>>> takes inputs from the host and exposes LDO outputs consumed by the BT and >>>> WLAN modules which can be powered-up and down independently. However >>>> a delay of 100ms must be respected between enabling the BT- and >>>> WLAN-enable GPIOs[*]. >>>> >>>> === >>>> >>>> This series is logically split into several sections. I'll go >>>> patch-by-patch and explain each step. >>>> >>>> Patch 1/18: >>>> >>>> This is a commit taken from the list by Jonathan Cameron that adds >>>> a __free() helper for OF nodes. Not strictly related to the series but >>>> until said commit ends in next, I need to carry it with this series. >>>> >>>> Patch 2/18: >>>> >>>> This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 >>>> and sm8550 reference platforms use it in the WCN7850 module. >>>> >>>> Patches 3/18-6/18: >>>> >>>> These contain all relevant DT bindings changes. We add new documents for >>>> the QCA6390 PMU and ATH12K devices as well as extend the bindings for the >>>> Qualcomm Bluetooth and ATH11K modules with regulators used by them in >>>> QCA6390. >>>> >>>> Patches 7/18-9/18: >>>> >>>> These contain changes to device-tree sources for the three platforms we >>>> work with in this series. As the WCN7850 module doesn't require any >>>> specific timings introducing dependencies between the Bluetooth and WLAN >>>> modules, while the QCA6390 does, we take two different approaches to how >>>> me model them in DT. >>>> >>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not >>>> necessary. The BT and WLAN devices on the device-tree are represented as >>>> consuming the inputs (relevant to the functionality of each) of the PMU >>>> directly. >>> >>> We are describing the hardware. From the hardware point of view, there >>> is a PMU. I think at some point we would really like to describe all >>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the >>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). >> >> While I agree with older WiFi+BT units, I don't think it's needed for >> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is >> transparent. > > I don't see any significant difference between WCN6750/WCN6855 and > WCN7850 from the PMU / power up point of view. Could you please point > me to the difference? > The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN and BT_EN ordering and the only requirement is to have all input regulators up before pulling up WLAN_EN and/or BT_EN. This makes the PMU transparent and BT and WLAN can be described as independent. Neil
On Mon, 19 Feb 2024 at 11:42, <neil.armstrong@linaro.org> wrote: > > On 19/02/2024 10:22, Dmitry Baryshkov wrote: > > On Mon, 19 Feb 2024 at 10:14, Neil Armstrong <neil.armstrong@linaro.org> wrote: > >> > >> On 18/02/2024 13:53, Dmitry Baryshkov wrote: > >>> On Fri, 16 Feb 2024 at 22:33, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >>>> > >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>> > >>>> First, I'd like to apologize for the somewhat chaotic previous iterations > >>>> of this series and improper versioning which was rightfully pointed out > >>>> to me. I figured that the scope changed so much that it didn't make sense > >>>> to consider previous submissions part of the same series as the original > >>>> RFC but others thought otherwise so this one becomes v5 and I'll keep the > >>>> versioning going forward. > >>>> > >>>> This is the summary of the work so far: > >>>> > >>>> v1: Original RFC: > >>>> > >>>> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ > >>>> > >>>> v2: First real patch series (should have been PATCH v2) adding what I > >>>> referred to back then as PCI power sequencing: > >>>> > >>>> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > >>>> > >>>> v3: RFC for the DT representation of the PMU supplying the WLAN and BT > >>>> modules inside the QCA6391 package (was largely separate from the > >>>> series but probably should have been called PATCH or RFC v3): > >>>> > >>>> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > >>>> > >>>> v4: Second attempt at the full series with changed scope (introduction of > >>>> the pwrseq subsystem, should have been RFC v4) > >>>> > >>>> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > >>>> > >>>> === > >>>> > >>>> With that out of the way, I'd like to get down to explaining the two > >>>> problems I'm trying to solve. > >>>> > >>>> Problem statement #1: Dynamic bus chicken-and-egg problem. > >>>> > >>>> Certain on-board PCI devices need to be powered up before they are can be > >>>> detected but their PCI drivers won't get bound until the device is > >>>> powered-up so enabling the relevant resources in the PCI device driver > >>>> itself is impossible. > >>>> > >>>> Problem statement #2: Sharing inter-dependent resources between devices. > >>>> > >>>> Certain devices that use separate drivers (often on different busses) > >>>> share resources (regulators, clocks, etc.). Typically these resources > >>>> are reference-counted but in some cases there are additional interactions > >>>> between them to consider, for example specific power-up sequence timings. > >>>> > >>>> === > >>>> > >>>> The reason for tackling both of these problems in a single series is the > >>>> fact the the platform I'm working on - Qualcomm RB5 - deals with both and > >>>> both need to be addressed in order to enable WLAN and Bluetooth support > >>>> upstream. > >>>> > >>>> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that > >>>> takes inputs from the host and exposes LDO outputs consumed by the BT and > >>>> WLAN modules which can be powered-up and down independently. However > >>>> a delay of 100ms must be respected between enabling the BT- and > >>>> WLAN-enable GPIOs[*]. > >>>> > >>>> === > >>>> > >>>> This series is logically split into several sections. I'll go > >>>> patch-by-patch and explain each step. > >>>> > >>>> Patch 1/18: > >>>> > >>>> This is a commit taken from the list by Jonathan Cameron that adds > >>>> a __free() helper for OF nodes. Not strictly related to the series but > >>>> until said commit ends in next, I need to carry it with this series. > >>>> > >>>> Patch 2/18: > >>>> > >>>> This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 > >>>> and sm8550 reference platforms use it in the WCN7850 module. > >>>> > >>>> Patches 3/18-6/18: > >>>> > >>>> These contain all relevant DT bindings changes. We add new documents for > >>>> the QCA6390 PMU and ATH12K devices as well as extend the bindings for the > >>>> Qualcomm Bluetooth and ATH11K modules with regulators used by them in > >>>> QCA6390. > >>>> > >>>> Patches 7/18-9/18: > >>>> > >>>> These contain changes to device-tree sources for the three platforms we > >>>> work with in this series. As the WCN7850 module doesn't require any > >>>> specific timings introducing dependencies between the Bluetooth and WLAN > >>>> modules, while the QCA6390 does, we take two different approaches to how > >>>> me model them in DT. > >>>> > >>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > >>>> necessary. The BT and WLAN devices on the device-tree are represented as > >>>> consuming the inputs (relevant to the functionality of each) of the PMU > >>>> directly. > >>> > >>> We are describing the hardware. From the hardware point of view, there > >>> is a PMU. I think at some point we would really like to describe all > >>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > >>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > >> > >> While I agree with older WiFi+BT units, I don't think it's needed for > >> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > >> transparent. > > > > I don't see any significant difference between WCN6750/WCN6855 and > > WCN7850 from the PMU / power up point of view. Could you please point > > me to the difference? > > > > The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > and BT_EN ordering and the only requirement is to have all input regulators > up before pulling up WLAN_EN and/or BT_EN. > > This makes the PMU transparent and BT and WLAN can be described as independent. From the hardware perspective, there is a PMU. It has several LDOs. So the device tree should have the same style as the previous generations.
On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > [snip] > > >>>> > > >>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > >>>> necessary. The BT and WLAN devices on the device-tree are represented as > > >>>> consuming the inputs (relevant to the functionality of each) of the PMU > > >>>> directly. > > >>> > > >>> We are describing the hardware. From the hardware point of view, there > > >>> is a PMU. I think at some point we would really like to describe all > > >>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > >>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > >> > > >> While I agree with older WiFi+BT units, I don't think it's needed for > > >> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > >> transparent. > > > > > > I don't see any significant difference between WCN6750/WCN6855 and > > > WCN7850 from the PMU / power up point of view. Could you please point > > > me to the difference? > > > > > > > The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > and BT_EN ordering and the only requirement is to have all input regulators > > up before pulling up WLAN_EN and/or BT_EN. > > > > This makes the PMU transparent and BT and WLAN can be described as independent. > > From the hardware perspective, there is a PMU. It has several LDOs. So > the device tree should have the same style as the previous > generations. > My thinking was this: yes, there is a PMU but describing it has no benefit (unlike QCA6x90). If we do describe, then we'll end up having to use pwrseq here despite it not being needed because now we won't be able to just get regulators from WLAN/BT drivers directly. So I also vote for keeping it this way. Let's go into the package detail only if it's required. Bartosz
On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > [snip] > > > > >>>> > > > >>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > > >>>> necessary. The BT and WLAN devices on the device-tree are represented as > > > >>>> consuming the inputs (relevant to the functionality of each) of the PMU > > > >>>> directly. > > > >>> > > > >>> We are describing the hardware. From the hardware point of view, there > > > >>> is a PMU. I think at some point we would really like to describe all > > > >>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > > >>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > > >> > > > >> While I agree with older WiFi+BT units, I don't think it's needed for > > > >> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > > >> transparent. > > > > > > > > I don't see any significant difference between WCN6750/WCN6855 and > > > > WCN7850 from the PMU / power up point of view. Could you please point > > > > me to the difference? > > > > > > > > > > The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > > and BT_EN ordering and the only requirement is to have all input regulators > > > up before pulling up WLAN_EN and/or BT_EN. > > > > > > This makes the PMU transparent and BT and WLAN can be described as independent. > > > > From the hardware perspective, there is a PMU. It has several LDOs. So > > the device tree should have the same style as the previous > > generations. > > > > My thinking was this: yes, there is a PMU but describing it has no > benefit (unlike QCA6x90). If we do describe, then we'll end up having > to use pwrseq here despite it not being needed because now we won't be > able to just get regulators from WLAN/BT drivers directly. > > So I also vote for keeping it this way. Let's go into the package > detail only if it's required. The WiFi / BT parts are not powered up by the board regulators. They are powered up by the PSU. So we are not describing it in the accurate way. Moreover, I think we definitely want to move BT driver to use only the pwrseq power up method. Doing it in the other way results in the code duplication and possible issues because of the regulator / pwrseq taking different code paths.
On 19/02/2024 13:33, Dmitry Baryshkov wrote: > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov >> <dmitry.baryshkov@linaro.org> wrote: >>> >> >> [snip] >> >>>>>>>> >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU >>>>>>>> directly. >>>>>>> >>>>>>> We are describing the hardware. From the hardware point of view, there >>>>>>> is a PMU. I think at some point we would really like to describe all >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). >>>>>> >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is >>>>>> transparent. >>>>> >>>>> I don't see any significant difference between WCN6750/WCN6855 and >>>>> WCN7850 from the PMU / power up point of view. Could you please point >>>>> me to the difference? >>>>> >>>> >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN >>>> and BT_EN ordering and the only requirement is to have all input regulators >>>> up before pulling up WLAN_EN and/or BT_EN. >>>> >>>> This makes the PMU transparent and BT and WLAN can be described as independent. >>> >>> From the hardware perspective, there is a PMU. It has several LDOs. So >>> the device tree should have the same style as the previous >>> generations. >>> >> >> My thinking was this: yes, there is a PMU but describing it has no >> benefit (unlike QCA6x90). If we do describe, then we'll end up having >> to use pwrseq here despite it not being needed because now we won't be >> able to just get regulators from WLAN/BT drivers directly. >> >> So I also vote for keeping it this way. Let's go into the package >> detail only if it's required. > > The WiFi / BT parts are not powered up by the board regulators. They > are powered up by the PSU. So we are not describing it in the accurate > way. I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in this situation the PCIe part is powered with the M.2 slot and the BT side is powered separately as we currently do it now. So yes there's a PMU, but it's not an always visible hardware part, from the SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. Neil > > Moreover, I think we definitely want to move BT driver to use only the > pwrseq power up method. Doing it in the other way results in the code > duplication and possible issues because of the regulator / pwrseq > taking different code paths. > >
On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >> > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > >> <dmitry.baryshkov@linaro.org> wrote: > >>> > >> > >> [snip] > >> > >>>>>>>> > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > >>>>>>>> directly. > >>>>>>> > >>>>>>> We are describing the hardware. From the hardware point of view, there > >>>>>>> is a PMU. I think at some point we would really like to describe all > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > >>>>>> > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > >>>>>> transparent. > >>>>> > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > >>>>> me to the difference? > >>>>> > >>>> > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > >>>> and BT_EN ordering and the only requirement is to have all input regulators > >>>> up before pulling up WLAN_EN and/or BT_EN. > >>>> > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > >>> > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > >>> the device tree should have the same style as the previous > >>> generations. > >>> > >> > >> My thinking was this: yes, there is a PMU but describing it has no > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > >> to use pwrseq here despite it not being needed because now we won't be > >> able to just get regulators from WLAN/BT drivers directly. > >> > >> So I also vote for keeping it this way. Let's go into the package > >> detail only if it's required. > > > > The WiFi / BT parts are not powered up by the board regulators. They > > are powered up by the PSU. So we are not describing it in the accurate > > way. > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > this situation the PCIe part is powered with the M.2 slot and the BT side > is powered separately as we currently do it now. QCA6390 can also be used as a discrete M.2 card. > So yes there's a PMU, but it's not an always visible hardware part, from the > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. From the hardware point: - There is a PMU - The PMU is connected to the board supplies - Both WiFi and BT parts are connected to the PMU - The BT_EN / WLAN_EN pins are not connected to the PMU So, not representing the PMU in the device tree is a simplification. > > Neil > > > > > Moreover, I think we definitely want to move BT driver to use only the > > pwrseq power up method. Doing it in the other way results in the code > > duplication and possible issues because of the regulator / pwrseq > > taking different code paths.
On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > >> > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > >> <dmitry.baryshkov@linaro.org> wrote: > > >>> > > >> > > >> [snip] > > >> > > >>>>>>>> > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > > >>>>>>>> directly. > > >>>>>>> > > >>>>>>> We are describing the hardware. From the hardware point of view, there > > >>>>>>> is a PMU. I think at some point we would really like to describe all > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > >>>>>> > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > >>>>>> transparent. > > >>>>> > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > > >>>>> me to the difference? > > >>>>> > > >>>> > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > >>>> and BT_EN ordering and the only requirement is to have all input regulators > > >>>> up before pulling up WLAN_EN and/or BT_EN. > > >>>> > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > > >>> > > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > > >>> the device tree should have the same style as the previous > > >>> generations. > > >>> > > >> > > >> My thinking was this: yes, there is a PMU but describing it has no > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > > >> to use pwrseq here despite it not being needed because now we won't be > > >> able to just get regulators from WLAN/BT drivers directly. > > >> > > >> So I also vote for keeping it this way. Let's go into the package > > >> detail only if it's required. > > > > > > The WiFi / BT parts are not powered up by the board regulators. They > > > are powered up by the PSU. So we are not describing it in the accurate > > > way. > > > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > > this situation the PCIe part is powered with the M.2 slot and the BT side > > is powered separately as we currently do it now. > > QCA6390 can also be used as a discrete M.2 card. > > > So yes there's a PMU, but it's not an always visible hardware part, from the > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. > > From the hardware point: > - There is a PMU > - The PMU is connected to the board supplies > - Both WiFi and BT parts are connected to the PMU > - The BT_EN / WLAN_EN pins are not connected to the PMU > > So, not representing the PMU in the device tree is a simplification. > What about the existing WLAN and BT users of similar packages? We would have to deprecate a lot of existing bindings. I don't think it's worth it. The WCN7850 is already described in bindings as consuming what is PMUs inputs and not its outputs. Bart > > > > Neil > > > > > > > > Moreover, I think we definitely want to move BT driver to use only the > > > pwrseq power up method. Doing it in the other way results in the code > > > duplication and possible issues because of the regulator / pwrseq > > > taking different code paths. > > -- > With best wishes > Dmitry
On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > >> > > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > > >> <dmitry.baryshkov@linaro.org> wrote: > > > >>> > > > >> > > > >> [snip] > > > >> > > > >>>>>>>> > > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > > > >>>>>>>> directly. > > > >>>>>>> > > > >>>>>>> We are describing the hardware. From the hardware point of view, there > > > >>>>>>> is a PMU. I think at some point we would really like to describe all > > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > > >>>>>> > > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > > >>>>>> transparent. > > > >>>>> > > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > > > >>>>> me to the difference? > > > >>>>> > > > >>>> > > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > > >>>> and BT_EN ordering and the only requirement is to have all input regulators > > > >>>> up before pulling up WLAN_EN and/or BT_EN. > > > >>>> > > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > > > >>> > > > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > > > >>> the device tree should have the same style as the previous > > > >>> generations. > > > >>> > > > >> > > > >> My thinking was this: yes, there is a PMU but describing it has no > > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > > > >> to use pwrseq here despite it not being needed because now we won't be > > > >> able to just get regulators from WLAN/BT drivers directly. > > > >> > > > >> So I also vote for keeping it this way. Let's go into the package > > > >> detail only if it's required. > > > > > > > > The WiFi / BT parts are not powered up by the board regulators. They > > > > are powered up by the PSU. So we are not describing it in the accurate > > > > way. > > > > > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > > > this situation the PCIe part is powered with the M.2 slot and the BT side > > > is powered separately as we currently do it now. > > > > QCA6390 can also be used as a discrete M.2 card. > > > > > So yes there's a PMU, but it's not an always visible hardware part, from the > > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. > > > > From the hardware point: > > - There is a PMU > > - The PMU is connected to the board supplies > > - Both WiFi and BT parts are connected to the PMU > > - The BT_EN / WLAN_EN pins are not connected to the PMU > > > > So, not representing the PMU in the device tree is a simplification. > > > > What about the existing WLAN and BT users of similar packages? We > would have to deprecate a lot of existing bindings. I don't think it's > worth it. We have bindings that are not reflecting the hardware. So yes, we should gradually update them once the powerseq is merged. > The WCN7850 is already described in bindings as consuming what is PMUs > inputs and not its outputs. So do WCN6855 and QCA6391 BlueTooth parts. > > Bart > > > > > > > Neil > > > > > > > > > > > Moreover, I think we definitely want to move BT driver to use only the > > > > pwrseq power up method. Doing it in the other way results in the code > > > > duplication and possible issues because of the regulator / pwrseq > > > > taking different code paths. > > > > -- > > With best wishes > > Dmitry
On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > > > > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > >> > > > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > > > >> <dmitry.baryshkov@linaro.org> wrote: > > > > >>> > > > > >> > > > > >> [snip] > > > > >> > > > > >>>>>>>> > > > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > > > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > > > > >>>>>>>> directly. > > > > >>>>>>> > > > > >>>>>>> We are describing the hardware. From the hardware point of view, there > > > > >>>>>>> is a PMU. I think at some point we would really like to describe all > > > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > > > >>>>>> > > > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > > > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > > > >>>>>> transparent. > > > > >>>>> > > > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > > > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > > > > >>>>> me to the difference? > > > > >>>>> > > > > >>>> > > > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > > > >>>> and BT_EN ordering and the only requirement is to have all input regulators > > > > >>>> up before pulling up WLAN_EN and/or BT_EN. > > > > >>>> > > > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > > > > >>> > > > > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > > > > >>> the device tree should have the same style as the previous > > > > >>> generations. > > > > >>> > > > > >> > > > > >> My thinking was this: yes, there is a PMU but describing it has no > > > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > > > > >> to use pwrseq here despite it not being needed because now we won't be > > > > >> able to just get regulators from WLAN/BT drivers directly. > > > > >> > > > > >> So I also vote for keeping it this way. Let's go into the package > > > > >> detail only if it's required. > > > > > > > > > > The WiFi / BT parts are not powered up by the board regulators. They > > > > > are powered up by the PSU. So we are not describing it in the accurate > > > > > way. > > > > > > > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > > > > this situation the PCIe part is powered with the M.2 slot and the BT side > > > > is powered separately as we currently do it now. > > > > > > QCA6390 can also be used as a discrete M.2 card. > > > > > > > So yes there's a PMU, but it's not an always visible hardware part, from the > > > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. > > > > > > From the hardware point: > > > - There is a PMU > > > - The PMU is connected to the board supplies > > > - Both WiFi and BT parts are connected to the PMU > > > - The BT_EN / WLAN_EN pins are not connected to the PMU > > > > > > So, not representing the PMU in the device tree is a simplification. > > > > > > > What about the existing WLAN and BT users of similar packages? We > > would have to deprecate a lot of existing bindings. I don't think it's > > worth it. > > We have bindings that are not reflecting the hardware. So yes, we > should gradually update them once the powerseq is merged. > > > The WCN7850 is already described in bindings as consuming what is PMUs > > inputs and not its outputs. > > So do WCN6855 and QCA6391 BlueTooth parts. > That is not true for the latter, this series is adding regulators for it. Bart > > > > Bart > > > > > > > > > > Neil > > > > > > > > > > > > > > Moreover, I think we definitely want to move BT driver to use only the > > > > > pwrseq power up method. Doing it in the other way results in the code > > > > > duplication and possible issues because of the regulator / pwrseq > > > > > taking different code paths. > > > > > > -- > > > With best wishes > > > Dmitry > > > > -- > With best wishes > Dmitry
On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > >> > > > > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > > > > >> <dmitry.baryshkov@linaro.org> wrote: > > > > > >>> > > > > > >> > > > > > >> [snip] > > > > > >> > > > > > >>>>>>>> > > > > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > > > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > > > > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > > > > > >>>>>>>> directly. > > > > > >>>>>>> > > > > > >>>>>>> We are describing the hardware. From the hardware point of view, there > > > > > >>>>>>> is a PMU. I think at some point we would really like to describe all > > > > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > > > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > > > > >>>>>> > > > > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > > > > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > > > > >>>>>> transparent. > > > > > >>>>> > > > > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > > > > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > > > > > >>>>> me to the difference? > > > > > >>>>> > > > > > >>>> > > > > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > > > > >>>> and BT_EN ordering and the only requirement is to have all input regulators > > > > > >>>> up before pulling up WLAN_EN and/or BT_EN. > > > > > >>>> > > > > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > > > > > >>> > > > > > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > > > > > >>> the device tree should have the same style as the previous > > > > > >>> generations. > > > > > >>> > > > > > >> > > > > > >> My thinking was this: yes, there is a PMU but describing it has no > > > > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > > > > > >> to use pwrseq here despite it not being needed because now we won't be > > > > > >> able to just get regulators from WLAN/BT drivers directly. > > > > > >> > > > > > >> So I also vote for keeping it this way. Let's go into the package > > > > > >> detail only if it's required. > > > > > > > > > > > > The WiFi / BT parts are not powered up by the board regulators. They > > > > > > are powered up by the PSU. So we are not describing it in the accurate > > > > > > way. > > > > > > > > > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > > > > > this situation the PCIe part is powered with the M.2 slot and the BT side > > > > > is powered separately as we currently do it now. > > > > > > > > QCA6390 can also be used as a discrete M.2 card. > > > > > > > > > So yes there's a PMU, but it's not an always visible hardware part, from the > > > > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. > > > > > > > > From the hardware point: > > > > - There is a PMU > > > > - The PMU is connected to the board supplies > > > > - Both WiFi and BT parts are connected to the PMU > > > > - The BT_EN / WLAN_EN pins are not connected to the PMU > > > > > > > > So, not representing the PMU in the device tree is a simplification. > > > > > > > > > > What about the existing WLAN and BT users of similar packages? We > > > would have to deprecate a lot of existing bindings. I don't think it's > > > worth it. > > > > We have bindings that are not reflecting the hardware. So yes, we > > should gradually update them once the powerseq is merged. > > > > > The WCN7850 is already described in bindings as consuming what is PMUs > > > inputs and not its outputs. > > > > So do WCN6855 and QCA6391 BlueTooth parts. > > > > That is not true for the latter, this series is adding regulators for it. But the bindings exist already, so you still have to extend it, deprecating regulator-less bindings. Bartosz, I really don't understand what is the issue there. There is a PMU. As such it should be represented in the DT and it can be handled by the same driver as you are adding for QCA6390. > > Bart > > > > > > > Bart > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > Moreover, I think we definitely want to move BT driver to use only the > > > > > > pwrseq power up method. Doing it in the other way results in the code > > > > > > duplication and possible issues because of the regulator / pwrseq > > > > > > taking different code paths. > > > > > > > > -- > > > > With best wishes > > > > Dmitry > > > > > > > > -- > > With best wishes > > Dmitry
On Thu, Feb 22, 2024 at 1:47 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > > > On 19/02/2024 13:33, Dmitry Baryshkov wrote: > > > > > > > On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > >> > > > > > > >> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov > > > > > > >> <dmitry.baryshkov@linaro.org> wrote: > > > > > > >>> > > > > > > >> > > > > > > >> [snip] > > > > > > >> > > > > > > >>>>>>>> > > > > > > >>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not > > > > > > >>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as > > > > > > >>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU > > > > > > >>>>>>>> directly. > > > > > > >>>>>>> > > > > > > >>>>>>> We are describing the hardware. From the hardware point of view, there > > > > > > >>>>>>> is a PMU. I think at some point we would really like to describe all > > > > > > >>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the > > > > > > >>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). > > > > > > >>>>>> > > > > > > >>>>>> While I agree with older WiFi+BT units, I don't think it's needed for > > > > > > >>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is > > > > > > >>>>>> transparent. > > > > > > >>>>> > > > > > > >>>>> I don't see any significant difference between WCN6750/WCN6855 and > > > > > > >>>>> WCN7850 from the PMU / power up point of view. Could you please point > > > > > > >>>>> me to the difference? > > > > > > >>>>> > > > > > > >>>> > > > > > > >>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN > > > > > > >>>> and BT_EN ordering and the only requirement is to have all input regulators > > > > > > >>>> up before pulling up WLAN_EN and/or BT_EN. > > > > > > >>>> > > > > > > >>>> This makes the PMU transparent and BT and WLAN can be described as independent. > > > > > > >>> > > > > > > >>> From the hardware perspective, there is a PMU. It has several LDOs. So > > > > > > >>> the device tree should have the same style as the previous > > > > > > >>> generations. > > > > > > >>> > > > > > > >> > > > > > > >> My thinking was this: yes, there is a PMU but describing it has no > > > > > > >> benefit (unlike QCA6x90). If we do describe, then we'll end up having > > > > > > >> to use pwrseq here despite it not being needed because now we won't be > > > > > > >> able to just get regulators from WLAN/BT drivers directly. > > > > > > >> > > > > > > >> So I also vote for keeping it this way. Let's go into the package > > > > > > >> detail only if it's required. > > > > > > > > > > > > > > The WiFi / BT parts are not powered up by the board regulators. They > > > > > > > are powered up by the PSU. So we are not describing it in the accurate > > > > > > > way. > > > > > > > > > > > > I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in > > > > > > this situation the PCIe part is powered with the M.2 slot and the BT side > > > > > > is powered separately as we currently do it now. > > > > > > > > > > QCA6390 can also be used as a discrete M.2 card. > > > > > > > > > > > So yes there's a PMU, but it's not an always visible hardware part, from the > > > > > > SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. > > > > > > > > > > From the hardware point: > > > > > - There is a PMU > > > > > - The PMU is connected to the board supplies > > > > > - Both WiFi and BT parts are connected to the PMU > > > > > - The BT_EN / WLAN_EN pins are not connected to the PMU > > > > > > > > > > So, not representing the PMU in the device tree is a simplification. > > > > > > > > > > > > > What about the existing WLAN and BT users of similar packages? We > > > > would have to deprecate a lot of existing bindings. I don't think it's > > > > worth it. > > > > > > We have bindings that are not reflecting the hardware. So yes, we > > > should gradually update them once the powerseq is merged. > > > > > > > The WCN7850 is already described in bindings as consuming what is PMUs > > > > inputs and not its outputs. > > > > > > So do WCN6855 and QCA6391 BlueTooth parts. > > > > > > > That is not true for the latter, this series is adding regulators for it. > > But the bindings exist already, so you still have to extend it, > deprecating regulator-less bindings. > > Bartosz, I really don't understand what is the issue there. There is a > PMU. As such it should be represented in the DT and it can be handled > by the same driver as you are adding for QCA6390. > The issue is that we'll pull in the pwrseq subsystem for WCN7850 which clearly does not require it in practice. I'd like to hear Krzysztof, Conor or Rob chime in here and make the decision on how to proceed. Bart > > > > Bart > > > > > > > > > > Bart > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > Moreover, I think we definitely want to move BT driver to use only the > > > > > > > pwrseq power up method. Doing it in the other way results in the code > > > > > > > duplication and possible issues because of the regulator / pwrseq > > > > > > > taking different code paths. > > > > > > > > > > -- > > > > > With best wishes > > > > > Dmitry > > > > > > > > > > > > -- > > > With best wishes > > > Dmitry > > > > -- > With best wishes > Dmitry
On 22/02/2024 13:50, Bartosz Golaszewski wrote: > On Thu, Feb 22, 2024 at 1:47 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>> >>> On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov >>> <dmitry.baryshkov@linaro.org> wrote: >>>> >>>> On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>> >>>>> On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov >>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>> >>>>>> On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: >>>>>>> >>>>>>> On 19/02/2024 13:33, Dmitry Baryshkov wrote: >>>>>>>> On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>>>> >>>>>>>>> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov >>>>>>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>>>>>> >>>>>>>>> >>>>>>>>> [snip] >>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not >>>>>>>>>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as >>>>>>>>>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU >>>>>>>>>>>>>>> directly. >>>>>>>>>>>>>> >>>>>>>>>>>>>> We are describing the hardware. From the hardware point of view, there >>>>>>>>>>>>>> is a PMU. I think at some point we would really like to describe all >>>>>>>>>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the >>>>>>>>>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). >>>>>>>>>>>>> >>>>>>>>>>>>> While I agree with older WiFi+BT units, I don't think it's needed for >>>>>>>>>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is >>>>>>>>>>>>> transparent. >>>>>>>>>>>> >>>>>>>>>>>> I don't see any significant difference between WCN6750/WCN6855 and >>>>>>>>>>>> WCN7850 from the PMU / power up point of view. Could you please point >>>>>>>>>>>> me to the difference? >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN >>>>>>>>>>> and BT_EN ordering and the only requirement is to have all input regulators >>>>>>>>>>> up before pulling up WLAN_EN and/or BT_EN. >>>>>>>>>>> >>>>>>>>>>> This makes the PMU transparent and BT and WLAN can be described as independent. >>>>>>>>>> >>>>>>>>>> From the hardware perspective, there is a PMU. It has several LDOs. So >>>>>>>>>> the device tree should have the same style as the previous >>>>>>>>>> generations. >>>>>>>>>> >>>>>>>>> >>>>>>>>> My thinking was this: yes, there is a PMU but describing it has no >>>>>>>>> benefit (unlike QCA6x90). If we do describe, then we'll end up having >>>>>>>>> to use pwrseq here despite it not being needed because now we won't be >>>>>>>>> able to just get regulators from WLAN/BT drivers directly. >>>>>>>>> >>>>>>>>> So I also vote for keeping it this way. Let's go into the package >>>>>>>>> detail only if it's required. >>>>>>>> >>>>>>>> The WiFi / BT parts are not powered up by the board regulators. They >>>>>>>> are powered up by the PSU. So we are not describing it in the accurate >>>>>>>> way. >>>>>>> >>>>>>> I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in >>>>>>> this situation the PCIe part is powered with the M.2 slot and the BT side >>>>>>> is powered separately as we currently do it now. >>>>>> >>>>>> QCA6390 can also be used as a discrete M.2 card. >>>>>> >>>>>>> So yes there's a PMU, but it's not an always visible hardware part, from the >>>>>>> SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. >>>>>> >>>>>> From the hardware point: >>>>>> - There is a PMU >>>>>> - The PMU is connected to the board supplies >>>>>> - Both WiFi and BT parts are connected to the PMU >>>>>> - The BT_EN / WLAN_EN pins are not connected to the PMU >>>>>> >>>>>> So, not representing the PMU in the device tree is a simplification. >>>>>> >>>>> >>>>> What about the existing WLAN and BT users of similar packages? We >>>>> would have to deprecate a lot of existing bindings. I don't think it's >>>>> worth it. >>>> >>>> We have bindings that are not reflecting the hardware. So yes, we >>>> should gradually update them once the powerseq is merged. >>>> >>>>> The WCN7850 is already described in bindings as consuming what is PMUs >>>>> inputs and not its outputs. >>>> >>>> So do WCN6855 and QCA6391 BlueTooth parts. >>>> >>> >>> That is not true for the latter, this series is adding regulators for it. >> >> But the bindings exist already, so you still have to extend it, >> deprecating regulator-less bindings. >> >> Bartosz, I really don't understand what is the issue there. There is a >> PMU. As such it should be represented in the DT and it can be handled >> by the same driver as you are adding for QCA6390. >> > > The issue is that we'll pull in the pwrseq subsystem for WCN7850 which > clearly does not require it in practice. > > I'd like to hear Krzysztof, Conor or Rob chime in here and make the > decision on how to proceed. For WCN7850 we'll be describing the PMU which is basically discrete, so we could also add dummy fixed regulators, and it would be the same. Neil > > Bart > >>> >>> Bart >>> >>>>> >>>>> Bart >>>>> >>>>>>> >>>>>>> Neil >>>>>>> >>>>>>>> >>>>>>>> Moreover, I think we definitely want to move BT driver to use only the >>>>>>>> pwrseq power up method. Doing it in the other way results in the code >>>>>>>> duplication and possible issues because of the regulator / pwrseq >>>>>>>> taking different code paths. >>>>>> >>>>>> -- >>>>>> With best wishes >>>>>> Dmitry >>>> >>>> >>>> >>>> -- >>>> With best wishes >>>> Dmitry >> >> >> >> -- >> With best wishes >> Dmitry
On 22/02/2024 13:50, Bartosz Golaszewski wrote: > On Thu, Feb 22, 2024 at 1:47 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Thu, 22 Feb 2024 at 14:27, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>> >>> On Thu, Feb 22, 2024 at 12:27 PM Dmitry Baryshkov >>> <dmitry.baryshkov@linaro.org> wrote: >>>> >>>> On Thu, 22 Feb 2024 at 13:00, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>> >>>>> On Mon, Feb 19, 2024 at 11:21 PM Dmitry Baryshkov >>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>> >>>>>> On Mon, 19 Feb 2024 at 19:18, <neil.armstrong@linaro.org> wrote: >>>>>>> >>>>>>> On 19/02/2024 13:33, Dmitry Baryshkov wrote: >>>>>>>> On Mon, 19 Feb 2024 at 14:23, Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>>>> >>>>>>>>> On Mon, Feb 19, 2024 at 11:26 AM Dmitry Baryshkov >>>>>>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>>>>>> >>>>>>>>> >>>>>>>>> [snip] >>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> For WCN7850 we hide the existence of the PMU as modeling it is simply not >>>>>>>>>>>>>>> necessary. The BT and WLAN devices on the device-tree are represented as >>>>>>>>>>>>>>> consuming the inputs (relevant to the functionality of each) of the PMU >>>>>>>>>>>>>>> directly. >>>>>>>>>>>>>> >>>>>>>>>>>>>> We are describing the hardware. From the hardware point of view, there >>>>>>>>>>>>>> is a PMU. I think at some point we would really like to describe all >>>>>>>>>>>>>> Qualcomm/Atheros WiFI+BT units using this PMU approach, including the >>>>>>>>>>>>>> older ath10k units present on RB3 (WCN3990) and db820c (QCA6174). >>>>>>>>>>>>> >>>>>>>>>>>>> While I agree with older WiFi+BT units, I don't think it's needed for >>>>>>>>>>>>> WCN7850 since BT+WiFi are now designed to be fully independent and PMU is >>>>>>>>>>>>> transparent. >>>>>>>>>>>> >>>>>>>>>>>> I don't see any significant difference between WCN6750/WCN6855 and >>>>>>>>>>>> WCN7850 from the PMU / power up point of view. Could you please point >>>>>>>>>>>> me to the difference? >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> The WCN7850 datasheet clearly states there's not contraint on the WLAN_EN >>>>>>>>>>> and BT_EN ordering and the only requirement is to have all input regulators >>>>>>>>>>> up before pulling up WLAN_EN and/or BT_EN. >>>>>>>>>>> >>>>>>>>>>> This makes the PMU transparent and BT and WLAN can be described as independent. >>>>>>>>>> >>>>>>>>>> From the hardware perspective, there is a PMU. It has several LDOs. So >>>>>>>>>> the device tree should have the same style as the previous >>>>>>>>>> generations. >>>>>>>>>> >>>>>>>>> >>>>>>>>> My thinking was this: yes, there is a PMU but describing it has no >>>>>>>>> benefit (unlike QCA6x90). If we do describe, then we'll end up having >>>>>>>>> to use pwrseq here despite it not being needed because now we won't be >>>>>>>>> able to just get regulators from WLAN/BT drivers directly. >>>>>>>>> >>>>>>>>> So I also vote for keeping it this way. Let's go into the package >>>>>>>>> detail only if it's required. >>>>>>>> >>>>>>>> The WiFi / BT parts are not powered up by the board regulators. They >>>>>>>> are powered up by the PSU. So we are not describing it in the accurate >>>>>>>> way. >>>>>>> >>>>>>> I disagree, the WCN7850 can also be used as a discrete PCIe M.2 card, and in >>>>>>> this situation the PCIe part is powered with the M.2 slot and the BT side >>>>>>> is powered separately as we currently do it now. >>>>>> >>>>>> QCA6390 can also be used as a discrete M.2 card. >>>>>> >>>>>>> So yes there's a PMU, but it's not an always visible hardware part, from the >>>>>>> SoC PoV, only the separate PCIe and BT subsystems are visible/controllable/powerable. >>>>>> >>>>>> From the hardware point: >>>>>> - There is a PMU >>>>>> - The PMU is connected to the board supplies >>>>>> - Both WiFi and BT parts are connected to the PMU >>>>>> - The BT_EN / WLAN_EN pins are not connected to the PMU >>>>>> >>>>>> So, not representing the PMU in the device tree is a simplification. >>>>>> >>>>> >>>>> What about the existing WLAN and BT users of similar packages? We >>>>> would have to deprecate a lot of existing bindings. I don't think it's >>>>> worth it. >>>> >>>> We have bindings that are not reflecting the hardware. So yes, we >>>> should gradually update them once the powerseq is merged. >>>> >>>>> The WCN7850 is already described in bindings as consuming what is PMUs >>>>> inputs and not its outputs. >>>> >>>> So do WCN6855 and QCA6391 BlueTooth parts. >>>> >>> >>> That is not true for the latter, this series is adding regulators for it. >> >> But the bindings exist already, so you still have to extend it, >> deprecating regulator-less bindings. >> >> Bartosz, I really don't understand what is the issue there. There is a >> PMU. As such it should be represented in the DT and it can be handled >> by the same driver as you are adding for QCA6390. >> > > The issue is that we'll pull in the pwrseq subsystem for WCN7850 which > clearly does not require it in practice. > > I'd like to hear Krzysztof, Conor or Rob chime in here and make the > decision on how to proceed. There's like 12 emails here, so please don't just point "MR X, please read everything to find the question I want to ask", but just ask the question with short intro. We all (and I bet you as well) are way too busy to read long threads... If I got it correctly, you ask if some other, existing QCA/WCN chips should be changed to this PMU approach? If yes, then: 1. It depends whether they have the PMU, so some sort of analysis of datasheet should be done. 2. You could but you don't have to. Bindings were done, they represent the hardware more-or-less, maybe less, but still good enough. 3. It does not have to impact actual behavior of Linux. You don't have to bind entire pwrseq driver to that QCA/WCN compatible. Anyway Linux behavior is here a bit separate question - it can change, it can stay the same, up to you. Best regards, Krzysztof
On Sat, 17 Feb 2024 at 02:03, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > First, I'd like to apologize for the somewhat chaotic previous iterations > of this series and improper versioning which was rightfully pointed out > to me. I figured that the scope changed so much that it didn't make sense > to consider previous submissions part of the same series as the original > RFC but others thought otherwise so this one becomes v5 and I'll keep the > versioning going forward. > > This is the summary of the work so far: > > v1: Original RFC: > > https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ > > v2: First real patch series (should have been PATCH v2) adding what I > referred to back then as PCI power sequencing: > > https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > > v3: RFC for the DT representation of the PMU supplying the WLAN and BT > modules inside the QCA6391 package (was largely separate from the > series but probably should have been called PATCH or RFC v3): > > https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > > v4: Second attempt at the full series with changed scope (introduction of > the pwrseq subsystem, should have been RFC v4) > > https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > > === > > With that out of the way, I'd like to get down to explaining the two > problems I'm trying to solve. > > Problem statement #1: Dynamic bus chicken-and-egg problem. > > Certain on-board PCI devices need to be powered up before they are can be > detected but their PCI drivers won't get bound until the device is > powered-up so enabling the relevant resources in the PCI device driver > itself is impossible. > > Problem statement #2: Sharing inter-dependent resources between devices. > > Certain devices that use separate drivers (often on different busses) > share resources (regulators, clocks, etc.). Typically these resources > are reference-counted but in some cases there are additional interactions > between them to consider, for example specific power-up sequence timings. > > === > > The reason for tackling both of these problems in a single series is the > fact the the platform I'm working on - Qualcomm RB5 - deals with both and > both need to be addressed in order to enable WLAN and Bluetooth support > upstream. > > The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that > takes inputs from the host and exposes LDO outputs consumed by the BT and > WLAN modules which can be powered-up and down independently. However > a delay of 100ms must be respected between enabling the BT- and > WLAN-enable GPIOs[*]. > > === > > This series is logically split into several sections. I'll go > patch-by-patch and explain each step. > > Patch 1/18: > > This is a commit taken from the list by Jonathan Cameron that adds > a __free() helper for OF nodes. Not strictly related to the series but > until said commit ends in next, I need to carry it with this series. > > Patch 2/18: > > This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 > and sm8550 reference platforms use it in the WCN7850 module. > > Patches 3/18-6/18: > > These contain all relevant DT bindings changes. We add new documents for > the QCA6390 PMU and ATH12K devices as well as extend the bindings for the > Qualcomm Bluetooth and ATH11K modules with regulators used by them in > QCA6390. > > Patches 7/18-9/18: > > These contain changes to device-tree sources for the three platforms we > work with in this series. As the WCN7850 module doesn't require any > specific timings introducing dependencies between the Bluetooth and WLAN > modules, while the QCA6390 does, we take two different approaches to how > me model them in DT. > > For WCN7850 we hide the existence of the PMU as modeling it is simply not > necessary. The BT and WLAN devices on the device-tree are represented as > consuming the inputs (relevant to the functionality of each) of the PMU > directly. > > For QCA6390 on RB5 we add the PMU node as a platform device. It consumes > regulators and GPIOs from the host and exposed regulators consumer in turn > by the BT and WLAN modules. This represents the internal structure of the > package. > > Patches 10/18-14/18: > > These contain the bulk of the PCI changes for this series. We introduce > a simple framework for powering up PCI devices before detecting them on > the bus and the first user of this library in the form of the WCN7850 PCI > power control driver. > > The general approach is as follows: PCI devices that need special > treatment before they can be powered up, scanned and bound to their PCI > drivers must be described on the device-tree as child nodes of the PCI > port node. These devices will be instantiated on the platform bus. They > will in fact be generic platform devices with the compatible of the form > used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We > add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl > drivers. These drivers are platform drivers that will now be matched > against the devices instantiated from port children just like any other > platform pairs. > > Both the power control platform device *AND* the associated PCI device > reuse the same OF node and have access to the same properties. The goal > of the platform driver is to request and bring up any required resources > and let the pwrctl framework know that it's now OK to rescan the bus and > detect the devices. When the device is bound, we are notified about it > by the PCI bus notifier event and can establish a device link between the > power control device and the PCI device so that any future extension for > power-management will already be able to work with the correct hierachy. > > The reusing of the OF node is the reason for the small changes to the PCI > OF core: as the bootloader can possibly leave the relevant regulators on > before booting linux, the PCI device can be detected before its platform > abstraction is probed. In this case, we find that device first and mark > its OF node as reused. The pwrctl framework handles the opposite case > (when the PCI device is detected only after the platform driver > successfully enabled it). > > Patches 15/18-16/18: > > These add a relatively simple power sequencing subsystem and the first > driver using it: the pwrseq module for the QCA6390 PMU. > > For the record: Bjorn suggested a different solution: a regulator driver > that would - based on which regulators are enabled by a consumer - enable > relevant resources (drive the enable GPIOs) while respecting the > HW-specific delays. This would however require significant and yet > unprecised changed to the regulator subsystem as well as be an abuse of > the regulator provider API akin to using the reset framework for power > sequencing as proposed before. > > Instead I'm proposing to add a subsystem that allows different devices to > use a shared power sequence split into consumer-specific as well as > common "units". > > A power sequence provider driver registers a set of units with pwrseq > core. Each unit can be enabled and disabled and contains an optional list > of other units which must be enabled before it itself can be. A unit > represents a discreet chunk of the power sequence. > > It also registers a list of targets: a target is an abstraction wrapping > a unit which allows consumers to tell pwrseq which unit they want to > reach. Real-life example is the driver we're adding here: there's a set > of common regulators, two PCIe-specific ones and two enable GPIOs: one > for Bluetooth and one for WLAN. > > The Bluetooth driver requests a descriptor to the power sequencer and > names the target it wants to reach: > > pwrseq = devm_pwrseq_get(dev, "bluetooth"); > > The pwrseq core then knows that when the driver calls: > > pwrseq_power_on(pwrseq); > > It must enable the "bluetooth-enable" unit but it depends on the > "regulators-common" unit so this one is enabled first. The provider > driver is also in charge of assuring an appropriate delay between > enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are > handled by the "wlan-enable" unit and so are not enabled until the WLAN > driver requests the "wlan" target to be powered on. > > Another thing worth discussing is the way we associate the consumer with > the relevant power sequencer. DT maintainers have expressed a discontent > with the existing mmc pwrseq bindings and have NAKed an earlier > initiative to introduce global pwrseq bindings to the kernel[1]. > > In this approach, we model the existing regulators and GPIOs in DT but > the pwrseq subsystem requires each provider to provide a .match() > callback. Whenever a consumer requests a power sequencer handle, we > iterate over the list of pwrseq drivers and call .match() for each. It's > up to the driver to verify in a platform-specific way whether it deals > with its consumer and let the core pwrseq code know. > > The advantage of this over reusing the regulator or reset subsystem is > that it's more generalized and can handle resources of all kinds as well > as deal with any kind of power-on sequences: for instance, Qualcomm has > a PCI switch they want a driver for but this switch requires enabling > some resources first (PCI pwrctl) and then configuring the device over > I2C (which can be handled by the pwrseq provider). > > Patch 17/18: > > This patch makes the Qualcomm Bluetooth driver get and use the power > sequencer for QCA6390. > > Patch 18/18: > > While tiny, this patch is possibly the highlight of the entire series. > It uses the two abstraction layers we introduced before to create an > elegant power sequencing PCI power control driver and supports the ath11k > module on QCA6390. > > With this series we can now enable BT and WLAN on several new Qualcomm > boards upstream. > > I tested the series on RB5 while Neil tested it on sm8650-qrd and > sm8550-qrd. Tested-by: Amit Pundir <amit.pundir@linaro.org> # On RB5 running AOSP > > Best Regards, > Bartosz Golaszewski > > It's hard to list the changes between versions here as the scope changed > significantly between each iteration and some versions were not even full > series but rather RFCs for parts of the solution. For this reason, I'll > only start listing changes starting from v6. > > [*] This is what the docs say. In practice it seems that this delay can be > ignored. However the subsequent model - QCA6490 - *does* require users to > respect it, so the problem remains valid. > > [1] https://lore.kernel.org/netdev/20210829131305.534417-1-dmitry.baryshkov@linaro.org/ > > Bartosz Golaszewski (15): > arm64: defconfig: enable ath12k as a module > dt-bindings: regulator: describe the PMU module of the QCA6390 package > dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390 > dt-bindings: new: wireless: qcom,ath11k: describe the ath11k on > QCA6390 > dt-bindings: new: wireless: describe the ath12k PCI module > arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 > PCI: hold the rescan mutex when scanning for the first time > PCI/pwrctl: reuse the OF node for power controlled devices > PCI/pwrctl: create platform devices for child OF nodes of the port > node > PCI/pwrctl: add PCI power control core code > PCI/pwrctl: add a power control driver for WCN7850 > power: sequencing: implement the pwrseq core > power: pwrseq: add a driver for the QCA6390 PMU module > Bluetooth: qca: use the power sequencer for QCA6390 > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > Jonathan Cameron (1): > of: Add cleanup.h based auto release via __free(device_node) markings. > > Neil Armstrong (2): > arm64: dts: qcom: sm8550-qrd: add the Wifi node > arm64: dts: qcom: sm8650-qrd: add the Wifi node > > .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + > .../net/wireless/qcom,ath11k-pci.yaml | 28 + > .../net/wireless/qcom,ath12k-pci.yaml | 103 ++ > .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 +++ > MAINTAINERS | 8 + > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 123 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 37 + > arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 29 + > arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + > arch/arm64/configs/defconfig | 1 + > drivers/bluetooth/hci_qca.c | 31 + > drivers/pci/Kconfig | 1 + > drivers/pci/Makefile | 1 + > drivers/pci/bus.c | 9 +- > drivers/pci/of.c | 14 +- > drivers/pci/probe.c | 2 + > drivers/pci/pwrctl/Kconfig | 25 + > drivers/pci/pwrctl/Makefile | 7 + > drivers/pci/pwrctl/core.c | 136 +++ > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 84 ++ > drivers/pci/pwrctl/pci-pwrctl-wcn7850.c | 202 ++++ > drivers/pci/remove.c | 2 + > drivers/power/Kconfig | 1 + > drivers/power/Makefile | 1 + > drivers/power/sequencing/Kconfig | 28 + > drivers/power/sequencing/Makefile | 6 + > drivers/power/sequencing/core.c | 1065 +++++++++++++++++ > drivers/power/sequencing/pwrseq-qca6390.c | 353 ++++++ > include/linux/of.h | 2 + > include/linux/pci-pwrctl.h | 51 + > include/linux/pwrseq/consumer.h | 56 + > include/linux/pwrseq/provider.h | 75 ++ > 34 files changed, 2678 insertions(+), 16 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-pci.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml > create mode 100644 drivers/pci/pwrctl/Kconfig > create mode 100644 drivers/pci/pwrctl/Makefile > create mode 100644 drivers/pci/pwrctl/core.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-wcn7850.c > create mode 100644 drivers/power/sequencing/Kconfig > create mode 100644 drivers/power/sequencing/Makefile > create mode 100644 drivers/power/sequencing/core.c > create mode 100644 drivers/power/sequencing/pwrseq-qca6390.c > create mode 100644 include/linux/pci-pwrctl.h > create mode 100644 include/linux/pwrseq/consumer.h > create mode 100644 include/linux/pwrseq/provider.h > > -- > 2.40.1 > >
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> First, I'd like to apologize for the somewhat chaotic previous iterations of this series and improper versioning which was rightfully pointed out to me. I figured that the scope changed so much that it didn't make sense to consider previous submissions part of the same series as the original RFC but others thought otherwise so this one becomes v5 and I'll keep the versioning going forward. This is the summary of the work so far: v1: Original RFC: https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/ v2: First real patch series (should have been PATCH v2) adding what I referred to back then as PCI power sequencing: https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ v3: RFC for the DT representation of the PMU supplying the WLAN and BT modules inside the QCA6391 package (was largely separate from the series but probably should have been called PATCH or RFC v3): https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ v4: Second attempt at the full series with changed scope (introduction of the pwrseq subsystem, should have been RFC v4) https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ === With that out of the way, I'd like to get down to explaining the two problems I'm trying to solve. Problem statement #1: Dynamic bus chicken-and-egg problem. Certain on-board PCI devices need to be powered up before they are can be detected but their PCI drivers won't get bound until the device is powered-up so enabling the relevant resources in the PCI device driver itself is impossible. Problem statement #2: Sharing inter-dependent resources between devices. Certain devices that use separate drivers (often on different busses) share resources (regulators, clocks, etc.). Typically these resources are reference-counted but in some cases there are additional interactions between them to consider, for example specific power-up sequence timings. === The reason for tackling both of these problems in a single series is the fact the the platform I'm working on - Qualcomm RB5 - deals with both and both need to be addressed in order to enable WLAN and Bluetooth support upstream. The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that takes inputs from the host and exposes LDO outputs consumed by the BT and WLAN modules which can be powered-up and down independently. However a delay of 100ms must be respected between enabling the BT- and WLAN-enable GPIOs[*]. === This series is logically split into several sections. I'll go patch-by-patch and explain each step. Patch 1/18: This is a commit taken from the list by Jonathan Cameron that adds a __free() helper for OF nodes. Not strictly related to the series but until said commit ends in next, I need to carry it with this series. Patch 2/18: This enables the ath12k PCI module in arm64 defconfig as Qualcomm sm8650 and sm8550 reference platforms use it in the WCN7850 module. Patches 3/18-6/18: These contain all relevant DT bindings changes. We add new documents for the QCA6390 PMU and ATH12K devices as well as extend the bindings for the Qualcomm Bluetooth and ATH11K modules with regulators used by them in QCA6390. Patches 7/18-9/18: These contain changes to device-tree sources for the three platforms we work with in this series. As the WCN7850 module doesn't require any specific timings introducing dependencies between the Bluetooth and WLAN modules, while the QCA6390 does, we take two different approaches to how me model them in DT. For WCN7850 we hide the existence of the PMU as modeling it is simply not necessary. The BT and WLAN devices on the device-tree are represented as consuming the inputs (relevant to the functionality of each) of the PMU directly. For QCA6390 on RB5 we add the PMU node as a platform device. It consumes regulators and GPIOs from the host and exposed regulators consumer in turn by the BT and WLAN modules. This represents the internal structure of the package. Patches 10/18-14/18: These contain the bulk of the PCI changes for this series. We introduce a simple framework for powering up PCI devices before detecting them on the bus and the first user of this library in the form of the WCN7850 PCI power control driver. The general approach is as follows: PCI devices that need special treatment before they can be powered up, scanned and bound to their PCI drivers must be described on the device-tree as child nodes of the PCI port node. These devices will be instantiated on the platform bus. They will in fact be generic platform devices with the compatible of the form used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl drivers. These drivers are platform drivers that will now be matched against the devices instantiated from port children just like any other platform pairs. Both the power control platform device *AND* the associated PCI device reuse the same OF node and have access to the same properties. The goal of the platform driver is to request and bring up any required resources and let the pwrctl framework know that it's now OK to rescan the bus and detect the devices. When the device is bound, we are notified about it by the PCI bus notifier event and can establish a device link between the power control device and the PCI device so that any future extension for power-management will already be able to work with the correct hierachy. The reusing of the OF node is the reason for the small changes to the PCI OF core: as the bootloader can possibly leave the relevant regulators on before booting linux, the PCI device can be detected before its platform abstraction is probed. In this case, we find that device first and mark its OF node as reused. The pwrctl framework handles the opposite case (when the PCI device is detected only after the platform driver successfully enabled it). Patches 15/18-16/18: These add a relatively simple power sequencing subsystem and the first driver using it: the pwrseq module for the QCA6390 PMU. For the record: Bjorn suggested a different solution: a regulator driver that would - based on which regulators are enabled by a consumer - enable relevant resources (drive the enable GPIOs) while respecting the HW-specific delays. This would however require significant and yet unprecised changed to the regulator subsystem as well as be an abuse of the regulator provider API akin to using the reset framework for power sequencing as proposed before. Instead I'm proposing to add a subsystem that allows different devices to use a shared power sequence split into consumer-specific as well as common "units". A power sequence provider driver registers a set of units with pwrseq core. Each unit can be enabled and disabled and contains an optional list of other units which must be enabled before it itself can be. A unit represents a discreet chunk of the power sequence. It also registers a list of targets: a target is an abstraction wrapping a unit which allows consumers to tell pwrseq which unit they want to reach. Real-life example is the driver we're adding here: there's a set of common regulators, two PCIe-specific ones and two enable GPIOs: one for Bluetooth and one for WLAN. The Bluetooth driver requests a descriptor to the power sequencer and names the target it wants to reach: pwrseq = devm_pwrseq_get(dev, "bluetooth"); The pwrseq core then knows that when the driver calls: pwrseq_power_on(pwrseq); It must enable the "bluetooth-enable" unit but it depends on the "regulators-common" unit so this one is enabled first. The provider driver is also in charge of assuring an appropriate delay between enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are handled by the "wlan-enable" unit and so are not enabled until the WLAN driver requests the "wlan" target to be powered on. Another thing worth discussing is the way we associate the consumer with the relevant power sequencer. DT maintainers have expressed a discontent with the existing mmc pwrseq bindings and have NAKed an earlier initiative to introduce global pwrseq bindings to the kernel[1]. In this approach, we model the existing regulators and GPIOs in DT but the pwrseq subsystem requires each provider to provide a .match() callback. Whenever a consumer requests a power sequencer handle, we iterate over the list of pwrseq drivers and call .match() for each. It's up to the driver to verify in a platform-specific way whether it deals with its consumer and let the core pwrseq code know. The advantage of this over reusing the regulator or reset subsystem is that it's more generalized and can handle resources of all kinds as well as deal with any kind of power-on sequences: for instance, Qualcomm has a PCI switch they want a driver for but this switch requires enabling some resources first (PCI pwrctl) and then configuring the device over I2C (which can be handled by the pwrseq provider). Patch 17/18: This patch makes the Qualcomm Bluetooth driver get and use the power sequencer for QCA6390. Patch 18/18: While tiny, this patch is possibly the highlight of the entire series. It uses the two abstraction layers we introduced before to create an elegant power sequencing PCI power control driver and supports the ath11k module on QCA6390. With this series we can now enable BT and WLAN on several new Qualcomm boards upstream. I tested the series on RB5 while Neil tested it on sm8650-qrd and sm8550-qrd. Best Regards, Bartosz Golaszewski It's hard to list the changes between versions here as the scope changed significantly between each iteration and some versions were not even full series but rather RFCs for parts of the solution. For this reason, I'll only start listing changes starting from v6. [*] This is what the docs say. In practice it seems that this delay can be ignored. However the subsequent model - QCA6490 - *does* require users to respect it, so the problem remains valid. [1] https://lore.kernel.org/netdev/20210829131305.534417-1-dmitry.baryshkov@linaro.org/ Bartosz Golaszewski (15): arm64: defconfig: enable ath12k as a module dt-bindings: regulator: describe the PMU module of the QCA6390 package dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390 dt-bindings: new: wireless: qcom,ath11k: describe the ath11k on QCA6390 dt-bindings: new: wireless: describe the ath12k PCI module arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 PCI: hold the rescan mutex when scanning for the first time PCI/pwrctl: reuse the OF node for power controlled devices PCI/pwrctl: create platform devices for child OF nodes of the port node PCI/pwrctl: add PCI power control core code PCI/pwrctl: add a power control driver for WCN7850 power: sequencing: implement the pwrseq core power: pwrseq: add a driver for the QCA6390 PMU module Bluetooth: qca: use the power sequencer for QCA6390 PCI/pwrctl: add a PCI power control driver for power sequenced devices Jonathan Cameron (1): of: Add cleanup.h based auto release via __free(device_node) markings. Neil Armstrong (2): arm64: dts: qcom: sm8550-qrd: add the Wifi node arm64: dts: qcom: sm8650-qrd: add the Wifi node .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + .../net/wireless/qcom,ath11k-pci.yaml | 28 + .../net/wireless/qcom,ath12k-pci.yaml | 103 ++ .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 +++ MAINTAINERS | 8 + arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 123 +- arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 37 + arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 29 + arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + arch/arm64/configs/defconfig | 1 + drivers/bluetooth/hci_qca.c | 31 + drivers/pci/Kconfig | 1 + drivers/pci/Makefile | 1 + drivers/pci/bus.c | 9 +- drivers/pci/of.c | 14 +- drivers/pci/probe.c | 2 + drivers/pci/pwrctl/Kconfig | 25 + drivers/pci/pwrctl/Makefile | 7 + drivers/pci/pwrctl/core.c | 136 +++ drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 84 ++ drivers/pci/pwrctl/pci-pwrctl-wcn7850.c | 202 ++++ drivers/pci/remove.c | 2 + drivers/power/Kconfig | 1 + drivers/power/Makefile | 1 + drivers/power/sequencing/Kconfig | 28 + drivers/power/sequencing/Makefile | 6 + drivers/power/sequencing/core.c | 1065 +++++++++++++++++ drivers/power/sequencing/pwrseq-qca6390.c | 353 ++++++ include/linux/of.h | 2 + include/linux/pci-pwrctl.h | 51 + include/linux/pwrseq/consumer.h | 56 + include/linux/pwrseq/provider.h | 75 ++ 34 files changed, 2678 insertions(+), 16 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-pci.yaml create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml create mode 100644 drivers/pci/pwrctl/Kconfig create mode 100644 drivers/pci/pwrctl/Makefile create mode 100644 drivers/pci/pwrctl/core.c create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c create mode 100644 drivers/pci/pwrctl/pci-pwrctl-wcn7850.c create mode 100644 drivers/power/sequencing/Kconfig create mode 100644 drivers/power/sequencing/Makefile create mode 100644 drivers/power/sequencing/core.c create mode 100644 drivers/power/sequencing/pwrseq-qca6390.c create mode 100644 include/linux/pci-pwrctl.h create mode 100644 include/linux/pwrseq/consumer.h create mode 100644 include/linux/pwrseq/provider.h