2024-05-28 19:05:05

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

Note: I am resending this series in its entirety once more for
discussions and reviews. If there won't be any major objections, I'll
then start sending individual bits and pieces to appropriate trees.

Merging strategy: The DT binding and DTS changes are a no-brainer, they
can go through the wireless, regulator and arm-msm trees separately. The
bluetooth and PCI changes have a build-time dependency on the power
sequencing code. The bluetooth changes also have a run-time dependency on
the PCI pwrctl part. In order to get it into next I plan to pick up the
power sequencing code into my own tree and maintain it. I can then
provide an immutable tag for the BT and PCI trees to pull. I wouldn't
stress about the BT runtime dependency as it will be fixed once all
changes are in next.

The actual cover letter follows:

--

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.

A similar design with a discreet PMU is also employed in other models of
the WCN family of chips although we can often do without the delays. With
this series we add support for the WCN7850 as well.

===

We introduce a new subsystem here - the power sequencing framework. The
qcom-wcn driver that we add is its first user. It implements the power-up
sequences for QCA6390 and WCN7850 chips. However - we only use it to
power-up the bluetooth module in the former. We use it to driver the WLAN
modules in both. The reason for this is that for WCN7850 we have
comprehensive bindings already upstream together with existing DT users.
Porting them to using the pwrseq subsystem can be done separately and in
an incremental manner once the subsystem itself is upstream. We will also
have to ensure backward DT compatibility. To avoid overcomplicating this
series, let's leave it out for now.

===

This series is logically split into several sections. I'll go
patch-by-patch and explain each step.

Patches 1/16-5/16:

These contain all relevant DT bindings changes. We add new documents for
the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
in QCA6390.

Patches 6/16-8/16:

These contain changes to device-tree sources for the three platforms we
work with in this series. We model the PMUs of the WLAN/BT chips as
top-level platform devices on the device tree. In order to limit the scope
of this series and not introduce an excessive amount of confusion with
deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
as is (meaning: they continue to consumer the GPIOs and power inputs from
the host). As the WCN7850 module doesn't require any specific timings, we can
incrementally change that later.

In both cases we add WLAN nodes that consume the power outputs of the PMU.
For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
of the PMU - we can do it as the bindings for this chip did not define any
supply handles prior to this series meaning we are able to get this correct
right away.

Patches 9/16-12/16:

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.

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).

Patch 13/16 - 14/16:

These add a relatively simple power sequencing subsystem and the first
driver using it: the pwrseq module for the PMUs on the WCN family of chips.

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 15:

This patch makes the Qualcomm Bluetooth driver get and use the power
sequencer for QCA6390.

Patch 16:

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 and ath12k on WCN7850.

With this series we can now enable BT and WLAN on several new Qualcomm
boards upstream.

Tested on RB5, sm8650-qrd, sm8650-hdk and sm8550-qrd.

Changelog:

Since v7:
- added DTS changes for sm8650-hdk
- added circular dependency detection for pwrseq units
- fixed a KASAN reported use-after-free error in remove path
- improve Kconfig descriptions
- fix typos in bindings and Kconfig
- fixed issues reported by smatch
- fix the unbind path in PCI pwrctl
- lots of minor improvements to the pwrseq core

Since v6:
- kernel doc fixes
- drop myself from the DT bindings maintainers list for ath12k
- wait until the PCI bridge device is fully added before creating the
PCI pwrctl platform devices for its sub-nodes, otherwise we may see
sysfs and procfs attribute failures (due to duplication, we're
basically trying to probe the same device twice at the same time)
- I kept the regulators for QCA6390's ath11k as required as they only
apply to this specific Qualcomm package

Since v5:
- unify the approach to modelling the WCN WLAN/BT chips by always exposing
the PMU node on the device tree and making the WLAN and BT nodes become
consumers of its power outputs; this includes a major rework of the DT
sources, bindings and driver code; there's no more a separate PCI
pwrctl driver for WCN7850, instead its power-up sequence was moved
into the pwrseq driver common for all WCN chips
- don't set load_uA from new regulator consumers
- fix reported kerneldoc issues
- drop voltage ranges for PMU outputs from DT
- many minor tweaks and reworks

v1: Original RFC:

https://lore.kernel.org/lkml/[email protected]/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/[email protected]/T/

v5: Two different ways of handling QCA6390 and WCN7850:

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

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
Bartosz Golaszewski (16):
regulator: dt-bindings: describe the PMU module of the QCA6390 package
regulator: dt-bindings: describe the PMU module of the WCN7850 package
dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
dt-bindings: net: wireless: describe the ath12k PCI module
arm64: dts: qcom: sm8550-qrd: add the Wifi node
arm64: dts: qcom: sm8650-qrd: add the Wifi node
arm64: dts: qcom: qrb5165-rb5: add the Wifi node
power: sequencing: implement the pwrseq core
power: pwrseq: add a driver for the PMU module on the QCom WCN chipsets
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 PCI power control driver for power sequenced devices
Bluetooth: qca: use the power sequencer for QCA6390

Neil Armstrong (1):
arm64: dts: qcom: sm8650-hdk: add the Wifi node

.../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 17 +
.../bindings/net/wireless/qcom,ath11k-pci.yaml | 46 +
.../bindings/net/wireless/qcom,ath12k.yaml | 99 ++
.../bindings/regulator/qcom,qca6390-pmu.yaml | 185 ++++
MAINTAINERS | 8 +
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +-
arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++
arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 89 ++
arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++
arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
drivers/bluetooth/hci_qca.c | 74 +-
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 | 17 +
drivers/pci/pwrctl/Makefile | 6 +
drivers/pci/pwrctl/core.c | 137 +++
drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++
drivers/pci/remove.c | 3 +-
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/sequencing/Kconfig | 28 +
drivers/power/sequencing/Makefile | 6 +
drivers/power/sequencing/core.c | 1105 ++++++++++++++++++++
drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++
include/linux/pci-pwrctl.h | 51 +
include/linux/pwrseq/consumer.h | 56 +
include/linux/pwrseq/provider.h | 75 ++
32 files changed, 2717 insertions(+), 34 deletions(-)
---
base-commit: 6dc544b66971c7f9909ff038b62149105272d26a
change-id: 20240527-pwrseq-76fc025248a2

Best regards,
--
Bartosz Golaszewski <[email protected]>



2024-05-28 19:05:33

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 02/17] regulator: dt-bindings: describe the PMU module of the WCN7850 package

From: Bartosz Golaszewski <[email protected]>

The WCN7850 package contains discreet modules for WLAN and Bluetooth. They
are powered by the Power Management Unit (PMU) that takes inputs from the
host and provides LDO outputs. Extend the bindings for QCA6390 to also
document this model.

Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Mark Brown <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/regulator/qcom,qca6390-pmu.yaml | 36 +++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
index 9d39ff9a75fd..2e543661a1e2 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
+++ b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
@@ -16,20 +16,37 @@ description:

properties:
compatible:
- const: qcom,qca6390-pmu
+ enum:
+ - qcom,qca6390-pmu
+ - qcom,wcn7850-pmu
+
+ vdd-supply:
+ description: VDD supply regulator handle

vddaon-supply:
description: VDD_AON supply regulator handle

+ vdddig-supply:
+ description: VDD_DIG supply regulator handle
+
vddpmu-supply:
description: VDD_PMU supply regulator handle

+ vddio1p2-supply:
+ description: VDD_IO_1P2 supply regulator handle
+
vddrfa0p95-supply:
description: VDD_RFA_0P95 supply regulator handle

+ vddrfa1p2-supply:
+ description: VDD_RFA_1P2 supply regulator handle
+
vddrfa1p3-supply:
description: VDD_RFA_1P3 supply regulator handle

+ vddrfa1p8-supply:
+ description: VDD_RFA_1P8 supply regulator handle
+
vddrfa1p9-supply:
description: VDD_RFA_1P9 supply regulator handle

@@ -50,6 +67,10 @@ properties:
maxItems: 1
description: GPIO line enabling the ATH11K Bluetooth module supplied by the PMU

+ clocks:
+ maxItems: 1
+ description: Reference clock handle
+
regulators:
type: object
description:
@@ -83,6 +104,19 @@ allOf:
- vddpcie1p3-supply
- vddpcie1p9-supply
- vddio-supply
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,wcn7850-pmu
+ then:
+ required:
+ - vdd-supply
+ - vddio-supply
+ - vddaon-supply
+ - vdddig-supply
+ - vddrfa1p2-supply
+ - vddrfa1p8-supply

additionalProperties: false


--
2.43.0


2024-05-28 19:06:11

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 03/17] dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390

From: Bartosz Golaszewski <[email protected]>

QCA6390 has a compatible listed in the bindings but is missing the
regulators description. Add the missing supply property and list the
required ones in the allOf section.

Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index 055a3351880b..48ac9f10ef05 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -62,6 +62,9 @@ properties:
vdddig-supply:
description: VDD_DIG supply regulator handle

+ vddbtcmx-supply:
+ description: VDD_BT_CMX supply regulator handle
+
vddbtcxmx-supply:
description: VDD_BT_CXMX supply regulator handle

@@ -184,6 +187,20 @@ allOf:
- vddrfa0p8-supply
- vddrfa1p2-supply
- vddrfa1p9-supply
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,qca6390-bt
+ then:
+ required:
+ - vddrfacmn-supply
+ - vddaon-supply
+ - vddbtcmx-supply
+ - vddrfa0p8-supply
+ - vddrfa1p2-supply
+ - vddrfa1p7-supply

examples:
- |

--
2.43.0


2024-05-28 19:06:43

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 04/17] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390

From: Bartosz Golaszewski <[email protected]>

Add a PCI compatible for the ATH11K module on QCA6390 and describe the
power inputs from the PMU that it consumes.

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
.../bindings/net/wireless/qcom,ath11k-pci.yaml | 46 ++++++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
index 41d023797d7d..8675d7d0215c 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k-pci.yaml
@@ -17,6 +17,7 @@ description: |
properties:
compatible:
enum:
+ - pci17cb,1101 # QCA6390
- pci17cb,1103 # WCN6855

reg:
@@ -28,10 +29,55 @@ properties:
string to uniquely identify variant of the calibration data for designs
with colliding bus and device ids

+ vddrfacmn-supply:
+ description: VDD_RFA_CMN supply regulator handle
+
+ vddaon-supply:
+ description: VDD_AON supply regulator handle
+
+ vddwlcx-supply:
+ description: VDD_WL_CX supply regulator handle
+
+ vddwlmx-supply:
+ description: VDD_WL_MX supply regulator handle
+
+ vddrfa0p8-supply:
+ description: VDD_RFA_0P8 supply regulator handle
+
+ vddrfa1p2-supply:
+ description: VDD_RFA_1P2 supply regulator handle
+
+ vddrfa1p7-supply:
+ description: VDD_RFA_1P7 supply regulator handle
+
+ vddpcie0p9-supply:
+ description: VDD_PCIE_0P9 supply regulator handle
+
+ vddpcie1p8-supply:
+ description: VDD_PCIE_1P8 supply regulator handle
+
required:
- compatible
- reg

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: pci17cb,1101
+ then:
+ required:
+ - vddrfacmn-supply
+ - vddaon-supply
+ - vddwlcx-supply
+ - vddwlmx-supply
+ - vddrfa0p8-supply
+ - vddrfa1p2-supply
+ - vddrfa1p7-supply
+ - vddpcie0p9-supply
+ - vddpcie1p8-supply
+
additionalProperties: false

examples:

--
2.43.0


2024-05-28 19:07:16

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 05/17] dt-bindings: net: wireless: describe the ath12k PCI module

From: Bartosz Golaszewski <[email protected]>

Add device-tree bindings for the ATH12K module found in the WCN7850
package.

Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/net/wireless/qcom,ath12k.yaml | 99 ++++++++++++++++++++++
1 file changed, 99 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
new file mode 100644
index 000000000000..1b5884015b15
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2024 Linaro Limited
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies ath12k wireless devices (PCIe)
+
+maintainers:
+ - Jeff Johnson <[email protected]>
+ - Kalle Valo <[email protected]>
+
+description:
+ Qualcomm Technologies IEEE 802.11be PCIe devices.
+
+properties:
+ compatible:
+ enum:
+ - pci17cb,1107 # WCN7850
+
+ reg:
+ maxItems: 1
+
+ vddaon-supply:
+ description: VDD_AON supply regulator handle
+
+ vddwlcx-supply:
+ description: VDD_WLCX supply regulator handle
+
+ vddwlmx-supply:
+ description: VDD_WLMX supply regulator handle
+
+ vddrfacmn-supply:
+ description: VDD_RFA_CMN supply regulator handle
+
+ vddrfa0p8-supply:
+ description: VDD_RFA_0P8 supply regulator handle
+
+ vddrfa1p2-supply:
+ description: VDD_RFA_1P2 supply regulator handle
+
+ vddrfa1p8-supply:
+ description: VDD_RFA_1P8 supply regulator handle
+
+ vddpcie0p9-supply:
+ description: VDD_PCIE_0P9 supply regulator handle
+
+ vddpcie1p8-supply:
+ description: VDD_PCIE_1P8 supply regulator handle
+
+required:
+ - compatible
+ - reg
+ - vddaon-supply
+ - vddwlcx-supply
+ - vddwlmx-supply
+ - vddrfacmn-supply
+ - vddrfa0p8-supply
+ - vddrfa1p2-supply
+ - vddrfa1p8-supply
+ - vddpcie0p9-supply
+ - vddpcie1p8-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,rpmh.h>
+ #include <dt-bindings/gpio/gpio.h>
+ pcie {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ pcie@0 {
+ device_type = "pci";
+ reg = <0x0 0x0 0x0 0x0 0x0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ bus-range = <0x01 0xff>;
+
+ wifi@0 {
+ compatible = "pci17cb,1107";
+ reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+ vddaon-supply = <&vreg_pmu_aon_0p59>;
+ vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+ vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+ vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+ vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+ vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+ vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>;
+ vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
+ vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
+ };
+ };
+ };

--
2.43.0


2024-05-28 19:07:39

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 06/17] arm64: dts: qcom: sm8550-qrd: add the Wifi node

From: Bartosz Golaszewski <[email protected]>

Describe the ath12k WLAN on-board the WCN7850 module present on the
board.

[Neil: authored the initial version of the change]
Co-developed-by: Neil Armstrong <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 +++++++++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 2ed1715000c9..7a15d472bd95 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -214,6 +214,68 @@ vph_pwr: vph-pwr-regulator {
regulator-always-on;
regulator-boot-on;
};
+
+ wcn7850-pmu {
+ compatible = "qcom,wcn7850-pmu";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_en>, <&pmk8550_sleep_clk>;
+
+ wlan-enable-gpios = <&tlmm 80 GPIO_ACTIVE_HIGH>;
+ /*
+ * TODO Add bt-enable-gpios once the Bluetooth driver is
+ * converted to using the power sequencer.
+ */
+
+ vdd-supply = <&vreg_s5g_0p85>;
+ vddio-supply = <&vreg_l15b_1p8>;
+ vddaon-supply = <&vreg_s2g_0p85>;
+ vdddig-supply = <&vreg_s4e_0p95>;
+ vddrfa1p2-supply = <&vreg_s4g_1p25>;
+ vddrfa1p8-supply = <&vreg_s6g_1p86>;
+
+ regulators {
+ vreg_pmu_rfa_cmn: ldo0 {
+ regulator-name = "vreg_pmu_rfa_cmn";
+ };
+
+ vreg_pmu_aon_0p59: ldo1 {
+ regulator-name = "vreg_pmu_aon_0p59";
+ };
+
+ vreg_pmu_wlcx_0p8: ldo2 {
+ regulator-name = "vreg_pmu_wlcx_0p8";
+ };
+
+ vreg_pmu_wlmx_0p85: ldo3 {
+ regulator-name = "vreg_pmu_wlmx_0p85";
+ };
+
+ vreg_pmu_btcmx_0p85: ldo4 {
+ regulator-name = "vreg_pmu_btcmx_0p85";
+ };
+
+ vreg_pmu_rfa_0p8: ldo5 {
+ regulator-name = "vreg_pmu_rfa_0p8";
+ };
+
+ vreg_pmu_rfa_1p2: ldo6 {
+ regulator-name = "vreg_pmu_rfa_1p2";
+ };
+
+ vreg_pmu_rfa_1p8: ldo7 {
+ regulator-name = "vreg_pmu_rfa_1p8";
+ };
+
+ vreg_pmu_pcie_0p9: ldo8 {
+ regulator-name = "vreg_pmu_pcie_0p9";
+ };
+
+ vreg_pmu_pcie_1p8: ldo9 {
+ regulator-name = "vreg_pmu_pcie_1p8";
+ };
+ };
+ };
};

&apps_rsc {
@@ -808,6 +870,23 @@ &pcie0 {
status = "okay";
};

+&pcieport0 {
+ wifi@0 {
+ compatible = "pci17cb,1107";
+ reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+ vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+ vddaon-supply = <&vreg_pmu_aon_0p59>;
+ vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+ vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+ vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+ vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+ vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>;
+ vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
+ vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
+ };
+};
+
&pcie0_phy {
vdda-phy-supply = <&vreg_l1e_0p88>;
vdda-pll-supply = <&vreg_l3e_1p2>;
@@ -891,6 +970,17 @@ &pon_resin {
status = "okay";
};

+&pmk8550_gpios {
+ pmk8550_sleep_clk: sleep-clk-state {
+ pins = "gpio3";
+ function = "func1";
+ input-disable;
+ output-enable;
+ bias-disable;
+ power-source = <0>;
+ };
+};
+
&qupv3_id_0 {
status = "okay";
};
@@ -1064,6 +1154,13 @@ wcd_default: wcd-reset-n-active-state {
bias-disable;
output-low;
};
+
+ wlan_en: wlan-en-state {
+ pins = "gpio80";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-down;
+ };
};

&uart7 {
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 79311a6bd1ad..0e616fdee798 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -1769,7 +1769,7 @@ pcie0: pcie@1c00000 {

status = "disabled";

- pcie@0 {
+ pcieport0: pcie@0 {
device_type = "pci";
reg = <0x0 0x0 0x0 0x0 0x0>;
bus-range = <0x01 0xff>;

--
2.43.0


2024-05-28 19:07:45

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 07/17] arm64: dts: qcom: sm8650-qrd: add the Wifi node

From: Bartosz Golaszewski <[email protected]>

Describe the ath12k WLAN on-board the WCN7850 module present on the
board.

[Neil: authored the initial version of the change]
Co-developed-by: Neil Armstrong <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 +++++++++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
index 98f6a272ce5a..6e3c4d8dcc19 100644
--- a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
@@ -203,6 +203,71 @@ wcd_codec_headset_in: endpoint {
};
};
};
+
+ wcn7850-pmu {
+ compatible = "qcom,wcn7850-pmu";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_en>;
+
+ wlan-enable-gpios = <&tlmm 16 GPIO_ACTIVE_HIGH>;
+ /*
+ * TODO Add bt-enable-gpios once the Bluetooth driver is
+ * converted to using the power sequencer.
+ */
+
+ vdd-supply = <&vreg_s4i_0p85>;
+ vddio-supply = <&vreg_l15b_1p8>;
+ vddio1p2-supply = <&vreg_l3c_1p2>;
+ vddaon-supply = <&vreg_s2c_0p8>;
+ vdddig-supply = <&vreg_s3c_0p9>;
+ vddrfa1p2-supply = <&vreg_s1c_1p2>;
+ vddrfa1p8-supply = <&vreg_s6c_1p8>;
+
+ clocks = <&rpmhcc RPMH_RF_CLK1>;
+
+ regulators {
+ vreg_pmu_rfa_cmn: ldo0 {
+ regulator-name = "vreg_pmu_rfa_cmn";
+ };
+
+ vreg_pmu_aon_0p59: ldo1 {
+ regulator-name = "vreg_pmu_aon_0p59";
+ };
+
+ vreg_pmu_wlcx_0p8: ldo2 {
+ regulator-name = "vreg_pmu_wlcx_0p8";
+ };
+
+ vreg_pmu_wlmx_0p85: ldo3 {
+ regulator-name = "vreg_pmu_wlmx_0p85";
+ };
+
+ vreg_pmu_btcmx_0p85: ldo4 {
+ regulator-name = "vreg_pmu_btcmx_0p85";
+ };
+
+ vreg_pmu_rfa_0p8: ldo5 {
+ regulator-name = "vreg_pmu_rfa_0p8";
+ };
+
+ vreg_pmu_rfa_1p2: ldo6 {
+ regulator-name = "vreg_pmu_rfa_1p2";
+ };
+
+ vreg_pmu_rfa_1p8: ldo7 {
+ regulator-name = "vreg_pmu_rfa_1p8";
+ };
+
+ vreg_pmu_pcie_0p9: ldo8 {
+ regulator-name = "vreg_pmu_pcie_0p9";
+ };
+
+ vreg_pmu_pcie_1p8: ldo9 {
+ regulator-name = "vreg_pmu_pcie_1p8";
+ };
+ };
+ };
};

&apps_rsc {
@@ -844,6 +909,23 @@ &pcie0 {
status = "okay";
};

+&pcieport0 {
+ wifi@0 {
+ compatible = "pci17cb,1107";
+ reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+ vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+ vddaon-supply = <&vreg_pmu_aon_0p59>;
+ vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+ vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+ vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+ vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+ vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>;
+ vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
+ vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
+ };
+};
+
&pcie0_phy {
vdda-phy-supply = <&vreg_l1i_0p88>;
vdda-pll-supply = <&vreg_l3i_1p2>;
@@ -1138,6 +1220,13 @@ wcd_default: wcd-reset-n-active-state {
bias-disable;
output-low;
};
+
+ wlan_en: wlan-en-state {
+ pins = "gpio16";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-down;
+ };
};

&uart14 {
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index d7c432552233..0fb971169f38 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2289,7 +2289,7 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,

status = "disabled";

- pcie@0 {
+ pcieport0: pcie@0 {
device_type = "pci";
reg = <0x0 0x0 0x0 0x0 0x0>;
bus-range = <0x01 0xff>;

--
2.43.0


2024-05-28 19:10:01

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 11/17] power: pwrseq: add a driver for the PMU module on the QCom WCN chipsets

From: Bartosz Golaszewski <[email protected]>

This adds the power sequencing driver for the PMU modules present on the
Qualcomm WCN Bluetooth and Wifi chipsets. It uses the pwrseq subsystem
and knows how to match the sequencer to the consumer device by verifying
the relevant properties and DT layout.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/power/sequencing/Kconfig | 16 ++
drivers/power/sequencing/Makefile | 2 +
drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 +++++++++++++++++++++++++++++
3 files changed, 354 insertions(+)

diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
index ba5732b1dbf8..f969374792f5 100644
--- a/drivers/power/sequencing/Kconfig
+++ b/drivers/power/sequencing/Kconfig
@@ -10,3 +10,19 @@ menuconfig POWER_SEQUENCING
during power-up.

If unsure, say no.
+
+if POWER_SEQUENCING
+
+config POWER_SEQUENCING_QCOM_WCN
+ tristate "Qualcomm WCN family PMU driver"
+ default m if ARCH_QCOM
+ help
+ Say Y here to enable the power sequencing driver for Qualcomm
+ WCN Bluetooth/WLAN chipsets.
+
+ Typically, a package from the Qualcomm WCN family contains the BT
+ and WLAN modules whose power is controlled by the PMU module. As the
+ former two share the power-up sequence which is executed by the PMU,
+ this driver is needed for correct power control.
+
+endif
diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
index dcdf8c0c159e..2eec2df7912d 100644
--- a/drivers/power/sequencing/Makefile
+++ b/drivers/power/sequencing/Makefile
@@ -2,3 +2,5 @@

obj-$(CONFIG_POWER_SEQUENCING) += pwrseq-core.o
pwrseq-core-y := core.o
+
+obj-$(CONFIG_POWER_SEQUENCING_QCOM_WCN) += pwrseq-qcom-wcn.o
diff --git a/drivers/power/sequencing/pwrseq-qcom-wcn.c b/drivers/power/sequencing/pwrseq-qcom-wcn.c
new file mode 100644
index 000000000000..7e6391e1b33f
--- /dev/null
+++ b/drivers/power/sequencing/pwrseq-qcom-wcn.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/jiffies.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pwrseq/provider.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+struct pwrseq_qcom_wcn_pdata {
+ const char *const *vregs;
+ size_t num_vregs;
+ unsigned int pwup_delay_msec;
+ unsigned int gpio_enable_delay;
+};
+
+struct pwrseq_qcom_wcn_ctx {
+ struct pwrseq_device *pwrseq;
+ struct device_node *of_node;
+ const struct pwrseq_qcom_wcn_pdata *pdata;
+ struct regulator_bulk_data *regs;
+ struct gpio_desc *bt_gpio;
+ struct gpio_desc *wlan_gpio;
+ struct clk *clk;
+ unsigned long last_gpio_enable;
+};
+
+static void pwrseq_qcom_wcn_ensure_gpio_delay(struct pwrseq_qcom_wcn_ctx *ctx)
+{
+ unsigned long diff_jiffies;
+ unsigned int diff_msecs;
+
+ if (!ctx->pdata->gpio_enable_delay)
+ return;
+
+ diff_jiffies = jiffies - ctx->last_gpio_enable;
+ diff_msecs = jiffies_to_msecs(diff_jiffies);
+
+ if (diff_msecs < ctx->pdata->gpio_enable_delay)
+ msleep(ctx->pdata->gpio_enable_delay - diff_msecs);
+}
+
+static int pwrseq_qcom_wcn_vregs_enable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ return regulator_bulk_enable(ctx->pdata->num_vregs, ctx->regs);
+}
+
+static int pwrseq_qcom_wcn_vregs_disable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ return regulator_bulk_disable(ctx->pdata->num_vregs, ctx->regs);
+}
+
+static const struct pwrseq_unit_data pwrseq_qcom_wcn_vregs_unit_data = {
+ .name = "regulators-enable",
+ .enable = pwrseq_qcom_wcn_vregs_enable,
+ .disable = pwrseq_qcom_wcn_vregs_disable,
+};
+
+static int pwrseq_qcom_wcn_clk_enable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ return clk_prepare_enable(ctx->clk);
+}
+
+static int pwrseq_qcom_wcn_clk_disable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ clk_disable_unprepare(ctx->clk);
+
+ return 0;
+}
+
+static const struct pwrseq_unit_data pwrseq_qcom_wcn_clk_unit_data = {
+ .name = "clock-enable",
+ .enable = pwrseq_qcom_wcn_clk_enable,
+ .disable = pwrseq_qcom_wcn_clk_disable,
+};
+
+static const struct pwrseq_unit_data *pwrseq_qcom_wcn_unit_deps[] = {
+ &pwrseq_qcom_wcn_vregs_unit_data,
+ &pwrseq_qcom_wcn_clk_unit_data,
+ NULL
+};
+
+static int pwrseq_qcom_wcn_bt_enable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ pwrseq_qcom_wcn_ensure_gpio_delay(ctx);
+ gpiod_set_value_cansleep(ctx->bt_gpio, 1);
+ ctx->last_gpio_enable = jiffies;
+
+ return 0;
+}
+
+static int pwrseq_qcom_wcn_bt_disable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ gpiod_set_value_cansleep(ctx->bt_gpio, 0);
+
+ return 0;
+}
+
+static const struct pwrseq_unit_data pwrseq_qcom_wcn_bt_unit_data = {
+ .name = "bluetooth-enable",
+ .deps = pwrseq_qcom_wcn_unit_deps,
+ .enable = pwrseq_qcom_wcn_bt_enable,
+ .disable = pwrseq_qcom_wcn_bt_disable,
+};
+
+static int pwrseq_qcom_wcn_wlan_enable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ pwrseq_qcom_wcn_ensure_gpio_delay(ctx);
+ gpiod_set_value_cansleep(ctx->wlan_gpio, 1);
+ ctx->last_gpio_enable = jiffies;
+
+ return 0;
+}
+
+static int pwrseq_qcom_wcn_wlan_disable(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ gpiod_set_value_cansleep(ctx->wlan_gpio, 0);
+
+ return 0;
+}
+
+static const struct pwrseq_unit_data pwrseq_qcom_wcn_wlan_unit_data = {
+ .name = "wlan-enable",
+ .deps = pwrseq_qcom_wcn_unit_deps,
+ .enable = pwrseq_qcom_wcn_wlan_enable,
+ .disable = pwrseq_qcom_wcn_wlan_disable,
+};
+
+static int pwrseq_qcom_wcn_pwup_delay(struct pwrseq_device *pwrseq)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+ if (ctx->pdata->pwup_delay_msec)
+ msleep(ctx->pdata->pwup_delay_msec);
+
+ return 0;
+}
+
+static const struct pwrseq_target_data pwrseq_qcom_wcn_bt_target_data = {
+ .name = "bluetooth",
+ .unit = &pwrseq_qcom_wcn_bt_unit_data,
+ .post_enable = pwrseq_qcom_wcn_pwup_delay,
+};
+
+static const struct pwrseq_target_data pwrseq_qcom_wcn_wlan_target_data = {
+ .name = "wlan",
+ .unit = &pwrseq_qcom_wcn_wlan_unit_data,
+ .post_enable = pwrseq_qcom_wcn_pwup_delay,
+};
+
+static const struct pwrseq_target_data *pwrseq_qcom_wcn_targets[] = {
+ &pwrseq_qcom_wcn_bt_target_data,
+ &pwrseq_qcom_wcn_wlan_target_data,
+ NULL
+};
+
+static const char *const pwrseq_qca6390_vregs[] = {
+ "vddio",
+ "vddaon",
+ "vddpmu",
+ "vddrfa0p95",
+ "vddrfa1p3",
+ "vddrfa1p9",
+ "vddpcie1p3",
+ "vddpcie1p9",
+};
+
+static const struct pwrseq_qcom_wcn_pdata pwrseq_qca6390_of_data = {
+ .vregs = pwrseq_qca6390_vregs,
+ .num_vregs = ARRAY_SIZE(pwrseq_qca6390_vregs),
+ .pwup_delay_msec = 60,
+ .gpio_enable_delay = 100,
+};
+
+static const char *const pwrseq_wcn7850_vregs[] = {
+ "vdd",
+ "vddio",
+ "vddio1p2",
+ "vddaon",
+ "vdddig",
+ "vddrfa1p2",
+ "vddrfa1p8",
+};
+
+static const struct pwrseq_qcom_wcn_pdata pwrseq_wcn7850_of_data = {
+ .vregs = pwrseq_wcn7850_vregs,
+ .num_vregs = ARRAY_SIZE(pwrseq_wcn7850_vregs),
+ .pwup_delay_msec = 50,
+};
+
+static int pwrseq_qcom_wcn_match(struct pwrseq_device *pwrseq,
+ struct device *dev)
+{
+ struct pwrseq_qcom_wcn_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+ struct device_node *dev_node = dev->of_node;
+
+ /*
+ * The PMU supplies power to the Bluetooth and WLAN modules. both
+ * consume the PMU AON output so check the presence of the
+ * 'vddaon-supply' property and whether it leads us to the right
+ * device.
+ */
+ if (!of_property_present(dev_node, "vddaon-supply"))
+ return 0;
+
+ struct device_node *reg_node __free(device_node) =
+ of_parse_phandle(dev_node, "vddaon-supply", 0);
+ if (!reg_node)
+ return 0;
+
+ /*
+ * `reg_node` is the PMU AON regulator, its parent is the `regulators`
+ * node and finally its grandparent is the PMU device node that we're
+ * looking for.
+ */
+ if (!reg_node->parent || !reg_node->parent->parent ||
+ reg_node->parent->parent != ctx->of_node)
+ return 0;
+
+ return 1;
+}
+
+static int pwrseq_qcom_wcn_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pwrseq_qcom_wcn_ctx *ctx;
+ struct pwrseq_config config;
+ int i, ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->of_node = dev->of_node;
+
+ ctx->pdata = of_device_get_match_data(dev);
+ if (!ctx->pdata)
+ return dev_err_probe(dev, -ENODEV,
+ "Failed to obtain platform data\n");
+
+ ctx->regs = devm_kcalloc(dev, ctx->pdata->num_vregs,
+ sizeof(*ctx->regs), GFP_KERNEL);
+ if (!ctx->regs)
+ return -ENOMEM;
+
+ for (i = 0; i < ctx->pdata->num_vregs; i++)
+ ctx->regs[i].supply = ctx->pdata->vregs[i];
+
+ ret = devm_regulator_bulk_get(dev, ctx->pdata->num_vregs, ctx->regs);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Failed to get all regulators\n");
+
+ ctx->bt_gpio = devm_gpiod_get_optional(dev, "bt-enable", GPIOD_OUT_LOW);
+ if (IS_ERR(ctx->bt_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->bt_gpio),
+ "Failed to get the Bluetooth enable GPIO\n");
+
+ ctx->wlan_gpio = devm_gpiod_get_optional(dev, "wlan-enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(ctx->wlan_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->wlan_gpio),
+ "Failed to get the WLAN enable GPIO\n");
+
+ ctx->clk = devm_clk_get_optional(dev, NULL);
+ if (IS_ERR(ctx->clk))
+ return dev_err_probe(dev, PTR_ERR(ctx->clk),
+ "Failed to get the reference clock\n");
+
+ memset(&config, 0, sizeof(config));
+
+ config.parent = dev;
+ config.owner = THIS_MODULE;
+ config.drvdata = ctx;
+ config.match = pwrseq_qcom_wcn_match;
+ config.targets = pwrseq_qcom_wcn_targets;
+
+ ctx->pwrseq = devm_pwrseq_device_register(dev, &config);
+ if (IS_ERR(ctx->pwrseq))
+ return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
+ "Failed to register the power sequencer\n");
+
+ return 0;
+}
+
+static const struct of_device_id pwrseq_qcom_wcn_of_match[] = {
+ {
+ .compatible = "qcom,qca6390-pmu",
+ .data = &pwrseq_qca6390_of_data,
+ },
+ {
+ .compatible = "qcom,wcn7850-pmu",
+ .data = &pwrseq_wcn7850_of_data,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pwrseq_qcom_wcn_of_match);
+
+static struct platform_driver pwrseq_qcom_wcn_driver = {
+ .driver = {
+ .name = "pwrseq-qcom_wcn",
+ .of_match_table = pwrseq_qcom_wcn_of_match,
+ },
+ .probe = pwrseq_qcom_wcn_probe,
+};
+module_platform_driver(pwrseq_qcom_wcn_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm WCN PMU power sequencing driver");
+MODULE_LICENSE("GPL");

--
2.43.0


2024-05-28 19:10:05

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 10/17] power: sequencing: implement the pwrseq core

From: Bartosz Golaszewski <[email protected]>

Implement the power sequencing subsystem allowing devices to share
complex powering-up and down procedures. It's split into the consumer
and provider parts but does not implement any new DT bindings so that
the actual power sequencing is never revealed in the DT representation.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
MAINTAINERS | 8 +
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/sequencing/Kconfig | 12 +
drivers/power/sequencing/Makefile | 4 +
drivers/power/sequencing/core.c | 1105 +++++++++++++++++++++++++++++++++++++
include/linux/pwrseq/consumer.h | 56 ++
include/linux/pwrseq/provider.h | 75 +++
8 files changed, 1262 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index dbc5d9ec3d20..dd129735e7c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17893,6 +17893,14 @@ F: include/linux/pm_*
F: include/linux/powercap.h
F: kernel/configs/nopm.config

+POWER SEQUENCING
+M: Bartosz Golaszewski <[email protected]>
+L: [email protected]
+S: Maintained
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
+F: drivers/power/sequencing/
+F: include/linux/pwrseq/
+
POWER STATE COORDINATION INTERFACE (PSCI)
M: Mark Rutland <[email protected]>
M: Lorenzo Pieralisi <[email protected]>
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 696bf77a7042..9a8e44ca9ae4 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
source "drivers/power/reset/Kconfig"
+source "drivers/power/sequencing/Kconfig"
source "drivers/power/supply/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index effbf0377f32..962a2cd30a51 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_POWER_RESET) += reset/
+obj-$(CONFIG_POWER_SEQUENCING) += sequencing/
obj-$(CONFIG_POWER_SUPPLY) += supply/
diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
new file mode 100644
index 000000000000..ba5732b1dbf8
--- /dev/null
+++ b/drivers/power/sequencing/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menuconfig POWER_SEQUENCING
+ tristate "Power Sequencing support"
+ help
+ Say Y here to enable the Power Sequencing subsystem.
+
+ This subsystem is designed to control power to devices that share
+ complex resources and/or require specific power sequences to be run
+ during power-up.
+
+ If unsure, say no.
diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
new file mode 100644
index 000000000000..dcdf8c0c159e
--- /dev/null
+++ b/drivers/power/sequencing/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_POWER_SEQUENCING) += pwrseq-core.o
+pwrseq-core-y := core.o
diff --git a/drivers/power/sequencing/core.c b/drivers/power/sequencing/core.c
new file mode 100644
index 000000000000..e07037ea5be0
--- /dev/null
+++ b/drivers/power/sequencing/core.c
@@ -0,0 +1,1105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#include <linux/bug.h>
+#include <linux/cleanup.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/lockdep.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/pwrseq/consumer.h>
+#include <linux/pwrseq/provider.h>
+#include <linux/radix-tree.h>
+#include <linux/rwsem.h>
+#include <linux/slab.h>
+
+/*
+ * Power-sequencing framework for linux.
+ *
+ * This subsystem allows power sequence providers to register a set of targets
+ * that consumers may request and power-up/down.
+ *
+ * Glossary:
+ *
+ * Unit - a unit is a discreet chunk of a power sequence. For instance one unit
+ * may enable a set of regulators, another may enable a specific GPIO. Units
+ * can define dependencies in the form of other units that must be enabled
+ * before it itself can be.
+ *
+ * Target - a target is a set of units (composed of the "final" unit and its
+ * dependencies) that a consumer selects by its name when requesting a handle
+ * to the power sequencer. Via the dependency system, multiple targets may
+ * share the same parts of a power sequence but ignore parts that are
+ * irrelevant.
+ *
+ * Descriptor - a handle passed by the pwrseq core to every consumer that
+ * serves as the entry point to the provider layer. It ensures coherence
+ * between different users and keeps reference counting consistent.
+ *
+ * Each provider must define a .match() callback whose role is to determine
+ * whether a potential consumer is in fact associated with this sequencer.
+ * This allows creating abstraction layers on top of regular device-tree
+ * resources like regulators, clocks and other nodes connected to the consumer
+ * via phandle.
+ */
+
+static DEFINE_IDA(pwrseq_ida);
+
+/*
+ * Protects the device list on the pwrseq bus from concurrent modifications
+ * but allows simultaneous read-only access.
+ */
+static DECLARE_RWSEM(pwrseq_sem);
+
+/**
+ * struct pwrseq_unit - Private power-sequence unit data.
+ * @ref: Reference count for this object. When it goes to 0, the object is
+ * destroyed.
+ * @name: Name of this target.
+ * @list: Link to siblings on the list of all units of a single sequencer.
+ * @deps: List of units on which this unit depends.
+ * @enable: Callback running the part of the power-on sequence provided by
+ * this unit.
+ * @disable: Callback running the part of the power-off sequence provided
+ * by this unit.
+ * @enable_count: Current number of users that enabled this unit. May be the
+ * consumer of the power sequencer or other units that depend
+ * on this one.
+ */
+struct pwrseq_unit {
+ struct kref ref;
+ const char *name;
+ struct list_head list;
+ struct list_head deps;
+ pwrseq_power_state_func enable;
+ pwrseq_power_state_func disable;
+ unsigned int enable_count;
+};
+
+static struct pwrseq_unit *pwrseq_unit_new(const struct pwrseq_unit_data *data)
+{
+ struct pwrseq_unit *unit;
+
+ unit = kzalloc(sizeof(*unit), GFP_KERNEL);
+ if (!unit)
+ return NULL;
+
+ unit->name = kstrdup_const(data->name, GFP_KERNEL);
+ if (!unit->name) {
+ kfree(unit);
+ return NULL;
+ }
+
+ kref_init(&unit->ref);
+ INIT_LIST_HEAD(&unit->deps);
+ unit->enable = data->enable;
+ unit->disable = data->disable;
+
+ return unit;
+}
+
+static struct pwrseq_unit *pwrseq_unit_incref(struct pwrseq_unit *unit)
+{
+ kref_get(&unit->ref);
+
+ return unit;
+}
+
+static void pwrseq_unit_release(struct kref *ref);
+
+static void pwrseq_unit_decref(struct pwrseq_unit *unit)
+{
+ kref_put(&unit->ref, pwrseq_unit_release);
+}
+
+/**
+ * struct pwrseq_unit_dep - Wrapper around a reference to the unit structure
+ * allowing to keep it on multiple dependency lists
+ * in different units.
+ * @list: Siblings on the list.
+ * @unit: Address of the referenced unit.
+ */
+struct pwrseq_unit_dep {
+ struct list_head list;
+ struct pwrseq_unit *unit;
+};
+
+static struct pwrseq_unit_dep *pwrseq_unit_dep_new(struct pwrseq_unit *unit)
+{
+ struct pwrseq_unit_dep *dep;
+
+ dep = kzalloc(sizeof(*dep), GFP_KERNEL);
+ if (!dep)
+ return NULL;
+
+ dep->unit = unit;
+
+ return dep;
+}
+
+static void pwrseq_unit_dep_free(struct pwrseq_unit_dep *ref)
+{
+ pwrseq_unit_decref(ref->unit);
+ kfree(ref);
+}
+
+static void pwrseq_unit_free_deps(struct list_head *list)
+{
+ struct pwrseq_unit_dep *dep, *next;
+
+ list_for_each_entry_safe(dep, next, list, list) {
+ list_del(&dep->list);
+ pwrseq_unit_dep_free(dep);
+ }
+}
+
+static void pwrseq_unit_release(struct kref *ref)
+{
+ struct pwrseq_unit *unit = container_of(ref, struct pwrseq_unit, ref);
+
+ pwrseq_unit_free_deps(&unit->deps);
+ list_del(&unit->list);
+ kfree_const(unit->name);
+ kfree(unit);
+}
+
+/**
+ * struct pwrseq_target - Private power-sequence target data.
+ * @list: Siblings on the list of all targets exposed by a power sequencer.
+ * @name: Name of the target.
+ * @unit: Final unit for this target.
+ * @post_enable: Callback run after the target unit has been enabled, *after*
+ * the state lock has been released. It's useful for implementing
+ * boot-up delays without blocking other users from powering up
+ * using the same power sequencer.
+ */
+struct pwrseq_target {
+ struct list_head list;
+ const char *name;
+ struct pwrseq_unit *unit;
+ pwrseq_power_state_func post_enable;
+};
+
+static struct pwrseq_target *
+pwrseq_target_new(const struct pwrseq_target_data *data)
+{
+ struct pwrseq_target *target;
+
+ target = kzalloc(sizeof(*target), GFP_KERNEL);
+ if (!target)
+ return NULL;
+
+ target->name = kstrdup_const(data->name, GFP_KERNEL);
+ if (!target->name) {
+ kfree(target);
+ return NULL;
+ }
+
+ target->post_enable = data->post_enable;
+
+ return target;
+}
+
+static void pwrseq_target_free(struct pwrseq_target *target)
+{
+ pwrseq_unit_decref(target->unit);
+ kfree_const(target->name);
+ kfree(target);
+}
+
+/**
+ * struct pwrseq_device - Private power sequencing data.
+ * @dev: Device struct associated with this sequencer.
+ * @id: Device ID.
+ * @owner: Prevents removal of active power sequencing providers.
+ * @rw_lock: Protects the device from being unregistered while in use.
+ * @state_lock: Prevents multiple users running the power sequence at the same
+ * time.
+ * @match: Power sequencer matching callback.
+ * @targets: List of targets exposed by this sequencer.
+ * @units: List of all units supported by this sequencer.
+ */
+struct pwrseq_device {
+ struct device dev;
+ int id;
+ struct module *owner;
+ struct rw_semaphore rw_lock;
+ struct mutex state_lock;
+ pwrseq_match_func match;
+ struct list_head targets;
+ struct list_head units;
+};
+
+static struct pwrseq_device *to_pwrseq_device(struct device *dev)
+{
+ return container_of(dev, struct pwrseq_device, dev);
+}
+
+static struct pwrseq_device *pwrseq_device_get(struct pwrseq_device *pwrseq)
+{
+ get_device(&pwrseq->dev);
+
+ return pwrseq;
+}
+
+static void pwrseq_device_put(struct pwrseq_device *pwrseq)
+{
+ put_device(&pwrseq->dev);
+}
+
+/**
+ * struct pwrseq_desc - Wraps access to the pwrseq_device and ensures that one
+ * user cannot break the reference counting for others.
+ * @pwrseq: Reference to the power sequencing device.
+ * @target: Reference to the target this descriptor allows to control.
+ * @powered_on: Power state set by the holder of the descriptor (not necessarily
+ * corresponding to the actual power state of the device).
+ */
+struct pwrseq_desc {
+ struct pwrseq_device *pwrseq;
+ struct pwrseq_target *target;
+ bool powered_on;
+};
+
+static const struct bus_type pwrseq_bus = {
+ .name = "pwrseq",
+};
+
+static void pwrseq_release(struct device *dev)
+{
+ struct pwrseq_device *pwrseq = to_pwrseq_device(dev);
+ struct pwrseq_target *target, *pos;
+
+ list_for_each_entry_safe(target, pos, &pwrseq->targets, list) {
+ list_del(&target->list);
+ pwrseq_target_free(target);
+ }
+
+ mutex_destroy(&pwrseq->state_lock);
+ ida_free(&pwrseq_ida, pwrseq->id);
+ kfree(pwrseq);
+}
+
+static const struct device_type pwrseq_device_type = {
+ .name = "power_sequencer",
+ .release = pwrseq_release,
+};
+
+static int pwrseq_check_unit_deps(const struct pwrseq_unit_data *data,
+ struct radix_tree_root *visited_units)
+{
+ const struct pwrseq_unit_data *tmp, **cur;
+ int ret;
+
+ ret = radix_tree_insert(visited_units, (unsigned long)data,
+ (void *)data);
+ if (ret)
+ return ret;
+
+ for (cur = data->deps; cur && *cur; cur++) {
+ tmp = radix_tree_lookup(visited_units, (unsigned long)*cur);
+ if (tmp) {
+ WARN(1, "Circular dependency in power sequencing flow detected!\n");
+ return -EINVAL;
+ }
+
+ ret = pwrseq_check_unit_deps(*cur, visited_units);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int pwrseq_check_target_deps(const struct pwrseq_target_data *data)
+{
+ struct radix_tree_root visited_units;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ int ret;
+
+ if (!data->unit)
+ return -EINVAL;
+
+ INIT_RADIX_TREE(&visited_units, GFP_KERNEL);
+ ret = pwrseq_check_unit_deps(data->unit, &visited_units);
+ radix_tree_for_each_slot(slot, &visited_units, &iter, 0)
+ radix_tree_delete(&visited_units, iter.index);
+
+ return ret;
+}
+
+static int pwrseq_unit_setup_deps(const struct pwrseq_unit_data **data,
+ struct list_head *dep_list,
+ struct list_head *unit_list,
+ struct radix_tree_root *processed_units);
+
+static struct pwrseq_unit *
+pwrseq_unit_setup(const struct pwrseq_unit_data *data,
+ struct list_head *unit_list,
+ struct radix_tree_root *processed_units)
+{
+ struct pwrseq_unit *unit;
+ int ret;
+
+ unit = radix_tree_lookup(processed_units, (unsigned long)data);
+ if (unit)
+ return pwrseq_unit_incref(unit);
+
+ unit = pwrseq_unit_new(data);
+ if (!unit)
+ return ERR_PTR(-ENOMEM);
+
+ if (data->deps) {
+ ret = pwrseq_unit_setup_deps(data->deps, &unit->deps,
+ unit_list, processed_units);
+ if (ret) {
+ pwrseq_unit_decref(unit);
+ return ERR_PTR(ret);
+ }
+ }
+
+ ret = radix_tree_insert(processed_units, (unsigned long)data, unit);
+ if (ret) {
+ pwrseq_unit_decref(unit);
+ return ERR_PTR(ret);
+ }
+
+ list_add_tail(&unit->list, unit_list);
+
+ return unit;
+}
+
+static int pwrseq_unit_setup_deps(const struct pwrseq_unit_data **data,
+ struct list_head *dep_list,
+ struct list_head *unit_list,
+ struct radix_tree_root *processed_units)
+{
+ const struct pwrseq_unit_data *pos;
+ struct pwrseq_unit_dep *dep;
+ struct pwrseq_unit *unit;
+ int i;
+
+ for (i = 0; data[i]; i++) {
+ pos = data[i];
+
+ unit = pwrseq_unit_setup(pos, unit_list, processed_units);
+ if (IS_ERR(unit))
+ return PTR_ERR(unit);
+
+ dep = pwrseq_unit_dep_new(unit);
+ if (!dep) {
+ pwrseq_unit_decref(unit);
+ return -ENOMEM;
+ }
+
+ list_add_tail(&dep->list, dep_list);
+ }
+
+ return 0;
+}
+
+static int pwrseq_do_setup_targets(const struct pwrseq_target_data **data,
+ struct pwrseq_device *pwrseq,
+ struct radix_tree_root *processed_units)
+{
+ const struct pwrseq_target_data *pos;
+ struct pwrseq_target *target;
+ int ret, i;
+
+ for (i = 0; data[i]; i++) {
+ pos = data[i];
+
+ ret = pwrseq_check_target_deps(pos);
+ if (ret)
+ return ret;
+
+ target = pwrseq_target_new(pos);
+ if (!target)
+ return -ENOMEM;
+
+ target->unit = pwrseq_unit_setup(pos->unit, &pwrseq->units,
+ processed_units);
+ if (IS_ERR(target->unit)) {
+ ret = PTR_ERR(target->unit);
+ pwrseq_target_free(target);
+ return ret;
+ }
+
+ list_add_tail(&target->list, &pwrseq->targets);
+ }
+
+ return 0;
+}
+
+static int pwrseq_setup_targets(const struct pwrseq_target_data **targets,
+ struct pwrseq_device *pwrseq)
+{
+ struct radix_tree_root processed_units;
+ struct radix_tree_iter iter;
+ void __rcu **slot;
+ int ret;
+
+ INIT_RADIX_TREE(&processed_units, GFP_KERNEL);
+ ret = pwrseq_do_setup_targets(targets, pwrseq, &processed_units);
+ radix_tree_for_each_slot(slot, &processed_units, &iter, 0)
+ radix_tree_delete(&processed_units, iter.index);
+
+ return ret;
+}
+
+/**
+ * pwrseq_device_register() - Register a new power sequencer.
+ * @config: Configuration of the new power sequencing device.
+ *
+ * The config structure is only used during the call and can be freed after
+ * the function returns. The config structure *must* have the parent device
+ * as well as the match() callback and at least one target set.
+ *
+ * Returns:
+ * Returns the address of the new pwrseq device or ERR_PTR() on failure.
+ */
+struct pwrseq_device *
+pwrseq_device_register(const struct pwrseq_config *config)
+{
+ struct pwrseq_device *pwrseq;
+ int ret, id;
+
+ if (!config->parent || !config->match || !config->targets ||
+ !config->targets[0])
+ return ERR_PTR(-EINVAL);
+
+ pwrseq = kzalloc(sizeof(*pwrseq), GFP_KERNEL);
+ if (!pwrseq)
+ return ERR_PTR(-ENOMEM);
+
+ pwrseq->dev.type = &pwrseq_device_type;
+ pwrseq->dev.bus = &pwrseq_bus;
+ pwrseq->dev.parent = config->parent;
+ device_set_node(&pwrseq->dev, dev_fwnode(config->parent));
+ dev_set_drvdata(&pwrseq->dev, config->drvdata);
+
+ id = ida_alloc(&pwrseq_ida, GFP_KERNEL);
+ if (id < 0) {
+ kfree(pwrseq);
+ return ERR_PTR(id);
+ }
+
+ pwrseq->id = id;
+
+ /*
+ * From this point onwards the device's release() callback is
+ * responsible for freeing resources.
+ */
+ device_initialize(&pwrseq->dev);
+
+ ret = dev_set_name(&pwrseq->dev, "pwrseq.%d", pwrseq->id);
+ if (ret)
+ goto err_put_pwrseq;
+
+ pwrseq->owner = config->owner ?: THIS_MODULE;
+ pwrseq->match = config->match;
+
+ init_rwsem(&pwrseq->rw_lock);
+ mutex_init(&pwrseq->state_lock);
+ INIT_LIST_HEAD(&pwrseq->targets);
+ INIT_LIST_HEAD(&pwrseq->units);
+
+ ret = pwrseq_setup_targets(config->targets, pwrseq);
+ if (ret)
+ goto err_put_pwrseq;
+
+ scoped_guard(rwsem_write, &pwrseq_sem) {
+ ret = device_add(&pwrseq->dev);
+ if (ret)
+ goto err_put_pwrseq;
+ }
+
+ return pwrseq;
+
+err_put_pwrseq:
+ pwrseq_device_put(pwrseq);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(pwrseq_device_register);
+
+/**
+ * pwrseq_device_unregister() - Unregister the power sequencer.
+ * @pwrseq: Power sequencer to unregister.
+ */
+void pwrseq_device_unregister(struct pwrseq_device *pwrseq)
+{
+ struct device *dev = &pwrseq->dev;
+ struct pwrseq_target *target;
+
+ scoped_guard(mutex, &pwrseq->state_lock) {
+ guard(rwsem_write)(&pwrseq->rw_lock);
+
+ list_for_each_entry(target, &pwrseq->targets, list)
+ WARN_ONCE(target->unit->enable_count,
+ "REMOVING POWER SEQUENCER WITH ACTIVE USERS\n");
+
+ guard(rwsem_write)(&pwrseq_sem);
+
+ device_del(dev);
+ }
+
+ pwrseq_device_put(pwrseq);
+}
+EXPORT_SYMBOL_GPL(pwrseq_device_unregister);
+
+static void devm_pwrseq_device_unregister(void *data)
+{
+ struct pwrseq_device *pwrseq = data;
+
+ pwrseq_device_unregister(pwrseq);
+}
+
+/**
+ * devm_pwrseq_device_register() - Managed variant of pwrseq_device_register().
+ * @dev: Managing device.
+ * @config: Configuration of the new power sequencing device.
+ *
+ * Returns:
+ * Returns the address of the new pwrseq device or ERR_PTR() on failure.
+ */
+struct pwrseq_device *
+devm_pwrseq_device_register(struct device *dev,
+ const struct pwrseq_config *config)
+{
+ struct pwrseq_device *pwrseq;
+ int ret;
+
+ pwrseq = pwrseq_device_register(config);
+ if (IS_ERR(pwrseq))
+ return pwrseq;
+
+ ret = devm_add_action_or_reset(dev, devm_pwrseq_device_unregister,
+ pwrseq);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return pwrseq;
+}
+EXPORT_SYMBOL_GPL(devm_pwrseq_device_register);
+
+/**
+ * pwrseq_device_get_drvdata() - Get the driver private data associated with
+ * this sequencer.
+ * @pwrseq: Power sequencer object.
+ *
+ * Returns:
+ * Address of the private driver data.
+ */
+void *pwrseq_device_get_drvdata(struct pwrseq_device *pwrseq)
+{
+ return dev_get_drvdata(&pwrseq->dev);
+}
+EXPORT_SYMBOL_GPL(pwrseq_device_get_drvdata);
+
+struct pwrseq_match_data {
+ struct pwrseq_desc *desc;
+ struct device *dev;
+ const char *target;
+};
+
+static int pwrseq_match_device(struct device *pwrseq_dev, void *data)
+{
+ struct pwrseq_device *pwrseq = to_pwrseq_device(pwrseq_dev);
+ struct pwrseq_match_data *match_data = data;
+ struct pwrseq_target *target;
+ int ret;
+
+ lockdep_assert_held_read(&pwrseq_sem);
+
+ guard(rwsem_read)(&pwrseq->rw_lock);
+ if (!device_is_registered(&pwrseq->dev))
+ return 0;
+
+ ret = pwrseq->match(pwrseq, match_data->dev);
+ if (ret <= 0)
+ return ret;
+
+ /* We got the matching device, let's find the right target. */
+ list_for_each_entry(target, &pwrseq->targets, list) {
+ if (strcmp(target->name, match_data->target))
+ continue;
+
+ match_data->desc->target = target;
+ }
+
+ /*
+ * This device does not have this target. No point in deferring as it
+ * will not get a new target dynamically later.
+ */
+ if (!match_data->desc->target)
+ return -ENOENT;
+
+ if (!try_module_get(pwrseq->owner))
+ return -EPROBE_DEFER;
+
+ match_data->desc->pwrseq = pwrseq_device_get(pwrseq);
+
+ return 1;
+}
+
+/**
+ * pwrseq_get() - Get the power sequencer associated with this device.
+ * @dev: Device for which to get the sequencer.
+ * @target: Name of the target exposed by the sequencer this device wants to
+ * reach.
+ *
+ * Returns:
+ * New power sequencer descriptor for use by the consumer driver or ERR_PTR()
+ * on failure.
+ */
+struct pwrseq_desc *pwrseq_get(struct device *dev, const char *target)
+{
+ struct pwrseq_match_data match_data;
+ int ret;
+
+ struct pwrseq_desc *desc __free(kfree) = kzalloc(sizeof(*desc),
+ GFP_KERNEL);
+ if (!desc)
+ return ERR_PTR(-ENOMEM);
+
+ match_data.desc = desc;
+ match_data.dev = dev;
+ match_data.target = target;
+
+ guard(rwsem_read)(&pwrseq_sem);
+
+ ret = bus_for_each_dev(&pwrseq_bus, NULL, &match_data,
+ pwrseq_match_device);
+ if (ret < 0)
+ return ERR_PTR(ret);
+ if (ret == 0)
+ /* No device matched. */
+ return ERR_PTR(-EPROBE_DEFER);
+
+ return no_free_ptr(desc);
+}
+EXPORT_SYMBOL_GPL(pwrseq_get);
+
+/**
+ * pwrseq_put() - Release the power sequencer descriptor.
+ * @desc: Descriptor to release.
+ */
+void pwrseq_put(struct pwrseq_desc *desc)
+{
+ struct pwrseq_device *pwrseq;
+
+ if (!desc)
+ return;
+
+ pwrseq = desc->pwrseq;
+
+ if (desc->powered_on)
+ pwrseq_power_off(desc);
+
+ kfree(desc);
+ module_put(pwrseq->owner);
+ pwrseq_device_put(pwrseq);
+}
+EXPORT_SYMBOL_GPL(pwrseq_put);
+
+static void devm_pwrseq_put(void *data)
+{
+ struct pwrseq_desc *desc = data;
+
+ pwrseq_put(desc);
+}
+
+/**
+ * devm_pwrseq_get() - Managed variant of pwrseq_get().
+ * @dev: Device for which to get the sequencer and which also manages its
+ * lifetime.
+ * @target: Name of the target exposed by the sequencer this device wants to
+ * reach.
+ *
+ * Returns:
+ * New power sequencer descriptor for use by the consumer driver or ERR_PTR()
+ * on failure.
+ */
+struct pwrseq_desc *devm_pwrseq_get(struct device *dev, const char *target)
+{
+ struct pwrseq_desc *desc;
+ int ret;
+
+ desc = pwrseq_get(dev, target);
+ if (IS_ERR(desc))
+ return desc;
+
+ ret = devm_add_action_or_reset(dev, devm_pwrseq_put, desc);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return desc;
+}
+EXPORT_SYMBOL_GPL(devm_pwrseq_get);
+
+static int pwrseq_unit_enable(struct pwrseq_device *pwrseq,
+ struct pwrseq_unit *target);
+static int pwrseq_unit_disable(struct pwrseq_device *pwrseq,
+ struct pwrseq_unit *target);
+
+static int pwrseq_unit_enable_deps(struct pwrseq_device *pwrseq,
+ struct list_head *list)
+{
+ struct pwrseq_unit_dep *pos;
+ int ret = 0;
+
+ list_for_each_entry(pos, list, list) {
+ ret = pwrseq_unit_enable(pwrseq, pos->unit);
+ if (ret) {
+ list_for_each_entry_continue_reverse(pos, list, list)
+ pwrseq_unit_disable(pwrseq, pos->unit);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int pwrseq_unit_disable_deps(struct pwrseq_device *pwrseq,
+ struct list_head *list)
+{
+ struct pwrseq_unit_dep *pos;
+ int ret = 0;
+
+ list_for_each_entry_reverse(pos, list, list) {
+ ret = pwrseq_unit_disable(pwrseq, pos->unit);
+ if (ret) {
+ list_for_each_entry_continue(pos, list, list)
+ pwrseq_unit_enable(pwrseq, pos->unit);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int pwrseq_unit_enable(struct pwrseq_device *pwrseq,
+ struct pwrseq_unit *unit)
+{
+ int ret;
+
+ lockdep_assert_held_read(&pwrseq->rw_lock);
+ lockdep_assert_held(&pwrseq->state_lock);
+
+ if (unit->enable_count != 0) {
+ unit->enable_count++;
+ return 0;
+ }
+
+ ret = pwrseq_unit_enable_deps(pwrseq, &unit->deps);
+ if (ret) {
+ dev_err(&pwrseq->dev,
+ "Failed to enable dependencies before power-on for target '%s': %d\n",
+ unit->name, ret);
+ return ret;
+ }
+
+ if (unit->enable) {
+ ret = unit->enable(pwrseq);
+ if (ret) {
+ dev_err(&pwrseq->dev,
+ "Failed to enable target '%s': %d\n",
+ unit->name, ret);
+ pwrseq_unit_disable_deps(pwrseq, &unit->deps);
+ return ret;
+ }
+ }
+
+ unit->enable_count++;
+
+ return 0;
+}
+
+static int pwrseq_unit_disable(struct pwrseq_device *pwrseq,
+ struct pwrseq_unit *unit)
+{
+ int ret;
+
+ lockdep_assert_held_read(&pwrseq->rw_lock);
+ lockdep_assert_held(&pwrseq->state_lock);
+
+ if (unit->enable_count == 0) {
+ WARN_ONCE(1, "Unmatched power-off for target '%s'\n",
+ unit->name);
+ return -EBUSY;
+ }
+
+ if (unit->enable_count != 1) {
+ unit->enable_count--;
+ return 0;
+ }
+
+ if (unit->disable) {
+ ret = unit->disable(pwrseq);
+ if (ret) {
+ dev_err(&pwrseq->dev,
+ "Failed to disable target '%s': %d\n",
+ unit->name, ret);
+ return ret;
+ }
+ }
+
+ ret = pwrseq_unit_disable_deps(pwrseq, &unit->deps);
+ if (ret) {
+ dev_err(&pwrseq->dev,
+ "Failed to disable dependencies after power-off for target '%s': %d\n",
+ unit->name, ret);
+ if (unit->enable)
+ unit->enable(pwrseq);
+ return ret;
+ }
+
+ unit->enable_count--;
+
+ return 0;
+}
+
+/**
+ * pwrseq_power_on() - Issue a power-on request on behalf of the consumer
+ * device.
+ * @desc: Descriptor referencing the power sequencer.
+ *
+ * This function tells the power sequencer that the consumer wants to be
+ * powered-up. The sequencer may already have powered-up the device in which
+ * case the function returns 0. If the power-up sequence is already in
+ * progress, the function will block until it's done and return 0. If this is
+ * the first request, the device will be powered up.
+ *
+ * Returns:
+ * 0 on success, negative error number on failure.
+ */
+int pwrseq_power_on(struct pwrseq_desc *desc)
+{
+ struct pwrseq_device *pwrseq;
+ struct pwrseq_target *target;
+ struct pwrseq_unit *unit;
+ int ret;
+
+ might_sleep();
+
+ if (!desc || desc->powered_on)
+ return 0;
+
+ pwrseq = desc->pwrseq;
+ target = desc->target;
+ unit = target->unit;
+
+ guard(rwsem_read)(&pwrseq->rw_lock);
+ if (!device_is_registered(&pwrseq->dev))
+ return -ENODEV;
+
+ scoped_guard(mutex, &pwrseq->state_lock) {
+ ret = pwrseq_unit_enable(pwrseq, unit);
+ if (!ret)
+ desc->powered_on = true;
+ }
+
+ if (target->post_enable) {
+ ret = target->post_enable(pwrseq);
+ if (ret) {
+ pwrseq_unit_disable(pwrseq, unit);
+ desc->powered_on = false;
+ }
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pwrseq_power_on);
+
+/**
+ * pwrseq_power_off() - Issue a power-off request on behalf of the consumer
+ * device.
+ * @desc: Descriptor referencing the power sequencer.
+ *
+ * This undoes the effects of pwrseq_power_on(). It issues a power-off request
+ * on behalf of the consumer and when the last remaining user does so, the
+ * power-down sequence will be started. If one is in progress, the function
+ * will block until it's complete and then return.
+ *
+ * Returns:
+ * 0 on success, negative error number on failure.
+ */
+int pwrseq_power_off(struct pwrseq_desc *desc)
+{
+ struct pwrseq_device *pwrseq;
+ struct pwrseq_unit *unit;
+ int ret;
+
+ might_sleep();
+
+ if (!desc || !desc->powered_on)
+ return 0;
+
+ pwrseq = desc->pwrseq;
+ unit = desc->target->unit;
+
+ guard(rwsem_read)(&pwrseq->rw_lock);
+ if (!device_is_registered(&pwrseq->dev))
+ return -ENODEV;
+
+ guard(mutex)(&pwrseq->state_lock);
+
+ ret = pwrseq_unit_disable(pwrseq, unit);
+ if (!ret)
+ desc->powered_on = false;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pwrseq_power_off);
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+struct pwrseq_debugfs_count_ctx {
+ struct device *dev;
+ loff_t index;
+};
+
+static int pwrseq_debugfs_seq_count(struct device *dev, void *data)
+{
+ struct pwrseq_debugfs_count_ctx *ctx = data;
+
+ ctx->dev = dev;
+
+ return ctx->index-- ? 0 : 1;
+}
+
+static void *pwrseq_debugfs_seq_start(struct seq_file *seq, loff_t *pos)
+{
+ struct pwrseq_debugfs_count_ctx ctx;
+
+ ctx.dev = NULL;
+ ctx.index = *pos;
+
+ /*
+ * We're holding the lock for the entire printout so no need to fiddle
+ * with device reference count.
+ */
+ down_read(&pwrseq_sem);
+
+ bus_for_each_dev(&pwrseq_bus, NULL, &ctx, pwrseq_debugfs_seq_count);
+ if (!ctx.index)
+ return NULL;
+
+ return ctx.dev;
+}
+
+static void *pwrseq_debugfs_seq_next(struct seq_file *seq, void *data,
+ loff_t *pos)
+{
+ struct device *curr = data;
+
+ ++*pos;
+
+ struct device *next __free(put_device) =
+ bus_find_next_device(&pwrseq_bus, curr);
+ return next;
+}
+
+static void pwrseq_debugfs_seq_show_target(struct seq_file *seq,
+ struct pwrseq_target *target)
+{
+ seq_printf(seq, " target: [%s] (target unit: [%s])\n",
+ target->name, target->unit->name);
+}
+
+static void pwrseq_debugfs_seq_show_unit(struct seq_file *seq,
+ struct pwrseq_unit *unit)
+{
+ struct pwrseq_unit_dep *ref;
+
+ seq_printf(seq, " unit: [%s] - enable count: %u\n",
+ unit->name, unit->enable_count);
+
+ if (list_empty(&unit->deps))
+ return;
+
+ seq_puts(seq, " dependencies:\n");
+ list_for_each_entry(ref, &unit->deps, list)
+ seq_printf(seq, " [%s]\n", ref->unit->name);
+}
+
+static int pwrseq_debugfs_seq_show(struct seq_file *seq, void *data)
+{
+ struct device *dev = data;
+ struct pwrseq_device *pwrseq = to_pwrseq_device(dev);
+ struct pwrseq_target *target;
+ struct pwrseq_unit *unit;
+
+ seq_printf(seq, "%s:\n", dev_name(dev));
+
+ seq_puts(seq, " targets:\n");
+ list_for_each_entry(target, &pwrseq->targets, list)
+ pwrseq_debugfs_seq_show_target(seq, target);
+
+ seq_puts(seq, " units:\n");
+ list_for_each_entry(unit, &pwrseq->units, list)
+ pwrseq_debugfs_seq_show_unit(seq, unit);
+
+ return 0;
+}
+
+static void pwrseq_debugfs_seq_stop(struct seq_file *seq, void *data)
+{
+ up_read(&pwrseq_sem);
+}
+
+static const struct seq_operations pwrseq_debugfs_sops = {
+ .start = pwrseq_debugfs_seq_start,
+ .next = pwrseq_debugfs_seq_next,
+ .show = pwrseq_debugfs_seq_show,
+ .stop = pwrseq_debugfs_seq_stop,
+};
+DEFINE_SEQ_ATTRIBUTE(pwrseq_debugfs);
+
+static struct dentry *pwrseq_debugfs_dentry;
+
+#endif /* CONFIG_DEBUG_FS */
+
+static int __init pwrseq_init(void)
+{
+ int ret;
+
+ ret = bus_register(&pwrseq_bus);
+ if (ret) {
+ pr_err("Failed to register the power sequencer bus\n");
+ return ret;
+ }
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+ pwrseq_debugfs_dentry = debugfs_create_file("pwrseq", 0444, NULL, NULL,
+ &pwrseq_debugfs_fops);
+#endif /* CONFIG_DEBUG_FS */
+
+ return 0;
+}
+subsys_initcall(pwrseq_init);
+
+static void __exit pwrseq_exit(void)
+{
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+ debugfs_remove_recursive(pwrseq_debugfs_dentry);
+#endif /* CONFIG_DEBUG_FS */
+
+ bus_unregister(&pwrseq_bus);
+}
+module_exit(pwrseq_exit);
+
+MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
+MODULE_DESCRIPTION("Power Sequencing subsystem core");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
new file mode 100644
index 000000000000..7d583b4f266e
--- /dev/null
+++ b/include/linux/pwrseq/consumer.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#ifndef __POWER_SEQUENCING_CONSUMER_H__
+#define __POWER_SEQUENCING_CONSUMER_H__
+
+#include <linux/err.h>
+
+struct device;
+struct pwrseq_desc;
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+
+struct pwrseq_desc * __must_check
+pwrseq_get(struct device *dev, const char *target);
+void pwrseq_put(struct pwrseq_desc *desc);
+
+struct pwrseq_desc * __must_check
+devm_pwrseq_get(struct device *dev, const char *target);
+
+int pwrseq_power_on(struct pwrseq_desc *desc);
+int pwrseq_power_off(struct pwrseq_desc *desc);
+
+#else /* CONFIG_POWER_SEQUENCING */
+
+static inline struct pwrseq_desc * __must_check
+pwrseq_get(struct device *dev, const char *target)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void pwrseq_put(struct pwrseq_desc *desc)
+{
+}
+
+static inline struct pwrseq_desc * __must_check
+devm_pwrseq_get(struct device *dev, const char *target)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline int pwrseq_power_on(struct pwrseq_desc *desc)
+{
+ return -ENOSYS;
+}
+
+static inline int pwrseq_power_off(struct pwrseq_desc *desc)
+{
+ return -ENOSYS;
+}
+
+#endif /* CONFIG_POWER_SEQUENCING */
+
+#endif /* __POWER_SEQUENCING_CONSUMER_H__ */
diff --git a/include/linux/pwrseq/provider.h b/include/linux/pwrseq/provider.h
new file mode 100644
index 000000000000..e627ed2f4d91
--- /dev/null
+++ b/include/linux/pwrseq/provider.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#ifndef __POWER_SEQUENCING_PROVIDER_H__
+#define __POWER_SEQUENCING_PROVIDER_H__
+
+struct device;
+struct module;
+struct pwrseq_device;
+
+typedef int (*pwrseq_power_state_func)(struct pwrseq_device *);
+typedef int (*pwrseq_match_func)(struct pwrseq_device *, struct device *);
+
+/**
+ * struct pwrseq_unit_data - Configuration of a single power sequencing
+ * unit.
+ * @name: Name of the unit.
+ * @deps: Units that must be enabled before this one and disabled after it
+ * in the order they come in this array.
+ * @enable: Callback running the part of the power-on sequence provided by
+ * this unit.
+ * @disable: Callback running the part of the power-off sequence provided
+ * by this unit.
+ */
+struct pwrseq_unit_data {
+ const char *name;
+ const struct pwrseq_unit_data **deps;
+ pwrseq_power_state_func enable;
+ pwrseq_power_state_func disable;
+};
+
+/**
+ * struct pwrseq_target_data - Configuration of a power sequencing target.
+ * @name: Name of the target.
+ * @unit: Final unit that this target must reach in order to be considered
+ * enabled.
+ * @post_enable: Callback run after the target unit has been enabled, *after*
+ * the state lock has been released. It's useful for implementing
+ * boot-up delays without blocking other users from powering up
+ * using the same power sequencer.
+ */
+struct pwrseq_target_data {
+ const char *name;
+ const struct pwrseq_unit_data *unit;
+ pwrseq_power_state_func post_enable;
+};
+
+/**
+ * struct pwrseq_config - Configuration used for registering a new provider.
+ * @parent: Parent device for the sequencer. Must be set.
+ * @owner: Module providing this device.
+ * @drvdata: Private driver data.
+ * @match: Provider callback used to match the consumer device to the sequencer.
+ * @targets: Array of targets for this power sequencer. Must be NULL-terminated.
+ */
+struct pwrseq_config {
+ struct device *parent;
+ struct module *owner;
+ void *drvdata;
+ pwrseq_match_func match;
+ const struct pwrseq_target_data **targets;
+};
+
+struct pwrseq_device *
+pwrseq_device_register(const struct pwrseq_config *config);
+void pwrseq_device_unregister(struct pwrseq_device *pwrseq);
+struct pwrseq_device *
+devm_pwrseq_device_register(struct device *dev,
+ const struct pwrseq_config *config);
+
+void *pwrseq_device_get_drvdata(struct pwrseq_device *pwrseq);
+
+#endif /* __POWER_SEQUENCING_PROVIDER_H__ */

--
2.43.0


2024-05-28 19:10:27

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 12/17] PCI: hold the rescan mutex when scanning for the first time

From: Bartosz Golaszewski <[email protected]>

With the introduction of PCI device power control drivers that will be
able to trigger the port rescan when probing, we need to hold the rescan
mutex during the initial pci_host_probe() too or the two could get in
each other's way.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pci/probe.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8e696e547565..604fc96b1098 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3072,7 +3072,9 @@ int pci_host_probe(struct pci_host_bridge *bridge)
struct pci_bus *bus, *child;
int ret;

+ pci_lock_rescan_remove();
ret = pci_scan_root_bus_bridge(bridge);
+ pci_unlock_rescan_remove();
if (ret < 0) {
dev_err(bridge->dev.parent, "Scanning root bridge failed");
return ret;

--
2.43.0


2024-05-28 19:11:14

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 14/17] PCI/pwrctl: create platform devices for child OF nodes of the port node

From: Bartosz Golaszewski <[email protected]>

In preparation for introducing PCI device power control - a set of
library functions that will allow powering-up of PCI devices before
they're detected on the PCI bus - we need to populate the devices
defined on the device-tree.

We are reusing the platform bus as it provides us with all the
infrastructure we need to match the pwrctl drivers against the
compatibles from OF nodes.

These platform devices will be probed by the driver core and bound to
the PCI pwrctl drivers we'll introduce later.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pci/bus.c | 9 +++++++++
drivers/pci/remove.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 826b5016a101..3e3517567721 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <linux/ioport.h>
#include <linux/of.h>
+#include <linux/of_platform.h>
#include <linux/proc_fs.h>
#include <linux/slab.h>

@@ -354,6 +355,14 @@ void pci_bus_add_device(struct pci_dev *dev)
pci_warn(dev, "device attach failed (%d)\n", retval);

pci_dev_assign_added(dev, true);
+
+ if (pci_is_bridge(dev)) {
+ retval = of_platform_populate(dev->dev.of_node, NULL, NULL,
+ &dev->dev);
+ if (retval)
+ pci_err(dev, "failed to populate child OF nodes (%d)\n",
+ retval);
+ }
}
EXPORT_SYMBOL_GPL(pci_bus_add_device);

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index d749ea8250d6..910387e5bdbf 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/pci.h>
#include <linux/module.h>
+#include <linux/of_platform.h>
#include "pci.h"

static void pci_free_resources(struct pci_dev *dev)
@@ -18,7 +19,7 @@ static void pci_stop_dev(struct pci_dev *dev)
pci_pme_active(dev, false);

if (pci_dev_is_added(dev)) {
-
+ of_platform_depopulate(&dev->dev);
device_release_driver(&dev->dev);
pci_proc_detach_device(dev);
pci_remove_sysfs_dev_files(dev);

--
2.43.0


2024-05-28 19:12:17

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

From: Bartosz Golaszewski <[email protected]>

Add a PCI power control driver that's capable of correctly powering up
devices using the power sequencing subsystem. The first users of this
driver are the ath11k module on QCA6390 and ath12k on WCN7850.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pci/pwrctl/Kconfig | 9 ++++
drivers/pci/pwrctl/Makefile | 2 +
drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+)

diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig
index 96195395af69..f1b824955d4b 100644
--- a/drivers/pci/pwrctl/Kconfig
+++ b/drivers/pci/pwrctl/Kconfig
@@ -5,4 +5,13 @@ menu "PCI Power control drivers"
config PCI_PWRCTL
tristate

+config PCI_PWRCTL_PWRSEQ
+ tristate "PCI Power Control driver using the Power Sequencing subsystem"
+ select POWER_SEQUENCING
+ select PCI_PWRCTL
+ default m if ((ATH11K_PCI || ATH12K) && ARCH_QCOM)
+ help
+ Enable support for the PCI power control driver for device
+ drivers using the Power Sequencing subsystem.
+
endmenu
diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile
index 52ae0640ef7b..d308aae4800c 100644
--- a/drivers/pci/pwrctl/Makefile
+++ b/drivers/pci/pwrctl/Makefile
@@ -2,3 +2,5 @@

obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o
pci-pwrctl-core-y := core.o
+
+obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o
diff --git a/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c b/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
new file mode 100644
index 000000000000..c7a113a76c0c
--- /dev/null
+++ b/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pci-pwrctl.h>
+#include <linux/platform_device.h>
+#include <linux/pwrseq/consumer.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+struct pci_pwrctl_pwrseq_data {
+ struct pci_pwrctl ctx;
+ struct pwrseq_desc *pwrseq;
+};
+
+static void devm_pci_pwrctl_pwrseq_power_off(void *data)
+{
+ struct pwrseq_desc *pwrseq = data;
+
+ pwrseq_power_off(pwrseq);
+}
+
+static int pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
+{
+ struct pci_pwrctl_pwrseq_data *data;
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
+ if (IS_ERR(data->pwrseq))
+ return dev_err_probe(dev, PTR_ERR(data->pwrseq),
+ "Failed to get the power sequencer\n");
+
+ ret = pwrseq_power_on(data->pwrseq);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to power-on the device\n");
+
+ ret = devm_add_action_or_reset(dev, devm_pci_pwrctl_pwrseq_power_off,
+ data->pwrseq);
+ if (ret)
+ return ret;
+
+ data->ctx.dev = dev;
+
+ ret = devm_pci_pwrctl_device_set_ready(dev, &data->ctx);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to register the pwrctl wrapper\n");
+
+ return 0;
+}
+
+static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
+ {
+ /* ATH11K in QCA6390 package. */
+ .compatible = "pci17cb,1101",
+ .data = "wlan",
+ },
+ {
+ /* ATH12K in WCN7850 package. */
+ .compatible = "pci17cb,1107",
+ .data = "wlan",
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pci_pwrctl_pwrseq_of_match);
+
+static struct platform_driver pci_pwrctl_pwrseq_driver = {
+ .driver = {
+ .name = "pci-pwrctl-pwrseq",
+ .of_match_table = pci_pwrctl_pwrseq_of_match,
+ },
+ .probe = pci_pwrctl_pwrseq_probe,
+};
+module_platform_driver(pci_pwrctl_pwrseq_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
+MODULE_DESCRIPTION("Generic PCI Power Control module for power sequenced devices");
+MODULE_LICENSE("GPL");

--
2.43.0


2024-05-28 19:12:17

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 15/17] PCI/pwrctl: add PCI power control core code

From: Bartosz Golaszewski <[email protected]>

Some PCI devices must be powered-on before they can be detected on the
bus. Introduce a simple framework reusing the existing PCI OF
infrastructure.

The way this works is: a DT node representing a PCI device connected to
the port can be matched against its power control platform driver. If
the match succeeds, the driver is responsible for powering-up the device
and calling pcie_pwrctl_device_set_ready() which will trigger a PCI bus
rescan as well as subscribe to PCI bus notifications.

When the device is detected and created, we'll make it consume the same
DT node that the platform device did. When the device is bound, we'll
create a device link between it and the parent power control device.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pci/Kconfig | 1 +
drivers/pci/Makefile | 1 +
drivers/pci/pwrctl/Kconfig | 8 +++
drivers/pci/pwrctl/Makefile | 4 ++
drivers/pci/pwrctl/core.c | 137 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci-pwrctl.h | 51 +++++++++++++++++
6 files changed, 202 insertions(+)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index d35001589d88..aa4d1833f442 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -296,5 +296,6 @@ source "drivers/pci/hotplug/Kconfig"
source "drivers/pci/controller/Kconfig"
source "drivers/pci/endpoint/Kconfig"
source "drivers/pci/switch/Kconfig"
+source "drivers/pci/pwrctl/Kconfig"

endif
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 175302036890..8ddad57934a6 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PCI) += access.o bus.o probe.o host-bridge.o \

obj-$(CONFIG_PCI) += msi/
obj-$(CONFIG_PCI) += pcie/
+obj-$(CONFIG_PCI) += pwrctl/

ifdef CONFIG_PCI
obj-$(CONFIG_PROC_FS) += proc.o
diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig
new file mode 100644
index 000000000000..96195395af69
--- /dev/null
+++ b/drivers/pci/pwrctl/Kconfig
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menu "PCI Power control drivers"
+
+config PCI_PWRCTL
+ tristate
+
+endmenu
diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile
new file mode 100644
index 000000000000..52ae0640ef7b
--- /dev/null
+++ b/drivers/pci/pwrctl/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o
+pci-pwrctl-core-y := core.o
diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
new file mode 100644
index 000000000000..feca26ad2f6a
--- /dev/null
+++ b/drivers/pci/pwrctl/core.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#include <linux/device.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/pci-pwrctl.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+ struct pci_pwrctl *pwrctl = container_of(nb, struct pci_pwrctl, nb);
+ struct device *dev = data;
+
+ if (dev_fwnode(dev) != dev_fwnode(pwrctl->dev))
+ return NOTIFY_DONE;
+
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ /*
+ * We will have two struct device objects bound to two different
+ * drivers on different buses but consuming the same DT node. We
+ * must not bind the pins twice in this case but only once for
+ * the first device to be added.
+ *
+ * If we got here then the PCI device is the second after the
+ * power control platform device. Mark its OF node as reused.
+ */
+ dev->of_node_reused = true;
+ break;
+ case BUS_NOTIFY_BOUND_DRIVER:
+ pwrctl->link = device_link_add(dev, pwrctl->dev,
+ DL_FLAG_AUTOREMOVE_CONSUMER);
+ if (!pwrctl->link)
+ dev_err(pwrctl->dev, "Failed to add device link\n");
+ break;
+ case BUS_NOTIFY_UNBOUND_DRIVER:
+ if (pwrctl->link)
+ device_link_remove(dev, pwrctl->dev);
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+/**
+ * pci_pwrctl_device_set_ready() - Notify the pwrctl subsystem that the PCI
+ * device is powered-up and ready to be detected.
+ *
+ * @pwrctl: PCI power control data.
+ *
+ * Returns:
+ * 0 on success, negative error number on error.
+ *
+ * Note:
+ * This function returning 0 doesn't mean the device was detected. It means,
+ * that the bus rescan was successfully started. The device will get bound to
+ * its PCI driver asynchronously.
+ */
+int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl)
+{
+ int ret;
+
+ if (!pwrctl->dev)
+ return -ENODEV;
+
+ pwrctl->nb.notifier_call = pci_pwrctl_notify;
+ ret = bus_register_notifier(&pci_bus_type, &pwrctl->nb);
+ if (ret)
+ return ret;
+
+ pci_lock_rescan_remove();
+ pci_rescan_bus(to_pci_dev(pwrctl->dev->parent)->bus);
+ pci_unlock_rescan_remove();
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_pwrctl_device_set_ready);
+
+/**
+ * pci_pwrctl_device_unset_ready() - Notify the pwrctl subsystem that the PCI
+ * device is about to be powered-down.
+ *
+ * @pwrctl: PCI power control data.
+ */
+void pci_pwrctl_device_unset_ready(struct pci_pwrctl *pwrctl)
+{
+ /*
+ * We don't have to delete the link here. Typically, this function
+ * is only called when the power control device is being detached. If
+ * it is being detached then the child PCI device must have already
+ * been unbound too or the device core wouldn't let us unbind.
+ */
+ bus_unregister_notifier(&pci_bus_type, &pwrctl->nb);
+}
+EXPORT_SYMBOL_GPL(pci_pwrctl_device_unset_ready);
+
+static void devm_pci_pwrctl_device_unset_ready(void *data)
+{
+ struct pci_pwrctl *pwrctl = data;
+
+ pci_pwrctl_device_unset_ready(pwrctl);
+}
+
+/**
+ * devm_pci_pwrctl_device_set_ready - Managed variant of
+ * pci_pwrctl_device_set_ready().
+ *
+ * @dev: Device managing this pwrctl provider.
+ * @pwrctl: PCI power control data.
+ *
+ * Returns:
+ * 0 on success, negative error number on error.
+ */
+int devm_pci_pwrctl_device_set_ready(struct device *dev,
+ struct pci_pwrctl *pwrctl)
+{
+ int ret;
+
+ ret = pci_pwrctl_device_set_ready(pwrctl);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(dev,
+ devm_pci_pwrctl_device_unset_ready,
+ pwrctl);
+}
+EXPORT_SYMBOL_GPL(devm_pci_pwrctl_device_set_ready);
+
+MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
+MODULE_DESCRIPTION("PCI Device Power Control core driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pci-pwrctl.h b/include/linux/pci-pwrctl.h
new file mode 100644
index 000000000000..45e9cfe740e4
--- /dev/null
+++ b/include/linux/pci-pwrctl.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ */
+
+#ifndef __PCI_PWRCTL_H__
+#define __PCI_PWRCTL_H__
+
+#include <linux/notifier.h>
+
+struct device;
+struct device_link;
+
+/*
+ * This is a simple framework for solving the issue of PCI devices that require
+ * certain resources (regulators, GPIOs, clocks) to be enabled before the
+ * device can actually be detected on the PCI bus.
+ *
+ * The idea is to reuse the platform bus to populate OF nodes describing the
+ * PCI device and its resources, let these platform devices probe and enable
+ * relevant resources and then trigger a rescan of the PCI bus allowing for the
+ * same device (with a second associated struct device) to be registered with
+ * the PCI subsystem.
+ *
+ * To preserve a correct hierarchy for PCI power management and device reset,
+ * we create a device link between the power control platform device (parent)
+ * and the supplied PCI device (child).
+ */
+
+/**
+ * struct pci_pwrctl - PCI device power control context.
+ * @dev: Address of the power controlling device.
+ *
+ * An object of this type must be allocated by the PCI power control device and
+ * passed to the pwrctl subsystem to trigger a bus rescan and setup a device
+ * link with the device once it's up.
+ */
+struct pci_pwrctl {
+ struct device *dev;
+
+ /* Private: don't use. */
+ struct notifier_block nb;
+ struct device_link *link;
+};
+
+int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl);
+void pci_pwrctl_device_unset_ready(struct pci_pwrctl *pwrctl);
+int devm_pci_pwrctl_device_set_ready(struct device *dev,
+ struct pci_pwrctl *pwrctl);
+
+#endif /* __PCI_PWRCTL_H__ */

--
2.43.0


2024-05-28 19:14:37

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 17/17] Bluetooth: qca: use the power sequencer for QCA6390

From: Bartosz Golaszewski <[email protected]>

Use the pwrseq subsystem's consumer API to run the power-up sequence for
the Bluetooth module of the QCA6390 package.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/bluetooth/hci_qca.c | 74 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 59 insertions(+), 15 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 384a2bbbf303..de4f88d835cb 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -28,6 +28,7 @@
#include <linux/of.h>
#include <linux/acpi.h>
#include <linux/platform_device.h>
+#include <linux/pwrseq/consumer.h>
#include <linux/regulator/consumer.h>
#include <linux/serdev.h>
#include <linux/mutex.h>
@@ -214,6 +215,7 @@ struct qca_power {
struct regulator_bulk_data *vreg_bulk;
int num_vregs;
bool vregs_on;
+ struct pwrseq_desc *pwrseq;
};

struct qca_serdev {
@@ -1684,6 +1686,27 @@ static bool qca_wakeup(struct hci_dev *hdev)
return wakeup;
}

+static int qca_port_reopen(struct hci_uart *hu)
+{
+ int ret;
+
+ /* Now the device is in ready state to communicate with host.
+ * To sync host with device we need to reopen port.
+ * Without this, we will have RTS and CTS synchronization
+ * issues.
+ */
+ serdev_device_close(hu->serdev);
+ ret = serdev_device_open(hu->serdev);
+ if (ret) {
+ bt_dev_err(hu->hdev, "failed to open port");
+ return ret;
+ }
+
+ hci_uart_set_flow_control(hu, false);
+
+ return 0;
+}
+
static int qca_regulator_init(struct hci_uart *hu)
{
enum qca_btsoc_type soc_type = qca_soc_type(hu);
@@ -1752,21 +1775,7 @@ static int qca_regulator_init(struct hci_uart *hu)
break;
}

- /* Now the device is in ready state to communicate with host.
- * To sync host with device we need to reopen port.
- * Without this, we will have RTS and CTS synchronization
- * issues.
- */
- serdev_device_close(hu->serdev);
- ret = serdev_device_open(hu->serdev);
- if (ret) {
- bt_dev_err(hu->hdev, "failed to open port");
- return ret;
- }
-
- hci_uart_set_flow_control(hu, false);
-
- return 0;
+ return qca_port_reopen(hu);
}

static int qca_power_on(struct hci_dev *hdev)
@@ -1794,6 +1803,17 @@ static int qca_power_on(struct hci_dev *hdev)
ret = qca_regulator_init(hu);
break;

+ case QCA_QCA6390:
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ ret = pwrseq_power_on(qcadev->bt_power->pwrseq);
+ if (ret)
+ return ret;
+
+ ret = qca_port_reopen(hu);
+ if (ret)
+ return ret;
+ break;
+
default:
qcadev = serdev_device_get_drvdata(hu->serdev);
if (qcadev->bt_en) {
@@ -2168,6 +2188,10 @@ static void qca_power_shutdown(struct hci_uart *hu)
}
break;

+ case QCA_QCA6390:
+ pwrseq_power_off(qcadev->bt_power->pwrseq);
+ break;
+
default:
gpiod_set_value_cansleep(qcadev->bt_en, 0);
}
@@ -2309,12 +2333,25 @@ static int qca_serdev_probe(struct serdev_device *serdev)
case QCA_WCN6750:
case QCA_WCN6855:
case QCA_WCN7850:
+ case QCA_QCA6390:
qcadev->bt_power = devm_kzalloc(&serdev->dev,
sizeof(struct qca_power),
GFP_KERNEL);
if (!qcadev->bt_power)
return -ENOMEM;
+ break;
+ default:
+ break;
+ }

+ switch (qcadev->btsoc_type) {
+ case QCA_WCN3988:
+ case QCA_WCN3990:
+ case QCA_WCN3991:
+ case QCA_WCN3998:
+ case QCA_WCN6750:
+ case QCA_WCN6855:
+ case QCA_WCN7850:
qcadev->bt_power->dev = &serdev->dev;
err = qca_init_regulators(qcadev->bt_power, data->vregs,
data->num_vregs);
@@ -2360,6 +2397,13 @@ static int qca_serdev_probe(struct serdev_device *serdev)
}
break;

+ case QCA_QCA6390:
+ qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
+ "bluetooth");
+ if (IS_ERR(qcadev->bt_power->pwrseq))
+ return PTR_ERR(qcadev->bt_power->pwrseq);
+ fallthrough;
+
default:
qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
GPIOD_OUT_LOW);

--
2.43.0


2024-05-28 20:05:43

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 09/17] arm64: dts: qcom: qrb5165-rb5: add the Wifi node

From: Bartosz Golaszewski <[email protected]>

Add a node for the PMU module of the QCA6391 present on the RB5 board.
Assign its LDO power outputs to the existing Bluetooth module. Add a
node for the PCIe port to sm8250.dtsi and define the WLAN node on it in
the board's .dts and also make it consume the power outputs of the PMU.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +++++++++++++++++++++++++++----
arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
2 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 70036a95cace..135bb00fe9c8 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -108,6 +108,67 @@ lt9611_3v3: lt9611-3v3 {
regulator-always-on;
};

+ qca6390-pmu {
+ compatible = "qcom,qca6390-pmu";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&bt_en_state>, <&wlan_en_state>;
+
+ vddaon-supply = <&vreg_s6a_0p95>;
+ vddpmu-supply = <&vreg_s2f_0p95>;
+ vddrfa0p95-supply = <&vreg_s2f_0p95>;
+ vddrfa1p3-supply = <&vreg_s8c_1p3>;
+ vddrfa1p9-supply = <&vreg_s5a_1p9>;
+ vddpcie1p3-supply = <&vreg_s8c_1p3>;
+ vddpcie1p9-supply = <&vreg_s5a_1p9>;
+ vddio-supply = <&vreg_s4a_1p8>;
+
+ wlan-enable-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>;
+ bt-enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
+
+ regulators {
+ vreg_pmu_rfa_cmn: ldo0 {
+ regulator-name = "vreg_pmu_rfa_cmn";
+ };
+
+ vreg_pmu_aon_0p59: ldo1 {
+ regulator-name = "vreg_pmu_aon_0p59";
+ };
+
+ vreg_pmu_wlcx_0p8: ldo2 {
+ regulator-name = "vreg_pmu_wlcx_0p8";
+ };
+
+ vreg_pmu_wlmx_0p85: ldo3 {
+ regulator-name = "vreg_pmu_wlmx_0p85";
+ };
+
+ vreg_pmu_btcmx_0p85: ldo4 {
+ regulator-name = "vreg_pmu_btcmx_0p85";
+ };
+
+ vreg_pmu_rfa_0p8: ldo5 {
+ regulator-name = "vreg_pmu_rfa_0p8";
+ };
+
+ vreg_pmu_rfa_1p2: ldo6 {
+ regulator-name = "vreg_pmu_rfa_1p2";
+ };
+
+ vreg_pmu_rfa_1p7: ldo7 {
+ regulator-name = "vreg_pmu_rfa_1p7";
+ };
+
+ vreg_pmu_pcie_0p9: ldo8 {
+ regulator-name = "vreg_pmu_pcie_0p9";
+ };
+
+ vreg_pmu_pcie_1p8: ldo9 {
+ regulator-name = "vreg_pmu_pcie_1p8";
+ };
+ };
+ };
+
thermal-zones {
conn-thermal {
polling-delay-passive = <0>;
@@ -734,6 +795,23 @@ &pcie0_phy {
vdda-pll-supply = <&vreg_l9a_1p2>;
};

+&pcieport0 {
+ wifi@0 {
+ compatible = "pci17cb,1101";
+ reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+ vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+ vddaon-supply = <&vreg_pmu_aon_0p59>;
+ vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
+ vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
+ vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+ vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+ vddrfa1p7-supply = <&vreg_pmu_rfa_1p7>;
+ vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
+ vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
+ };
+};
+
&pcie1 {
status = "okay";
};
@@ -1303,6 +1381,14 @@ sdc2_card_det_n: sd-card-det-n-state {
function = "gpio";
bias-pull-up;
};
+
+ wlan_en_state: wlan-default-state {
+ pins = "gpio20";
+ function = "gpio";
+ drive-strength = <16>;
+ output-low;
+ bias-pull-up;
+ };
};

&uart6 {
@@ -1311,17 +1397,12 @@ &uart6 {
bluetooth {
compatible = "qcom,qca6390-bt";

- pinctrl-names = "default";
- pinctrl-0 = <&bt_en_state>;
-
- enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
-
- vddio-supply = <&vreg_s4a_1p8>;
- vddpmu-supply = <&vreg_s2f_0p95>;
- vddaon-supply = <&vreg_s6a_0p95>;
- vddrfa0p9-supply = <&vreg_s2f_0p95>;
- vddrfa1p3-supply = <&vreg_s8c_1p3>;
- vddrfa1p9-supply = <&vreg_s5a_1p9>;
+ vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
+ vddaon-supply = <&vreg_pmu_aon_0p59>;
+ vddbtcmx-supply = <&vreg_pmu_btcmx_0p85>;
+ vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
+ vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
+ vddrfa1p7-supply = <&vreg_pmu_rfa_1p7>;
};
};

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 759e0822b3ac..b32bd849df44 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2204,7 +2204,7 @@ pcie0: pcie@1c00000 {

status = "disabled";

- pcie@0 {
+ pcieport0: pcie@0 {
device_type = "pci";
reg = <0x0 0x0 0x0 0x0 0x0>;
bus-range = <0x01 0xff>;

--
2.43.0


2024-05-28 20:12:42

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 01/17] regulator: dt-bindings: describe the PMU module of the QCA6390 package

From: Bartosz Golaszewski <[email protected]>

The QCA6390 package contains discreet modules for WLAN and Bluetooth. They
are powered by the Power Management Unit (PMU) that takes inputs from the
host and provides LDO outputs. This document describes this module.

Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Mark Brown <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/regulator/qcom,qca6390-pmu.yaml | 151 +++++++++++++++++++++
1 file changed, 151 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
new file mode 100644
index 000000000000..9d39ff9a75fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/qcom,qca6390-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. QCA6390 PMU Regulators
+
+maintainers:
+ - Bartosz Golaszewski <[email protected]>
+
+description:
+ The QCA6390 package contains discreet modules for WLAN and Bluetooth. They
+ are powered by the Power Management Unit (PMU) that takes inputs from the
+ host and provides LDO outputs. This document describes this module.
+
+properties:
+ compatible:
+ const: qcom,qca6390-pmu
+
+ vddaon-supply:
+ description: VDD_AON supply regulator handle
+
+ vddpmu-supply:
+ description: VDD_PMU supply regulator handle
+
+ vddrfa0p95-supply:
+ description: VDD_RFA_0P95 supply regulator handle
+
+ vddrfa1p3-supply:
+ description: VDD_RFA_1P3 supply regulator handle
+
+ vddrfa1p9-supply:
+ description: VDD_RFA_1P9 supply regulator handle
+
+ vddpcie1p3-supply:
+ description: VDD_PCIE_1P3 supply regulator handle<S-Del>
+
+ vddpcie1p9-supply:
+ description: VDD_PCIE_1P9 supply regulator handle
+
+ vddio-supply:
+ description: VDD_IO supply regulator handle
+
+ wlan-enable-gpios:
+ maxItems: 1
+ description: GPIO line enabling the ATH11K WLAN module supplied by the PMU
+
+ bt-enable-gpios:
+ maxItems: 1
+ description: GPIO line enabling the ATH11K Bluetooth module supplied by the PMU
+
+ regulators:
+ type: object
+ description:
+ LDO outputs of the PMU
+
+ patternProperties:
+ "^ldo[0-9]$":
+ $ref: regulator.yaml#
+ type: object
+ unevaluatedProperties: false
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - regulators
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,qca6390-pmu
+ then:
+ required:
+ - vddaon-supply
+ - vddpmu-supply
+ - vddrfa0p95-supply
+ - vddrfa1p3-supply
+ - vddrfa1p9-supply
+ - vddpcie1p3-supply
+ - vddpcie1p9-supply
+ - vddio-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ pmu {
+ compatible = "qcom,qca6390-pmu";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&bt_en_state>, <&wlan_en_state>;
+
+ vddaon-supply = <&vreg_s6a_0p95>;
+ vddpmu-supply = <&vreg_s2f_0p95>;
+ vddrfa0p95-supply = <&vreg_s2f_0p95>;
+ vddrfa1p3-supply = <&vreg_s8c_1p3>;
+ vddrfa1p9-supply = <&vreg_s5a_1p9>;
+ vddpcie1p3-supply = <&vreg_s8c_1p3>;
+ vddpcie1p9-supply = <&vreg_s5a_1p9>;
+ vddio-supply = <&vreg_s4a_1p8>;
+
+ wlan-enable-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>;
+ bt-enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
+
+ regulators {
+ vreg_pmu_rfa_cmn: ldo0 {
+ regulator-name = "vreg_pmu_rfa_cmn";
+ };
+
+ vreg_pmu_aon_0p59: ldo1 {
+ regulator-name = "vreg_pmu_aon_0p59";
+ };
+
+ vreg_pmu_wlcx_0p8: ldo2 {
+ regulator-name = "vreg_pmu_wlcx_0p8";
+ };
+
+ vreg_pmu_wlmx_0p85: ldo3 {
+ regulator-name = "vreg_pmu_wlmx_0p85";
+ };
+
+ vreg_pmu_btcmx_0p85: ldo4 {
+ regulator-name = "vreg_pmu_btcmx_0p85";
+ };
+
+ vreg_pmu_rfa_0p8: ldo5 {
+ regulator-name = "vreg_pmu_rfa_0p8";
+ };
+
+ vreg_pmu_rfa_1p2: ldo6 {
+ regulator-name = "vreg_pmu_rfa_1p2";
+ };
+
+ vreg_pmu_rfa_1p7: ldo7 {
+ regulator-name = "vreg_pmu_rfa_1p7";
+ };
+
+ vreg_pmu_pcie_0p9: ldo8 {
+ regulator-name = "vreg_pmu_pcie_0p9";
+ };
+
+ vreg_pmu_pcie_1p8: ldo9 {
+ regulator-name = "vreg_pmu_pcie_1p8";
+ };
+ };
+ };

--
2.43.0


2024-05-28 21:11:24

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v8 13/17] PCI/pwrctl: reuse the OF node for power controlled devices

From: Bartosz Golaszewski <[email protected]>

With PCI power control we deal with two struct device objects bound to
two different drivers but consuming the same OF node. We must not bind
the pinctrl twice. To that end: before setting the OF node of the newly
instantiated PCI device, check if a platform device consuming the same
OF node doesn't already exist on the platform bus and - if so - mark the
PCI device as reusing the OF node.

Tested-by: Amit Pundir <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pci/of.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 51e3dd0ea5ab..b908fe1ae951 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -6,6 +6,7 @@
*/
#define pr_fmt(fmt) "PCI: OF: " fmt

+#include <linux/cleanup.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
#include <linux/pci.h>
@@ -13,6 +14,7 @@
#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/platform_device.h>
#include "pci.h"

#ifdef CONFIG_PCI
@@ -25,16 +27,20 @@
*/
int pci_set_of_node(struct pci_dev *dev)
{
- struct device_node *node;
-
if (!dev->bus->dev.of_node)
return 0;

- node = of_pci_find_child_device(dev->bus->dev.of_node, dev->devfn);
+ struct device_node *node __free(device_node) =
+ of_pci_find_child_device(dev->bus->dev.of_node, dev->devfn);
if (!node)
return 0;

- device_set_node(&dev->dev, of_fwnode_handle(node));
+ struct device *pdev __free(put_device) =
+ bus_find_device_by_of_node(&platform_bus_type, node);
+ if (pdev)
+ dev->bus->dev.of_node_reused = true;
+
+ device_set_node(&dev->dev, of_fwnode_handle(no_free_ptr(node)));
return 0;
}


--
2.43.0


2024-05-30 11:02:55

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On 28/05/2024 21:03, Bartosz Golaszewski wrote:
> Note: I am resending this series in its entirety once more for
> discussions and reviews. If there won't be any major objections, I'll
> then start sending individual bits and pieces to appropriate trees.
>
> Merging strategy: The DT binding and DTS changes are a no-brainer, they
> can go through the wireless, regulator and arm-msm trees separately. The
> bluetooth and PCI changes have a build-time dependency on the power
> sequencing code. The bluetooth changes also have a run-time dependency on
> the PCI pwrctl part. In order to get it into next I plan to pick up the
> power sequencing code into my own tree and maintain it. I can then
> provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> stress about the BT runtime dependency as it will be fixed once all
> changes are in next.
>
> The actual cover letter follows:
>
> --
>
> 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.
>
> A similar design with a discreet PMU is also employed in other models of
> the WCN family of chips although we can often do without the delays. With
> this series we add support for the WCN7850 as well.
>
> ===
>
> We introduce a new subsystem here - the power sequencing framework. The
> qcom-wcn driver that we add is its first user. It implements the power-up
> sequences for QCA6390 and WCN7850 chips. However - we only use it to
> power-up the bluetooth module in the former. We use it to driver the WLAN
> modules in both. The reason for this is that for WCN7850 we have
> comprehensive bindings already upstream together with existing DT users.
> Porting them to using the pwrseq subsystem can be done separately and in
> an incremental manner once the subsystem itself is upstream. We will also
> have to ensure backward DT compatibility. To avoid overcomplicating this
> series, let's leave it out for now.
>
> ===
>
> This series is logically split into several sections. I'll go
> patch-by-patch and explain each step.
>
> Patches 1/16-5/16:
>
> These contain all relevant DT bindings changes. We add new documents for
> the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
> for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
> in QCA6390.
>
> Patches 6/16-8/16:
>
> These contain changes to device-tree sources for the three platforms we
> work with in this series. We model the PMUs of the WLAN/BT chips as
> top-level platform devices on the device tree. In order to limit the scope
> of this series and not introduce an excessive amount of confusion with
> deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
> as is (meaning: they continue to consumer the GPIOs and power inputs from
> the host). As the WCN7850 module doesn't require any specific timings, we can
> incrementally change that later.
>
> In both cases we add WLAN nodes that consume the power outputs of the PMU.
> For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
> of the PMU - we can do it as the bindings for this chip did not define any
> supply handles prior to this series meaning we are able to get this correct
> right away.
>
> Patches 9/16-12/16:
>
> 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.
>
> 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).
>
> Patch 13/16 - 14/16:
>
> These add a relatively simple power sequencing subsystem and the first
> driver using it: the pwrseq module for the PMUs on the WCN family of chips.
>
> 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 15:
>
> This patch makes the Qualcomm Bluetooth driver get and use the power
> sequencer for QCA6390.
>
> Patch 16:
>
> 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 and ath12k on WCN7850.
>
> With this series we can now enable BT and WLAN on several new Qualcomm
> boards upstream.
>
> Tested on RB5, sm8650-qrd, sm8650-hdk and sm8550-qrd.
>
> Changelog:
>
> Since v7:
> - added DTS changes for sm8650-hdk
> - added circular dependency detection for pwrseq units
> - fixed a KASAN reported use-after-free error in remove path
> - improve Kconfig descriptions
> - fix typos in bindings and Kconfig
> - fixed issues reported by smatch
> - fix the unbind path in PCI pwrctl
> - lots of minor improvements to the pwrseq core
>
> Since v6:
> - kernel doc fixes
> - drop myself from the DT bindings maintainers list for ath12k
> - wait until the PCI bridge device is fully added before creating the
> PCI pwrctl platform devices for its sub-nodes, otherwise we may see
> sysfs and procfs attribute failures (due to duplication, we're
> basically trying to probe the same device twice at the same time)
> - I kept the regulators for QCA6390's ath11k as required as they only
> apply to this specific Qualcomm package
>
> Since v5:
> - unify the approach to modelling the WCN WLAN/BT chips by always exposing
> the PMU node on the device tree and making the WLAN and BT nodes become
> consumers of its power outputs; this includes a major rework of the DT
> sources, bindings and driver code; there's no more a separate PCI
> pwrctl driver for WCN7850, instead its power-up sequence was moved
> into the pwrseq driver common for all WCN chips
> - don't set load_uA from new regulator consumers
> - fix reported kerneldoc issues
> - drop voltage ranges for PMU outputs from DT
> - many minor tweaks and reworks
>
> v1: Original RFC:
>
> https://lore.kernel.org/lkml/[email protected]/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/[email protected]/T/
>
> v5: Two different ways of handling QCA6390 and WCN7850:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> Bartosz Golaszewski (16):
> regulator: dt-bindings: describe the PMU module of the QCA6390 package
> regulator: dt-bindings: describe the PMU module of the WCN7850 package
> dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
> dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
> dt-bindings: net: wireless: describe the ath12k PCI module
> arm64: dts: qcom: sm8550-qrd: add the Wifi node
> arm64: dts: qcom: sm8650-qrd: add the Wifi node
> arm64: dts: qcom: qrb5165-rb5: add the Wifi node
> power: sequencing: implement the pwrseq core
> power: pwrseq: add a driver for the PMU module on the QCom WCN chipsets
> 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 PCI power control driver for power sequenced devices
> Bluetooth: qca: use the power sequencer for QCA6390
>
> Neil Armstrong (1):
> arm64: dts: qcom: sm8650-hdk: add the Wifi node
>
> .../bindings/net/bluetooth/qualcomm-bluetooth.yaml | 17 +
> .../bindings/net/wireless/qcom,ath11k-pci.yaml | 46 +
> .../bindings/net/wireless/qcom,ath12k.yaml | 99 ++
> .../bindings/regulator/qcom,qca6390-pmu.yaml | 185 ++++
> MAINTAINERS | 8 +
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +-
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 89 ++
> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
> drivers/bluetooth/hci_qca.c | 74 +-
> 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 | 17 +
> drivers/pci/pwrctl/Makefile | 6 +
> drivers/pci/pwrctl/core.c | 137 +++
> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++
> drivers/pci/remove.c | 3 +-
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/sequencing/Kconfig | 28 +
> drivers/power/sequencing/Makefile | 6 +
> drivers/power/sequencing/core.c | 1105 ++++++++++++++++++++
> drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++
> include/linux/pci-pwrctl.h | 51 +
> include/linux/pwrseq/consumer.h | 56 +
> include/linux/pwrseq/provider.h | 75 ++
> 32 files changed, 2717 insertions(+), 34 deletions(-)
> ---
> base-commit: 6dc544b66971c7f9909ff038b62149105272d26a
> change-id: 20240527-pwrseq-76fc025248a2
>
> Best regards,

Tested-by: Neil Armstrong <[email protected]> # on SM8550-QRD
Tested-by: Neil Armstrong <[email protected]> # on SM8650-QRD
Tested-by: Neil Armstrong <[email protected]> # on SM8650-HDK

Thanks,
Neil

2024-06-04 17:20:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On Tue, May 28, 2024 at 09:03:08PM +0200, Bartosz Golaszewski wrote:
> Note: I am resending this series in its entirety once more for
> discussions and reviews. If there won't be any major objections, I'll
> then start sending individual bits and pieces to appropriate trees.
>
> Merging strategy: The DT binding and DTS changes are a no-brainer, they
> can go through the wireless, regulator and arm-msm trees separately. The
> bluetooth and PCI changes have a build-time dependency on the power
> sequencing code. The bluetooth changes also have a run-time dependency on
> the PCI pwrctl part. In order to get it into next I plan to pick up the
> power sequencing code into my own tree and maintain it. I can then
> provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> stress about the BT runtime dependency as it will be fixed once all
> changes are in next.
> ...

> ---
> base-commit: 6dc544b66971c7f9909ff038b62149105272d26a
> change-id: 20240527-pwrseq-76fc025248a2

What does this apply to? I don't know what 6dc544b66971 is; it
doesn't seem to be in upstream or linux-next.

2024-06-04 17:30:30

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 01/17] regulator: dt-bindings: describe the PMU module of the QCA6390 package

On Tue, May 28, 2024 at 09:03:09PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> The QCA6390 package contains discreet modules for WLAN and Bluetooth. They

s/discreet/discrete/

> are powered by the Power Management Unit (PMU) that takes inputs from the
> host and provides LDO outputs. This document describes this module.

LDO? Again below, but maybe this is obvious to everybody.

"This document describes this module" seems possibly unnecessary.

> +description:
> + The QCA6390 package contains discreet modules for WLAN and Bluetooth. They

s/discreet/discrete/

> + are powered by the Power Management Unit (PMU) that takes inputs from the
> + host and provides LDO outputs. This document describes this module.

> + vddpcie1p3-supply:
> + description: VDD_PCIE_1P3 supply regulator handle<S-Del>

s/<S-Del>// ?

2024-06-04 17:46:58

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 10/17] power: sequencing: implement the pwrseq core

On Tue, May 28, 2024 at 09:03:18PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Implement the power sequencing subsystem allowing devices to share
> complex powering-up and down procedures. It's split into the consumer
> and provider parts but does not implement any new DT bindings so that
> the actual power sequencing is never revealed in the DT representation.

> +++ b/drivers/power/sequencing/core.c

> + * Unit - a unit is a discreet chunk of a power sequence. For instance one unit

s/discreet/discrete/

> +static struct pwrseq_unit *pwrseq_unit_incref(struct pwrseq_unit *unit)
> +{
> + kref_get(&unit->ref);
> +
> + return unit;
> +}
> +
> +static void pwrseq_unit_release(struct kref *ref);
> +
> +static void pwrseq_unit_decref(struct pwrseq_unit *unit)
> +{
> + kref_put(&unit->ref, pwrseq_unit_release);
> +}

No existing callers of kref_get() and kref_put() use names that
include "incref" or "decref". Many include "get" and "put", so maybe
there would be some value in using that pattern?

2024-06-04 20:34:51

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On Tue, Jun 4, 2024 at 7:19 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Tue, May 28, 2024 at 09:03:08PM +0200, Bartosz Golaszewski wrote:
> > Note: I am resending this series in its entirety once more for
> > discussions and reviews. If there won't be any major objections, I'll
> > then start sending individual bits and pieces to appropriate trees.
> >
> > Merging strategy: The DT binding and DTS changes are a no-brainer, they
> > can go through the wireless, regulator and arm-msm trees separately. The
> > bluetooth and PCI changes have a build-time dependency on the power
> > sequencing code. The bluetooth changes also have a run-time dependency on
> > the PCI pwrctl part. In order to get it into next I plan to pick up the
> > power sequencing code into my own tree and maintain it. I can then
> > provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> > stress about the BT runtime dependency as it will be fixed once all
> > changes are in next.
> > ...
>
> > ---
> > base-commit: 6dc544b66971c7f9909ff038b62149105272d26a
> > change-id: 20240527-pwrseq-76fc025248a2
>
> What does this apply to? I don't know what 6dc544b66971 is; it
> doesn't seem to be in upstream or linux-next.

It's next-20240528 but it also applies to today's next without
conflicts. What do you want me to base the PCI part when resending?

Bart

2024-06-04 23:35:08

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Wed, 5 Jun 2024 at 02:23, Bjorn Helgaas <[email protected]> wrote:
>
> On Tue, May 28, 2024 at 09:03:24PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > Add a PCI power control driver that's capable of correctly powering up
> > devices using the power sequencing subsystem. The first users of this
> > driver are the ath11k module on QCA6390 and ath12k on WCN7850.
>
> Can you add a little detail here about what benefit we will see from
> this driver? E.g., something that doesn't work correctly now, but
> will work with this driver?
>
> > +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> > + {
> > + /* ATH11K in QCA6390 package. */
> > + .compatible = "pci17cb,1101",
> > + .data = "wlan",
> > + },
> > + {
> > + /* ATH12K in WCN7850 package. */
> > + .compatible = "pci17cb,1107",
> > + .data = "wlan",
> > + },
>
> IIUC, "pci17cb,1101" and "pci17cb,1107" exist partly so we can check
> that a DTS conforms to the schema, e.g., a "pci17cb,1101" node
> contains all the required regulators. For that use, we obviously need
> a very specific "compatible" string.
>
> Is there any opportunity to add a more generic "compatible" string in
> addition to those so this list doesn't have to be updated for every
> PMU? The .data here is "wlan" in both cases, and for this purpose, we
> don't care whether it's "pci17cb,1101" or "pci17cb,1107".

These two devices have different set of regulators and different
requirements to power them on.

--
With best wishes
Dmitry

2024-06-05 02:14:31

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Wed, Jun 05, 2024 at 02:34:52AM +0300, Dmitry Baryshkov wrote:
> On Wed, 5 Jun 2024 at 02:23, Bjorn Helgaas <[email protected]> wrote:
> >
> > On Tue, May 28, 2024 at 09:03:24PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > Add a PCI power control driver that's capable of correctly powering up
> > > devices using the power sequencing subsystem. The first users of this
> > > driver are the ath11k module on QCA6390 and ath12k on WCN7850.

> > > +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> > > + {
> > > + /* ATH11K in QCA6390 package. */
> > > + .compatible = "pci17cb,1101",
> > > + .data = "wlan",
> > > + },
> > > + {
> > > + /* ATH12K in WCN7850 package. */
> > > + .compatible = "pci17cb,1107",
> > > + .data = "wlan",
> > > + },
> >
> > IIUC, "pci17cb,1101" and "pci17cb,1107" exist partly so we can check
> > that a DTS conforms to the schema, e.g., a "pci17cb,1101" node
> > contains all the required regulators. For that use, we obviously need
> > a very specific "compatible" string.
> >
> > Is there any opportunity to add a more generic "compatible" string in
> > addition to those so this list doesn't have to be updated for every
> > PMU? The .data here is "wlan" in both cases, and for this purpose, we
> > don't care whether it's "pci17cb,1101" or "pci17cb,1107".
>
> These two devices have different set of regulators and different
> requirements to power them on.

Right, but I don't think pci_pwrctl_pwrseq_probe() knows about those
different sets. It basically looks like:

pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
{
struct pci_pwrctl_pwrseq_data *data;
struct device *dev = &pdev->dev;

data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
pwrseq_power_on(data->pwrseq);
data->ctx.dev = dev;
devm_pci_pwrctl_device_set_ready(dev, &data->ctx);
}

I think of_device_get_match_data(dev) will return "wlan" for both
"pci17cb,1101" and "pci17cb,1107", so devm_pwrseq_get(),
pwrseq_power_on(), and devm_pci_pwrctl_device_set_ready() don't see
the distinction between them.

Of course, they also get "dev", so they can find the device-specifc
stuff that way, but I think that's on the drivers/power/sequencing/
side, not in this pci-pwrctl-pwrseq driver itself.

So what if there were a more generic "compatible" string, e.g., if the
DT contained something like this:

wifi@0 {
compatible = "pci17cb,1101", "wlan-pwrseq";
...
}

and pci_pwrctl_pwrseq_of_match[] had this:

{ .compatible = "wlan-pwrseq", .data = "wlan", }

Wouldn't this pci-pwrctl-pwrseq driver work the same? I'm not a DT
whiz, so likely I'm missing something, but it would be nice if we
didn't have to update this very generic-looking driver to add every
device that needs it.

Bjorn

2024-06-05 06:53:39

by Ratheesh Kannoth

[permalink] [raw]
Subject: Re: [PATCH v8 10/17] power: sequencing: implement the pwrseq core

On 2024-05-29 at 00:33:18, Bartosz Golaszewski ([email protected]) wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Implement the power sequencing subsystem allowing devices to share
> complex powering-up and down procedures. It's split into the consumer
> and provider parts but does not implement any new DT bindings so that
> the actual power sequencing is never revealed in the DT representation.
>
> Tested-by: Amit Pundir <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> MAINTAINERS | 8 +
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/sequencing/Kconfig | 12 +
> drivers/power/sequencing/Makefile | 4 +
> drivers/power/sequencing/core.c | 1105 +++++++++++++++++++++++++++++++++++++
> include/linux/pwrseq/consumer.h | 56 ++
> include/linux/pwrseq/provider.h | 75 +++
> 8 files changed, 1262 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dbc5d9ec3d20..dd129735e7c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17893,6 +17893,14 @@ F: include/linux/pm_*
> F: include/linux/powercap.h
> F: kernel/configs/nopm.config
>
> +POWER SEQUENCING
> +M: Bartosz Golaszewski <[email protected]>
> +L: [email protected]
> +S: Maintained
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
> +F: drivers/power/sequencing/
> +F: include/linux/pwrseq/
> +
> POWER STATE COORDINATION INTERFACE (PSCI)
> M: Mark Rutland <[email protected]>
> M: Lorenzo Pieralisi <[email protected]>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 696bf77a7042..9a8e44ca9ae4 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> source "drivers/power/reset/Kconfig"
> +source "drivers/power/sequencing/Kconfig"
> source "drivers/power/supply/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index effbf0377f32..962a2cd30a51 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_POWER_RESET) += reset/
> +obj-$(CONFIG_POWER_SEQUENCING) += sequencing/
> obj-$(CONFIG_POWER_SUPPLY) += supply/
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> new file mode 100644
> index 000000000000..ba5732b1dbf8
> --- /dev/null
> +++ b/drivers/power/sequencing/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +menuconfig POWER_SEQUENCING
> + tristate "Power Sequencing support"
> + help
> + Say Y here to enable the Power Sequencing subsystem.
> +
> + This subsystem is designed to control power to devices that share
> + complex resources and/or require specific power sequences to be run
> + during power-up.
> +
> + If unsure, say no.
> diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
> new file mode 100644
> index 000000000000..dcdf8c0c159e
> --- /dev/null
> +++ b/drivers/power/sequencing/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_POWER_SEQUENCING) += pwrseq-core.o
> +pwrseq-core-y := core.o
> diff --git a/drivers/power/sequencing/core.c b/drivers/power/sequencing/core.c
> new file mode 100644
> index 000000000000..e07037ea5be0
> --- /dev/null
> +++ b/drivers/power/sequencing/core.c
> @@ -0,0 +1,1105 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Linaro Ltd.
> + */
> +
> +#include <linux/bug.h>
> +#include <linux/cleanup.h>
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/idr.h>
> +#include <linux/kernel.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/lockdep.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/pwrseq/consumer.h>
> +#include <linux/pwrseq/provider.h>
> +#include <linux/radix-tree.h>
> +#include <linux/rwsem.h>
> +#include <linux/slab.h>
> +
> +/*
> + * Power-sequencing framework for linux.
> + *
> + * This subsystem allows power sequence providers to register a set of targets
> + * that consumers may request and power-up/down.
> + *
> + * Glossary:
> + *
> + * Unit - a unit is a discreet chunk of a power sequence. For instance one unit
> + * may enable a set of regulators, another may enable a specific GPIO. Units
> + * can define dependencies in the form of other units that must be enabled
> + * before it itself can be.
> + *
> + * Target - a target is a set of units (composed of the "final" unit and its
> + * dependencies) that a consumer selects by its name when requesting a handle
> + * to the power sequencer. Via the dependency system, multiple targets may
> + * share the same parts of a power sequence but ignore parts that are
> + * irrelevant.
> + *
> + * Descriptor - a handle passed by the pwrseq core to every consumer that
> + * serves as the entry point to the provider layer. It ensures coherence
> + * between different users and keeps reference counting consistent.
> + *
> + * Each provider must define a .match() callback whose role is to determine
> + * whether a potential consumer is in fact associated with this sequencer.
> + * This allows creating abstraction layers on top of regular device-tree
> + * resources like regulators, clocks and other nodes connected to the consumer
> + * via phandle.
> + */
> +
> +static DEFINE_IDA(pwrseq_ida);
> +
> +/*
> + * Protects the device list on the pwrseq bus from concurrent modifications
> + * but allows simultaneous read-only access.
> + */
> +static DECLARE_RWSEM(pwrseq_sem);
> +
> +/**
> + * struct pwrseq_unit - Private power-sequence unit data.
> + * @ref: Reference count for this object. When it goes to 0, the object is
> + * destroyed.
> + * @name: Name of this target.
> + * @list: Link to siblings on the list of all units of a single sequencer.
> + * @deps: List of units on which this unit depends.
> + * @enable: Callback running the part of the power-on sequence provided by
> + * this unit.
> + * @disable: Callback running the part of the power-off sequence provided
> + * by this unit.
> + * @enable_count: Current number of users that enabled this unit. May be the
> + * consumer of the power sequencer or other units that depend
> + * on this one.
> + */
> +struct pwrseq_unit {
> + struct kref ref;
> + const char *name;
> + struct list_head list;
> + struct list_head deps;
> + pwrseq_power_state_func enable;
> + pwrseq_power_state_func disable;
> + unsigned int enable_count;
> +};
> +
> +static struct pwrseq_unit *pwrseq_unit_new(const struct pwrseq_unit_data *data)
> +{
> + struct pwrseq_unit *unit;
> +
> + unit = kzalloc(sizeof(*unit), GFP_KERNEL);
> + if (!unit)
> + return NULL;
> +
> + unit->name = kstrdup_const(data->name, GFP_KERNEL);
> + if (!unit->name) {
> + kfree(unit);
> + return NULL;
> + }
> +
> + kref_init(&unit->ref);
> + INIT_LIST_HEAD(&unit->deps);
> + unit->enable = data->enable;
> + unit->disable = data->disable;
> +
> + return unit;
> +}
> +
> +static struct pwrseq_unit *pwrseq_unit_incref(struct pwrseq_unit *unit)
> +{
> + kref_get(&unit->ref);
> +
> + return unit;
> +}
> +
> +static void pwrseq_unit_release(struct kref *ref);
> +
> +static void pwrseq_unit_decref(struct pwrseq_unit *unit)
> +{
> + kref_put(&unit->ref, pwrseq_unit_release);
> +}
> +
> +/**
> + * struct pwrseq_unit_dep - Wrapper around a reference to the unit structure
> + * allowing to keep it on multiple dependency lists
> + * in different units.
> + * @list: Siblings on the list.
> + * @unit: Address of the referenced unit.
> + */
> +struct pwrseq_unit_dep {
> + struct list_head list;
> + struct pwrseq_unit *unit;
> +};
> +
> +static struct pwrseq_unit_dep *pwrseq_unit_dep_new(struct pwrseq_unit *unit)
nit. pwrseq_unit_dep_alloc/create rhymes well with pwrseq_unit_dep_free(),
> +{
> + struct pwrseq_unit_dep *dep;
> +
> + dep = kzalloc(sizeof(*dep), GFP_KERNEL);
> + if (!dep)
> + return NULL;
> +
> + dep->unit = unit;
> +
> + return dep;
> +}
> +
> +static void pwrseq_unit_dep_free(struct pwrseq_unit_dep *ref)
> +{
> + pwrseq_unit_decref(ref->unit);
> + kfree(ref);
> +}
> +
> +static void pwrseq_unit_free_deps(struct list_head *list)
> +{
> + struct pwrseq_unit_dep *dep, *next;
> +
> + list_for_each_entry_safe(dep, next, list, list) {
no need of 'locks' to protect against simutaneous 'add' ?
> + list_del(&dep->list);
> + pwrseq_unit_dep_free(dep);
> + }
> +}
> +
> +static void pwrseq_unit_release(struct kref *ref)
> +{
> + struct pwrseq_unit *unit = container_of(ref, struct pwrseq_unit, ref);
> +
> + pwrseq_unit_free_deps(&unit->deps);
> + list_del(&unit->list);
> + kfree_const(unit->name);
> + kfree(unit);
> +}
> +
> +/**
> + * struct pwrseq_target - Private power-sequence target data.
> + * @list: Siblings on the list of all targets exposed by a power sequencer.
> + * @name: Name of the target.
> + * @unit: Final unit for this target.
> + * @post_enable: Callback run after the target unit has been enabled, *after*
> + * the state lock has been released. It's useful for implementing
> + * boot-up delays without blocking other users from powering up
> + * using the same power sequencer.
> + */
> +struct pwrseq_target {
> + struct list_head list;
> + const char *name;
> + struct pwrseq_unit *unit;
> + pwrseq_power_state_func post_enable;
> +};
> +
> +static struct pwrseq_target *
> +pwrseq_target_new(const struct pwrseq_target_data *data)
> +{
> + struct pwrseq_target *target;
> +
> + target = kzalloc(sizeof(*target), GFP_KERNEL);
> + if (!target)
> + return NULL;
> +
> + target->name = kstrdup_const(data->name, GFP_KERNEL);
> + if (!target->name) {
> + kfree(target);
> + return NULL;
> + }
> +
> + target->post_enable = data->post_enable;
> +
> + return target;
> +}
> +
> +static void pwrseq_target_free(struct pwrseq_target *target)
> +{
> + pwrseq_unit_decref(target->unit);
> + kfree_const(target->name);
> + kfree(target);
> +}
> +
> +/**
> + * struct pwrseq_device - Private power sequencing data.
> + * @dev: Device struct associated with this sequencer.
> + * @id: Device ID.
> + * @owner: Prevents removal of active power sequencing providers.
> + * @rw_lock: Protects the device from being unregistered while in use.
> + * @state_lock: Prevents multiple users running the power sequence at the same
> + * time.
> + * @match: Power sequencer matching callback.
> + * @targets: List of targets exposed by this sequencer.
> + * @units: List of all units supported by this sequencer.
> + */
> +struct pwrseq_device {
> + struct device dev;
> + int id;
> + struct module *owner;
> + struct rw_semaphore rw_lock;
> + struct mutex state_lock;
> + pwrseq_match_func match;
> + struct list_head targets;
> + struct list_head units;
> +};
> +
> +static struct pwrseq_device *to_pwrseq_device(struct device *dev)
> +{
> + return container_of(dev, struct pwrseq_device, dev);
> +}
> +
> +static struct pwrseq_device *pwrseq_device_get(struct pwrseq_device *pwrseq)
> +{
> + get_device(&pwrseq->dev);
> +
> + return pwrseq;
> +}
> +
> +static void pwrseq_device_put(struct pwrseq_device *pwrseq)
> +{
> + put_device(&pwrseq->dev);
> +}
> +
> +/**
> + * struct pwrseq_desc - Wraps access to the pwrseq_device and ensures that one
> + * user cannot break the reference counting for others.
> + * @pwrseq: Reference to the power sequencing device.
> + * @target: Reference to the target this descriptor allows to control.
> + * @powered_on: Power state set by the holder of the descriptor (not necessarily
> + * corresponding to the actual power state of the device).
> + */
> +struct pwrseq_desc {
> + struct pwrseq_device *pwrseq;
> + struct pwrseq_target *target;
> + bool powered_on;
> +};
> +
> +static const struct bus_type pwrseq_bus = {
> + .name = "pwrseq",
> +};
> +
> +static void pwrseq_release(struct device *dev)
> +{
> + struct pwrseq_device *pwrseq = to_pwrseq_device(dev);
> + struct pwrseq_target *target, *pos;
> +
> + list_for_each_entry_safe(target, pos, &pwrseq->targets, list) {
> + list_del(&target->list);
> + pwrseq_target_free(target);
> + }
> +
> + mutex_destroy(&pwrseq->state_lock);
> + ida_free(&pwrseq_ida, pwrseq->id);
> + kfree(pwrseq);
> +}
> +
> +static const struct device_type pwrseq_device_type = {
> + .name = "power_sequencer",
> + .release = pwrseq_release,
> +};
> +
> +static int pwrseq_check_unit_deps(const struct pwrseq_unit_data *data,
> + struct radix_tree_root *visited_units)
> +{
> + const struct pwrseq_unit_data *tmp, **cur;
> + int ret;
> +
> + ret = radix_tree_insert(visited_units, (unsigned long)data,
> + (void *)data);
> + if (ret)
> + return ret;
> +
> + for (cur = data->deps; cur && *cur; cur++) {
> + tmp = radix_tree_lookup(visited_units, (unsigned long)*cur);
> + if (tmp) {
> + WARN(1, "Circular dependency in power sequencing flow detected!\n");
> + return -EINVAL;
> + }
> +
> + ret = pwrseq_check_unit_deps(*cur, visited_units);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int pwrseq_check_target_deps(const struct pwrseq_target_data *data)
> +{
> + struct radix_tree_root visited_units;
> + struct radix_tree_iter iter;
> + void __rcu **slot;
> + int ret;
> +
> + if (!data->unit)
> + return -EINVAL;
> +
> + INIT_RADIX_TREE(&visited_units, GFP_KERNEL);
> + ret = pwrseq_check_unit_deps(data->unit, &visited_units);
> + radix_tree_for_each_slot(slot, &visited_units, &iter, 0)
> + radix_tree_delete(&visited_units, iter.index);
> +
> + return ret;
> +}
> +
> +static int pwrseq_unit_setup_deps(const struct pwrseq_unit_data **data,
> + struct list_head *dep_list,
> + struct list_head *unit_list,
> + struct radix_tree_root *processed_units);
> +
> +static struct pwrseq_unit *
> +pwrseq_unit_setup(const struct pwrseq_unit_data *data,
> + struct list_head *unit_list,
> + struct radix_tree_root *processed_units)
> +{
> + struct pwrseq_unit *unit;
> + int ret;
> +
> + unit = radix_tree_lookup(processed_units, (unsigned long)data);
> + if (unit)
> + return pwrseq_unit_incref(unit);
> +
> + unit = pwrseq_unit_new(data);
> + if (!unit)
> + return ERR_PTR(-ENOMEM);
> +
> + if (data->deps) {
> + ret = pwrseq_unit_setup_deps(data->deps, &unit->deps,
> + unit_list, processed_units);
> + if (ret) {
> + pwrseq_unit_decref(unit);
> + return ERR_PTR(ret);
> + }
> + }
> +
> + ret = radix_tree_insert(processed_units, (unsigned long)data, unit);
> + if (ret) {
> + pwrseq_unit_decref(unit);
> + return ERR_PTR(ret);
> + }
> +
> + list_add_tail(&unit->list, unit_list);
> +
> + return unit;
> +}
> +
> +static int pwrseq_unit_setup_deps(const struct pwrseq_unit_data **data,
> + struct list_head *dep_list,
> + struct list_head *unit_list,
> + struct radix_tree_root *processed_units)
> +{
> + const struct pwrseq_unit_data *pos;
> + struct pwrseq_unit_dep *dep;
> + struct pwrseq_unit *unit;
> + int i;
> +
> + for (i = 0; data[i]; i++) {
Can we add range for i ? just depending on data[i] to be zero looks to be risky.

> + pos = data[i];
> +
> + unit = pwrseq_unit_setup(pos, unit_list, processed_units);
> + if (IS_ERR(unit))
> + return PTR_ERR(unit);
> +
> + dep = pwrseq_unit_dep_new(unit);
> + if (!dep) {
> + pwrseq_unit_decref(unit);
This frees only one 'unit'. is there any chance for multiple 'unit', then better clean
up here ?
> + return -ENOMEM;
> + }
> +
> + list_add_tail(&dep->list, dep_list);
> + }
> +
> + return 0;
> +}
> +
> +static int pwrseq_do_setup_targets(const struct pwrseq_target_data **data,
> + struct pwrseq_device *pwrseq,
> + struct radix_tree_root *processed_units)
> +{
> + const struct pwrseq_target_data *pos;
> + struct pwrseq_target *target;
> + int ret, i;
> +
> + for (i = 0; data[i]; i++) {
> + pos = data[i];
> +
> + ret = pwrseq_check_target_deps(pos);
> + if (ret)
> + return ret;
> +
> + target = pwrseq_target_new(pos);
> + if (!target)
> + return -ENOMEM;
> +
> + target->unit = pwrseq_unit_setup(pos->unit, &pwrseq->units,
> + processed_units);
> + if (IS_ERR(target->unit)) {
> + ret = PTR_ERR(target->unit);
> + pwrseq_target_free(target);
> + return ret;
> + }
> +
> + list_add_tail(&target->list, &pwrseq->targets);
> + }
> +
> + return 0;
> +}
> +
> +static int pwrseq_setup_targets(const struct pwrseq_target_data **targets,
> + struct pwrseq_device *pwrseq)
> +{
> + struct radix_tree_root processed_units;
> + struct radix_tree_iter iter;
> + void __rcu **slot;
> + int ret;
> +
> + INIT_RADIX_TREE(&processed_units, GFP_KERNEL);
> + ret = pwrseq_do_setup_targets(targets, pwrseq, &processed_units);
> + radix_tree_for_each_slot(slot, &processed_units, &iter, 0)
> + radix_tree_delete(&processed_units, iter.index);
> +
> + return ret;
> +}
> +
> +/**
> + * pwrseq_device_register() - Register a new power sequencer.
> + * @config: Configuration of the new power sequencing device.
> + *
> + * The config structure is only used during the call and can be freed after
> + * the function returns. The config structure *must* have the parent device
> + * as well as the match() callback and at least one target set.
> + *
> + * Returns:
> + * Returns the address of the new pwrseq device or ERR_PTR() on failure.
> + */
> +struct pwrseq_device *
> +pwrseq_device_register(const struct pwrseq_config *config)
> +{
> + struct pwrseq_device *pwrseq;
> + int ret, id;
> +
> + if (!config->parent || !config->match || !config->targets ||
> + !config->targets[0])
> + return ERR_PTR(-EINVAL);
> +
> + pwrseq = kzalloc(sizeof(*pwrseq), GFP_KERNEL);
> + if (!pwrseq)
> + return ERR_PTR(-ENOMEM);
> +
> + pwrseq->dev.type = &pwrseq_device_type;
> + pwrseq->dev.bus = &pwrseq_bus;
> + pwrseq->dev.parent = config->parent;
> + device_set_node(&pwrseq->dev, dev_fwnode(config->parent));
> + dev_set_drvdata(&pwrseq->dev, config->drvdata);
> +
> + id = ida_alloc(&pwrseq_ida, GFP_KERNEL);
> + if (id < 0) {
> + kfree(pwrseq);
> + return ERR_PTR(id);
> + }
> +
> + pwrseq->id = id;
> +
> + /*
> + * From this point onwards the device's release() callback is
> + * responsible for freeing resources.
> + */
> + device_initialize(&pwrseq->dev);
> +
> + ret = dev_set_name(&pwrseq->dev, "pwrseq.%d", pwrseq->id);
> + if (ret)
> + goto err_put_pwrseq;
> +
> + pwrseq->owner = config->owner ?: THIS_MODULE;
> + pwrseq->match = config->match;
> +
> + init_rwsem(&pwrseq->rw_lock);
> + mutex_init(&pwrseq->state_lock);
> + INIT_LIST_HEAD(&pwrseq->targets);
> + INIT_LIST_HEAD(&pwrseq->units);
> +
> + ret = pwrseq_setup_targets(config->targets, pwrseq);
> + if (ret)
> + goto err_put_pwrseq;
> +
> + scoped_guard(rwsem_write, &pwrseq_sem) {
> + ret = device_add(&pwrseq->dev);
> + if (ret)
> + goto err_put_pwrseq;
> + }
> +
> + return pwrseq;
> +
> +err_put_pwrseq:
no need to kfree(pwrseq) ?
> + pwrseq_device_put(pwrseq);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_device_register);
> +
> +/**
> + * pwrseq_device_unregister() - Unregister the power sequencer.
> + * @pwrseq: Power sequencer to unregister.
> + */
> +void pwrseq_device_unregister(struct pwrseq_device *pwrseq)
> +{
> + struct device *dev = &pwrseq->dev;
> + struct pwrseq_target *target;
> +
> + scoped_guard(mutex, &pwrseq->state_lock) {
> + guard(rwsem_write)(&pwrseq->rw_lock);
> +
> + list_for_each_entry(target, &pwrseq->targets, list)
> + WARN_ONCE(target->unit->enable_count,
> + "REMOVING POWER SEQUENCER WITH ACTIVE USERS\n");
> +
> + guard(rwsem_write)(&pwrseq_sem);
> +
> + device_del(dev);
> + }
> +
> + pwrseq_device_put(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_device_unregister);
> +
> +static void devm_pwrseq_device_unregister(void *data)
> +{
> + struct pwrseq_device *pwrseq = data;
> +
> + pwrseq_device_unregister(pwrseq);
> +}
> +
> +/**
> + * devm_pwrseq_device_register() - Managed variant of pwrseq_device_register().
> + * @dev: Managing device.
> + * @config: Configuration of the new power sequencing device.
> + *
> + * Returns:
> + * Returns the address of the new pwrseq device or ERR_PTR() on failure.
> + */
> +struct pwrseq_device *
> +devm_pwrseq_device_register(struct device *dev,
> + const struct pwrseq_config *config)
> +{
> + struct pwrseq_device *pwrseq;
> + int ret;
> +
> + pwrseq = pwrseq_device_register(config);
> + if (IS_ERR(pwrseq))
> + return pwrseq;
> +
> + ret = devm_add_action_or_reset(dev, devm_pwrseq_device_unregister,
> + pwrseq);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return pwrseq;
> +}
> +EXPORT_SYMBOL_GPL(devm_pwrseq_device_register);
> +
> +/**
> + * pwrseq_device_get_drvdata() - Get the driver private data associated with
> + * this sequencer.
> + * @pwrseq: Power sequencer object.
> + *
> + * Returns:
> + * Address of the private driver data.
> + */
> +void *pwrseq_device_get_drvdata(struct pwrseq_device *pwrseq)
> +{
> + return dev_get_drvdata(&pwrseq->dev);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_device_get_drvdata);
> +
> +struct pwrseq_match_data {
> + struct pwrseq_desc *desc;
> + struct device *dev;
> + const char *target;
> +};
> +
> +static int pwrseq_match_device(struct device *pwrseq_dev, void *data)
> +{
> + struct pwrseq_device *pwrseq = to_pwrseq_device(pwrseq_dev);
> + struct pwrseq_match_data *match_data = data;
> + struct pwrseq_target *target;
> + int ret;
> +
> + lockdep_assert_held_read(&pwrseq_sem);
> +
> + guard(rwsem_read)(&pwrseq->rw_lock);
> + if (!device_is_registered(&pwrseq->dev))
> + return 0;
> +
> + ret = pwrseq->match(pwrseq, match_data->dev);
> + if (ret <= 0)
> + return ret;
> +
> + /* We got the matching device, let's find the right target. */
> + list_for_each_entry(target, &pwrseq->targets, list) {
> + if (strcmp(target->name, match_data->target))
> + continue;
> +
> + match_data->desc->target = target;
> + }
> +
> + /*
> + * This device does not have this target. No point in deferring as it
> + * will not get a new target dynamically later.
> + */
> + if (!match_data->desc->target)
> + return -ENOENT;
> +
> + if (!try_module_get(pwrseq->owner))
> + return -EPROBE_DEFER;
> +
> + match_data->desc->pwrseq = pwrseq_device_get(pwrseq);
> +
> + return 1;
> +}
> +
> +/**
> + * pwrseq_get() - Get the power sequencer associated with this device.
> + * @dev: Device for which to get the sequencer.
> + * @target: Name of the target exposed by the sequencer this device wants to
> + * reach.
> + *
> + * Returns:
> + * New power sequencer descriptor for use by the consumer driver or ERR_PTR()
> + * on failure.
> + */
> +struct pwrseq_desc *pwrseq_get(struct device *dev, const char *target)
> +{
> + struct pwrseq_match_data match_data;
> + int ret;
> +
> + struct pwrseq_desc *desc __free(kfree) = kzalloc(sizeof(*desc),
> + GFP_KERNEL);
> + if (!desc)
> + return ERR_PTR(-ENOMEM);
> +
> + match_data.desc = desc;
> + match_data.dev = dev;
> + match_data.target = target;
> +
> + guard(rwsem_read)(&pwrseq_sem);
> +
> + ret = bus_for_each_dev(&pwrseq_bus, NULL, &match_data,
> + pwrseq_match_device);
> + if (ret < 0)
> + return ERR_PTR(ret);
> + if (ret == 0)
> + /* No device matched. */
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + return no_free_ptr(desc);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_get);
> +
> +/**
> + * pwrseq_put() - Release the power sequencer descriptor.
> + * @desc: Descriptor to release.
> + */
> +void pwrseq_put(struct pwrseq_desc *desc)
> +{
> + struct pwrseq_device *pwrseq;
> +
> + if (!desc)
> + return;
> +
> + pwrseq = desc->pwrseq;
> +
> + if (desc->powered_on)
> + pwrseq_power_off(desc);
> +
> + kfree(desc);
> + module_put(pwrseq->owner);
> + pwrseq_device_put(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_put);
> +
> +static void devm_pwrseq_put(void *data)
> +{
> + struct pwrseq_desc *desc = data;
> +
> + pwrseq_put(desc);
> +}
> +
> +/**
> + * devm_pwrseq_get() - Managed variant of pwrseq_get().
> + * @dev: Device for which to get the sequencer and which also manages its
> + * lifetime.
> + * @target: Name of the target exposed by the sequencer this device wants to
> + * reach.
> + *
> + * Returns:
> + * New power sequencer descriptor for use by the consumer driver or ERR_PTR()
> + * on failure.
> + */
> +struct pwrseq_desc *devm_pwrseq_get(struct device *dev, const char *target)
> +{
> + struct pwrseq_desc *desc;
> + int ret;
> +
> + desc = pwrseq_get(dev, target);
> + if (IS_ERR(desc))
> + return desc;
> +
> + ret = devm_add_action_or_reset(dev, devm_pwrseq_put, desc);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return desc;
> +}
> +EXPORT_SYMBOL_GPL(devm_pwrseq_get);
> +
> +static int pwrseq_unit_enable(struct pwrseq_device *pwrseq,
> + struct pwrseq_unit *target);
> +static int pwrseq_unit_disable(struct pwrseq_device *pwrseq,
> + struct pwrseq_unit *target);
> +
> +static int pwrseq_unit_enable_deps(struct pwrseq_device *pwrseq,
> + struct list_head *list)
> +{
> + struct pwrseq_unit_dep *pos;
> + int ret = 0;
> +
> + list_for_each_entry(pos, list, list) {
> + ret = pwrseq_unit_enable(pwrseq, pos->unit);
> + if (ret) {
> + list_for_each_entry_continue_reverse(pos, list, list)
> + pwrseq_unit_disable(pwrseq, pos->unit);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int pwrseq_unit_disable_deps(struct pwrseq_device *pwrseq,
> + struct list_head *list)
> +{
> + struct pwrseq_unit_dep *pos;
> + int ret = 0;
> +
> + list_for_each_entry_reverse(pos, list, list) {
> + ret = pwrseq_unit_disable(pwrseq, pos->unit);
> + if (ret) {
> + list_for_each_entry_continue(pos, list, list)
> + pwrseq_unit_enable(pwrseq, pos->unit);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int pwrseq_unit_enable(struct pwrseq_device *pwrseq,
> + struct pwrseq_unit *unit)
> +{
> + int ret;
> +
> + lockdep_assert_held_read(&pwrseq->rw_lock);
> + lockdep_assert_held(&pwrseq->state_lock);
> +
> + if (unit->enable_count != 0) {
> + unit->enable_count++;
> + return 0;
> + }
> +
> + ret = pwrseq_unit_enable_deps(pwrseq, &unit->deps);
> + if (ret) {
> + dev_err(&pwrseq->dev,
> + "Failed to enable dependencies before power-on for target '%s': %d\n",
> + unit->name, ret);
> + return ret;
> + }
> +
> + if (unit->enable) {
> + ret = unit->enable(pwrseq);
> + if (ret) {
> + dev_err(&pwrseq->dev,
> + "Failed to enable target '%s': %d\n",
> + unit->name, ret);
> + pwrseq_unit_disable_deps(pwrseq, &unit->deps);
> + return ret;
> + }
> + }
> +
> + unit->enable_count++;
> +
> + return 0;
> +}
> +
> +static int pwrseq_unit_disable(struct pwrseq_device *pwrseq,
> + struct pwrseq_unit *unit)
> +{
> + int ret;
> +
> + lockdep_assert_held_read(&pwrseq->rw_lock);
> + lockdep_assert_held(&pwrseq->state_lock);
> +
> + if (unit->enable_count == 0) {
> + WARN_ONCE(1, "Unmatched power-off for target '%s'\n",
> + unit->name);
> + return -EBUSY;
> + }
> +
> + if (unit->enable_count != 1) {
> + unit->enable_count--;
> + return 0;
> + }
> +
> + if (unit->disable) {
> + ret = unit->disable(pwrseq);
> + if (ret) {
> + dev_err(&pwrseq->dev,
> + "Failed to disable target '%s': %d\n",
> + unit->name, ret);
> + return ret;
> + }
> + }
> +
> + ret = pwrseq_unit_disable_deps(pwrseq, &unit->deps);
> + if (ret) {
> + dev_err(&pwrseq->dev,
> + "Failed to disable dependencies after power-off for target '%s': %d\n",
> + unit->name, ret);
> + if (unit->enable)
> + unit->enable(pwrseq);
> + return ret;
> + }
> +
> + unit->enable_count--;
> +
> + return 0;
> +}
> +
> +/**
> + * pwrseq_power_on() - Issue a power-on request on behalf of the consumer
> + * device.
> + * @desc: Descriptor referencing the power sequencer.
> + *
> + * This function tells the power sequencer that the consumer wants to be
> + * powered-up. The sequencer may already have powered-up the device in which
> + * case the function returns 0. If the power-up sequence is already in
> + * progress, the function will block until it's done and return 0. If this is
> + * the first request, the device will be powered up.
> + *
> + * Returns:
> + * 0 on success, negative error number on failure.
> + */
> +int pwrseq_power_on(struct pwrseq_desc *desc)
> +{
> + struct pwrseq_device *pwrseq;
> + struct pwrseq_target *target;
> + struct pwrseq_unit *unit;
> + int ret;
> +
> + might_sleep();
> +
> + if (!desc || desc->powered_on)
> + return 0;
> +
> + pwrseq = desc->pwrseq;
> + target = desc->target;
> + unit = target->unit;
> +
> + guard(rwsem_read)(&pwrseq->rw_lock);
> + if (!device_is_registered(&pwrseq->dev))
> + return -ENODEV;
> +
> + scoped_guard(mutex, &pwrseq->state_lock) {
> + ret = pwrseq_unit_enable(pwrseq, unit);
> + if (!ret)
> + desc->powered_on = true;
> + }
> +
> + if (target->post_enable) {
> + ret = target->post_enable(pwrseq);
> + if (ret) {
> + pwrseq_unit_disable(pwrseq, unit);
> + desc->powered_on = false;
> + }
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_power_on);
> +
> +/**
> + * pwrseq_power_off() - Issue a power-off request on behalf of the consumer
> + * device.
> + * @desc: Descriptor referencing the power sequencer.
> + *
> + * This undoes the effects of pwrseq_power_on(). It issues a power-off request
> + * on behalf of the consumer and when the last remaining user does so, the
> + * power-down sequence will be started. If one is in progress, the function
> + * will block until it's complete and then return.
> + *
> + * Returns:
> + * 0 on success, negative error number on failure.
> + */
> +int pwrseq_power_off(struct pwrseq_desc *desc)
> +{
> + struct pwrseq_device *pwrseq;
> + struct pwrseq_unit *unit;
> + int ret;
> +
> + might_sleep();
> +
> + if (!desc || !desc->powered_on)
> + return 0;
> +
> + pwrseq = desc->pwrseq;
> + unit = desc->target->unit;
> +
> + guard(rwsem_read)(&pwrseq->rw_lock);
> + if (!device_is_registered(&pwrseq->dev))
> + return -ENODEV;
> +
> + guard(mutex)(&pwrseq->state_lock);
> +
> + ret = pwrseq_unit_disable(pwrseq, unit);
> + if (!ret)
> + desc->powered_on = false;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_power_off);
> +
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +
> +struct pwrseq_debugfs_count_ctx {
> + struct device *dev;
> + loff_t index;
> +};
> +
> +static int pwrseq_debugfs_seq_count(struct device *dev, void *data)
> +{
> + struct pwrseq_debugfs_count_ctx *ctx = data;
> +
> + ctx->dev = dev;
> +
> + return ctx->index-- ? 0 : 1;
> +}
> +
> +static void *pwrseq_debugfs_seq_start(struct seq_file *seq, loff_t *pos)
> +{
> + struct pwrseq_debugfs_count_ctx ctx;
> +
> + ctx.dev = NULL;
> + ctx.index = *pos;
> +
> + /*
> + * We're holding the lock for the entire printout so no need to fiddle
> + * with device reference count.
> + */
> + down_read(&pwrseq_sem);
> +
> + bus_for_each_dev(&pwrseq_bus, NULL, &ctx, pwrseq_debugfs_seq_count);
> + if (!ctx.index)
> + return NULL;
> +
> + return ctx.dev;
> +}
> +
> +static void *pwrseq_debugfs_seq_next(struct seq_file *seq, void *data,
> + loff_t *pos)
> +{
> + struct device *curr = data;
> +
> + ++*pos;
> +
> + struct device *next __free(put_device) =
> + bus_find_next_device(&pwrseq_bus, curr);
> + return next;
> +}
> +
> +static void pwrseq_debugfs_seq_show_target(struct seq_file *seq,
> + struct pwrseq_target *target)
> +{
> + seq_printf(seq, " target: [%s] (target unit: [%s])\n",
> + target->name, target->unit->name);
> +}
> +
> +static void pwrseq_debugfs_seq_show_unit(struct seq_file *seq,
> + struct pwrseq_unit *unit)
> +{
> + struct pwrseq_unit_dep *ref;
> +
> + seq_printf(seq, " unit: [%s] - enable count: %u\n",
> + unit->name, unit->enable_count);
> +
> + if (list_empty(&unit->deps))
> + return;
> +
> + seq_puts(seq, " dependencies:\n");
> + list_for_each_entry(ref, &unit->deps, list)
> + seq_printf(seq, " [%s]\n", ref->unit->name);
> +}
> +
> +static int pwrseq_debugfs_seq_show(struct seq_file *seq, void *data)
> +{
> + struct device *dev = data;
> + struct pwrseq_device *pwrseq = to_pwrseq_device(dev);
> + struct pwrseq_target *target;
> + struct pwrseq_unit *unit;
> +
> + seq_printf(seq, "%s:\n", dev_name(dev));
> +
> + seq_puts(seq, " targets:\n");
> + list_for_each_entry(target, &pwrseq->targets, list)
> + pwrseq_debugfs_seq_show_target(seq, target);
> +
> + seq_puts(seq, " units:\n");
> + list_for_each_entry(unit, &pwrseq->units, list)
> + pwrseq_debugfs_seq_show_unit(seq, unit);
> +
> + return 0;
> +}
> +
> +static void pwrseq_debugfs_seq_stop(struct seq_file *seq, void *data)
> +{
> + up_read(&pwrseq_sem);
> +}
> +
> +static const struct seq_operations pwrseq_debugfs_sops = {
> + .start = pwrseq_debugfs_seq_start,
> + .next = pwrseq_debugfs_seq_next,
> + .show = pwrseq_debugfs_seq_show,
> + .stop = pwrseq_debugfs_seq_stop,
> +};
> +DEFINE_SEQ_ATTRIBUTE(pwrseq_debugfs);
> +
> +static struct dentry *pwrseq_debugfs_dentry;
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
> +static int __init pwrseq_init(void)
> +{
> + int ret;
> +
> + ret = bus_register(&pwrseq_bus);
> + if (ret) {
> + pr_err("Failed to register the power sequencer bus\n");
> + return ret;
> + }
> +
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + pwrseq_debugfs_dentry = debugfs_create_file("pwrseq", 0444, NULL, NULL,
> + &pwrseq_debugfs_fops);
> +#endif /* CONFIG_DEBUG_FS */
> +
> + return 0;
> +}
> +subsys_initcall(pwrseq_init);
> +
> +static void __exit pwrseq_exit(void)
> +{
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + debugfs_remove_recursive(pwrseq_debugfs_dentry);
> +#endif /* CONFIG_DEBUG_FS */
> +
> + bus_unregister(&pwrseq_bus);
> +}
> +module_exit(pwrseq_exit);
> +
> +MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
> +MODULE_DESCRIPTION("Power Sequencing subsystem core");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
> new file mode 100644
> index 000000000000..7d583b4f266e
> --- /dev/null
> +++ b/include/linux/pwrseq/consumer.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2024 Linaro Ltd.
> + */
> +
> +#ifndef __POWER_SEQUENCING_CONSUMER_H__
> +#define __POWER_SEQUENCING_CONSUMER_H__
> +
> +#include <linux/err.h>
> +
> +struct device;
> +struct pwrseq_desc;
> +
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> +
> +struct pwrseq_desc * __must_check
> +pwrseq_get(struct device *dev, const char *target);
> +void pwrseq_put(struct pwrseq_desc *desc);
> +
> +struct pwrseq_desc * __must_check
> +devm_pwrseq_get(struct device *dev, const char *target);
> +
> +int pwrseq_power_on(struct pwrseq_desc *desc);
> +int pwrseq_power_off(struct pwrseq_desc *desc);
> +
> +#else /* CONFIG_POWER_SEQUENCING */
> +
> +static inline struct pwrseq_desc * __must_check
> +pwrseq_get(struct device *dev, const char *target)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline void pwrseq_put(struct pwrseq_desc *desc)
> +{
> +}
> +
> +static inline struct pwrseq_desc * __must_check
> +devm_pwrseq_get(struct device *dev, const char *target)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline int pwrseq_power_on(struct pwrseq_desc *desc)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int pwrseq_power_off(struct pwrseq_desc *desc)
> +{
> + return -ENOSYS;
> +}
> +
> +#endif /* CONFIG_POWER_SEQUENCING */
> +
> +#endif /* __POWER_SEQUENCING_CONSUMER_H__ */
> diff --git a/include/linux/pwrseq/provider.h b/include/linux/pwrseq/provider.h
> new file mode 100644
> index 000000000000..e627ed2f4d91
> --- /dev/null
> +++ b/include/linux/pwrseq/provider.h
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2024 Linaro Ltd.
> + */
> +
> +#ifndef __POWER_SEQUENCING_PROVIDER_H__
> +#define __POWER_SEQUENCING_PROVIDER_H__
> +
> +struct device;
> +struct module;
> +struct pwrseq_device;
> +
> +typedef int (*pwrseq_power_state_func)(struct pwrseq_device *);
> +typedef int (*pwrseq_match_func)(struct pwrseq_device *, struct device *);
> +
> +/**
> + * struct pwrseq_unit_data - Configuration of a single power sequencing
> + * unit.
> + * @name: Name of the unit.
> + * @deps: Units that must be enabled before this one and disabled after it
> + * in the order they come in this array.
> + * @enable: Callback running the part of the power-on sequence provided by
> + * this unit.
> + * @disable: Callback running the part of the power-off sequence provided
> + * by this unit.
> + */
> +struct pwrseq_unit_data {
> + const char *name;
> + const struct pwrseq_unit_data **deps;
> + pwrseq_power_state_func enable;
> + pwrseq_power_state_func disable;
> +};
> +
> +/**
> + * struct pwrseq_target_data - Configuration of a power sequencing target.
> + * @name: Name of the target.
> + * @unit: Final unit that this target must reach in order to be considered
> + * enabled.
> + * @post_enable: Callback run after the target unit has been enabled, *after*
> + * the state lock has been released. It's useful for implementing
> + * boot-up delays without blocking other users from powering up
> + * using the same power sequencer.
> + */
> +struct pwrseq_target_data {
> + const char *name;
> + const struct pwrseq_unit_data *unit;
> + pwrseq_power_state_func post_enable;
> +};
> +
> +/**
> + * struct pwrseq_config - Configuration used for registering a new provider.
> + * @parent: Parent device for the sequencer. Must be set.
> + * @owner: Module providing this device.
> + * @drvdata: Private driver data.
> + * @match: Provider callback used to match the consumer device to the sequencer.
> + * @targets: Array of targets for this power sequencer. Must be NULL-terminated.
> + */
> +struct pwrseq_config {
> + struct device *parent;
> + struct module *owner;
> + void *drvdata;
> + pwrseq_match_func match;
> + const struct pwrseq_target_data **targets;
> +};
> +
> +struct pwrseq_device *
> +pwrseq_device_register(const struct pwrseq_config *config);
> +void pwrseq_device_unregister(struct pwrseq_device *pwrseq);
> +struct pwrseq_device *
> +devm_pwrseq_device_register(struct device *dev,
> + const struct pwrseq_config *config);
> +
> +void *pwrseq_device_get_drvdata(struct pwrseq_device *pwrseq);
> +
> +#endif /* __POWER_SEQUENCING_PROVIDER_H__ */
>
> --
> 2.43.0
>

2024-06-05 07:43:35

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 10/17] power: sequencing: implement the pwrseq core

On Wed, Jun 5, 2024 at 8:52 AM Ratheesh Kannoth <[email protected]> wrote:
>
> On 2024-05-29 at 00:33:18, Bartosz Golaszewski ([email protected]) wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > Implement the power sequencing subsystem allowing devices to share
> > complex powering-up and down procedures. It's split into the consumer
> > and provider parts but does not implement any new DT bindings so that
> > the actual power sequencing is never revealed in the DT representation.
> >
> > Tested-by: Amit Pundir <[email protected]>
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > ---
> > +
> > +static struct pwrseq_unit_dep *pwrseq_unit_dep_new(struct pwrseq_unit *unit)
> nit. pwrseq_unit_dep_alloc/create rhymes well with pwrseq_unit_dep_free(),

So what?

> > +static void pwrseq_unit_free_deps(struct list_head *list)
> > +{
> > + struct pwrseq_unit_dep *dep, *next;
> > +
> > + list_for_each_entry_safe(dep, next, list, list) {
> no need of 'locks' to protect against simutaneous 'add' ?

No, this only happens once during release.

> > +
> > +static int pwrseq_unit_setup_deps(const struct pwrseq_unit_data **data,
> > + struct list_head *dep_list,
> > + struct list_head *unit_list,
> > + struct radix_tree_root *processed_units)
> > +{
> > + const struct pwrseq_unit_data *pos;
> > + struct pwrseq_unit_dep *dep;
> > + struct pwrseq_unit *unit;
> > + int i;
> > +
> > + for (i = 0; data[i]; i++) {
> Can we add range for i ? just depending on data[i] to be zero looks to be risky.
>

Why? It's perfectly normal to expect users to end the array with a
NULL pointer. The docs say these arrays must be NULL-terminated.

> > + pos = data[i];
> > +
> > + unit = pwrseq_unit_setup(pos, unit_list, processed_units);
> > + if (IS_ERR(unit))
> > + return PTR_ERR(unit);
> > +
> > + dep = pwrseq_unit_dep_new(unit);
> > + if (!dep) {
> > + pwrseq_unit_decref(unit);
> This frees only one 'unit'. is there any chance for multiple 'unit', then better clean
> up here ?

The references to those will be dropped in pwrseq_release().

> > +
> > + /*
> > + * From this point onwards the device's release() callback is
> > + * responsible for freeing resources.
> > + */
> > + device_initialize(&pwrseq->dev);
> > +
> > + ret = dev_set_name(&pwrseq->dev, "pwrseq.%d", pwrseq->id);
> > + if (ret)
> > + goto err_put_pwrseq;
> > +
> > + pwrseq->owner = config->owner ?: THIS_MODULE;
> > + pwrseq->match = config->match;
> > +
> > + init_rwsem(&pwrseq->rw_lock);
> > + mutex_init(&pwrseq->state_lock);
> > + INIT_LIST_HEAD(&pwrseq->targets);
> > + INIT_LIST_HEAD(&pwrseq->units);
> > +
> > + ret = pwrseq_setup_targets(config->targets, pwrseq);
> > + if (ret)
> > + goto err_put_pwrseq;
> > +
> > + scoped_guard(rwsem_write, &pwrseq_sem) {
> > + ret = device_add(&pwrseq->dev);
> > + if (ret)
> > + goto err_put_pwrseq;
> > + }
> > +
> > + return pwrseq;
> > +
> > +err_put_pwrseq:
> no need to kfree(pwrseq) ?

It's literally put on the next line?

Bart

2024-06-05 08:47:57

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Wed, Jun 5, 2024 at 4:13 AM Bjorn Helgaas <[email protected]> wrote:
>
> On Wed, Jun 05, 2024 at 02:34:52AM +0300, Dmitry Baryshkov wrote:
> > On Wed, 5 Jun 2024 at 02:23, Bjorn Helgaas <[email protected]> wrote:
> > >
> > > On Tue, May 28, 2024 at 09:03:24PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > Add a PCI power control driver that's capable of correctly powering up
> > > > devices using the power sequencing subsystem. The first users of this
> > > > driver are the ath11k module on QCA6390 and ath12k on WCN7850.
>
> > > > +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> > > > + {
> > > > + /* ATH11K in QCA6390 package. */
> > > > + .compatible = "pci17cb,1101",
> > > > + .data = "wlan",
> > > > + },
> > > > + {
> > > > + /* ATH12K in WCN7850 package. */
> > > > + .compatible = "pci17cb,1107",
> > > > + .data = "wlan",
> > > > + },
> > >
> > > IIUC, "pci17cb,1101" and "pci17cb,1107" exist partly so we can check
> > > that a DTS conforms to the schema, e.g., a "pci17cb,1101" node
> > > contains all the required regulators. For that use, we obviously need
> > > a very specific "compatible" string.
> > >
> > > Is there any opportunity to add a more generic "compatible" string in
> > > addition to those so this list doesn't have to be updated for every
> > > PMU? The .data here is "wlan" in both cases, and for this purpose, we
> > > don't care whether it's "pci17cb,1101" or "pci17cb,1107".
> >
> > These two devices have different set of regulators and different
> > requirements to power them on.
>
> Right, but I don't think pci_pwrctl_pwrseq_probe() knows about those
> different sets. It basically looks like:
>
> pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
> {
> struct pci_pwrctl_pwrseq_data *data;
> struct device *dev = &pdev->dev;
>
> data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
> pwrseq_power_on(data->pwrseq);
> data->ctx.dev = dev;
> devm_pci_pwrctl_device_set_ready(dev, &data->ctx);
> }
>
> I think of_device_get_match_data(dev) will return "wlan" for both
> "pci17cb,1101" and "pci17cb,1107", so devm_pwrseq_get(),
> pwrseq_power_on(), and devm_pci_pwrctl_device_set_ready() don't see
> the distinction between them.
>

These are only the first two users of this generic driver. We may end
up adding more that will use different targets or even extend the
match data with additional fields.

> Of course, they also get "dev", so they can find the device-specifc
> stuff that way, but I think that's on the drivers/power/sequencing/
> side, not in this pci-pwrctl-pwrseq driver itself.
>
> So what if there were a more generic "compatible" string, e.g., if the
> DT contained something like this:
>
> wifi@0 {
> compatible = "pci17cb,1101", "wlan-pwrseq";

What even is "pwrseq" in the context of the hardware description? DT
maintainers would like to have a word with you. :)

> ...
> }
>
> and pci_pwrctl_pwrseq_of_match[] had this:
>
> { .compatible = "wlan-pwrseq", .data = "wlan", }
>
> Wouldn't this pci-pwrctl-pwrseq driver work the same? I'm not a DT
> whiz, so likely I'm missing something, but it would be nice if we
> didn't have to update this very generic-looking driver to add every
> device that needs it.
>

Device-tree describes hardware, not the implementation. You can see
elsewhere in this series that we have the PMU described as a PMIC on
the device tree but we never register with the regulator subsystem nor
do we create actual regulators in C. The HW description does not have
to match the C implementation 1:1 but has to be accurate. There's not
such HW component as "wlan-pwrseq". If you want a good example of such
generic fallback - it'll be the C45 ethernet PHYs as they actually
exist: there's a HW definition of what a broader C45 PHY is, even
though it can be extended in concrete HW designs.

I'd leave this as is.

Bart

2024-06-05 17:47:24

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Wed, Jun 05, 2024 at 10:47:32AM +0200, Bartosz Golaszewski wrote:
> On Wed, Jun 5, 2024 at 4:13 AM Bjorn Helgaas <[email protected]> wrote:
> > On Wed, Jun 05, 2024 at 02:34:52AM +0300, Dmitry Baryshkov wrote:
> > > On Wed, 5 Jun 2024 at 02:23, Bjorn Helgaas <[email protected]> wrote:
> > > > On Tue, May 28, 2024 at 09:03:24PM +0200, Bartosz Golaszewski wrote:
> > > > > From: Bartosz Golaszewski <[email protected]>
> > > > >
> > > > > Add a PCI power control driver that's capable of correctly powering up
> > > > > devices using the power sequencing subsystem. The first users of this
> > > > > driver are the ath11k module on QCA6390 and ath12k on WCN7850.
> >
> > > > > +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> > > > > + {
> > > > > + /* ATH11K in QCA6390 package. */
> > > > > + .compatible = "pci17cb,1101",
> > > > > + .data = "wlan",
> > > > > + },
> > > > > + {
> > > > > + /* ATH12K in WCN7850 package. */
> > > > > + .compatible = "pci17cb,1107",
> > > > > + .data = "wlan",
> > > > > + },
> > > >
> > > > IIUC, "pci17cb,1101" and "pci17cb,1107" exist partly so we can check
> > > > that a DTS conforms to the schema, e.g., a "pci17cb,1101" node
> > > > contains all the required regulators. For that use, we obviously need
> > > > a very specific "compatible" string.
> > > >
> > > > Is there any opportunity to add a more generic "compatible" string in
> > > > addition to those so this list doesn't have to be updated for every
> > > > PMU? The .data here is "wlan" in both cases, and for this purpose, we
> > > > don't care whether it's "pci17cb,1101" or "pci17cb,1107".
> > >
> > > These two devices have different set of regulators and different
> > > requirements to power them on.
> >
> > Right, but I don't think pci_pwrctl_pwrseq_probe() knows about those
> > different sets. It basically looks like:
> >
> > pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
> > {
> > struct pci_pwrctl_pwrseq_data *data;
> > struct device *dev = &pdev->dev;
> >
> > data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
> > pwrseq_power_on(data->pwrseq);
> > data->ctx.dev = dev;
> > devm_pci_pwrctl_device_set_ready(dev, &data->ctx);
> > }
> >
> > I think of_device_get_match_data(dev) will return "wlan" for both
> > "pci17cb,1101" and "pci17cb,1107", so devm_pwrseq_get(),
> > pwrseq_power_on(), and devm_pci_pwrctl_device_set_ready() don't see
> > the distinction between them.
>
> These are only the first two users of this generic driver. We may end
> up adding more that will use different targets or even extend the
> match data with additional fields.

If that were the only reason, I would suggest waiting to add the
specific device strings until we need the functionality, but it sounds
like there are other stronger reasons.

> > Of course, they also get "dev", so they can find the device-specifc
> > stuff that way, but I think that's on the drivers/power/sequencing/
> > side, not in this pci-pwrctl-pwrseq driver itself.
> >
> > So what if there were a more generic "compatible" string, e.g., if the
> > DT contained something like this:
> >
> > wifi@0 {
> > compatible = "pci17cb,1101", "wlan-pwrseq";
>
> What even is "pwrseq" in the context of the hardware description? DT
> maintainers would like to have a word with you. :)

There are "compatible" strings like "simple-bus", "simple-mfd", and
"syscon" that allow drivers to bind and provide generic functionality
when they don't need to know the exact hardware.

> > and pci_pwrctl_pwrseq_of_match[] had this:
> >
> > { .compatible = "wlan-pwrseq", .data = "wlan", }
> >
> > Wouldn't this pci-pwrctl-pwrseq driver work the same? I'm not a DT
> > whiz, so likely I'm missing something, but it would be nice if we
> > didn't have to update this very generic-looking driver to add every
> > device that needs it.

Do you have any other ideas to reduce the churn in this file? It just
seems weird to have to add an ID to this file without adding any
actual code or data related to it.

We should probably also add a pattern to MAINTAINERS so
get_maintainers.pl on this file will show you as a maintainer.

Bjorn

2024-06-05 17:56:16

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On Tue, Jun 04, 2024 at 08:24:34PM +0200, Bartosz Golaszewski wrote:
> On Tue, Jun 4, 2024 at 7:19 PM Bjorn Helgaas <[email protected]> wrote:
> >
> > On Tue, May 28, 2024 at 09:03:08PM +0200, Bartosz Golaszewski wrote:
> > > Note: I am resending this series in its entirety once more for
> > > discussions and reviews. If there won't be any major objections, I'll
> > > then start sending individual bits and pieces to appropriate trees.
> > >
> > > Merging strategy: The DT binding and DTS changes are a no-brainer, they
> > > can go through the wireless, regulator and arm-msm trees separately. The
> > > bluetooth and PCI changes have a build-time dependency on the power
> > > sequencing code. The bluetooth changes also have a run-time dependency on
> > > the PCI pwrctl part. In order to get it into next I plan to pick up the
> > > power sequencing code into my own tree and maintain it. I can then
> > > provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> > > stress about the BT runtime dependency as it will be fixed once all
> > > changes are in next.
> > > ...
> >
> > > ---
> > > base-commit: 6dc544b66971c7f9909ff038b62149105272d26a
> > > change-id: 20240527-pwrseq-76fc025248a2
> >
> > What does this apply to? I don't know what 6dc544b66971 is; it
> > doesn't seem to be in upstream or linux-next.
>
> It's next-20240528 but it also applies to today's next without
> conflicts. What do you want me to base the PCI part when resending?

For PCI, we usually apply things on topic branches based on -rc1, so
that's the easiest.

2024-06-05 17:58:10

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 15/17] PCI/pwrctl: add PCI power control core code

On Tue, May 28, 2024 at 09:03:23PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Some PCI devices must be powered-on before they can be detected on the
> bus. Introduce a simple framework reusing the existing PCI OF
> infrastructure.
>
> The way this works is: a DT node representing a PCI device connected to
> the port can be matched against its power control platform driver. If
> the match succeeds, the driver is responsible for powering-up the device
> and calling pcie_pwrctl_device_set_ready() which will trigger a PCI bus

s/pcie_pwrctl_device_set_ready/pci_pwrctl_device_set_ready/

> rescan as well as subscribe to PCI bus notifications.
>
> When the device is detected and created, we'll make it consume the same
> DT node that the platform device did. When the device is bound, we'll
> create a device link between it and the parent power control device.

2024-06-05 18:10:30

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Wed, Jun 5, 2024 at 7:47 PM Bjorn Helgaas <[email protected]> wrote:
>
> > >
> > > wifi@0 {
> > > compatible = "pci17cb,1101", "wlan-pwrseq";
> >
> > What even is "pwrseq" in the context of the hardware description? DT
> > maintainers would like to have a word with you. :)
>
> There are "compatible" strings like "simple-bus", "simple-mfd", and
> "syscon" that allow drivers to bind and provide generic functionality
> when they don't need to know the exact hardware.
>

There's a difference however: a "simple bus" is a thing. A "simple
multifunction device" is also an actual thing. A "pwrseq" or
"power-sequencer" is not a thing, it's a functionality. And we don't
describe it in device-tree. Rob has said before that he regrets having
merged the mmc pwrseq bindings back in the day and that he wouldn't do
it again now because it describes what HW does and not what it is. In
this case the PMU is simply a PMIC and the bindings I'm proposing
describe it as such. But what you're proposing is even worse: this is
the ath1x module of the larger chipset (power sequencee rather than
sequencer) so naming it "wlan-pwrseq" makes absolutely no sense at
all. It's a PCI device whose ID is 0x17cb1101 and the device tree
describes it as such.

> > > and pci_pwrctl_pwrseq_of_match[] had this:
> > >
> > > { .compatible = "wlan-pwrseq", .data = "wlan", }
> > >
> > > Wouldn't this pci-pwrctl-pwrseq driver work the same? I'm not a DT
> > > whiz, so likely I'm missing something, but it would be nice if we
> > > didn't have to update this very generic-looking driver to add every
> > > device that needs it.
>
> Do you have any other ideas to reduce the churn in this file? It just
> seems weird to have to add an ID to this file without adding any
> actual code or data related to it.
>

Is it really that much churn though? You'd save 4 lines of code? I
think this is premature optimization, we'll see about unifying it when
we have several models supported, right now with two, I'd just leave
it as is and not seek perfection.

> We should probably also add a pattern to MAINTAINERS so
> get_maintainers.pl on this file will show you as a maintainer.
>

Makes sense.

Bartosz

> Bjorn

2024-06-05 18:11:35

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 15/17] PCI/pwrctl: add PCI power control core code

On Wed, Jun 5, 2024 at 7:58 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Tue, May 28, 2024 at 09:03:23PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > Some PCI devices must be powered-on before they can be detected on the
> > bus. Introduce a simple framework reusing the existing PCI OF
> > infrastructure.
> >
> > The way this works is: a DT node representing a PCI device connected to
> > the port can be matched against its power control platform driver. If
> > the match succeeds, the driver is responsible for powering-up the device
> > and calling pcie_pwrctl_device_set_ready() which will trigger a PCI bus
>
> s/pcie_pwrctl_device_set_ready/pci_pwrctl_device_set_ready/
>

Ah that's a leftover from when it was PCIe specific. Thanks.

Bart

> > rescan as well as subscribe to PCI bus notifications.
> >
> > When the device is detected and created, we'll make it consume the same
> > DT node that the platform device did. When the device is bound, we'll
> > create a device link between it and the parent power control device.

2024-06-11 22:54:38

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On Tue, May 28, 2024 at 09:03:08PM +0200, Bartosz Golaszewski wrote:
> Note: I am resending this series in its entirety once more for
> discussions and reviews. If there won't be any major objections, I'll
> then start sending individual bits and pieces to appropriate trees.
>
> Merging strategy: The DT binding and DTS changes are a no-brainer, they
> can go through the wireless, regulator and arm-msm trees separately. The
> bluetooth and PCI changes have a build-time dependency on the power
> sequencing code. The bluetooth changes also have a run-time dependency on
> the PCI pwrctl part. In order to get it into next I plan to pick up the
> power sequencing code into my own tree and maintain it. I can then
> provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> stress about the BT runtime dependency as it will be fixed once all
> changes are in next.

The PCI changes are very self-contained and any conflicts will be
trivial, so rather than messing with an immutable tag, how about if I
just ack them and you can include them in your tree directly?

2024-06-11 22:55:54

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 12/17] PCI: hold the rescan mutex when scanning for the first time

On Tue, May 28, 2024 at 09:03:20PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> With the introduction of PCI device power control drivers that will be
> able to trigger the port rescan when probing, we need to hold the rescan
> mutex during the initial pci_host_probe() too or the two could get in
> each other's way.
>
> Tested-by: Amit Pundir <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

With s/hold the/Hold the/ in subject to match history,

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/probe.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8e696e547565..604fc96b1098 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3072,7 +3072,9 @@ int pci_host_probe(struct pci_host_bridge *bridge)
> struct pci_bus *bus, *child;
> int ret;
>
> + pci_lock_rescan_remove();
> ret = pci_scan_root_bus_bridge(bridge);
> + pci_unlock_rescan_remove();
> if (ret < 0) {
> dev_err(bridge->dev.parent, "Scanning root bridge failed");
> return ret;
>
> --
> 2.43.0
>

2024-06-11 22:57:04

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 14/17] PCI/pwrctl: create platform devices for child OF nodes of the port node

On Tue, May 28, 2024 at 09:03:22PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> In preparation for introducing PCI device power control - a set of
> library functions that will allow powering-up of PCI devices before
> they're detected on the PCI bus - we need to populate the devices
> defined on the device-tree.
>
> We are reusing the platform bus as it provides us with all the
> infrastructure we need to match the pwrctl drivers against the
> compatibles from OF nodes.
>
> These platform devices will be probed by the driver core and bound to
> the PCI pwrctl drivers we'll introduce later.
>
> Tested-by: Amit Pundir <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

With s/create/Create/ in subject,

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/bus.c | 9 +++++++++
> drivers/pci/remove.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 826b5016a101..3e3517567721 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -12,6 +12,7 @@
> #include <linux/errno.h>
> #include <linux/ioport.h>
> #include <linux/of.h>
> +#include <linux/of_platform.h>
> #include <linux/proc_fs.h>
> #include <linux/slab.h>
>
> @@ -354,6 +355,14 @@ void pci_bus_add_device(struct pci_dev *dev)
> pci_warn(dev, "device attach failed (%d)\n", retval);
>
> pci_dev_assign_added(dev, true);
> +
> + if (pci_is_bridge(dev)) {
> + retval = of_platform_populate(dev->dev.of_node, NULL, NULL,
> + &dev->dev);
> + if (retval)
> + pci_err(dev, "failed to populate child OF nodes (%d)\n",
> + retval);
> + }
> }
> EXPORT_SYMBOL_GPL(pci_bus_add_device);
>
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index d749ea8250d6..910387e5bdbf 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <linux/pci.h>
> #include <linux/module.h>
> +#include <linux/of_platform.h>
> #include "pci.h"
>
> static void pci_free_resources(struct pci_dev *dev)
> @@ -18,7 +19,7 @@ static void pci_stop_dev(struct pci_dev *dev)
> pci_pme_active(dev, false);
>
> if (pci_dev_is_added(dev)) {
> -
> + of_platform_depopulate(&dev->dev);
> device_release_driver(&dev->dev);
> pci_proc_detach_device(dev);
> pci_remove_sysfs_dev_files(dev);
>
> --
> 2.43.0
>

2024-06-11 22:58:14

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 16/17] PCI/pwrctl: add a PCI power control driver for power sequenced devices

On Tue, May 28, 2024 at 09:03:24PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Add a PCI power control driver that's capable of correctly powering up
> devices using the power sequencing subsystem. The first users of this
> driver are the ath11k module on QCA6390 and ath12k on WCN7850.
>
> Tested-by: Amit Pundir <[email protected]>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

With s/add/Add/ in subject,

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/pwrctl/Kconfig | 9 ++++
> drivers/pci/pwrctl/Makefile | 2 +
> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++++++++++++++++++++++++++++++++++
> 3 files changed, 100 insertions(+)
>
> diff --git a/drivers/pci/pwrctl/Kconfig b/drivers/pci/pwrctl/Kconfig
> index 96195395af69..f1b824955d4b 100644
> --- a/drivers/pci/pwrctl/Kconfig
> +++ b/drivers/pci/pwrctl/Kconfig
> @@ -5,4 +5,13 @@ menu "PCI Power control drivers"
> config PCI_PWRCTL
> tristate
>
> +config PCI_PWRCTL_PWRSEQ
> + tristate "PCI Power Control driver using the Power Sequencing subsystem"
> + select POWER_SEQUENCING
> + select PCI_PWRCTL
> + default m if ((ATH11K_PCI || ATH12K) && ARCH_QCOM)
> + help
> + Enable support for the PCI power control driver for device
> + drivers using the Power Sequencing subsystem.
> +
> endmenu
> diff --git a/drivers/pci/pwrctl/Makefile b/drivers/pci/pwrctl/Makefile
> index 52ae0640ef7b..d308aae4800c 100644
> --- a/drivers/pci/pwrctl/Makefile
> +++ b/drivers/pci/pwrctl/Makefile
> @@ -2,3 +2,5 @@
>
> obj-$(CONFIG_PCI_PWRCTL) += pci-pwrctl-core.o
> pci-pwrctl-core-y := core.o
> +
> +obj-$(CONFIG_PCI_PWRCTL_PWRSEQ) += pci-pwrctl-pwrseq.o
> diff --git a/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c b/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
> new file mode 100644
> index 000000000000..c7a113a76c0c
> --- /dev/null
> +++ b/drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Linaro Ltd.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pci-pwrctl.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwrseq/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +struct pci_pwrctl_pwrseq_data {
> + struct pci_pwrctl ctx;
> + struct pwrseq_desc *pwrseq;
> +};
> +
> +static void devm_pci_pwrctl_pwrseq_power_off(void *data)
> +{
> + struct pwrseq_desc *pwrseq = data;
> +
> + pwrseq_power_off(pwrseq);
> +}
> +
> +static int pci_pwrctl_pwrseq_probe(struct platform_device *pdev)
> +{
> + struct pci_pwrctl_pwrseq_data *data;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->pwrseq = devm_pwrseq_get(dev, of_device_get_match_data(dev));
> + if (IS_ERR(data->pwrseq))
> + return dev_err_probe(dev, PTR_ERR(data->pwrseq),
> + "Failed to get the power sequencer\n");
> +
> + ret = pwrseq_power_on(data->pwrseq);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to power-on the device\n");
> +
> + ret = devm_add_action_or_reset(dev, devm_pci_pwrctl_pwrseq_power_off,
> + data->pwrseq);
> + if (ret)
> + return ret;
> +
> + data->ctx.dev = dev;
> +
> + ret = devm_pci_pwrctl_device_set_ready(dev, &data->ctx);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register the pwrctl wrapper\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pci_pwrctl_pwrseq_of_match[] = {
> + {
> + /* ATH11K in QCA6390 package. */
> + .compatible = "pci17cb,1101",
> + .data = "wlan",
> + },
> + {
> + /* ATH12K in WCN7850 package. */
> + .compatible = "pci17cb,1107",
> + .data = "wlan",
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, pci_pwrctl_pwrseq_of_match);
> +
> +static struct platform_driver pci_pwrctl_pwrseq_driver = {
> + .driver = {
> + .name = "pci-pwrctl-pwrseq",
> + .of_match_table = pci_pwrctl_pwrseq_of_match,
> + },
> + .probe = pci_pwrctl_pwrseq_probe,
> +};
> +module_platform_driver(pci_pwrctl_pwrseq_driver);
> +
> +MODULE_AUTHOR("Bartosz Golaszewski <[email protected]>");
> +MODULE_DESCRIPTION("Generic PCI Power Control module for power sequenced devices");
> +MODULE_LICENSE("GPL");
>
> --
> 2.43.0
>

2024-06-12 07:11:20

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v8 00/17] power: sequencing: implement the subsystem and add first users

On Wed, Jun 12, 2024 at 12:54 AM Bjorn Helgaas <[email protected]> wrote:
>
> On Tue, May 28, 2024 at 09:03:08PM +0200, Bartosz Golaszewski wrote:
> > Note: I am resending this series in its entirety once more for
> > discussions and reviews. If there won't be any major objections, I'll
> > then start sending individual bits and pieces to appropriate trees.
> >
> > Merging strategy: The DT binding and DTS changes are a no-brainer, they
> > can go through the wireless, regulator and arm-msm trees separately. The
> > bluetooth and PCI changes have a build-time dependency on the power
> > sequencing code. The bluetooth changes also have a run-time dependency on
> > the PCI pwrctl part. In order to get it into next I plan to pick up the
> > power sequencing code into my own tree and maintain it. I can then
> > provide an immutable tag for the BT and PCI trees to pull. I wouldn't
> > stress about the BT runtime dependency as it will be fixed once all
> > changes are in next.
>
> The PCI changes are very self-contained and any conflicts will be
> trivial, so rather than messing with an immutable tag, how about if I
> just ack them and you can include them in your tree directly?

Sure, if you're convinced that eventual conflicts in PCI core won't be
an issue then it's even better.

Bart