[PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Yixun Lan]

The PCIe port require 3.3v power regulator for device to work properly, So
explicitly add it to fix the DT warning:

arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: pcie@ca400000 (spacemit,k1-pcie): pcie@0: 'vpcie3v3-supply' is a required property
        from schema $id: http://devicetree.org/schemas/pci/spacemit,k1-pcie-host.yaml

Fixes: 0be016a4b5d1 ("riscv: dts: spacemit: PCIe and PHY-related updates")
Reported-by: Conor Dooley <conor@kernel.org>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
This patch try to add power regulator to PCIe port node, thus will also
fix the dts warning when doing DT schema check.

Please also note driver of spacemit PCIe request 3.3v power regulator, see
drivers/pci/controller/dwc/pcie-spacemit-k1.c:312, it's not necessary
because the controller itself doesn't need it to work. But I do see other
drivers that request the regulator in host controller driver, for example,
to power it up early before PHY initilization. I'd intentionally leave
this problem out of this fix, and will do further check & address it
later.

Problem reported by Conor Dooley, so I added a R-Y tag for him here.
---
 arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
index 5971605754b3..51f6c6a774b0 100644
--- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
@@ -305,6 +305,7 @@ &pcie1_phy {
 
 &pcie1_port {
 	phys = <&pcie1_phy>;
+	vpcie3v3-supply = <&pcie_vcc_3v3>;
 };
 
 &pcie1 {
@@ -320,6 +321,7 @@ &pcie2_phy {
 
 &pcie2_port {
 	phys = <&pcie2_phy>;
+	vpcie3v3-supply = <&pcie_vcc_3v3>;
 };
 
 &pcie2 {

---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260226-k1-pcie-fix-pwr-7f26118121ac

Best regards,
-- 
Yixun Lan <dlan@kernel.org>

Re: [PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Chukun Pan]

Hi,

> &pcie1_port {
>  	phys = <&pcie1_phy>;
> +	vpcie3v3-supply = <&pcie_vcc_3v3>;
>  };
> 
>  &pcie1 {
> @@ -320,6 +321,7 @@ &pcie2_phy {
> 
>  &pcie2_port {
>  	phys = <&pcie2_phy>;
> +	vpcie3v3-supply = <&pcie_vcc_3v3>;
>  };

```
&pcie1 {
	vpcie3v3-supply = <&pcie_vcc_3v3>;
	status = "okay";
};
```

According to DT binding, the vpcie3v3-supply of the &pciex node should
be moved to the &pciex_port node. This is simply a duplication of the
property.

But do we really need this pcie_port (PCIe bridge)?

The PCIe bridge node (pcie@0) was treated as a platform device, but it
did not define the interrupts property, which resulted in the following
warning: `[    2.897980] irq: no irq domain found for pcie@0 !`

Would it be better to submit a patch to remove this pcie_port?

```
-       ret = k1_pcie_parse_port(k1);
-       if (ret)
-               return dev_err_probe(dev, ret, "failed to parse root port\n");
+       k1->phy = devm_phy_get(dev, "pcie-phy");
+       if (IS_ERR(k1->phy))
+               return dev_err_probe(dev, PTR_ERR(k1->phy), "missing PHY\n");
```

I have tested this change and it works.

Thanks,
Chukun

Re: [PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Yixun Lan]

On Thu, 26 Feb 2026 08:17:55 +0000, Yixun Lan wrote:
> The PCIe port require 3.3v power regulator for device to work properly, So
> explicitly add it to fix the DT warning:
> 
> arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dtb: pcie@ca400000 (spacemit,k1-pcie): pcie@0: 'vpcie3v3-supply' is a required property
>         from schema $id: http://devicetree.org/schemas/pci/spacemit,k1-pcie-host.yaml
> 
> 
> [...]

Applied, thanks!

[1/1] riscv: dts: spacemit: pcie: fix missing power regulator
      https://github.com/spacemit-com/linux/commit/8a9071299dec817a544c0fb48f7302396fafdc4b

Best regards,
-- 
Yixun Lan <dlan@kernel.org>

Re: [PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Yixun Lan]

Hi Chukun,

Sorry, I missed your mail due recent problem of my client..
On 11:05 Mon 02 Mar     , Chukun Pan wrote:
> Hi,
> 
> > &pcie1_port {
> >  	phys = <&pcie1_phy>;
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> >  };
> > 
> >  &pcie1 {
> > @@ -320,6 +321,7 @@ &pcie2_phy {
> > 
> >  &pcie2_port {
> >  	phys = <&pcie2_phy>;
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> >  };
> 
> ```
> &pcie1 {
> 	vpcie3v3-supply = <&pcie_vcc_3v3>;
> 	status = "okay";
> };
> ```
> 
> According to DT binding, the vpcie3v3-supply of the &pciex node should
> be moved to the &pciex_port node. This is simply a duplication of the
> property.
> 
I have confidence that pcie port need to add a regulator to provide 
supply for devices..

But, I'm not sure whether it's ok to remove regulator from &pciex node,
it's possibly a 'yes' answer, but to convince me, I'd like to see a real
test case to prove it: e.g, power supply for pciex is actually off before 
the driver initialization, then run regular procedure as it should, if
all works fine

Btw, different drivers request same regulator is ok, and is quite normal,
can't draw a conclusion that it's a duplication, as they may be used for
different reasons

> But do we really need this pcie_port (PCIe bridge)?
> 
> The PCIe bridge node (pcie@0) was treated as a platform device, but it
> did not define the interrupts property, which resulted in the following
> warning: `[    2.897980] irq: no irq domain found for pcie@0 !`
> 
> Would it be better to submit a patch to remove this pcie_port?
> 
> ```
> -       ret = k1_pcie_parse_port(k1);
> -       if (ret)
> -               return dev_err_probe(dev, ret, "failed to parse root port\n");
> +       k1->phy = devm_phy_get(dev, "pcie-phy");
> +       if (IS_ERR(k1->phy))
> +               return dev_err_probe(dev, PTR_ERR(k1->phy), "missing PHY\n");
> ```

I've not really looked at this, and not an expert on this area, so will
leave this to Alex or PCIe maintainers..

> I have tested this change and it works.
> 
> Thanks,
> Chukun
> 

-- 
Yixun Lan (dlan)

Re: [PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Yao Zi]

On Mon, Mar 02, 2026 at 11:05:11AM +0800, Chukun Pan wrote:
> Hi,
> 
> > &pcie1_port {
> >  	phys = <&pcie1_phy>;
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> >  };
> > 
> >  &pcie1 {
> > @@ -320,6 +321,7 @@ &pcie2_phy {
> > 
> >  &pcie2_port {
> >  	phys = <&pcie2_phy>;
> > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> >  };
> 
> ```
> &pcie1 {
> 	vpcie3v3-supply = <&pcie_vcc_3v3>;
> 	status = "okay";
> };
> ```
> 
> According to DT binding, the vpcie3v3-supply of the &pciex node should
> be moved to the &pciex_port node. This is simply a duplication of the
> property.
> 
> But do we really need this pcie_port (PCIe bridge)?

This schema is required to be taken during review of the PCIe driver[1],
and it should be the future way to handle Root Port specific properties
through pwctrl-slot driver instead of the host driver.

> The PCIe bridge node (pcie@0) was treated as a platform device, but it
> did not define the interrupts property, which resulted in the following
> warning: `[    2.897980] irq: no irq domain found for pcie@0 !`
> 
> Would it be better to submit a patch to remove this pcie_port?

Thus I don't think it's a good idea. We should go back and investigate
a proper fix for the irq domain problem.

> ```
> -       ret = k1_pcie_parse_port(k1);
> -       if (ret)
> -               return dev_err_probe(dev, ret, "failed to parse root port\n");
> +       k1->phy = devm_phy_get(dev, "pcie-phy");
> +       if (IS_ERR(k1->phy))
> +               return dev_err_probe(dev, PTR_ERR(k1->phy), "missing PHY\n");
> ```
> 
> I have tested this change and it works.
> 
> Thanks,
> Chukun

Best regards,
Yao Zi

[1]: https://lore.kernel.org/linux-pci/u53qfrubgrcamiz35ox6lcdpp5bbzfwcsic466z5r6yyx6xz3n@c64nw2pegtfe/

Re: [PATCH] riscv: dts: spacemit: pcie: fix missing power regulator

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3582 bytes --]

On Mon, Mar 02, 2026 at 11:36:55AM +0800, Yixun Lan wrote:
> Hi Chukun,
> 
> Sorry, I missed your mail due recent problem of my client..
> On 11:05 Mon 02 Mar     , Chukun Pan wrote:
> > Hi,
> > 
> > > &pcie1_port {
> > >  	phys = <&pcie1_phy>;
> > > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> > >  };
> > > 
> > >  &pcie1 {
> > > @@ -320,6 +321,7 @@ &pcie2_phy {
> > > 
> > >  &pcie2_port {
> > >  	phys = <&pcie2_phy>;
> > > +	vpcie3v3-supply = <&pcie_vcc_3v3>;
> > >  };
> > 
> > ```
> > &pcie1 {
> > 	vpcie3v3-supply = <&pcie_vcc_3v3>;
> > 	status = "okay";
> > };
> > ```
> > 
> > According to DT binding, the vpcie3v3-supply of the &pciex node should
> > be moved to the &pciex_port node. This is simply a duplication of the
> > property.
> > 
> I have confidence that pcie port need to add a regulator to provide 
> supply for devices..
> 
> But, I'm not sure whether it's ok to remove regulator from &pciex node,
> it's possibly a 'yes' answer, but to convince me, I'd like to see a real
> test case to prove it: e.g, power supply for pciex is actually off before 
> the driver initialization, then run regular procedure as it should, if
> all works fine

I'm not really sure what this is about. Whether the supply can be off
before driver probe has nothing to do with what node the supply should
be in. If you're concerned about removing it from the pcie node causing
it to not be enabled during probe, then the driver should probably reach
into the port node, get the regulator and call enable() on it?
As I said when reporting this, either you need to change the driver and
dts, or change the binding. If the supply actually is provided to the
controller and port, then you need to change the binding to have a
supply in the controller node and change the driver to make sure that
both the controller node supply and the port node supply are enabled as
someone could opt to provide them from different regulators.

Looking at the bpif3 schematic
https://drive.google.com/file/d/19iLJ5xnCB_oK8VeQjkPGjzAn39WYyylv/view
I see nowhere where that 3v3 supply is actually provided to the k1,
which I would expect if it were the supply for the controller itself.
Instead, the m2 and minipcie slots are where I see that supply provided
in the schematic. To me, that sounds like the port is what needs the
supply, not the controller, but I'm not super familiar with pci
devicetree stuff unfortunately.

> Btw, different drivers request same regulator is ok, and is quite normal,
> can't draw a conclusion that it's a duplication, as they may be used for
> different reasons
> 
> > But do we really need this pcie_port (PCIe bridge)?
> > 
> > The PCIe bridge node (pcie@0) was treated as a platform device, but it
> > did not define the interrupts property, which resulted in the following
> > warning: `[    2.897980] irq: no irq domain found for pcie@0 !`
> > 
> > Would it be better to submit a patch to remove this pcie_port?
> > 
> > ```
> > -       ret = k1_pcie_parse_port(k1);
> > -       if (ret)
> > -               return dev_err_probe(dev, ret, "failed to parse root port\n");
> > +       k1->phy = devm_phy_get(dev, "pcie-phy");
> > +       if (IS_ERR(k1->phy))
> > +               return dev_err_probe(dev, PTR_ERR(k1->phy), "missing PHY\n");
> > ```
> 
> I've not really looked at this, and not an expert on this area, so will
> leave this to Alex or PCIe maintainers..
> 
> > I have tested this change and it works.
> > 
> > Thanks,
> > Chukun
> > 
> 
> -- 
> Yixun Lan (dlan)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]