| Message ID | 20240712091008.14815-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] power sequencing updates for v6.11-rc1 | expand |
The pull request you sent on Fri, 12 Jul 2024 11:10:08 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/pwrseq-updates-for-v6.11-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e763c9ec71dd462337d0b671ec5014b737c5342e
Thank you!
On Fri, 12 Jul 2024 at 02:13, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > This PR contains the core power sequencing framework, the first driver, PCI > changes using the pwrseq library (blessed by Bjorn Helgaas) and some fixes > that came later. Hmm. Let's see how this all works out, but I already found an annoyance. It first asks me about the new PCI power sequencing driver. And then it asks me separately if I want the power sequencing support. Now, either this should (a) not ask about the generic power sequencing support at all, and just select if if a driver that is enabled needs it OR (b) it should ask about power sequencing support and then if you say "N", it should not ask about the drivers. But asking *twice* is definitely not kosher. Linus
On Mon, 15 Jul 2024 at 19:17, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > But asking *twice* is definitely not kosher. .. and obviously it's only "twice" right now. If every driver continues this pattern, we'll have "n+1" questions. Linus
On Mon, 15 Jul 2024 at 19:17, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Hmm. Let's see how this all works out, but I already found an annoyance. .. and another one. On my Altra box, commit 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node") causes annoying messages at bootup: pci 000c:00:01.0: failed to populate child OF nodes (-22) pci 000c:00:02.0: failed to populate child OF nodes (-22) .. repeat for every PCI bridge .. for no obvious reason. FWIW, -22 is -EINVAL. Linus
On Mon, Jul 15, 2024 at 09:29:34PM -0700, Linus Torvalds wrote: > On Mon, 15 Jul 2024 at 19:17, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > Hmm. Let's see how this all works out, but I already found an annoyance. > > .. and another one. > > On my Altra box, commit 8fb18619d910 ("PCI/pwrctl: Create platform > devices for child OF nodes of the port node") causes annoying messages > at bootup: > > pci 000c:00:01.0: failed to populate child OF nodes (-22) > pci 000c:00:02.0: failed to populate child OF nodes (-22) > .. repeat for every PCI bridge .. > > for no obvious reason. > > FWIW, -22 is -EINVAL. > So we did see these error messages on non-CONFIG_OF platforms, and a fix was merged as well with commit, 50b040ef3732 ("PCI/pwrctl: only call of_platform_populate() if CONFIG_OF is enabled") But apparently, the fix assumed that all CONFIG_OF platforms (selected in defconfig) have 'dev.of_node' populated. And your platforms being an ARM64 one, has CONFIG_OF selected ARM64 defconfig, but uses ACPI instead of devicetree. So you don't have 'dev.of_node', which is a valid configuration btw (we failed to spot it). And in other places of these of_ APIs, we do have checks for 'dev.of_node'. So for this issue, below diff should be sufficient: diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 3bab78cc68f7..abe826bb5840 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -350,7 +350,7 @@ void pci_bus_add_device(struct pci_dev *dev) pci_dev_assign_added(dev, true); - if (IS_ENABLED(CONFIG_OF) && pci_is_bridge(dev)) { + if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node && pci_is_bridge(dev)) { retval = of_platform_populate(dev->dev.of_node, NULL, NULL, &dev->dev); if (retval) Let me know if it works, I can spin a patch. - Mani
On Tue, Jul 16, 2024 at 4:17 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Fri, 12 Jul 2024 at 02:13, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > This PR contains the core power sequencing framework, the first driver, PCI > > changes using the pwrseq library (blessed by Bjorn Helgaas) and some fixes > > that came later. > > Hmm. Let's see how this all works out, but I already found an annoyance. > > It first asks me about the new PCI power sequencing driver. > > And then it asks me separately if I want the power sequencing support. > > Now, either this should > > (a) not ask about the generic power sequencing support at all, and > just select if if a driver that is enabled needs it > > OR > > (b) it should ask about power sequencing support and then if you say > "N", it should not ask about the drivers. > > But asking *twice* is definitely not kosher. > > Linus I didn't notice it because I almost always use menuconfig. I'll look into it. Bartosz
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Linus, I'm sending this early as this is the initial pull-request for a new driver subsystem living under drivers/power/sequencing/. I'll try to be brief here and allow myself to link the cover letter from the last time the series was sent in its entirety[1] (as opposed to smaller chunks targetting specific maintainers) for a very detailed description of the problem and the solution. I'll just stick to the key points below. This has been in development since last year's Linux Plumbers Conference and was inspired by the need to enable support upstream for Bluetooth/WLAN chips on Qualcomm platforms. The main problem we're fixing is powering up devices which are represented as separate objects in the kernel (binding to different drivers) but which share parts of the power-up sequence and thus need some kind of a mediator who knows the possible interactions and can assure they don't interfere with neither device's bring up. An example of such an inter-driver interaction is the WCN family of BT/WLAN chips from Qualcomm of which some models require the user to observe a certain delay between driving the bt-enable and wlan-enable GPIOs. This is not a new problem but up to this point all attempts at addressing it ended up hitting one wall or another and being dropped. The main obstacle was the fact that most these attempts tried to introduce the concept of a "power sequence" into the device-tree bindings which breaks the main DT rule: describe the hardware, not its behavior. The solution I proposed focuses on making the power sequencer drivers interpret the actual HW description flexibly. More details on that are in the linked cover letter. The second problem fixed here is powering up PCI devices before they are detected on the bus. This is achieved by creating special platform devices for device-tree nodes describing hard-wired PCI devices which bind to the so-called PCI power control drivers which enable required resources and trigger a bus rescan once the controlled device is up then setup the correct devlink hierarchy for power-management. By combining the two new frameworks we implemented the power sequencing PCI power control driver which is capable of powering up the WLAN modules of the QCom WCN family of chipsets. All this has spent a significant amount of time in linux-next and enabled WLAN/BT support on several Qualcomm platforms. To further prove that this is useful and needed: right after this was picked up into next, I was sent a series using the subsystem for a similar use-case on Amlogic platforms[2]. This PR contains the core power sequencing framework, the first driver, PCI changes using the pwrseq library (blessed by Bjorn Helgaas) and some fixes that came later. You'll also see the pwrseq core pulled into the Bluetooth tree to satisfy the build-time dependency on power sequencing in the hci_qca driver which uses the same power sequence provider as the PCI pwrctl driver added in this PR. More changes that don't have build-time dependencies on pwrseq are scattered across three other maintainer trees: there will be DT bindings in the regulator and wireless trees and DTS changes in the arm64 tree. Please consider pulling for v6.11. Best Regards, Bartosz Golaszewski [1] https://lore.kernel.org/all/20240528-pwrseq-v8-0-d354d52b763c@linaro.org/ [2] https://lore.kernel.org/lkml/20240705-pwrseq-v1-0-31829b47fc72@amlogic.com/ The following changes since commit 83a7eefedc9b56fe7bfeff13b6c7356688ffa670: Linux 6.10-rc3 (2024-06-09 14:19:43 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/pwrseq-updates-for-v6.11-rc1 for you to fetch changes up to 50b040ef373293b4ae2ecdc5873daa4656724868: PCI/pwrctl: only call of_platform_populate() if CONFIG_OF is enabled (2024-07-08 21:15:26 +0200) ---------------------------------------------------------------- pwrseq updates for v6.11-rc1 - add the pwrseq core framework - add the first power sequencing driver: pwrseq-qcom-wcn - add power control (pwrctl) changes to PCI core - add the first PCI pwrctl power sequencing driver ---------------------------------------------------------------- Bartosz Golaszewski (7): 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 Bert Karwatzki (1): PCI/pwrctl: only call of_platform_populate() if CONFIG_OF is enabled Krzysztof Kozlowski (1): power: sequencing: simplify returning pointer without cleanup MAINTAINERS | 16 + 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 | 29 + 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 ++ 20 files changed, 1950 insertions(+), 5 deletions(-) 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