[PATCH v3 0/4] Add ext. port vbus handling for onboard-dev

[PATCH v3 0/4] Add ext. port vbus handling for onboard-dev

[Marco Felsch]

Hi,

the purpose of this series is to make it possible to switch the vbus of
the downstream ports of an onboard-dev hub via ext. host controlled
regulators.

Changelog:
v3:
- fix dt-bindings issues

v2:
- Link: https://lore.kernel.org/all/20250327172803.3404615-1-m.felsch@pengutronix.de/
- fix compile time errors in case the module builds

v1:
- Link: https://lore.kernel.org/all/20240807-b4-v6-10-topic-usb-onboard-dev-v1-0-f33ce21353c9@pengutronix.de/

---
Marco Felsch (4):
      usb: port: track the disabled state
      usb: hub: add infrastructure to pass onboard_dev port features
      dt-bindings: usb: microchip,usb2514: add support for port vbus-supply
      usb: misc: onboard_dev: add ext-vbus-supply handling

 .../devicetree/bindings/usb/microchip,usb2514.yaml |  6 ++
 drivers/usb/core/hub.c                             | 55 ++++++++++++-
 drivers/usb/core/hub.h                             |  4 +
 drivers/usb/core/port.c                            |  6 ++
 drivers/usb/misc/onboard_usb_dev.c                 | 95 ++++++++++++++++++++++
 drivers/usb/misc/onboard_usb_dev.h                 |  3 +
 include/linux/usb.h                                |  3 +
 7 files changed, 170 insertions(+), 2 deletions(-)
---
base-commit: 038d61fd642278bab63ee8ef722c50d10ab01e8f
change-id: 20250821-v6-16-topic-usb-onboard-dev-b8d4d1d8a086

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>

[PATCH v3 1/4] usb: port: track the disabled state

[Marco Felsch]

The disable state isn't tracked at the moment, instead the state is
directly passed to the hub driver. Change this behavior to only trigger
the hub if a state change happened. Exit early in case of no state
changes but don't return an error.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/usb/core/hub.h  | 2 ++
 drivers/usb/core/port.c | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 9ebc5ef54a325d63e01b0deb59a1853d2b13c8d5..297adf2c6078809ca582104f228e5222c464f999 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -97,6 +97,7 @@ struct usb_hub {
  * @usb3_lpm_u2_permit: whether USB3 U2 LPM is permitted.
  * @early_stop: whether port initialization will be stopped earlier.
  * @ignore_event: whether events of the port are ignored.
+ * @disabled: whether the port is disabled
  */
 struct usb_port {
 	struct usb_device *child;
@@ -118,6 +119,7 @@ struct usb_port {
 	unsigned int is_superspeed:1;
 	unsigned int usb3_lpm_u1_permit:1;
 	unsigned int usb3_lpm_u2_permit:1;
+	unsigned int disabled:1;
 };
 
 #define to_usb_port(_dev) \
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index f54198171b6a3fb49c5f74f4a8a303b422d099eb..cae08a9a71e65c42fb9a8f5369060bd82b8daebb 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
 	if (rc)
 		return rc;
 
+	/* Early quit if no change was detected */
+	if (port_dev->disabled == disabled)
+		return count;
+
 	hub_get(hub);
 	rc = usb_autopm_get_interface(intf);
 	if (rc < 0)
@@ -148,6 +152,8 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
 			usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
 	}
 
+	port_dev->disabled = disabled;
+
 	if (!rc)
 		rc = count;
 

-- 
2.39.5

[PATCH v3 2/4] usb: hub: add infrastructure to pass onboard_dev port features

[Marco Felsch]

On board devices may require special handling for en-/disable port
features due to PCB design decisions e.g. enable/disable the VBUS power
on the port via a host controlled regulator or GPIO.

This commit adds the necessary infrastructure to prepare the common code
base for such use-cases.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/usb/core/hub.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/core/hub.h |  2 ++
 include/linux/usb.h    |  3 +++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 256fe8c86828d51c33442345acdb7f3fe80a98ce..50ae500e2ad3711996609fc84806803449a5bf16 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -457,9 +457,19 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
  */
 int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+	int ret;
+
+	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
 		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
 		NULL, 0, 1000);
+	if (ret)
+		return ret;
+
+	if (hub->onboard_hub_clear_port_feature)
+		ret = hub->onboard_hub_clear_port_feature(hdev, feature, port1);
+
+	return ret;
 }
 
 /*
@@ -467,9 +477,19 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
  */
 static int set_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+	int ret;
+
+	ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
 		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
 		NULL, 0, 1000);
+	if (ret)
+		return ret;
+
+	if (hub->onboard_hub_set_port_feature)
+		ret = hub->onboard_hub_set_port_feature(hdev, feature, port1);
+
+	return ret;
 }
 
 static char *to_led_name(int selector)
@@ -6544,6 +6564,37 @@ void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
 	}
 }
 
+/**
+ * usb_hub_register_port_feature_hooks - Register port set/get feature hooks
+ * @hdev: USB device belonging to the usb hub
+ * @set_port_feature: set_feature hook which gets called by the hub core
+ * @clear_port_feature: clear_feature hook which gets called by the hub core
+ *
+ * Register set/get_port_feature hooks for a onboard_dev hub.
+ */
+void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
+		int (*set_port_feature)(struct usb_device *, int, int),
+		int (*clear_port_feature)(struct usb_device *, int, int))
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+
+	if (WARN_ON_ONCE(is_root_hub(hdev) || !hub))
+		return;
+
+	if (set_port_feature)
+		hub->onboard_hub_set_port_feature = set_port_feature;
+	if (clear_port_feature)
+		hub->onboard_hub_clear_port_feature = clear_port_feature;
+
+	/*
+	 * Keep it simple for now. Just check the power state and re-sync it
+	 * after adding the hooks since the onboard-dev may do some additional
+	 * logic e.g. controlling regulators.
+	 */
+	hub_power_on(hub, false);
+}
+EXPORT_SYMBOL_GPL(usb_hub_register_port_feature_hooks);
+
 #ifdef CONFIG_ACPI
 /**
  * usb_get_hub_port_acpi_handle - Get the usb port's acpi handle
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 297adf2c6078809ca582104f228e5222c464f999..31800b5922ba896dc7cac5f9e3ed1a77e7c5a801 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -76,6 +76,8 @@ struct usb_hub {
 	struct timer_list	irq_urb_retry;
 	struct usb_port		**ports;
 	struct list_head        onboard_devs;
+	int (*onboard_hub_set_port_feature)(struct usb_device *udev, int feature, int port1);
+	int (*onboard_hub_clear_port_feature)(struct usb_device *udev, int feature, int port1);
 };
 
 /**
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 92c752f5446ff37ef09b9296f7711e1a622680ea..1415a826953ac5488619073e2f659188f7988d6e 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -928,6 +928,9 @@ int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,
 		struct usb_dev_state *owner);
 int usb_hub_release_port(struct usb_device *hdev, unsigned port1,
 		struct usb_dev_state *owner);
+void usb_hub_register_port_feature_hooks(struct usb_device *hdev,
+		int (*set_port_feature)(struct usb_device *, int, int),
+		int (*clear_port_feature)(struct usb_device *, int, int));
 
 /**
  * usb_make_path - returns stable device path in the usb tree

-- 
2.39.5

[PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Marco Felsch]

Some PCB designs don't connect the USB hub port power control GPIO and
instead make use of a host controllable regulator. Add support for this
use-case by introducing portX-vbus-supply property.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/devicetree/bindings/usb/microchip,usb2514.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
index 4e3901efed3fcd4fbbd8cb777f9df4fcadf2ca00..ac1e5f1a5ea2e66c61ce92154385952b15e78e55 100644
--- a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
+++ b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
@@ -49,6 +49,12 @@ patternProperties:
     $ref: /schemas/usb/usb-device.yaml
     additionalProperties: true
 
+  "^port[1-7]-vbus-supply$":
+    type: object
+    description:
+      Regulator controlling the USB VBUS on portX. Only required if the host
+      controls the portX VBUS.
+
 allOf:
   - $ref: usb-device.yaml#
   - if:

-- 
2.39.5

[PATCH v3 4/4] usb: misc: onboard_dev: add ext-vbus-supply handling

[Marco Felsch]

Add support to power the port VBUS via host controlled regulators since
some embedded hub PCB designs don't connect the dedicated USB hub port
power GPIOs accordingly.

To support the above use-case this commits adds support to parse the OF
information and setup the regulators accordingly within the platform
driver part. Furthermore the usb driver registers the set/clear features
hooks via the new usb_hub_register_port_feature_hooks() if the
onboard_dev is a hub. Afterwards all generic hub handling is passed to
the onboard_dev driver too which allows us to control the regulators.

At the moment this feature is limited to the following hubs:
  - usb424,2412
  - usb424,2414
  - usb424,2417

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/usb/misc/onboard_usb_dev.c | 95 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/misc/onboard_usb_dev.h |  3 ++
 2 files changed, 98 insertions(+)

diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index 5b481876af1b2c10ce625fcf0fb8bfbe8905aa8c..bcb227c8dca2cecb3e49abfd41dcbe4e586d7d07 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -49,6 +49,8 @@ static DECLARE_WORK(attach_usb_driver_work, onboard_dev_attach_usb_driver);
 
 /************************** Platform driver **************************/
 
+#define MAX_DOWNSTREAM_PORTS	7
+
 struct usbdev_node {
 	struct usb_device *udev;
 	struct list_head list;
@@ -65,6 +67,7 @@ struct onboard_dev {
 	struct list_head udev_list;
 	struct mutex lock;
 	struct clk *clk;
+	struct regulator *ext_vbus_supplies[MAX_DOWNSTREAM_PORTS];
 };
 
 static int onboard_dev_get_regulators(struct onboard_dev *onboard_dev)
@@ -226,6 +229,53 @@ static int onboard_dev_add_usbdev(struct onboard_dev *onboard_dev,
 	return err;
 }
 
+static int onboard_dev_port_power(struct onboard_dev *onboard_dev, int port1,
+				  bool enable)
+{
+	struct regulator *vbus_supply;
+	unsigned int port = port1 - 1;
+
+	if (WARN_ON(port >= MAX_DOWNSTREAM_PORTS))
+		return -EINVAL;
+
+	vbus_supply = onboard_dev->ext_vbus_supplies[port];
+
+	/* External supplies are optional */
+	if (!vbus_supply)
+		return 0;
+
+	if (enable)
+		return regulator_enable(vbus_supply);
+
+	return regulator_disable(vbus_supply);
+}
+
+static int onboard_dev_add_ext_vbus_supplies(struct onboard_dev *onboard_dev)
+{
+	struct device *dev = onboard_dev->dev;
+	unsigned int i;
+
+	if (!onboard_dev->pdata->support_ext_vbus_supplies)
+		return 0;
+
+	for (i = 0; i < MAX_DOWNSTREAM_PORTS; i++) {
+		char supply_name[] = "portX-vbus";
+		struct regulator *reg;
+
+		sprintf(supply_name, "port%u-vbus", i + 1);
+		reg = devm_regulator_get_optional(dev, supply_name);
+		if (!IS_ERR(reg)) {
+			onboard_dev->ext_vbus_supplies[i] = reg;
+		} else {
+			if (PTR_ERR(reg) != -ENODEV)
+				return dev_err_probe(dev, PTR_ERR(reg),
+						     "failed to get %s-supply\n", supply_name);
+		}
+	}
+
+	return 0;
+}
+
 static void onboard_dev_remove_usbdev(struct onboard_dev *onboard_dev,
 				      const struct usb_device *udev)
 {
@@ -460,6 +510,10 @@ static int onboard_dev_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(onboard_dev->reset_gpio),
 				     "failed to get reset GPIO\n");
 
+	err = onboard_dev_add_ext_vbus_supplies(onboard_dev);
+	if (err)
+		return err;
+
 	mutex_init(&onboard_dev->lock);
 	INIT_LIST_HEAD(&onboard_dev->udev_list);
 
@@ -573,6 +627,44 @@ static struct platform_driver onboard_dev_driver = {
 #define VENDOR_ID_VIA		0x2109
 #define VENDOR_ID_XMOS		0x20B1
 
+static int onboard_dev_port_feature(struct usb_device *udev, bool set,
+				    int feature, int port1)
+{
+	struct device *dev = &udev->dev;
+	struct onboard_dev *onboard_dev = dev_get_drvdata(dev);
+
+	/*
+	 * Check usb_hub_register_port_feature_hooks() if you want to extent
+	 * the list of handled features. At the moment only power is synced
+	 * after adding the hook.
+	 */
+	switch (feature) {
+	case USB_PORT_FEAT_POWER:
+		return onboard_dev_port_power(onboard_dev, port1, set);
+	default:
+		return 0;
+	}
+}
+
+static int
+onboard_dev_set_port_feature(struct usb_device *udev, int feature, int port1)
+{
+	return onboard_dev_port_feature(udev, true, feature, port1);
+}
+
+static int
+onboard_dev_clear_port_feature(struct usb_device *udev, int feature, int port1)
+{
+	return onboard_dev_port_feature(udev, false, feature, port1);
+}
+
+static void
+onboard_dev_register_hub_hooks(struct usb_device *udev)
+{
+	usb_hub_register_port_feature_hooks(udev, onboard_dev_set_port_feature,
+					    onboard_dev_clear_port_feature);
+}
+
 /*
  * Returns the onboard_dev platform device that is associated with the USB
  * device passed as parameter.
@@ -632,6 +724,9 @@ static int onboard_dev_usbdev_probe(struct usb_device *udev)
 
 	dev_set_drvdata(dev, onboard_dev);
 
+	if (onboard_dev->pdata->is_hub)
+		onboard_dev_register_hub_hooks(udev);
+
 	err = onboard_dev_add_usbdev(onboard_dev, udev);
 	if (err)
 		return err;
diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h
index e017b8e22f936be66da73789abb4f620e6af4d6a..75aa2ab9297e272a98de15270d3595d7df03fb9c 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -14,6 +14,7 @@ struct onboard_dev_pdata {
 	unsigned int num_supplies;	/* number of supplies */
 	const char * const supply_names[MAX_SUPPLIES];
 	bool is_hub;
+	bool support_ext_vbus_supplies;
 };
 
 static const struct onboard_dev_pdata microchip_usb424_data = {
@@ -21,6 +22,7 @@ static const struct onboard_dev_pdata microchip_usb424_data = {
 	.num_supplies = 1,
 	.supply_names = { "vdd" },
 	.is_hub = true,
+	.support_ext_vbus_supplies = true,
 };
 
 static const struct onboard_dev_pdata microchip_usb2514_data = {
@@ -28,6 +30,7 @@ static const struct onboard_dev_pdata microchip_usb2514_data = {
 	.num_supplies = 2,
 	.supply_names = { "vdd", "vdda" },
 	.is_hub = true,
+	.support_ext_vbus_supplies = true,
 };
 
 static const struct onboard_dev_pdata microchip_usb5744_data = {

-- 
2.39.5

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Krzysztof Kozlowski]

On Thu, Aug 21, 2025 at 06:31:57PM +0200, Marco Felsch wrote:
> Some PCB designs don't connect the USB hub port power control GPIO and
> instead make use of a host controllable regulator. Add support for this
> use-case by introducing portX-vbus-supply property.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/usb/microchip,usb2514.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> index 4e3901efed3fcd4fbbd8cb777f9df4fcadf2ca00..ac1e5f1a5ea2e66c61ce92154385952b15e78e55 100644
> --- a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> +++ b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> @@ -49,6 +49,12 @@ patternProperties:
>      $ref: /schemas/usb/usb-device.yaml
>      additionalProperties: true
>  
> +  "^port[1-7]-vbus-supply$":
> +    type: object
> +    description:
> +      Regulator controlling the USB VBUS on portX. Only required if the host
> +      controls the portX VBUS.

Your commit msg should briefly describe status of previous discussion:
why Rob's comment was not applied. Otherwise we repeat: this looks like
property of specific port.

The binding does not list ports now, but lists hard-wired devices, so my
question is now: is this per hard-wired device or per port (even if port
is hot-pluggable)?

Best regards,
Krzysztof

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Marco Felsch]

On 25-08-22, Krzysztof Kozlowski wrote:
> On Thu, Aug 21, 2025 at 06:31:57PM +0200, Marco Felsch wrote:
> > Some PCB designs don't connect the USB hub port power control GPIO and
> > instead make use of a host controllable regulator. Add support for this
> > use-case by introducing portX-vbus-supply property.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/usb/microchip,usb2514.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > index 4e3901efed3fcd4fbbd8cb777f9df4fcadf2ca00..ac1e5f1a5ea2e66c61ce92154385952b15e78e55 100644
> > --- a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > +++ b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > @@ -49,6 +49,12 @@ patternProperties:
> >      $ref: /schemas/usb/usb-device.yaml
> >      additionalProperties: true
> >  
> > +  "^port[1-7]-vbus-supply$":
> > +    type: object
> > +    description:
> > +      Regulator controlling the USB VBUS on portX. Only required if the host
> > +      controls the portX VBUS.
> 
> Your commit msg should briefly describe status of previous discussion:
> why Rob's comment was not applied. Otherwise we repeat: this looks like
> property of specific port.

I answered Rob on my v1 but got no feedback. My v2 caused an issue found
by Rob's test bot. Therefore I thought he is okay and applied the
patchset for testing.

At least to me it's unclear when Rob's test bot is executed.

> The binding does not list ports now, but lists hard-wired devices, so my
> question is now: is this per hard-wired device or per port (even if port
> is hot-pluggable)?

Sorry but I don't get you. The binding lists the regulators required to
enable/disable the hub downstream port VBUS. These regulators are
controlled by an external party e.g. the CPU instead of the USB hub
itself. The connection from the CPU to the regulator which controlls the
+5V usb-connector pin is hard-wired, yes.

Regards,
  Marco

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Rob Herring]

On Fri, Aug 22, 2025 at 12:30:05PM +0200, Marco Felsch wrote:
> On 25-08-22, Krzysztof Kozlowski wrote:
> > On Thu, Aug 21, 2025 at 06:31:57PM +0200, Marco Felsch wrote:
> > > Some PCB designs don't connect the USB hub port power control GPIO and
> > > instead make use of a host controllable regulator. Add support for this
> > > use-case by introducing portX-vbus-supply property.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  Documentation/devicetree/bindings/usb/microchip,usb2514.yaml | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > index 4e3901efed3fcd4fbbd8cb777f9df4fcadf2ca00..ac1e5f1a5ea2e66c61ce92154385952b15e78e55 100644
> > > --- a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > @@ -49,6 +49,12 @@ patternProperties:
> > >      $ref: /schemas/usb/usb-device.yaml
> > >      additionalProperties: true
> > >  
> > > +  "^port[1-7]-vbus-supply$":
> > > +    type: object
> > > +    description:
> > > +      Regulator controlling the USB VBUS on portX. Only required if the host
> > > +      controls the portX VBUS.
> > 
> > Your commit msg should briefly describe status of previous discussion:
> > why Rob's comment was not applied. Otherwise we repeat: this looks like
> > property of specific port.
> 
> I answered Rob on my v1 but got no feedback. 

I just read it and don't understand. You don't have to have all 
properties for a driver in the node associated with the driver. The 
driver can freely look in the child nodes or anywhere else in the whole 
tree if needed. Is that what you meant?

For USB hubs we generally define child nodes for each port. Some of the 
hub bindings don't because they are incomplete. If you have a per port 
property, then the DT property belongs in the port's node.

> My v2 caused an issue found
> by Rob's test bot. Therefore I thought he is okay and applied the
> patchset for testing.

Other way around. If it doesn't pass tests, I don't look at it. (Well, I 
do, but don't expect a reply.)

> At least to me it's unclear when Rob's test bot is executed.

When you submit something. It's all automatic, though sometimes the 
emails are delayed. Results are always in PW within 1-2 hours (unless 
someone patch bombs us with a large series).

> 
> > The binding does not list ports now, but lists hard-wired devices, so my
> > question is now: is this per hard-wired device or per port (even if port
> > is hot-pluggable)?
> 
> Sorry but I don't get you. The binding lists the regulators required to
> enable/disable the hub downstream port VBUS. These regulators are
> controlled by an external party e.g. the CPU instead of the USB hub
> itself. The connection from the CPU to the regulator which controlls the
> +5V usb-connector pin is hard-wired, yes.
> 
> Regards,
>   Marco

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Krzysztof Kozlowski]

On 22/08/2025 12:30, Marco Felsch wrote:
>> The binding does not list ports now, but lists hard-wired devices, so my
>> question is now: is this per hard-wired device or per port (even if port
>> is hot-pluggable)?
> 
> Sorry but I don't get you. The binding lists the regulators required to
> enable/disable the hub downstream port VBUS. These regulators are

Is the port an external facing connector or a hard-wired USB device
(please read the binding)?

> controlled by an external party e.g. the CPU instead of the USB hub
> itself. The connection from the CPU to the regulator which controlls the
> +5V usb-connector pin is hard-wired, yes.

I speak about USB devices.

Best regards,
Krzysztof

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Marco Felsch]

On 25-08-22, Rob Herring wrote:
> On Fri, Aug 22, 2025 at 12:30:05PM +0200, Marco Felsch wrote:
> > On 25-08-22, Krzysztof Kozlowski wrote:
> > > On Thu, Aug 21, 2025 at 06:31:57PM +0200, Marco Felsch wrote:
> > > > Some PCB designs don't connect the USB hub port power control GPIO and
> > > > instead make use of a host controllable regulator. Add support for this
> > > > use-case by introducing portX-vbus-supply property.
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  Documentation/devicetree/bindings/usb/microchip,usb2514.yaml | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > > index 4e3901efed3fcd4fbbd8cb777f9df4fcadf2ca00..ac1e5f1a5ea2e66c61ce92154385952b15e78e55 100644
> > > > --- a/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > > +++ b/Documentation/devicetree/bindings/usb/microchip,usb2514.yaml
> > > > @@ -49,6 +49,12 @@ patternProperties:
> > > >      $ref: /schemas/usb/usb-device.yaml
> > > >      additionalProperties: true
> > > >  
> > > > +  "^port[1-7]-vbus-supply$":
> > > > +    type: object
> > > > +    description:
> > > > +      Regulator controlling the USB VBUS on portX. Only required if the host
> > > > +      controls the portX VBUS.
> > > 
> > > Your commit msg should briefly describe status of previous discussion:
> > > why Rob's comment was not applied. Otherwise we repeat: this looks like
> > > property of specific port.
> > 
> > I answered Rob on my v1 but got no feedback. 
> 
> I just read it and don't understand. You don't have to have all 
> properties for a driver in the node associated with the driver. The 
> driver can freely look in the child nodes or anywhere else in the whole 
> tree if needed. Is that what you meant?
> 
> For USB hubs we generally define child nodes for each port. Some of the 
> hub bindings don't because they are incomplete. If you have a per port 
> property, then the DT property belongs in the port's node.

The problem was, that the regulator API didn't supported to search for
regulators which don't belong to its own DT node.

I wasn't sure if this is intented or not. Now I see that, the regulator
API gained the support for this use-case else I would have pinged Mark
if we need to add support for it.

I will change the "vbus-supply" to be specified within the port DT
nodes.

> > My v2 caused an issue found
> > by Rob's test bot. Therefore I thought he is okay and applied the
> > patchset for testing.
> 
> Other way around. If it doesn't pass tests, I don't look at it. (Well, I 
> do, but don't expect a reply.)

Okay, thanks for the clarification.

> > At least to me it's unclear when Rob's test bot is executed.
> 
> When you submit something. It's all automatic, though sometimes the 
> emails are delayed. Results are always in PW within 1-2 hours (unless 
> someone patch bombs us with a large series).

Thanks.

Regards,
  Marco

Re: [PATCH v3 3/4] dt-bindings: usb: microchip,usb2514: add support for port vbus-supply

[Marco Felsch]

On 25-08-24, Krzysztof Kozlowski wrote:
> On 22/08/2025 12:30, Marco Felsch wrote:
> >> The binding does not list ports now, but lists hard-wired devices, so my
> >> question is now: is this per hard-wired device or per port (even if port
> >> is hot-pluggable)?
> > 
> > Sorry but I don't get you. The binding lists the regulators required to
> > enable/disable the hub downstream port VBUS. These regulators are
> 
> Is the port an external facing connector or a hard-wired USB device
> (please read the binding)?

It's completely irrelevant isn't it? The host is in charge of enabling
the VBUS supply via a dedicated GPIO (e.g. a I2C GPIO expander). If the
VBUS is off, no device appear, if it is on, the device gets powered and
appears within the system. If no device is plugged yet and the VBUS is
enabled, the device gets enumerated immediatly.

Normally the VBUS supplies are controlled by the HUB control signals,
but unfortunately our design didn't used these and yes in my case it's a
hard-wired device.

Generally speaking I don't see how this will make a difference for
hard-wired or hot-pluggable devices.

Regards,
  Marco