Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-28 Thread Shawn Lin

在 2026/01/28 星期三 5:53, Neil Armstrong 写道:

On 1/27/26 16:53, Manivannan Sadhasivam wrote:

On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:
Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host 
Controller
power control which connects over PCIe and requires specific power 
supplies

to start up.



This driver only handles the supplies. So why can't you use the existing
pwrctrl-slot driver as a fallback?


It would fit with no change, but the name "slot" doesn't match the goal 
here,

it's not a slot at all, it's an actual pcie IC.



How about renaming slot.cto something like pci-pwrctrl-simple.c, 
especially if most power sequences fit into this category? This would 
follow the naming example seen in other subsystems, such as 
drivers/mmc/core/pwrseq_simple.c.



Neil



- Mani


Signed-off-by: Neil Armstrong 
---
  drivers/pci/pwrctrl/Kconfig | 10 
  drivers/pci/pwrctrl/Makefile    |  2 +
  drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c | 88 ++ 
+++

  3 files changed, 100 insertions(+)

diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
index e0f999f299bb..5a94e60d0d3e 100644
--- a/drivers/pci/pwrctrl/Kconfig
+++ b/drivers/pci/pwrctrl/Kconfig
@@ -11,6 +11,16 @@ config PCI_PWRCTRL_PWRSEQ
  select POWER_SEQUENCING
  select PCI_PWRCTRL
+config PCI_PWRCTRL_UPD720201
+    tristate "PCI Power Control driver for the UPD720201 USB3 Host 
Controller"

+    select PCI_PWRCTRL
+    help
+  Say Y here to enable the PCI Power Control driver of the 
UPD720201

+  USB3 Host Controller.
+
+  The voltage regulators powering the rails of the PCI slots
+  are expected to be defined in the devicetree node of the PCI 
device.

+
  config PCI_PWRCTRL_SLOT
  tristate "PCI Power Control driver for PCI slots"
  select PCI_PWRCTRL
diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
index 13b02282106c..a99f85de8a3d 100644
--- a/drivers/pci/pwrctrl/Makefile
+++ b/drivers/pci/pwrctrl/Makefile
@@ -5,6 +5,8 @@ pci-pwrctrl-core-y    := core.o
  obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ)    += pci-pwrctrl-pwrseq.o
+obj-$(CONFIG_PCI_PWRCTRL_UPD720201)    += pci-pwrctrl-upd720201.o
+
  obj-$(CONFIG_PCI_PWRCTRL_SLOT)    += pci-pwrctrl-slot.o
  pci-pwrctrl-slot-y    := slot.o
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c b/drivers/ 
pci/pwrctrl/pci-pwrctrl-upd720201.c

new file mode 100644
index ..db96bbb69c21
--- /dev/null
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Based on upd720201.c:
+ * Copyright (C) 2024 Linaro Ltd.
+ * Author: Manivannan Sadhasivam 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pci_pwrctrl_upd720201_data {
+    struct pci_pwrctrl ctx;
+    struct regulator_bulk_data *supplies;
+    int num_supplies;
+};
+
+static void devm_pci_pwrctrl_upd720201_power_off(void *data)
+{
+    struct pci_pwrctrl_upd720201_data *upd720201 = data;
+
+    regulator_bulk_disable(upd720201->num_supplies, upd720201- 
>supplies);

+    regulator_bulk_free(upd720201->num_supplies, upd720201->supplies);
+}
+
+static int pci_pwrctrl_upd720201_probe(struct platform_device *pdev)
+{
+    struct pci_pwrctrl_upd720201_data *upd720201;
+    struct device *dev = &pdev->dev;
+    int ret;
+
+    upd720201 = devm_kzalloc(dev, sizeof(*upd720201), GFP_KERNEL);
+    if (!upd720201)
+    return -ENOMEM;
+
+    ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
+    &upd720201->supplies);
+    if (ret < 0) {
+    dev_err_probe(dev, ret, "Failed to get upd720201 
regulators\n");

+    return ret;
+    }
+
+    upd720201->num_supplies = ret;
+    ret = regulator_bulk_enable(upd720201->num_supplies, upd720201- 
>supplies);

+    if (ret < 0) {
+    dev_err_probe(dev, ret, "Failed to enable upd720201 
regulators\n");
+    regulator_bulk_free(upd720201->num_supplies, upd720201- 
>supplies);

+    return ret;
+    }
+
+    ret = devm_add_action_or_reset(dev, 
devm_pci_pwrctrl_upd720201_power_off,

+   upd720201);
+    if (ret)
+    return ret;
+
+    pci_pwrctrl_init(&upd720201->ctx, dev);
+
+    ret = devm_pci_pwrctrl_device_set_ready(dev, &upd720201->ctx);
+    if (ret)
+    return dev_err_probe(dev, ret, "Failed to register pwrctrl 
driver\n");

+
+    return 0;
+}
+
+static const struct of_device_id pci_pwrctrl_upd720201_of_match[] = {
+    {
+    .compatible = "pci1912,0014",
+    },
+    { }
+};
+MODULE_DEVICE_TABLE(of, pci_pwrctrl_upd720201_of_match);
+
+static struct platform_driver pci_pwrctrl_upd720201_driver = {
+    .driver = {
+    .name = "pci-pwrctrl-upd720201",
+    .of_match_table = pci_pwrctrl_upd720201_of_match,
+    },
+    .probe = pci_pwrctrl_upd720201_probe,
+};
+module_platform_driver(pci_pwrctrl_upd720201_driver);
+
+MODULE_AUTHOR("Neil Armstrong ");
+MO

Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-28 Thread Neil Armstrong

On 1/28/26 08:57, Manivannan Sadhasivam wrote:

On Wed, Jan 28, 2026 at 02:22:50PM +0800, Shawn Lin wrote:

在 2026/01/28 星期三 5:53, Neil Armstrong 写道:

On 1/27/26 16:53, Manivannan Sadhasivam wrote:

On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:

Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host
Controller
power control which connects over PCIe and requires specific
power supplies
to start up.



This driver only handles the supplies. So why can't you use the existing
pwrctrl-slot driver as a fallback?


It would fit with no change, but the name "slot" doesn't match the goal
here,
it's not a slot at all, it's an actual pcie IC.



How about renaming slot.cto something like pci-pwrctrl-simple.c, especially
if most power sequences fit into this category? This would follow the naming
example seen in other subsystems, such as drivers/mmc/core/pwrseq_simple.c.



Yes. There is no point in duplicating the drivers just for a different name.
Slot driver is relatively new. So I don't think there would be issues in
renaming the module name.

I'd prefer for 'pci-pwrctrl-generic.ko' for module name and 'generic.c' for
driver name.


Will do

Thanks,
Neil



- Mani





Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-28 Thread Bartosz Golaszewski
On Tue, 27 Jan 2026 10:57:29 +0100, Neil Armstrong
 said:
> Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host Controller
> power control which connects over PCIe and requires specific power supplies
> to start up.
>
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/pci/pwrctrl/Kconfig | 10 
>  drivers/pci/pwrctrl/Makefile|  2 +
>  drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c | 88 
> +
>  3 files changed, 100 insertions(+)
>
> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
> index e0f999f299bb..5a94e60d0d3e 100644
> --- a/drivers/pci/pwrctrl/Kconfig
> +++ b/drivers/pci/pwrctrl/Kconfig
> @@ -11,6 +11,16 @@ config PCI_PWRCTRL_PWRSEQ
>   select POWER_SEQUENCING
>   select PCI_PWRCTRL
>
> +config PCI_PWRCTRL_UPD720201
> + tristate "PCI Power Control driver for the UPD720201 USB3 Host 
> Controller"
> + select PCI_PWRCTRL
> + help
> +   Say Y here to enable the PCI Power Control driver of the UPD720201
> +   USB3 Host Controller.
> +
> +   The voltage regulators powering the rails of the PCI slots
> +   are expected to be defined in the devicetree node of the PCI device.
> +
>  config PCI_PWRCTRL_SLOT
>   tristate "PCI Power Control driver for PCI slots"
>   select PCI_PWRCTRL
> diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
> index 13b02282106c..a99f85de8a3d 100644
> --- a/drivers/pci/pwrctrl/Makefile
> +++ b/drivers/pci/pwrctrl/Makefile
> @@ -5,6 +5,8 @@ pci-pwrctrl-core-y:= core.o
>
>  obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ) += pci-pwrctrl-pwrseq.o
>
> +obj-$(CONFIG_PCI_PWRCTRL_UPD720201)  += pci-pwrctrl-upd720201.o
> +
>  obj-$(CONFIG_PCI_PWRCTRL_SLOT)   += pci-pwrctrl-slot.o
>  pci-pwrctrl-slot-y   := slot.o
>
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c 
> b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> new file mode 100644
> index ..db96bbb69c21
> --- /dev/null
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Based on upd720201.c:
> + * Copyright (C) 2024 Linaro Ltd.
> + * Author: Manivannan Sadhasivam 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct pci_pwrctrl_upd720201_data {
> + struct pci_pwrctrl ctx;
> + struct regulator_bulk_data *supplies;
> + int num_supplies;
> +};
> +
> +static void devm_pci_pwrctrl_upd720201_power_off(void *data)
> +{
> + struct pci_pwrctrl_upd720201_data *upd720201 = data;
> +
> + regulator_bulk_disable(upd720201->num_supplies, upd720201->supplies);
> + regulator_bulk_free(upd720201->num_supplies, upd720201->supplies);
> +}
> +
> +static int pci_pwrctrl_upd720201_probe(struct platform_device *pdev)
> +{
> + struct pci_pwrctrl_upd720201_data *upd720201;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + upd720201 = devm_kzalloc(dev, sizeof(*upd720201), GFP_KERNEL);
> + if (!upd720201)
> + return -ENOMEM;
> +
> + ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
> + &upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to get upd720201 regulators\n");

dev_err_probe()?

> + return ret;
> + }
> +
> + upd720201->num_supplies = ret;
> + ret = regulator_bulk_enable(upd720201->num_supplies, 
> upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to enable upd720201 
> regulators\n");
> + regulator_bulk_free(upd720201->num_supplies, 
> upd720201->supplies);
> + return ret;

Save a line by returning dev_err_probe() here?

> + }
> +
> + ret = devm_add_action_or_reset(dev, 
> devm_pci_pwrctrl_upd720201_power_off,
> +upd720201);
> + if (ret)
> + return ret;
> +
> + pci_pwrctrl_init(&upd720201->ctx, dev);
> +
> + ret = devm_pci_pwrctrl_device_set_ready(dev, &upd720201->ctx);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register pwrctrl 
> driver\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pci_pwrctrl_upd720201_of_match[] = {
> + {
> + .compatible = "pci1912,0014",
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, pci_pwrctrl_upd720201_of_match);
> +
> +static struct platform_driver pci_pwrctrl_upd720201_driver = {
> + .driver = {
> + .name = "pci-pwrctrl-upd720201",
> + .of_match_table = pci_pwrctrl_upd720201_of_match,
> + },
> + .probe = pci_pwrctrl_upd720201_probe,
> +};
> +module_platform_driver(pci_pwrctrl_upd720201_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong ");
> +MODULE_DESCRIPTION("PCI Power Control driver for UPD720201 USB3 Host 
> Controller");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34

Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-27 Thread Manivannan Sadhasivam
On Wed, Jan 28, 2026 at 02:22:50PM +0800, Shawn Lin wrote:
> 在 2026/01/28 星期三 5:53, Neil Armstrong 写道:
> > On 1/27/26 16:53, Manivannan Sadhasivam wrote:
> > > On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:
> > > > Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host
> > > > Controller
> > > > power control which connects over PCIe and requires specific
> > > > power supplies
> > > > to start up.
> > > > 
> > > 
> > > This driver only handles the supplies. So why can't you use the existing
> > > pwrctrl-slot driver as a fallback?
> > 
> > It would fit with no change, but the name "slot" doesn't match the goal
> > here,
> > it's not a slot at all, it's an actual pcie IC.
> > 
> 
> How about renaming slot.cto something like pci-pwrctrl-simple.c, especially
> if most power sequences fit into this category? This would follow the naming
> example seen in other subsystems, such as drivers/mmc/core/pwrseq_simple.c.
> 

Yes. There is no point in duplicating the drivers just for a different name.
Slot driver is relatively new. So I don't think there would be issues in
renaming the module name.

I'd prefer for 'pci-pwrctrl-generic.ko' for module name and 'generic.c' for
driver name.

- Mani

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


Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-27 Thread Bjorn Helgaas
On Tue, Jan 27, 2026 at 10:53:50PM +0100, Neil Armstrong wrote:
> On 1/27/26 16:53, Manivannan Sadhasivam wrote:
> > On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:
> > > Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host 
> > > Controller
> > > power control which connects over PCIe and requires specific power 
> > > supplies
> > > to start up.
> > 
> > This driver only handles the supplies. So why can't you use the existing
> > pwrctrl-slot driver as a fallback?
> 
> It would fit with no change, but the name "slot" doesn't match the
> goal here, it's not a slot at all, it's an actual pcie IC.

"slot" is probably an overly specific name.  Maybe we can solve it
with a better name.


Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-27 Thread Neil Armstrong

On 1/27/26 16:53, Manivannan Sadhasivam wrote:

On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:

Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host Controller
power control which connects over PCIe and requires specific power supplies
to start up.



This driver only handles the supplies. So why can't you use the existing
pwrctrl-slot driver as a fallback?


It would fit with no change, but the name "slot" doesn't match the goal here,
it's not a slot at all, it's an actual pcie IC.

Neil



- Mani


Signed-off-by: Neil Armstrong 
---
  drivers/pci/pwrctrl/Kconfig | 10 
  drivers/pci/pwrctrl/Makefile|  2 +
  drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c | 88 +
  3 files changed, 100 insertions(+)

diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
index e0f999f299bb..5a94e60d0d3e 100644
--- a/drivers/pci/pwrctrl/Kconfig
+++ b/drivers/pci/pwrctrl/Kconfig
@@ -11,6 +11,16 @@ config PCI_PWRCTRL_PWRSEQ
select POWER_SEQUENCING
select PCI_PWRCTRL
  
+config PCI_PWRCTRL_UPD720201

+   tristate "PCI Power Control driver for the UPD720201 USB3 Host 
Controller"
+   select PCI_PWRCTRL
+   help
+ Say Y here to enable the PCI Power Control driver of the UPD720201
+ USB3 Host Controller.
+
+ The voltage regulators powering the rails of the PCI slots
+ are expected to be defined in the devicetree node of the PCI device.
+
  config PCI_PWRCTRL_SLOT
tristate "PCI Power Control driver for PCI slots"
select PCI_PWRCTRL
diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
index 13b02282106c..a99f85de8a3d 100644
--- a/drivers/pci/pwrctrl/Makefile
+++ b/drivers/pci/pwrctrl/Makefile
@@ -5,6 +5,8 @@ pci-pwrctrl-core-y  := core.o
  
  obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ)	+= pci-pwrctrl-pwrseq.o
  
+obj-$(CONFIG_PCI_PWRCTRL_UPD720201)	+= pci-pwrctrl-upd720201.o

+
  obj-$(CONFIG_PCI_PWRCTRL_SLOT)+= pci-pwrctrl-slot.o
  pci-pwrctrl-slot-y:= slot.o
  
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c

new file mode 100644
index ..db96bbb69c21
--- /dev/null
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Based on upd720201.c:
+ * Copyright (C) 2024 Linaro Ltd.
+ * Author: Manivannan Sadhasivam 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pci_pwrctrl_upd720201_data {
+   struct pci_pwrctrl ctx;
+   struct regulator_bulk_data *supplies;
+   int num_supplies;
+};
+
+static void devm_pci_pwrctrl_upd720201_power_off(void *data)
+{
+   struct pci_pwrctrl_upd720201_data *upd720201 = data;
+
+   regulator_bulk_disable(upd720201->num_supplies, upd720201->supplies);
+   regulator_bulk_free(upd720201->num_supplies, upd720201->supplies);
+}
+
+static int pci_pwrctrl_upd720201_probe(struct platform_device *pdev)
+{
+   struct pci_pwrctrl_upd720201_data *upd720201;
+   struct device *dev = &pdev->dev;
+   int ret;
+
+   upd720201 = devm_kzalloc(dev, sizeof(*upd720201), GFP_KERNEL);
+   if (!upd720201)
+   return -ENOMEM;
+
+   ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
+   &upd720201->supplies);
+   if (ret < 0) {
+   dev_err_probe(dev, ret, "Failed to get upd720201 regulators\n");
+   return ret;
+   }
+
+   upd720201->num_supplies = ret;
+   ret = regulator_bulk_enable(upd720201->num_supplies, 
upd720201->supplies);
+   if (ret < 0) {
+   dev_err_probe(dev, ret, "Failed to enable upd720201 
regulators\n");
+   regulator_bulk_free(upd720201->num_supplies, 
upd720201->supplies);
+   return ret;
+   }
+
+   ret = devm_add_action_or_reset(dev, 
devm_pci_pwrctrl_upd720201_power_off,
+  upd720201);
+   if (ret)
+   return ret;
+
+   pci_pwrctrl_init(&upd720201->ctx, dev);
+
+   ret = devm_pci_pwrctrl_device_set_ready(dev, &upd720201->ctx);
+   if (ret)
+   return dev_err_probe(dev, ret, "Failed to register pwrctrl 
driver\n");
+
+   return 0;
+}
+
+static const struct of_device_id pci_pwrctrl_upd720201_of_match[] = {
+   {
+   .compatible = "pci1912,0014",
+   },
+   { }
+};
+MODULE_DEVICE_TABLE(of, pci_pwrctrl_upd720201_of_match);
+
+static struct platform_driver pci_pwrctrl_upd720201_driver = {
+   .driver = {
+   .name = "pci-pwrctrl-upd720201",
+   .of_match_table = pci_pwrctrl_upd720201_of_match,
+   },
+   .probe = pci_pwrctrl_upd720201_probe,
+};
+module_platform_driver(pci_pwrctrl_upd720201_driver);
+
+MODULE_AUTHOR("Neil Armstrong ");
+MODULE_DESCRIPTION("PCI Power Control drive

Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-27 Thread Bjorn Helgaas
On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:
> Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host Controller
> power control which connects over PCIe and requires specific power supplies
> to start up.

s/fo /for /

In subject, "PCI/pwrctrl: Add ..." to match history (see
"git log --oneline drivers/pci/pwrctrl/")

> Signed-off-by: Neil Armstrong 
> ---
>  drivers/pci/pwrctrl/Kconfig | 10 
>  drivers/pci/pwrctrl/Makefile|  2 +
>  drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c | 88 
> +
>  3 files changed, 100 insertions(+)
> 
> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
> index e0f999f299bb..5a94e60d0d3e 100644
> --- a/drivers/pci/pwrctrl/Kconfig
> +++ b/drivers/pci/pwrctrl/Kconfig
> @@ -11,6 +11,16 @@ config PCI_PWRCTRL_PWRSEQ
>   select POWER_SEQUENCING
>   select PCI_PWRCTRL
>  
> +config PCI_PWRCTRL_UPD720201
> + tristate "PCI Power Control driver for the UPD720201 USB3 Host 
> Controller"
> + select PCI_PWRCTRL
> + help
> +   Say Y here to enable the PCI Power Control driver of the UPD720201
> +   USB3 Host Controller.
> +
> +   The voltage regulators powering the rails of the PCI slots
> +   are expected to be defined in the devicetree node of the PCI device.

I assume this is a function of the platform design, not an intrinsic
feature of UPD720201?  I.e., my guess is that this driver is not
required for every platform that includes a UPD720201 device?

Maybe this is just another way of asking Mani's question about using
pwrctrl-slot.  *Every* device requires specific power supplies to
start up (re patch 1/7), and this driver doesn't appear to depend on
anything unique about UPD720201.

>  config PCI_PWRCTRL_SLOT
>   tristate "PCI Power Control driver for PCI slots"
>   select PCI_PWRCTRL
> diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
> index 13b02282106c..a99f85de8a3d 100644
> --- a/drivers/pci/pwrctrl/Makefile
> +++ b/drivers/pci/pwrctrl/Makefile
> @@ -5,6 +5,8 @@ pci-pwrctrl-core-y:= core.o
>  
>  obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ) += pci-pwrctrl-pwrseq.o
>  
> +obj-$(CONFIG_PCI_PWRCTRL_UPD720201)  += pci-pwrctrl-upd720201.o
> +
>  obj-$(CONFIG_PCI_PWRCTRL_SLOT)   += pci-pwrctrl-slot.o
>  pci-pwrctrl-slot-y   := slot.o
>  
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c 
> b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> new file mode 100644
> index ..db96bbb69c21
> --- /dev/null
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Based on upd720201.c:
> + * Copyright (C) 2024 Linaro Ltd.
> + * Author: Manivannan Sadhasivam 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct pci_pwrctrl_upd720201_data {
> + struct pci_pwrctrl ctx;
> + struct regulator_bulk_data *supplies;
> + int num_supplies;
> +};

To match recent rework of nearby drivers:

s/pci_pwrctrl_upd720201_data/upd720201_pwrctrl/
s/ctx/pwrctrl/

> +static void devm_pci_pwrctrl_upd720201_power_off(void *data)

and:

s/devm_pci_pwrctrl_upd720201_power_off/devm_upd720201_release/
s/pci_pwrctrl_upd720201_probe/upd720201_pwrctrl_probe/

Might be more opportunities to be more similar to slot.c and
pci-pwrctrl-tc9563.c, e.g., adding:

  upd720201->pwrctrl.power_on = ...;
  upd720201->pwrctrl.power_off = ...;

(would have to be based on pci/pwrctrl branch, which is where this
patch would be applied)

> +{
> + struct pci_pwrctrl_upd720201_data *upd720201 = data;
> +
> + regulator_bulk_disable(upd720201->num_supplies, upd720201->supplies);
> + regulator_bulk_free(upd720201->num_supplies, upd720201->supplies);
> +}
> +
> +static int pci_pwrctrl_upd720201_probe(struct platform_device *pdev)
> +{
> + struct pci_pwrctrl_upd720201_data *upd720201;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + upd720201 = devm_kzalloc(dev, sizeof(*upd720201), GFP_KERNEL);
> + if (!upd720201)
> + return -ENOMEM;
> +
> + ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
> + &upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to get upd720201 regulators\n");
> + return ret;
> + }
> +
> + upd720201->num_supplies = ret;
> + ret = regulator_bulk_enable(upd720201->num_supplies, 
> upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to enable upd720201 
> regulators\n");
> + regulator_bulk_free(upd720201->num_supplies, 
> upd720201->supplies);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, 
> devm_pci_pwrctrl_upd720201_power_off,
> +upd720201);
> + if (ret)
> + re

Re: [PATCH v2 2/7] pci: pwrctrl: add PCI pwrctrl driver for the UPD720201/UPD720202 USB 3.0 xHCI Host Controller

2026-01-27 Thread Manivannan Sadhasivam
On Tue, Jan 27, 2026 at 10:57:29AM +0100, Neil Armstrong wrote:
> Add support fo the Renesas UPD720201/UPD720202 USB 3.0 xHCI Host Controller
> power control which connects over PCIe and requires specific power supplies
> to start up.
> 

This driver only handles the supplies. So why can't you use the existing
pwrctrl-slot driver as a fallback?

- Mani

> Signed-off-by: Neil Armstrong 
> ---
>  drivers/pci/pwrctrl/Kconfig | 10 
>  drivers/pci/pwrctrl/Makefile|  2 +
>  drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c | 88 
> +
>  3 files changed, 100 insertions(+)
> 
> diff --git a/drivers/pci/pwrctrl/Kconfig b/drivers/pci/pwrctrl/Kconfig
> index e0f999f299bb..5a94e60d0d3e 100644
> --- a/drivers/pci/pwrctrl/Kconfig
> +++ b/drivers/pci/pwrctrl/Kconfig
> @@ -11,6 +11,16 @@ config PCI_PWRCTRL_PWRSEQ
>   select POWER_SEQUENCING
>   select PCI_PWRCTRL
>  
> +config PCI_PWRCTRL_UPD720201
> + tristate "PCI Power Control driver for the UPD720201 USB3 Host 
> Controller"
> + select PCI_PWRCTRL
> + help
> +   Say Y here to enable the PCI Power Control driver of the UPD720201
> +   USB3 Host Controller.
> +
> +   The voltage regulators powering the rails of the PCI slots
> +   are expected to be defined in the devicetree node of the PCI device.
> +
>  config PCI_PWRCTRL_SLOT
>   tristate "PCI Power Control driver for PCI slots"
>   select PCI_PWRCTRL
> diff --git a/drivers/pci/pwrctrl/Makefile b/drivers/pci/pwrctrl/Makefile
> index 13b02282106c..a99f85de8a3d 100644
> --- a/drivers/pci/pwrctrl/Makefile
> +++ b/drivers/pci/pwrctrl/Makefile
> @@ -5,6 +5,8 @@ pci-pwrctrl-core-y:= core.o
>  
>  obj-$(CONFIG_PCI_PWRCTRL_PWRSEQ) += pci-pwrctrl-pwrseq.o
>  
> +obj-$(CONFIG_PCI_PWRCTRL_UPD720201)  += pci-pwrctrl-upd720201.o
> +
>  obj-$(CONFIG_PCI_PWRCTRL_SLOT)   += pci-pwrctrl-slot.o
>  pci-pwrctrl-slot-y   := slot.o
>  
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c 
> b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> new file mode 100644
> index ..db96bbb69c21
> --- /dev/null
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-upd720201.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Based on upd720201.c:
> + * Copyright (C) 2024 Linaro Ltd.
> + * Author: Manivannan Sadhasivam 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct pci_pwrctrl_upd720201_data {
> + struct pci_pwrctrl ctx;
> + struct regulator_bulk_data *supplies;
> + int num_supplies;
> +};
> +
> +static void devm_pci_pwrctrl_upd720201_power_off(void *data)
> +{
> + struct pci_pwrctrl_upd720201_data *upd720201 = data;
> +
> + regulator_bulk_disable(upd720201->num_supplies, upd720201->supplies);
> + regulator_bulk_free(upd720201->num_supplies, upd720201->supplies);
> +}
> +
> +static int pci_pwrctrl_upd720201_probe(struct platform_device *pdev)
> +{
> + struct pci_pwrctrl_upd720201_data *upd720201;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + upd720201 = devm_kzalloc(dev, sizeof(*upd720201), GFP_KERNEL);
> + if (!upd720201)
> + return -ENOMEM;
> +
> + ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
> + &upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to get upd720201 regulators\n");
> + return ret;
> + }
> +
> + upd720201->num_supplies = ret;
> + ret = regulator_bulk_enable(upd720201->num_supplies, 
> upd720201->supplies);
> + if (ret < 0) {
> + dev_err_probe(dev, ret, "Failed to enable upd720201 
> regulators\n");
> + regulator_bulk_free(upd720201->num_supplies, 
> upd720201->supplies);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, 
> devm_pci_pwrctrl_upd720201_power_off,
> +upd720201);
> + if (ret)
> + return ret;
> +
> + pci_pwrctrl_init(&upd720201->ctx, dev);
> +
> + ret = devm_pci_pwrctrl_device_set_ready(dev, &upd720201->ctx);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register pwrctrl 
> driver\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pci_pwrctrl_upd720201_of_match[] = {
> + {
> + .compatible = "pci1912,0014",
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, pci_pwrctrl_upd720201_of_match);
> +
> +static struct platform_driver pci_pwrctrl_upd720201_driver = {
> + .driver = {
> + .name = "pci-pwrctrl-upd720201",
> + .of_match_table = pci_pwrctrl_upd720201_of_match,
> + },
> + .probe = pci_pwrctrl_upd720201_probe,
> +};
> +module_platform_driver(pci_pwrctrl_upd720201_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong ");
> +MODULE_DESCRIPTION("PCI Power Control driver for UPD720201