[PATCH v1 0/2] PCI: j721e: Add voltage regulator support

[PATCH v1 0/2] PCI: j721e: Add voltage regulator support

[Vitor Soares]

From: Vitor Soares <vitor.soares@toradex.com>

Some PCIe endpoints or slots connected to the TI J721E PCIe root complex
may require external voltage regulators to provide 1.5V, 3.3V, or 12V
supplies. These regulators depend on the specific board design — for
example, M.2 or miniPCIe connectors often need 3.3V or 1.5V, while
full-size PCIe slots may also require 12V.

This series adds bindings and driver support for these optional regulators.
When present, the driver enables them automatically using
devm_regulator_get_enable_optional(), ensuring proper cleanup on removal.

Tested on a Toradex Aquila AM69 platform with a Wi-Fi PCIe endpoint
requiring 3.3V.

These changes are based on upstream discussion:
https://lore.kernel.org/linux-pci/20231105092908.3792-1-wsa+renesas@sang-engineering.com/

Vitor Soares (2):
  dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies
  PCI: j721e: Add support for optional regulator supplies

 .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 14 ++++++++++++++
 drivers/pci/controller/cadence/pci-j721e.c         | 13 +++++++++++++
 2 files changed, 27 insertions(+)

-- 
2.51.0

[PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies

[Vitor Soares]

From: Vitor Soares <vitor.soares@toradex.com>

Add optional regulator supply properties for PCIe endpoints on TI SoCs.
Some boards provide dedicated regulators for PCIe devices, such as
1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
(for high-power devices). These supplies are now described as optional
properties to allow the driver to control endpoint power where supported.

Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
---
 .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
index c704099f134b..a20b03406448 100644
--- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
@@ -110,6 +110,18 @@ properties:
       interrupts:
         maxItems: 1
 
+  vpcie1v5-supply:
+    description: 1.5V regulator used to power PCIe interfaces,
+                 typically present on miniPCIe slots.
+
+  vpcie3v3-supply:
+    description: 3.3V regulator used to power PCIe interfaces
+                 or endpoint connectors such as M.2 or miniPCIe.
+
+  vpcie12v-supply:
+    description: 12V regulator used to power PCIe slots that
+                 require higher-voltage devices (e.g. full-size cards).
+
 allOf:
   - $ref: cdns-pcie-host.yaml#
   - if:
@@ -202,5 +214,7 @@ examples:
             ranges = <0x01000000 0x0 0x10001000  0x00 0x10001000  0x0 0x0010000>,
                      <0x02000000 0x0 0x10011000  0x00 0x10011000  0x0 0x7fef000>;
             dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>;
+            vpcie3v3-supply = <&pcie_3v3>;
+            vpcie12v-supply = <&pcie_12v>;
         };
     };
-- 
2.51.0

Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies

[Krzysztof Kozlowski]

On Tue, Oct 14, 2025 at 12:25:48PM +0100, Vitor Soares wrote:
> From: Vitor Soares <vitor.soares@toradex.com>
> 
> Add optional regulator supply properties for PCIe endpoints on TI SoCs.
> Some boards provide dedicated regulators for PCIe devices, such as
> 1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
> (for high-power devices). These supplies are now described as optional
> properties to allow the driver to control endpoint power where supported.

Last sentence is completely redundant. Please do not describe DT, we
all can read the patch. Driver is irrelevant here.

How you described here and in descriptions, suggests these are rather
port properties, not the controller.

> 
> Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> ---
>  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> index c704099f134b..a20b03406448 100644
> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> @@ -110,6 +110,18 @@ properties:
>        interrupts:
>          maxItems: 1
>  
> +  vpcie1v5-supply:

How is it called in this device datasheet (not the board schematics)?

> +    description: 1.5V regulator used to power PCIe interfaces,
> +                 typically present on miniPCIe slots.

Best regards,
Krzysztof

Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies

[Vitor Soares]

Hi Krzysztof,

Thank you for the feedback.

On Mon, 2025-10-20 at 13:14 +0200, Krzysztof Kozlowski wrote:
> On Tue, Oct 14, 2025 at 12:25:48PM +0100, Vitor Soares wrote:
> > From: Vitor Soares <vitor.soares@toradex.com>
> > 
> > Add optional regulator supply properties for PCIe endpoints on TI SoCs.
> > Some boards provide dedicated regulators for PCIe devices, such as
> > 1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
> > (for high-power devices). These supplies are now described as optional
> > properties to allow the driver to control endpoint power where supported.
> 
> Last sentence is completely redundant. Please do not describe DT, we
> all can read the patch. Driver is irrelevant here.
> 
> 
Ack, I will remove last sentence.

> 
> How you described here and in descriptions, suggests these are rather
> port properties, not the controller.

You are right - these supplies power the PCIe slot/connector, not the controller
itself. However, as per my understanding, the current kernel practice is to
place slot supplies in the root complex node rather than the endpoint node. as
seen in e.g.:
- imx6q-pcie.yaml
- rockchip-dw-pcie.yaml
- rcar-pci-host.yaml

This seems consistent with those existing bindings, but please let me know if
I’m overlooking something specific to this case.

> 
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> > ---
> >  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > index c704099f134b..a20b03406448 100644
> > --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > @@ -110,6 +110,18 @@ properties:
> >        interrupts:
> >          maxItems: 1
> >  
> > +  vpcie1v5-supply:
> 
> How is it called in this device datasheet (not the board schematics)?

The TI SoC datasheet describes the controller interface but doesn’t define these
external supply rails - they are board-level regulators specific to the slot.

> 
> > +    description: 1.5V regulator used to power PCIe interfaces,
> > +                 typically present on miniPCIe slots.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Vitor Soares

Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies

[Manivannan Sadhasivam]

On Mon, Oct 27, 2025 at 11:22:26PM +0000, Vitor Soares wrote:
> Hi Krzysztof,
> 
> Thank you for the feedback.
> 
> On Mon, 2025-10-20 at 13:14 +0200, Krzysztof Kozlowski wrote:
> > On Tue, Oct 14, 2025 at 12:25:48PM +0100, Vitor Soares wrote:
> > > From: Vitor Soares <vitor.soares@toradex.com>
> > > 
> > > Add optional regulator supply properties for PCIe endpoints on TI SoCs.
> > > Some boards provide dedicated regulators for PCIe devices, such as
> > > 1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
> > > (for high-power devices). These supplies are now described as optional
> > > properties to allow the driver to control endpoint power where supported.
> > 
> > Last sentence is completely redundant. Please do not describe DT, we
> > all can read the patch. Driver is irrelevant here.
> > 
> > 
> Ack, I will remove last sentence.
> 
> > 
> > How you described here and in descriptions, suggests these are rather
> > port properties, not the controller.
> 
> You are right - these supplies power the PCIe slot/connector, not the controller
> itself. However, as per my understanding, the current kernel practice is to
> place slot supplies in the root complex node rather than the endpoint node. as
> seen in e.g.:
> - imx6q-pcie.yaml
> - rockchip-dw-pcie.yaml
> - rcar-pci-host.yaml
> 
> This seems consistent with those existing bindings, but please let me know if
> I’m overlooking something specific to this case.
> 

We do not properly document it, but defining the slot supplies in host bridge
(controller) node is deprecated. Some bindings still do it for legacy reasons,
but the new ones should define them in the Root Port nodes as they belong to. We
do not have a separate DT node for PCI slots, but rather reuse the Root Port
node.

There are also bindings that define supplies in the endpoint node. They do it
for devices directly connected to the PCI bus without a connector (like in PCB).

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies

[Vitor Soares]

On Tue, 2025-10-28 at 11:11 +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 27, 2025 at 11:22:26PM +0000, Vitor Soares wrote:
> > Hi Krzysztof,
> > 
> > Thank you for the feedback.
> > 
> > On Mon, 2025-10-20 at 13:14 +0200, Krzysztof Kozlowski wrote:
> > > On Tue, Oct 14, 2025 at 12:25:48PM +0100, Vitor Soares wrote:
> > > > From: Vitor Soares <vitor.soares@toradex.com>
> > > > 
> > > > Add optional regulator supply properties for PCIe endpoints on TI SoCs.
> > > > Some boards provide dedicated regulators for PCIe devices, such as
> > > > 1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
> > > > (for high-power devices). These supplies are now described as optional
> > > > properties to allow the driver to control endpoint power where
> > > > supported.
> > > 
> > > Last sentence is completely redundant. Please do not describe DT, we
> > > all can read the patch. Driver is irrelevant here.
> > > 
> > > 
> > Ack, I will remove last sentence.
> > 
> > > 
> > > How you described here and in descriptions, suggests these are rather
> > > port properties, not the controller.
> > 
> > You are right - these supplies power the PCIe slot/connector, not the
> > controller
> > itself. However, as per my understanding, the current kernel practice is to
> > place slot supplies in the root complex node rather than the endpoint node.
> > as
> > seen in e.g.:
> > - imx6q-pcie.yaml
> > - rockchip-dw-pcie.yaml
> > - rcar-pci-host.yaml
> > 
> > This seems consistent with those existing bindings, but please let me know
> > if
> > I’m overlooking something specific to this case.
> > 
> 
> We do not properly document it, but defining the slot supplies in host bridge
> (controller) node is deprecated. Some bindings still do it for legacy reasons,
> but the new ones should define them in the Root Port nodes as they belong to.
> We
> do not have a separate DT node for PCI slots, but rather reuse the Root Port
> node.
> 
> There are also bindings that define supplies in the endpoint node. They do it
> for devices directly connected to the PCI bus without a connector (like in
> PCB).
> 
> - Mani
> 

Thanks for the clarification and context. From what I understand, the
recommendation is to define the supply regulators under the individual root port
node rather than in the host bridge (controller) node, as the supplies
conceptually belong to each port rather than the controller itself.

On the j721e PCIe controller, the current driver implementation assumes a single
root port and doesn’t parse child port nodes. To follow the new convention, I’d
need to refactor the driver to support root port subnodes, and I wonder if the
PHY reference and reset should also be moved to the Root Port node in that case.

Could you please point me to an example of a PCIe controller binding or driver
that already follows this approach?

Best regards,
Vitor Soares

[PATCH v1 2/2] PCI: j721e: Add support for optional regulator supplies

[Vitor Soares]

From: Vitor Soares <vitor.soares@toradex.com>

Some boards require external regulators to power PCIe endpoints.
Add support for optional 1.5V, 3.3V, and 12V supplies, which may be
defined in the device tree as vpcie1v5-supply, vpcie3v3-supply, and
vpcie12v-supply.

Use devm_regulator_get_enable_optional() to obtain and enable each
supply, so it will be automatically disabled when the driver is
removed.

Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
---
 drivers/pci/controller/cadence/pci-j721e.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 5bc5ab20aa6d..f29ce2aef04e 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 
 #include "../../pci.h"
 #include "pcie-cadence.h"
@@ -467,6 +468,10 @@ static const struct of_device_id of_j721e_pcie_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_j721e_pcie_match);
 
+static const char * const j721e_pcie_supplies[] = {
+	"vpcie12v", "vpcie3v3", "vpcie1v5"
+};
+
 static int j721e_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -480,6 +485,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
 	struct gpio_desc *gpiod;
 	void __iomem *base;
 	struct clk *clk;
+	unsigned int i;
 	u32 num_lanes;
 	u32 mode;
 	int ret;
@@ -565,6 +571,13 @@ static int j721e_pcie_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	for (i = 0; i < ARRAY_SIZE(j721e_pcie_supplies); i++) {
+		ret = devm_regulator_get_enable_optional(dev, j721e_pcie_supplies[i]);
+		if (ret < 0 && ret != -ENODEV)
+			return dev_err_probe(dev, ret, "can't enable regulator %s\n",
+					     j721e_pcie_supplies[i]);
+	}
+
 	dev_set_drvdata(dev, pcie);
 	pm_runtime_enable(dev);
 	ret = pm_runtime_get_sync(dev);
-- 
2.51.0

Re: [PATCH v1 2/2] PCI: j721e: Add support for optional regulator supplies

[Manivannan Sadhasivam]

On Tue, Oct 14, 2025 at 12:25:49PM +0100, Vitor Soares wrote:
> From: Vitor Soares <vitor.soares@toradex.com>
> 
> Some boards require external regulators to power PCIe endpoints.
> Add support for optional 1.5V, 3.3V, and 12V supplies, which may be
> defined in the device tree as vpcie1v5-supply, vpcie3v3-supply, and
> vpcie12v-supply.
> 
> Use devm_regulator_get_enable_optional() to obtain and enable each
> supply, so it will be automatically disabled when the driver is
> removed.
> 
> Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> ---
>  drivers/pci/controller/cadence/pci-j721e.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 5bc5ab20aa6d..f29ce2aef04e 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -21,6 +21,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include "../../pci.h"
>  #include "pcie-cadence.h"
> @@ -467,6 +468,10 @@ static const struct of_device_id of_j721e_pcie_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_j721e_pcie_match);
>  
> +static const char * const j721e_pcie_supplies[] = {
> +	"vpcie12v", "vpcie3v3", "vpcie1v5"
> +};

Please don't hardcode the supplies in driver. The DT binding should make sure
the relevant supplies are passed (including the optional ones). Just use
of_regulator_bulk_get_all() to acquire all the passed supplies.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v1 2/2] PCI: j721e: Add support for optional regulator supplies

[Vitor Soares]

Hello Mani,

Thank you for the feedback.

On Tue, 2025-10-21 at 07:36 +0530, Manivannan Sadhasivam wrote:
> On Tue, Oct 14, 2025 at 12:25:49PM +0100, Vitor Soares wrote:
> > From: Vitor Soares <vitor.soares@toradex.com>
> > 
> > Some boards require external regulators to power PCIe endpoints.
> > Add support for optional 1.5V, 3.3V, and 12V supplies, which may be
> > defined in the device tree as vpcie1v5-supply, vpcie3v3-supply, and
> > vpcie12v-supply.
> > 
> > Use devm_regulator_get_enable_optional() to obtain and enable each
> > supply, so it will be automatically disabled when the driver is
> > removed.
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> > ---
> >  drivers/pci/controller/cadence/pci-j721e.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/cadence/pci-j721e.c
> > b/drivers/pci/controller/cadence/pci-j721e.c
> > index 5bc5ab20aa6d..f29ce2aef04e 100644
> > --- a/drivers/pci/controller/cadence/pci-j721e.c
> > +++ b/drivers/pci/controller/cadence/pci-j721e.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> >  
> >  #include "../../pci.h"
> >  #include "pcie-cadence.h"
> > @@ -467,6 +468,10 @@ static const struct of_device_id of_j721e_pcie_match[]
> > = {
> >  };
> >  MODULE_DEVICE_TABLE(of, of_j721e_pcie_match);
> >  
> > +static const char * const j721e_pcie_supplies[] = {
> > +       "vpcie12v", "vpcie3v3", "vpcie1v5"
> > +};
> 
> Please don't hardcode the supplies in driver. The DT binding should make sure
> the relevant supplies are passed (including the optional ones). Just use
> of_regulator_bulk_get_all() to acquire all the passed supplies.
> 
> - Mani
> 

I checked the bulk regulator APIs as suggested and of_regulator_bulk_get_all()
does handle optional supplies correctly, however it is not a managed function
and doesn't enable the  regulators automatically.

To use of_regulator_bulk_get_all(), I would need to:
- Manually enable regulators with regulator_bulk_enable()
- Add cleanup/disable logic in remove path
- Handle error cleanup path manually

This would actually make the code more complex and error-prone compared to the
current approach using devm_regulator_get_enable_optional(), which provides
managed cleanup and automatic enable for optional supplies.

I also checked devm_regulator_bulk_get_enable(), it treats all supplies as
required and needs the supplies name as well.

Unless there is a devm_regulator_bulk_get_optional_enable() API I'm not aware
of, the current per-supply approach is the standard kernel pattern for this use
case. Would you still prefer the bulk approach despite these limitations?

Best regards,
Vitor Soares

Re: [PATCH v1 2/2] PCI: j721e: Add support for optional regulator supplies

[Manivannan Sadhasivam]

On Mon, Oct 27, 2025 at 08:24:12PM +0000, Vitor Soares wrote:
> Hello Mani,
> 
> Thank you for the feedback.
> 
> On Tue, 2025-10-21 at 07:36 +0530, Manivannan Sadhasivam wrote:
> > On Tue, Oct 14, 2025 at 12:25:49PM +0100, Vitor Soares wrote:
> > > From: Vitor Soares <vitor.soares@toradex.com>
> > > 
> > > Some boards require external regulators to power PCIe endpoints.
> > > Add support for optional 1.5V, 3.3V, and 12V supplies, which may be
> > > defined in the device tree as vpcie1v5-supply, vpcie3v3-supply, and
> > > vpcie12v-supply.
> > > 
> > > Use devm_regulator_get_enable_optional() to obtain and enable each
> > > supply, so it will be automatically disabled when the driver is
> > > removed.
> > > 
> > > Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> > > ---
> > >  drivers/pci/controller/cadence/pci-j721e.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c
> > > b/drivers/pci/controller/cadence/pci-j721e.c
> > > index 5bc5ab20aa6d..f29ce2aef04e 100644
> > > --- a/drivers/pci/controller/cadence/pci-j721e.c
> > > +++ b/drivers/pci/controller/cadence/pci-j721e.c
> > > @@ -21,6 +21,7 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > >  
> > >  #include "../../pci.h"
> > >  #include "pcie-cadence.h"
> > > @@ -467,6 +468,10 @@ static const struct of_device_id of_j721e_pcie_match[]
> > > = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, of_j721e_pcie_match);
> > >  
> > > +static const char * const j721e_pcie_supplies[] = {
> > > +       "vpcie12v", "vpcie3v3", "vpcie1v5"
> > > +};
> > 
> > Please don't hardcode the supplies in driver. The DT binding should make sure
> > the relevant supplies are passed (including the optional ones). Just use
> > of_regulator_bulk_get_all() to acquire all the passed supplies.
> > 
> > - Mani
> > 
> 
> I checked the bulk regulator APIs as suggested and of_regulator_bulk_get_all()
> does handle optional supplies correctly, however it is not a managed function
> and doesn't enable the  regulators automatically.
> 
> To use of_regulator_bulk_get_all(), I would need to:
> - Manually enable regulators with regulator_bulk_enable()
> - Add cleanup/disable logic in remove path
> - Handle error cleanup path manually
> 
> This would actually make the code more complex and error-prone compared to the
> current approach using devm_regulator_get_enable_optional(), which provides
> managed cleanup and automatic enable for optional supplies.
> 
> I also checked devm_regulator_bulk_get_enable(), it treats all supplies as
> required and needs the supplies name as well.
> 
> Unless there is a devm_regulator_bulk_get_optional_enable() API I'm not aware
> of, the current per-supply approach is the standard kernel pattern for this use
> case. Would you still prefer the bulk approach despite these limitations?
> 

Fine then. If you do not have a reason to manualy turn off/on the supplies
(during suspend/resume), then it might be better to use
devm_regulator_get_enable_optional() for now.

I'd love to add the managed version of of_regulator_bulk_get_all(), but I guess
Mark wouldn't want it.

- Mani

-- 
மணிவண்ணன் சதாசிவம்