| Message ID | 20240201155532.49707-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | power: sequencing: implement the subsystem and add first users | expand |
On Thu, Feb 01, 2024 at 04:55:23PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > We now have 3 RFC and 1 PATCH versions of these patches on the list in under a month. Please at least add a version to your subject... > I'd like to preface the cover letter by saying right away that this > series is not complete. It's an RFC that presents my approach and is sent > to the list for discussion. There are no DT bindings nor docs in > Documentation/ yet. Please review it as an RFC and not an upstreambound > series. If the approach is accepted as correct, I'll add missing bits. > > The RFC[1] presenting my proposed device-tree representation of the > QCA6391 package present on the RB5 board - while not really officially > accepted - was not outright rejected which is a good sign. > > This series incorporates it and builds a proposed power sequencing > subsystem together with the first dedicated driver around it. Then it > adds first two users: the Bluetooth and WLAN modules of the QCA6391. > > The Bluetooth part is pretty straightforward. The WLAN however is a PCIe > device and as such needs to be powered-up *before* it's detected on the > PCI bus. To that end, we modify the PCI core to instantiate platform > devices for existing DT child nodes of the PCIe ports. For those nodes > for which a power-sequencing driver exists, we bind it and let it probe. > The driver then triggers a rescan of the PCI bus with the aim of > detecting the now powered-on device. The device will consume the same DT > node as the platform, power-sequencing device. We use device links to > make the latter become the parent of the former. > > The main advantage of the above approach (both for PCI as well as > generic power sequencers) is that we don't introduce significant changes > in DT bindings and don't introduce new properties. We merely define new > resources. > How can we tell? There are still no Documentation/dt-bindings changes in your series. Regards, Bjorn > [1] https://lore.kernel.org/all/CAMRc=MckG32DQv7b1AQL-mbnYdx4fsdYWtLwCyXc5Ma7EeSAKw@mail.gmail.com/T/#md5dc62007d12f6833d4e51658b14e0493954ba68 > > Bartosz Golaszewski (9): > of: provide a cleanup helper for OF nodes > arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 > power: sequencing: new subsystem > power: pwrseq: add a driver for the QCA6390 PMU module > Bluetooth: qca: use the power sequencer for QCA6390 > PCI: create platform devices for child OF nodes of the port node > PCI: hold the rescan mutex when scanning for the first time > PCI/pwrctl: add PCI power control core code > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 128 +++++- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + > drivers/bluetooth/hci_qca.c | 30 ++ > drivers/pci/Kconfig | 1 + > drivers/pci/Makefile | 1 + > drivers/pci/bus.c | 9 +- > drivers/pci/probe.c | 2 + > drivers/pci/pwrctl/Kconfig | 17 + > drivers/pci/pwrctl/Makefile | 4 + > drivers/pci/pwrctl/core.c | 82 ++++ > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 83 ++++ > 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 | 482 ++++++++++++++++++++++ > drivers/power/sequencing/pwrseq-qca6390.c | 232 +++++++++++ > include/linux/of.h | 4 + > include/linux/pci-pwrctl.h | 24 ++ > include/linux/pwrseq/consumer.h | 53 +++ > include/linux/pwrseq/provider.h | 41 ++ > 22 files changed, 1229 insertions(+), 12 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-qca6390.c > create mode 100644 include/linux/pci-pwrctl.h > create mode 100644 include/linux/pwrseq/consumer.h > create mode 100644 include/linux/pwrseq/provider.h > > -- > 2.40.1 >
On Thu, Feb 01, 2024 at 04:55:23PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > I'd like to preface the cover letter by saying right away that this > series is not complete. It's an RFC that presents my approach and is sent > to the list for discussion. There are no DT bindings nor docs in > Documentation/ yet. Please review it as an RFC and not an upstreambound > series. If the approach is accepted as correct, I'll add missing bits. > > The RFC[1] presenting my proposed device-tree representation of the > QCA6391 package present on the RB5 board - while not really officially > accepted - was not outright rejected which is a good sign. > > This series incorporates it and builds a proposed power sequencing > subsystem together with the first dedicated driver around it. Then it > adds first two users: the Bluetooth and WLAN modules of the QCA6391. > > The Bluetooth part is pretty straightforward. The WLAN however is a PCIe > device and as such needs to be powered-up *before* it's detected on the > PCI bus. To that end, we modify the PCI core to instantiate platform > devices for existing DT child nodes of the PCIe ports. For those nodes > for which a power-sequencing driver exists, we bind it and let it probe. > The driver then triggers a rescan of the PCI bus with the aim of > detecting the now powered-on device. The device will consume the same DT > node as the platform, power-sequencing device. We use device links to > make the latter become the parent of the former. > > The main advantage of the above approach (both for PCI as well as > generic power sequencers) is that we don't introduce significant changes > in DT bindings and don't introduce new properties. We merely define new > resources. > > [1] https://lore.kernel.org/all/CAMRc=MckG32DQv7b1AQL-mbnYdx4fsdYWtLwCyXc5Ma7EeSAKw@mail.gmail.com/T/#md5dc62007d12f6833d4e51658b14e0493954ba68 FWIW, booting RB5 with this patch series does give me working working WiFI. But I do see the following splat during boot: [ 5.880411] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0' [ 5.891938] CPU: 5 PID: 68 Comm: kworker/u16:4 Not tainted 6.8.0-rc2-next-20240131-00009-g079fdad54c8f #199 [ 5.901927] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT) [ 5.908808] Workqueue: events_unbound async_run_entry_fn [ 5.914274] Call trace: [ 5.916794] dump_backtrace+0xec/0x108 [ 5.920649] show_stack+0x18/0x24 [ 5.924062] dump_stack_lvl+0x50/0x68 [ 5.927826] dump_stack+0x18/0x24 [ 5.931238] sysfs_warn_dup+0x64/0x80 [ 5.935004] sysfs_create_bin_file+0xf4/0x130 [ 5.939480] pci_create_attr+0x100/0x168 [ 5.943509] pci_create_sysfs_dev_files+0x6c/0xc0 [ 5.948337] pci_bus_add_device+0x60/0x114 [ 5.952551] pci_bus_add_devices+0x4c/0x7c [ 5.956762] pci_host_probe+0x138/0x188 [ 5.960700] dw_pcie_host_init+0x290/0x334 [ 5.964914] qcom_pcie_probe+0x1f8/0x23c [ 5.968942] platform_probe+0xa8/0xd0 [ 5.972707] really_probe+0x130/0x2e4 [ 5.976469] __driver_probe_device+0xa0/0x128 [ 5.980944] driver_probe_device+0x3c/0x1f8 [ 5.985245] __device_attach_driver+0x118/0x140 [ 5.989897] bus_for_each_drv+0xf4/0x14c [ 5.993923] __device_attach_async_helper+0x78/0xd0 [ 5.998937] async_run_entry_fn+0x24/0xdc [ 6.003051] process_scheduled_works+0x210/0x328 [ 6.007793] worker_thread+0x28c/0x450 [ 6.011642] kthread+0xfc/0x184 [ 6.014865] ret_from_fork+0x10/0x20 [ 6.018572] ------------[ cut here ]------------ [ 6.023339] proc_dir_entry '0000:00/00.0' already registered Regards, Bjorn
On Fri, Feb 2, 2024 at 1:40 AM Bjorn Andersson <andersson@kernel.org> wrote: > > On Thu, Feb 01, 2024 at 04:55:23PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > We now have 3 RFC and 1 PATCH versions of these patches on the list in > under a month. Please at least add a version to your subject... > So there was an RFC for the PCI power sequencing (now renamed to PCI power control - pci_pwrctl), then a proper series for it (with changes listed). Then an RFC with just proposed DT changes (sent mostly to DT maintainers to clear it with them) and now an RFC with power sequencing + PCI power control. Hard to figure out how to version it if these are pretty much separate developments. I'll provide links to everything next time. > > I'd like to preface the cover letter by saying right away that this > > series is not complete. It's an RFC that presents my approach and is sent > > to the list for discussion. There are no DT bindings nor docs in > > Documentation/ yet. Please review it as an RFC and not an upstreambound > > series. If the approach is accepted as correct, I'll add missing bits. > > > > The RFC[1] presenting my proposed device-tree representation of the > > QCA6391 package present on the RB5 board - while not really officially > > accepted - was not outright rejected which is a good sign. > > > > This series incorporates it and builds a proposed power sequencing > > subsystem together with the first dedicated driver around it. Then it > > adds first two users: the Bluetooth and WLAN modules of the QCA6391. > > > > The Bluetooth part is pretty straightforward. The WLAN however is a PCIe > > device and as such needs to be powered-up *before* it's detected on the > > PCI bus. To that end, we modify the PCI core to instantiate platform > > devices for existing DT child nodes of the PCIe ports. For those nodes > > for which a power-sequencing driver exists, we bind it and let it probe. > > The driver then triggers a rescan of the PCI bus with the aim of > > detecting the now powered-on device. The device will consume the same DT > > node as the platform, power-sequencing device. We use device links to > > make the latter become the parent of the former. > > > > The main advantage of the above approach (both for PCI as well as > > generic power sequencers) is that we don't introduce significant changes > > in DT bindings and don't introduce new properties. We merely define new > > resources. > > > > How can we tell? There are still no Documentation/dt-bindings changes in > your series. Noted. Bartosz > > Regards, > Bjorn > > > [1] https://lore.kernel.org/all/CAMRc=MckG32DQv7b1AQL-mbnYdx4fsdYWtLwCyXc5Ma7EeSAKw@mail.gmail.com/T/#md5dc62007d12f6833d4e51658b14e0493954ba68 > > > > Bartosz Golaszewski (9): > > of: provide a cleanup helper for OF nodes > > arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 > > power: sequencing: new subsystem > > power: pwrseq: add a driver for the QCA6390 PMU module > > Bluetooth: qca: use the power sequencer for QCA6390 > > PCI: create platform devices for child OF nodes of the port node > > PCI: hold the rescan mutex when scanning for the first time > > PCI/pwrctl: add PCI power control core code > > PCI/pwrctl: add a PCI power control driver for power sequenced devices > > > > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 128 +++++- > > arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + > > drivers/bluetooth/hci_qca.c | 30 ++ > > drivers/pci/Kconfig | 1 + > > drivers/pci/Makefile | 1 + > > drivers/pci/bus.c | 9 +- > > drivers/pci/probe.c | 2 + > > drivers/pci/pwrctl/Kconfig | 17 + > > drivers/pci/pwrctl/Makefile | 4 + > > drivers/pci/pwrctl/core.c | 82 ++++ > > drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 83 ++++ > > 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 | 482 ++++++++++++++++++++++ > > drivers/power/sequencing/pwrseq-qca6390.c | 232 +++++++++++ > > include/linux/of.h | 4 + > > include/linux/pci-pwrctl.h | 24 ++ > > include/linux/pwrseq/consumer.h | 53 +++ > > include/linux/pwrseq/provider.h | 41 ++ > > 22 files changed, 1229 insertions(+), 12 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-qca6390.c > > create mode 100644 include/linux/pci-pwrctl.h > > create mode 100644 include/linux/pwrseq/consumer.h > > create mode 100644 include/linux/pwrseq/provider.h > > > > -- > > 2.40.1 > >
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> I'd like to preface the cover letter by saying right away that this series is not complete. It's an RFC that presents my approach and is sent to the list for discussion. There are no DT bindings nor docs in Documentation/ yet. Please review it as an RFC and not an upstreambound series. If the approach is accepted as correct, I'll add missing bits. The RFC[1] presenting my proposed device-tree representation of the QCA6391 package present on the RB5 board - while not really officially accepted - was not outright rejected which is a good sign. This series incorporates it and builds a proposed power sequencing subsystem together with the first dedicated driver around it. Then it adds first two users: the Bluetooth and WLAN modules of the QCA6391. The Bluetooth part is pretty straightforward. The WLAN however is a PCIe device and as such needs to be powered-up *before* it's detected on the PCI bus. To that end, we modify the PCI core to instantiate platform devices for existing DT child nodes of the PCIe ports. For those nodes for which a power-sequencing driver exists, we bind it and let it probe. The driver then triggers a rescan of the PCI bus with the aim of detecting the now powered-on device. The device will consume the same DT node as the platform, power-sequencing device. We use device links to make the latter become the parent of the former. The main advantage of the above approach (both for PCI as well as generic power sequencers) is that we don't introduce significant changes in DT bindings and don't introduce new properties. We merely define new resources. [1] https://lore.kernel.org/all/CAMRc=MckG32DQv7b1AQL-mbnYdx4fsdYWtLwCyXc5Ma7EeSAKw@mail.gmail.com/T/#md5dc62007d12f6833d4e51658b14e0493954ba68 Bartosz Golaszewski (9): of: provide a cleanup helper for OF nodes arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391 power: sequencing: new subsystem power: pwrseq: add a driver for the QCA6390 PMU module Bluetooth: qca: use the power sequencer for QCA6390 PCI: create platform devices for child OF nodes of the port node PCI: hold the rescan mutex when scanning for the first time PCI/pwrctl: add PCI power control core code PCI/pwrctl: add a PCI power control driver for power sequenced devices arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 128 +++++- arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 + drivers/bluetooth/hci_qca.c | 30 ++ drivers/pci/Kconfig | 1 + drivers/pci/Makefile | 1 + drivers/pci/bus.c | 9 +- drivers/pci/probe.c | 2 + drivers/pci/pwrctl/Kconfig | 17 + drivers/pci/pwrctl/Makefile | 4 + drivers/pci/pwrctl/core.c | 82 ++++ drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 83 ++++ 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 | 482 ++++++++++++++++++++++ drivers/power/sequencing/pwrseq-qca6390.c | 232 +++++++++++ include/linux/of.h | 4 + include/linux/pci-pwrctl.h | 24 ++ include/linux/pwrseq/consumer.h | 53 +++ include/linux/pwrseq/provider.h | 41 ++ 22 files changed, 1229 insertions(+), 12 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-qca6390.c create mode 100644 include/linux/pci-pwrctl.h create mode 100644 include/linux/pwrseq/consumer.h create mode 100644 include/linux/pwrseq/provider.h