Messages in this thread | | | Date | Sat, 6 Apr 2024 11:03:36 +0800 | From | Xilin Wu <> | Subject | Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users |
| |
On 2024/3/25 21:16, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> > > Note: I dropped most of the the review and test tags on purpose, the code > changed significantly and warrants a new round of reviews and tests. > > === > > 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 and sm8550-qrd. > > Changelog: > > 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/20240104130123.37115-1-brgl@bgdev.pl/T/ > > v2: First real patch series (should have been PATCH v2) adding what I > referred to back then as PCI power sequencing: > > https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/ > > v3: RFC for the DT representation of the PMU supplying the WLAN and BT > modules inside the QCA6391 package (was largely separate from the > series but probably should have been called PATCH or RFC v3): > > https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/ > > v4: Second attempt at the full series with changed scope (introduction of > the pwrseq subsystem, should have been RFC v4) > > https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/ > > v5: Two different ways of handling QCA6390 and WCN7850: > > https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/ > > 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 > 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 > power: sequencing: implement the pwrseq core > power: pwrseq: add a driver for the PMU module on the QCom WCN > chipsets > Bluetooth: qca: use the power sequencer for QCA6390 > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > .../net/bluetooth/qualcomm-bluetooth.yaml | 17 + > .../net/wireless/qcom,ath11k-pci.yaml | 46 + > .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++ > .../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 | 10 + > arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++ > arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 + > arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++ > arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 + > 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 | 136 +++ > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++ > drivers/pci/remove.c | 2 + > drivers/power/Kconfig | 1 + > drivers/power/Makefile | 1 + > drivers/power/sequencing/Kconfig | 28 + > drivers/power/sequencing/Makefile | 6 + > drivers/power/sequencing/core.c | 1065 +++++++++++++++++ > drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++ > include/linux/pci-pwrctl.h | 51 + > include/linux/pwrseq/consumer.h | 56 + > include/linux/pwrseq/provider.h | 75 ++ > 31 files changed, 2614 insertions(+), 31 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml > create mode 100644 drivers/pci/pwrctl/Kconfig > create mode 100644 drivers/pci/pwrctl/Makefile > create mode 100644 drivers/pci/pwrctl/core.c > create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c > create mode 100644 drivers/power/sequencing/Kconfig > create mode 100644 drivers/power/sequencing/Makefile > create mode 100644 drivers/power/sequencing/core.c > create mode 100644 drivers/power/sequencing/pwrseq-qcom-wcn.c > create mode 100644 include/linux/pci-pwrctl.h > create mode 100644 include/linux/pwrseq/consumer.h > create mode 100644 include/linux/pwrseq/provider.h
I tested the patchset on SM8550 and it does give me working WiFi. However I seethe following warnings during boot.
[ 5.973011] mhi mhi0: Requested to power ON [ 6.597591] mhi mhi0: Power on setup success [ 6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0' [ 6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1 [ 6.597635] Hardware name: AYN Odin 2 (DT) [ 6.597637] Workqueue: async async_run_entry_fn [ 6.597645] Call trace: [ 6.597646] dump_backtrace+0xa0/0x128 [ 6.597649] show_stack+0x20/0x38 [ 6.597650] dump_stack_lvl+0x74/0x90 [ 6.597653] dump_stack+0x18/0x28 [ 6.597654] sysfs_warn_dup+0x6c/0x90 [ 6.597658] sysfs_add_bin_file_mode_ns+0xdc/0x100 [ 6.597660] sysfs_create_bin_file+0x7c/0xb8 [ 6.597662] pci_create_attr+0xb4/0x1a8 [ 6.597665] pci_create_resource_files+0x64/0xd0 [ 6.597667] pci_create_sysfs_dev_files+0x24/0x40 [ 6.597669] pci_bus_add_device+0x54/0x138 [ 6.597670] pci_bus_add_devices+0x40/0x98 [ 6.597672] pci_host_probe+0x70/0xf0 [ 6.597673] dw_pcie_host_init+0x248/0x658 [ 6.597676] qcom_pcie_probe+0x234/0x330 [ 6.597677] platform_probe+0x70/0xd8 [ 6.597680] really_probe+0xc8/0x3a0 [ 6.597681] __driver_probe_device+0x84/0x170 [ 6.597682] driver_probe_device+0x44/0x120 [ 6.597683] __device_attach_driver+0xc4/0x168 [ 6.597684] bus_for_each_drv+0x8c/0xf0 [ 6.597686] __device_attach_async_helper+0xb4/0x118 [ 6.597687] async_run_entry_fn+0x40/0x178 [ 6.597689] process_one_work+0x16c/0x410 [ 6.597691] worker_thread+0x284/0x3a0 [ 6.597693] kthread+0x118/0x128 [ 6.597693] ret_from_fork+0x10/0x20 [ 6.597698] ------------[ cut here ]------------ [ 6.597698] proc_dir_entry '0000:00/00.0' already registered [ 6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0 [ 6.597713] Modules linked in: [ 6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1 [ 6.597715] Hardware name: AYN Odin 2 (DT) [ 6.597716] Workqueue: async async_run_entry_fn [ 6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) [ 6.597719] pc : proc_register+0x138/0x1d0 [ 6.597721] lr : proc_register+0x138/0x1d0 [ 6.597723] sp : ffff800081e3b9a0 [ 6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0 [ 6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540 [ 6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004 [ 6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff [ 6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910 [ 6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165 [ 6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528 [ 6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000 [ 6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff [ 6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000 [ 6.597737] Call trace: [ 6.597737] proc_register+0x138/0x1d0 [ 6.597739] proc_create_data+0x48/0x78 [ 6.597741] pci_proc_attach_device+0x84/0x118 [ 6.597743] pci_bus_add_device+0x5c/0x138 [ 6.597744] pci_bus_add_devices+0x40/0x98 [ 6.597745] pci_host_probe+0x70/0xf0 [ 6.597746] dw_pcie_host_init+0x248/0x658 [ 6.597748] qcom_pcie_probe+0x234/0x330 [ 6.597749] platform_probe+0x70/0xd8 [ 6.597750] really_probe+0xc8/0x3a0 [ 6.597751] __driver_probe_device+0x84/0x170 [ 6.597752] driver_probe_device+0x44/0x120 [ 6.597753] __device_attach_driver+0xc4/0x168 [ 6.597754] bus_for_each_drv+0x8c/0xf0 [ 6.597756] __device_attach_async_helper+0xb4/0x118 [ 6.597757] async_run_entry_fn+0x40/0x178 [ 6.597759] process_one_work+0x16c/0x410 [ 6.597760] worker_thread+0x284/0x3a0 [ 6.597761] kthread+0x118/0x128 [ 6.597762] ret_from_fork+0x10/0x20 [ 6.597763] ---[ end trace 0000000000000000 ]---
This probably only occurs when the relevant drivers on compiled as built-in. Similar behavior has been noticed before as well:
https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a
Thanks, Xilin
| |