Re: [PATCH 0/4] PCI/pwrctrl: Rework pwrctrl driver integration and add driver for PCI slot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/10/2024 5:55 PM, Manivannan Sadhasivam via B4 Relay wrote:
Hi,

This series reworks the PCI pwrctrl integration (again) by moving the creation
and removal of pwrctrl devices to pci_scan_device() and pci_destroy_dev() APIs.
This is based on the suggestion provided by Lukas Wunner [1][2]. With this
change, it is now possible to create pwrctrl devices for PCI bridges as well.
This is required to control the power state of the PCI slots in a system. Since
the PCI slots are not explicitly defined in devicetree, the agreement is to
define the supplies for PCI slots in PCI bridge nodes itself [3].

Based on this, a pwrctrl driver to control the supplies of PCI slots are also
added in patch 4. With this driver, it is now possible to control the voltage
regulators powering the PCI slots defined in PCI bridge nodes as below:

```
pcie@0 {
	compatible "pciclass,0604"
	...

	vpcie12v-supply = <&vpcie12v_reg>;
	vpcie3v3-supply = <&vpcie3v3_reg>;
	vpcie3v3aux-supply = <&vpcie3v3aux_reg>;
};
```

To make use of this driver, the PCI bridge DT node should also have the
compatible "pciclass,0604". But adding this compatible triggers the following
checkpatch warning:

WARNING: DT compatible string vendor "pciclass" appears un-documented --
check ./Documentation/devicetree/bindings/vendor-prefixes.yaml

For fixing it, I added patch 3. But due to some reason, checkpatch is not
picking the 'pciclass' vendor prefix alone, and requires adding the full
compatible 'pciclass,0604' in the vendor-prefixes list. Since my perl skills are
not great, I'm leaving it in the hands of Rob to fix the checkpatch script.

[1] https://lore.kernel.org/linux-pci/Z0yLDBMAsh0yKWf2@xxxxxxxxx
[2] https://lore.kernel.org/linux-pci/Z0xAdQ2ozspEnV5g@xxxxxxxxx
[3] https://github.com/devicetree-org/dt-schema/issues/145

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
---
Manivannan Sadhasivam (4):
       PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()
       PCI/pwrctrl: Move pci_pwrctrl_unregister() to pci_destroy_dev()
       dt-bindings: vendor-prefixes: Document the 'pciclass' prefix
       PCI/pwrctrl: Add pwrctrl driver for PCI Slots

  .../devicetree/bindings/vendor-prefixes.yaml       |  2 +-
  drivers/pci/bus.c                                  | 43 ----------
  drivers/pci/probe.c                                | 34 ++++++++
  drivers/pci/pwrctrl/Kconfig                        | 11 +++
  drivers/pci/pwrctrl/Makefile                       |  3 +
  drivers/pci/pwrctrl/core.c                         |  2 +-
  drivers/pci/pwrctrl/slot.c                         | 93 ++++++++++++++++++++++
  drivers/pci/remove.c                               |  2 +-
  8 files changed, 144 insertions(+), 46 deletions(-)
---
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
change-id: 20241210-pci-pwrctrl-slot-02c0ec63172f

Best regards,

Hi Mani

PCIe3 is able to link up after applying your patch. Slot power is turned on correctly.
But see “NULL pointer dereference” when I try to remove device.

root@linaro-gnome:/sys/bus/pci/devices/0003:00:00.0# echo 1 > remove

[   38.752976] ------------[ cut here ]------------
[   38.757726] WARNING: CPU: 1 PID: 816 at drivers/regulator/core.c:5857 regulator_unregister+0x13c/0x160 [   38.767288] Modules linked in: phy_qcom_qmp_combo aux_bridge drm_kms_helper drm nvme backlight pinctrl_sm8550_lpass_lpi pci_pwrctl_slot pci_pwrctrl_core nvme_core phy_qcom_edp phy_qcom_eusb2_repeater dispcc_x1e80100 pinctrl_lpass_lpi phy_qcom_snps_eusb2 lpasscc_sc8280xp typec gpucc_x1e80100 phy_qcom_qmp_pcie [   38.795279] CPU: 1 UID: 0 PID: 816 Comm: bash Not tainted 6.12.0-next-20241128-00005-g6178bf6ce3c2-dirty #50 [   38.805359] Hardware name: Qualcomm IDP, BIOS 6.0.240607.BOOT.MXF.2.4-00348.1-HAMOA-1.67705.7 06/ 7/2024 [   38.815088] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   38.822239] pc : regulator_unregister+0x13c/0x160
[   38.827081] lr : regulator_unregister+0xc0/0x160
[   38.831829] sp : ffff800082ad39e0
[   38.835238] x29: ffff800082ad39e0 x28: ffff67874a4b1140 x27: 0000000000000000 [   38.842563] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000 [   38.849895] x23: 0000000000000001 x22: ffff800082ad3a88 x21: 0000000000000000 [   38.857220] x20: ffffb72b1c7de2b0 x19: ffff678740f60000 x18: ffffffffffffffff [   38.864545] x17: 2e30303030646231 x16: ffffb72b1a68d54c x15: 3030303064424337 [   38.871867] x14: 000000000000000b x13: ffff6787402b6010 x12: 0000000000000000 [   38.879200] x11: 0000000000000000 x10: ffffb72b1a689fe8 x9 : ffffb72b1a689bc8 [   38.886525] x8 : ffff678740e5b2c0 x7 : ffff800082ad3a88 x6 : ffff678740e5b2c0 [   38.893849] x5 : ffff678740e5b2c0 x4 : ffff678740e5b2c0 x3 : ffff678740e5b2c0 [   38.901172] x2 : ffff67874a4b1140 x1 : 0000000000000000 x0 : 0000000000000007
[   38.908496] Call trace:
[   38.911019]  regulator_unregister+0x13c/0x160 (P)
[   38.915856]  regulator_unregister+0xc0/0x160 (L)
[   38.920600]  devm_rdev_release+0x14/0x20
[   38.924634]  devres_release_all+0xa0/0x100
[   38.928845]  device_unbind_cleanup+0x18/0x60
[   38.933239]  device_release_driver_internal+0x1ec/0x228
[   38.938606]  device_release_driver+0x18/0x24
[   38.942998]  bus_remove_device+0xcc/0x10c
[   38.947122]  device_del+0x14c/0x404
[   38.950705]  device_unregister+0x18/0x34
[   38.954738]  of_device_unregister+0x14/0x20
[   38.959041]  pci_remove_bus_device+0x110/0x1b0
[   38.963612]  pci_remove_bus_device+0x38/0x1b0
[   38.968094]  pci_stop_and_remove_bus_device_locked+0x28/0x3c
[   38.973907]  remove_store+0x94/0xa4
[   38.977494]  dev_attr_store+0x18/0x2c
[   38.981263]  sysfs_kf_write+0x44/0x54
[   38.985037]  kernfs_fop_write_iter+0x118/0x1a8
[   38.989607]  vfs_write+0x2b0/0x35c
[   38.993107]  ksys_write+0x68/0xfc
[   38.996519]  __arm64_sys_write+0x1c/0x28
[   39.000552]  invoke_syscall+0x48/0x110
[   39.004411]  el0_svc_common.constprop.0+0x40/0xe8
[   39.009244]  do_el0_svc+0x20/0x2c
[   39.012655]  el0_svc+0x30/0xd0
[   39.015805]  el0t_64_sync_handler+0x144/0x168
[   39.020283]  el0t_64_sync+0x198/0x19c
[   39.024052] ---[ end trace 0000000000000000 ]---
[   39.028897] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000c0
[   39.037932] Mem abort info:
[   39.040823]   ESR = 0x0000000096000004
[   39.044691]   EC = 0x25: DABT (current EL), IL = 32 bits
[   39.050161]   SET = 0, FnV = 0
[   39.053316]   EA = 0, S1PTW = 0
[   39.056557]   FSC = 0x04: level 0 translation fault
[   39.061585] Data abort info:
[   39.064554]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   39.070190]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   39.075395]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   39.080861] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000889f93000
[   39.087484] [00000000000000c0] pgd=0000000000000000, p4d=0000000000000000
[   39.094467] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[   39.100903] Modules linked in: phy_qcom_qmp_combo aux_bridge drm_kms_helper drm nvme backlight pinctrl_sm8550_lpass_lpi pci_pwrctl_slot pci_pwrctrl_core nvme_core phy_qcom_edp phy_qcom_eusb2_repeater dispcc_x1e80100 pinctrl_lpass_lpi phy_qcom_snps_eusb2 lpasscc_sc8280xp typec gpucc_x1e80100 phy_qcom_qmp_pcie [   39.128882] CPU: 1 UID: 0 PID: 816 Comm: bash Tainted: G W          6.12.0-next-20241128-00005-g6178bf6ce3c2-dirty #50
[   39.140480] Tainted: [W]=WARN
[   39.143540] Hardware name: Qualcomm IDP, BIOS 6.0.240607.BOOT.MXF.2.4-00348.1-HAMOA-1.67705.7 06/ 7/2024 [   39.153273] pstate: 21400005 (nzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   39.160421] pc : pci_remove_bus_device+0x120/0x1b0
[   39.165341] lr : pci_remove_bus_device+0x110/0x1b0
[   39.170261] sp : ffff800082ad3c00
[   39.173676] x29: ffff800082ad3c00 x28: ffff67874a4b1140 x27: 0000000000000000 [   39.181004] x26: 0000000000000000 x25: 0000000000000000 x24: ffff67874cb2caa0 [   39.188334] x23: ffff800082ad3d88 x22: 0000000000000000 x21: ffff6787458f00c8 [   39.195661] x20: ffff6787458f0000 x19: ffffb72b1c5e88d8 x18: ffffffffffffffff [   39.202984] x17: 74616c703d4d4554 x16: 5359534255530079 x15: 6d6d75642d676572 [   39.210311] x14: ffffb72b1ca01098 x13: 0000000000000040 x12: 0000000000000228 [   39.217638] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000 [   39.224964] x8 : 0000000000000000 x7 : ffff678740d3ebc8 x6 : ffff678745584b40 [   39.232296] x5 : ffff6787411c64e0 x4 : ffff6787411c64e0 x3 : ffff678741256679 [   39.239626] x2 : ffff678740e5b048 x1 : 0000000000000008 x0 : 00000000000000c0
[   39.246951] Call trace:
[   39.249469]  pci_remove_bus_device+0x120/0x1b0 (P)
[   39.254399]  pci_remove_bus_device+0x110/0x1b0 (L)
[   39.259326]  pci_remove_bus_device+0x38/0x1b0
[   39.263801]  pci_stop_and_remove_bus_device_locked+0x28/0x3c
[   39.269620]  remove_store+0x94/0xa4
[   39.273209]  dev_attr_store+0x18/0x2c
[   39.276972]  sysfs_kf_write+0x44/0x54
[   39.280735]  kernfs_fop_write_iter+0x118/0x1a8
[   39.285305]  vfs_write+0x2b0/0x35c
[   39.288808]  ksys_write+0x68/0xfc
[   39.292221]  __arm64_sys_write+0x1c/0x28
[   39.296258]  invoke_syscall+0x48/0x110
[   39.300119]  el0_svc_common.constprop.0+0x40/0xe8
[   39.304952]  do_el0_svc+0x20/0x2c
[   39.308365]  el0_svc+0x30/0xd0
[   39.311515]  el0t_64_sync_handler+0x144/0x168
[   39.316002]  el0t_64_sync+0x198/0x19c
[   39.319766] Code: f9414aa0 d503201f d2800101 91030000 (f821101f)
[   39.326029] ---[ end trace 0000000000000000 ]---

Thanks,
Qiang





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux