Re: [PATCH] riscv: dts: spacemit: drop unnecessary power supply from PCIe

[Yixun Lan <dlan@kernel.org>]

Hi

On 00:54 Thu 14 May     , sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `vpcie3v3-supply` from the PCIe controller node breaks power sequencing because the `spacemit,k1-pcie` driver expects it on the controller node.
> --
> 
> commit ab7558e23a3bb4900f7961804151870f4c1f99fe
> Author: Yixun Lan <dlan@kernel.org>
> 
> riscv: dts: spacemit: drop unnecessary power supply from PCIe
> 
> This commit removes the vpcie3v3-supply property from the PCIe controller
> nodes in the Banana Pi F3 and Milk-V Jupiter device trees. The commit message
> states that the supply belongs to the child port node rather than the Root
> Complex, aligning with the dt-binding documentation.
> 
> Link: https://lore.kernel.org/all/20251113214540.2623070-4-elder@riscstar.com/
> 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > index e20daa50a152f..c70081856606f 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > @@ -316,7 +316,6 @@ &pcie1_port {
> >  };
> >  
> >  &pcie1 {
> > -	vpcie3v3-supply = <&pcie_vcc_3v3>;
> 
> Will removing the vpcie3v3-supply property from the controller node break
> power sequencing?
> 
No, I don't think so, the PCIe RC do not require external power supply

> Looking at drivers/pci/controller/dwc/pcie-spacemit-k1.c, the host driver
> calls devm_regulator_get_enable(dev, "vpcie3v3") during k1_pcie_probe().
> 
Right, I think the vpcie3v3 supply should be removed from host driver

> Without this property on the controller node, won't the driver fall back
> to a dummy regulator?
> 
Currently, yes if above code not removed, it will fallback to dummy
regulator if no vpcie3v3-supply specified

> Since the spacemit-k1-pcie driver does not currently integrate with the PCI
> power control framework (by calling pci_pwrctrl_create_devices()), does any
> driver in the kernel actually control the power supply for the child port node?
> 
Yes, the PCIe sub ports node will request the power supply, in 
drivers/pci/pwrctrl/generic.c:119
 slot_pwrctrl_probe() -> of_regulator_buck_get_all()

> While this issue might be masked on the Banana Pi F3 and Milk-V Jupiter
> boards because their vpcie3v3 regulators are defined as regulator-always-on,
> does this change break dynamic power management for the PCIe slots?
> 
> Should the host driver be updated to adopt the pci_pwrctrl framework before
> dropping this property from the device tree?
> 
The host driver already adopt pci_pwrctrl framework

> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260513-k1-pcie-fix-supply-v1-1-4eecbdd44ebe@kernel.org?part=1

-- 
Yixun Lan (dlan)