[PATCH v5 0/2] Implement vbus support for HD3SS3220 port controller

[PATCH v5 0/2] Implement vbus support for HD3SS3220 port controller

[Krishna Kurapati]

As per the data sheet of HD3SS3220:

"Upon detecting a UFP device, HD3SS3220 will keep ID pin high if VBUS is
not at VSafe0V. Once VBUS is at VSafe0V, the HD3SS3220 will assert ID pin
low. This is done to enforce Type-C requirement that VBUS must be at
VSafe0V before re-enabling VBUS"

This series implements support to read ID pin state and accordingly enable
VBUS.

Changes in v5:
- Modified error handling in driver as per comments received on v4.

Link to v4:
https://lore.kernel.org/all/20251025122854.1163275-1-krishna.kurapati@oss.qualcomm.com/

Changes in v4:
- Modified logic to check for vbus supply. Directly checking first remote
  endpoint.
- Used of_regulator_get_optional instead of of_regulator_get

Link to v3:
https://lore.kernel.org/all/20251024181832.2744502-1-krishna.kurapati@oss.qualcomm.com/

Changes in v3:
- Removed vbus supply from hd3ss3220 bindings.
- Implemented getting vbus from connector node.

Link to v2:
https://lore.kernel.org/all/20251008175750.1770454-1-krishna.kurapati@oss.qualcomm.com/

Changes in v2:
- Fixed inclusion of header files appropriately.
- Modified commit text for driver patch.

Link to v1:
https://lore.kernel.org/all/20251002172539.586538-1-krishna.kurapati@oss.qualcomm.com/

Krishna Kurapati (2):
  dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
  usb: typec: hd3ss3220: Enable VBUS based on ID pin state

 .../devicetree/bindings/usb/ti,hd3ss3220.yaml |  8 ++
 drivers/usb/typec/hd3ss3220.c                 | 74 +++++++++++++++++++
 2 files changed, 82 insertions(+)

-- 
2.34.1

[PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state

[Krishna Kurapati]

Update the bindings to support reading ID state and VBUS, as per the
HD3SS3220 data sheet. The ID pin is kept high if VBUS is not at VSafe0V and
asserted low once VBUS is at VSafe0V, enforcing the Type-C requirement that
VBUS must be at VSafe0V before re-enabling VBUS.

Add id-gpios property to describe the input gpio for USB ID pin.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
index bec1c8047bc0..06099e93c6c3 100644
--- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
@@ -25,6 +25,14 @@ properties:
   interrupts:
     maxItems: 1
 
+  id-gpios:
+    description:
+      An input gpio for USB ID pin. Upon detecting a UFP device, HD3SS3220
+      will keep ID pin high if VBUS is not at VSafe0V. Once VBUS is at VSafe0V,
+      the HD3SS3220 will assert ID pin low. This is done to enforce Type-C
+      requirement that VBUS must be at VSafe0V before re-enabling VBUS.
+    maxItems: 1
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
     description: OF graph bindings (specified in bindings/graph.txt) that model
-- 
2.34.1

[PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Krishna Kurapati]

There is a ID pin present on HD3SS3220 controller that can be routed
to SoC. As per the datasheet:

"Upon detecting a UFP device, HD3SS3220 will keep ID pin high if VBUS is
not at VSafe0V. Once VBUS is at VSafe0V, the HD3SS3220 will assert ID pin
low. This is done to enforce Type-C requirement that VBUS must be at
VSafe0V before re-enabling VBUS"

Add support to read the ID pin state and enable VBUS accordingly.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 drivers/usb/typec/hd3ss3220.c | 74 +++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 3ecc688dda82..8614c71d7ae5 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -15,6 +15,9 @@
 #include <linux/usb/typec.h>
 #include <linux/delay.h>
 #include <linux/workqueue.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_graph.h>
 
 #define HD3SS3220_REG_CN_STAT		0x08
 #define HD3SS3220_REG_CN_STAT_CTRL	0x09
@@ -54,6 +57,11 @@ struct hd3ss3220 {
 	struct delayed_work output_poll_work;
 	enum usb_role role_state;
 	bool poll;
+
+	struct gpio_desc *id_gpiod;
+	int id_irq;
+
+	struct regulator *vbus;
 };
 
 static int hd3ss3220_set_power_opmode(struct hd3ss3220 *hd3ss3220, int power_opmode)
@@ -319,6 +327,48 @@ static const struct regmap_config config = {
 	.max_register = 0x0A,
 };
 
+static irqreturn_t hd3ss3220_id_isr(int irq, void *dev_id)
+{
+	struct hd3ss3220 *hd3ss3220 = dev_id;
+	int ret;
+	int id;
+
+	if (!hd3ss3220->vbus)
+		return IRQ_HANDLED;
+
+	id = hd3ss3220->id_gpiod ? gpiod_get_value_cansleep(hd3ss3220->id_gpiod) : 1;
+
+	if (!id) {
+		ret = regulator_enable(hd3ss3220->vbus);
+		if (ret)
+			dev_err(hd3ss3220->dev, "enable vbus regulator failed\n");
+	} else {
+		regulator_disable(hd3ss3220->vbus);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
+{
+	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
+	struct device_node *np;
+
+	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
+	if (!np) {
+		dev_err(hd3ss3220->dev, "failed to get device node");
+		return -ENODEV;
+	}
+
+	hd3ss3220->vbus = of_regulator_get_optional(hd3ss3220->dev, np, "vbus");
+	if (IS_ERR(hd3ss3220->vbus))
+		hd3ss3220->vbus = NULL;
+
+	of_node_put(np);
+
+	return 0;
+}
+
 static int hd3ss3220_probe(struct i2c_client *client)
 {
 	struct typec_capability typec_cap = { };
@@ -354,6 +404,30 @@ static int hd3ss3220_probe(struct i2c_client *client)
 		hd3ss3220->role_sw = usb_role_switch_get(hd3ss3220->dev);
 	}
 
+	hd3ss3220->id_gpiod = devm_gpiod_get_optional(hd3ss3220->dev, "id", GPIOD_IN);
+	if (IS_ERR(hd3ss3220->id_gpiod))
+		return PTR_ERR(hd3ss3220->id_gpiod);
+
+	if (hd3ss3220->id_gpiod) {
+		hd3ss3220->id_irq = gpiod_to_irq(hd3ss3220->id_gpiod);
+		if (hd3ss3220->id_irq < 0)
+			return dev_err_probe(hd3ss3220->dev,
+					     hd3ss3220->id_irq, "failed to get ID gpio\n");
+
+		ret = devm_request_threaded_irq(hd3ss3220->dev,
+						hd3ss3220->id_irq, NULL,
+						hd3ss3220_id_isr,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						dev_name(hd3ss3220->dev), hd3ss3220);
+		if (ret < 0)
+			return dev_err_probe(hd3ss3220->dev, ret, "failed to get ID irq\n");
+	}
+
+	ret = hd3ss3220_get_vbus_supply(hd3ss3220);
+	if (ret)
+		return dev_err_probe(hd3ss3220->dev, ret, "failed to get vbus\n");
+
 	if (IS_ERR(hd3ss3220->role_sw)) {
 		ret = PTR_ERR(hd3ss3220->role_sw);
 		goto err_put_fwnode;
-- 
2.34.1

RE: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state

[Biju Das]

> -----Original Message-----
> From: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> Sent: 27 October 2025 07:28
.kernel.org; linux-kernel@vger.kernel.org; Krishna
> Kurapati <krishna.kurapati@oss.qualcomm.com>
> Subject: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
> 
> Update the bindings to support reading ID state and VBUS, as per the
> HD3SS3220 data sheet. The ID pin is kept high if VBUS is not at VSafe0V and asserted low once VBUS is
> at VSafe0V, enforcing the Type-C requirement that VBUS must be at VSafe0V before re-enabling VBUS.
> 
> Add id-gpios property to describe the input gpio for USB ID pin.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> index bec1c8047bc0..06099e93c6c3 100644
> --- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> @@ -25,6 +25,14 @@ properties:
>    interrupts:
>      maxItems: 1
> 
> +  id-gpios:
> +    description:
> +      An input gpio for USB ID pin. Upon detecting a UFP device, HD3SS3220
> +      will keep ID pin high if VBUS is not at VSafe0V. Once VBUS is at VSafe0V,
> +      the HD3SS3220 will assert ID pin low. This is done to enforce Type-C
> +      requirement that VBUS must be at VSafe0V before re-enabling VBUS.
> +    maxItems: 1
> +

Maybe to help DT users, add an example for this use case??

Cheers,
Biju

>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>      description: OF graph bindings (specified in bindings/graph.txt) that model
> --
> 2.34.1

Re: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state

[Krishna Kurapati PSSNV]

On 10/27/2025 1:02 PM, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> Sent: 27 October 2025 07:28
> .kernel.org; linux-kernel@vger.kernel.org; Krishna
>> Kurapati <krishna.kurapati@oss.qualcomm.com>
>> Subject: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
>>
>> Update the bindings to support reading ID state and VBUS, as per the
>> HD3SS3220 data sheet. The ID pin is kept high if VBUS is not at VSafe0V and asserted low once VBUS is
>> at VSafe0V, enforcing the Type-C requirement that VBUS must be at VSafe0V before re-enabling VBUS.
>>
>> Add id-gpios property to describe the input gpio for USB ID pin.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
>> b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
>> index bec1c8047bc0..06099e93c6c3 100644
>> --- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
>> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
>> @@ -25,6 +25,14 @@ properties:
>>     interrupts:
>>       maxItems: 1
>>
>> +  id-gpios:
>> +    description:
>> +      An input gpio for USB ID pin. Upon detecting a UFP device, HD3SS3220
>> +      will keep ID pin high if VBUS is not at VSafe0V. Once VBUS is at VSafe0V,
>> +      the HD3SS3220 will assert ID pin low. This is done to enforce Type-C
>> +      requirement that VBUS must be at VSafe0V before re-enabling VBUS.
>> +    maxItems: 1
>> +
> 
> Maybe to help DT users, add an example for this use case??
> 
Hi Biju,

  Adding GPIO to dt is a generic thing. Also this is an optional 
proprety. Can we skip adding an example.

Regards,
Krishna,

RE: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state

[Biju Das]

> -----Original Message-----
> From: Krishna Kurapati PSSNV <krishna.kurapati@oss.qualcomm.com>
> Sent: 27 October 2025 08:32
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: linux-usb@vger.kernel.org; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> Heikki Krogerus <heikki.krogerus@linux.intel.com>; Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com>; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
> 
> 
> 
> On 10/27/2025 1:02 PM, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> >> Sent: 27 October 2025 07:28
> > .kernel.org; linux-kernel@vger.kernel.org; Krishna
> >> Kurapati <krishna.kurapati@oss.qualcomm.com>
> >> Subject: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support
> >> for VBUS based on ID state
> >>
> >> Update the bindings to support reading ID state and VBUS, as per the
> >> HD3SS3220 data sheet. The ID pin is kept high if VBUS is not at
> >> VSafe0V and asserted low once VBUS is at VSafe0V, enforcing the Type-C requirement that VBUS must
> be at VSafe0V before re-enabling VBUS.
> >>
> >> Add id-gpios property to describe the input gpio for USB ID pin.
> >>
> >> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> >> ---
> >>   Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> >> b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> >> index bec1c8047bc0..06099e93c6c3 100644
> >> --- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> >> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> >> @@ -25,6 +25,14 @@ properties:
> >>     interrupts:
> >>       maxItems: 1
> >>
> >> +  id-gpios:
> >> +    description:
> >> +      An input gpio for USB ID pin. Upon detecting a UFP device, HD3SS3220
> >> +      will keep ID pin high if VBUS is not at VSafe0V. Once VBUS is at VSafe0V,
> >> +      the HD3SS3220 will assert ID pin low. This is done to enforce Type-C
> >> +      requirement that VBUS must be at VSafe0V before re-enabling VBUS.
> >> +    maxItems: 1
> >> +
> >
> > Maybe to help DT users, add an example for this use case??
> >
> Hi Biju,
> 
>   Adding GPIO to dt is a generic thing. Also this is an optional proprety. Can we skip adding an
> example.

OK for me. Just thought that example will cover regulator usage as well for DT user.

Cheers,
Biju

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Heikki Krogerus]

Hi Krishna,

> +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
> +{
> +	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
> +	struct device_node *np;
> +
> +	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
> +	if (!np) {
> +		dev_err(hd3ss3220->dev, "failed to get device node");
> +		return -ENODEV;
> +	}

So I guess that's the connector node. Why can't you just place the
regulator reference to the hd3ss3220 controller node instead of the
connector like the port controllers do?

That would allow us to do a simple devm_regulator_get_optional() call
that's not tied to DT only.

> +	hd3ss3220->vbus = of_regulator_get_optional(hd3ss3220->dev, np, "vbus");
> +	if (IS_ERR(hd3ss3220->vbus))
> +		hd3ss3220->vbus = NULL;
> +
> +	of_node_put(np);
> +
> +	return 0;
> +}
> +
>  static int hd3ss3220_probe(struct i2c_client *client)
>  {
>  	struct typec_capability typec_cap = { };

<snip>

> +	ret = hd3ss3220_get_vbus_supply(hd3ss3220);
> +	if (ret)
> +		return dev_err_probe(hd3ss3220->dev, ret, "failed to get vbus\n");

I think you have resource leaks here. I'm pretty sure you need to do
regulator_put() somewhere. You are also leaking the connector fwnode
that was acquired just before this point..

>  	if (IS_ERR(hd3ss3220->role_sw)) {
>  		ret = PTR_ERR(hd3ss3220->role_sw);
>  		goto err_put_fwnode;

Get the regulator here after the above condition. Then add a label for
the regulator_put(). And you already have the handle to the connector
fwnode so use that one instead of getting it again:

        hd3ss3220->vbus = of_regulator_get_optional(hd3ss3220->dev, to_of_node(connector), "vbus");

But do it like that only if you really can't place the vbus regulator
reference to the controller node. I would really prefer that we could
do a simple:

        hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");

thanks,

-- 
heikki

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Krishna Kurapati PSSNV]

On 10/27/2025 3:17 PM, Heikki Krogerus wrote:
> Hi Krishna,
> 
>> +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
>> +{
>> +	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
>> +	struct device_node *np;
>> +
>> +	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
>> +	if (!np) {
>> +		dev_err(hd3ss3220->dev, "failed to get device node");
>> +		return -ENODEV;
>> +	}
> 
> So I guess that's the connector node. Why can't you just place the
> regulator reference to the hd3ss3220 controller node instead of the
> connector like the port controllers do?
> 
> That would allow us to do a simple devm_regulator_get_optional() call
> that's not tied to DT only.
> 

I did that in v1:
https://lore.kernel.org/all/20251002172539.586538-3-krishna.kurapati@oss.qualcomm.com/

But Dmitry mentioned that vbus supply must be in connector node:
https://lore.kernel.org/all/cbpne2d7yr2vpxmrrveqajlp3irzsglxroxyyjmviuci2ewted@6ewwp6yyybk5/

So keeping it in connector node.

>> +	hd3ss3220->vbus = of_regulator_get_optional(hd3ss3220->dev, np, "vbus");
>> +	if (IS_ERR(hd3ss3220->vbus))
>> +		hd3ss3220->vbus = NULL;
>> +
>> +	of_node_put(np);
>> +
>> +	return 0;
>> +}
>> +
>>   static int hd3ss3220_probe(struct i2c_client *client)
>>   {
>>   	struct typec_capability typec_cap = { };
> 
> <snip>
> 
>> +	ret = hd3ss3220_get_vbus_supply(hd3ss3220);
>> +	if (ret)
>> +		return dev_err_probe(hd3ss3220->dev, ret, "failed to get vbus\n");
> 
> I think you have resource leaks here. I'm pretty sure you need to do
> regulator_put() somewhere. You are also leaking the connector fwnode
> that was acquired just before this point..
> 

ACK. Will do regulator_put in cleanup path.
For device node of connector, i am doing of_node_put above.

>>   	if (IS_ERR(hd3ss3220->role_sw)) {
>>   		ret = PTR_ERR(hd3ss3220->role_sw);
>>   		goto err_put_fwnode;
> 
> Get the regulator here after the above condition. Then add a label for
> the regulator_put(). And you already have the handle to the connector
> fwnode so use that one instead of getting it again:
> 
>          hd3ss3220->vbus = of_regulator_get_optional(hd3ss3220->dev, to_of_node(connector), "vbus");
> 
> But do it like that only if you really can't place the vbus regulator
> reference to the controller node. I would really prefer that we could
> do a simple:
> 
>          hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");

ACK.

Thanks for the review.

Regards,
Krishna,

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Dmitry Baryshkov]

On Mon, Oct 27, 2025 at 11:47:13AM +0200, Heikki Krogerus wrote:
> Hi Krishna,
> 
> > +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
> > +{
> > +	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
> > +	struct device_node *np;
> > +
> > +	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
> > +	if (!np) {
> > +		dev_err(hd3ss3220->dev, "failed to get device node");
> > +		return -ENODEV;
> > +	}
> 
> So I guess that's the connector node. Why can't you just place the
> regulator reference to the hd3ss3220 controller node instead of the
> connector like the port controllers do?
> 
> That would allow us to do a simple devm_regulator_get_optional() call
> that's not tied to DT only.

But we have devm_of_regulator_get_optional(), it was mentioned in the
previous email if I'm not mistaken. If we need, we should add
devm_fwnode_regulator_get(_optional).

vbus supply is described as a part of the usb-c-connector schema, so
it is not that logical to describe it as a part of the Type-C
controller.


-- 
With best wishes
Dmitry

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Heikki Krogerus]

On Mon, Oct 27, 2025 at 03:19:04PM +0200, Dmitry Baryshkov wrote:
> On Mon, Oct 27, 2025 at 11:47:13AM +0200, Heikki Krogerus wrote:
> > Hi Krishna,
> > 
> > > +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
> > > +{
> > > +	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
> > > +	struct device_node *np;
> > > +
> > > +	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
> > > +	if (!np) {
> > > +		dev_err(hd3ss3220->dev, "failed to get device node");
> > > +		return -ENODEV;
> > > +	}
> > 
> > So I guess that's the connector node. Why can't you just place the
> > regulator reference to the hd3ss3220 controller node instead of the
> > connector like the port controllers do?
> > 
> > That would allow us to do a simple devm_regulator_get_optional() call
> > that's not tied to DT only.
> 
> But we have devm_of_regulator_get_optional(), it was mentioned in the
> previous email if I'm not mistaken. If we need, we should add
> devm_fwnode_regulator_get(_optional).
> 
> vbus supply is described as a part of the usb-c-connector schema, so
> it is not that logical to describe it as a part of the Type-C
> controller.

Okay, got it. This is OK by me then.

Thanks Dmitry,

-- 
heikki

Re: [PATCH v5 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state

[Rob Herring (Arm)]

On Mon, 27 Oct 2025 12:57:40 +0530, Krishna Kurapati wrote:
> Update the bindings to support reading ID state and VBUS, as per the
> HD3SS3220 data sheet. The ID pin is kept high if VBUS is not at VSafe0V and
> asserted low once VBUS is at VSafe0V, enforcing the Type-C requirement that
> VBUS must be at VSafe0V before re-enabling VBUS.
> 
> Add id-gpios property to describe the input gpio for USB ID pin.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Krishna Kurapati PSSNV]

On 10/27/2025 6:49 PM, Dmitry Baryshkov wrote:
> On Mon, Oct 27, 2025 at 11:47:13AM +0200, Heikki Krogerus wrote:
>> Hi Krishna,
>>
>>> +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
>>> +{
>>> +	struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
>>> +	struct device_node *np;
>>> +
>>> +	np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
>>> +	if (!np) {
>>> +		dev_err(hd3ss3220->dev, "failed to get device node");
>>> +		return -ENODEV;
>>> +	}
>>
>> So I guess that's the connector node. Why can't you just place the
>> regulator reference to the hd3ss3220 controller node instead of the
>> connector like the port controllers do?
>>
>> That would allow us to do a simple devm_regulator_get_optional() call
>> that's not tied to DT only.
> 
> But we have devm_of_regulator_get_optional(), it was mentioned in the
> previous email if I'm not mistaken. If we need, we should add
> devm_fwnode_regulator_get(_optional).
> 
> vbus supply is described as a part of the usb-c-connector schema, so
> it is not that logical to describe it as a part of the Type-C
> controller.
> 
> 

I tried the following as suggested:

hd3ss3220->vbus =  devm_of_regulator_get_optional(hd3ss3220->dev, 
to_of_node(connector),
if (IS_ERR(hd3ss3220->vbus))
   hd3ss3220->vbus = NULL;

If there is a vbus supply I see its returning proper handle pointer.
Else it returned ENODEV. (which is fine for our case as there is no vbus 
in DT).

Can I mark the function as a void one. Instead of returning any int 
value, would it be fine if to just mark vbus as NULL and proceed ?

Regards,
Krishna,

Re: [PATCH v5 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state

[Dmitry Baryshkov]

On 31/10/2025 08:45, Krishna Kurapati PSSNV wrote:
> 
> 
> On 10/27/2025 6:49 PM, Dmitry Baryshkov wrote:
>> On Mon, Oct 27, 2025 at 11:47:13AM +0200, Heikki Krogerus wrote:
>>> Hi Krishna,
>>>
>>>> +static int hd3ss3220_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
>>>> +{
>>>> +    struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
>>>> +    struct device_node *np;
>>>> +
>>>> +    np = of_graph_get_remote_node(hd3ss3220_node, 0, 0);
>>>> +    if (!np) {
>>>> +        dev_err(hd3ss3220->dev, "failed to get device node");
>>>> +        return -ENODEV;
>>>> +    }
>>>
>>> So I guess that's the connector node. Why can't you just place the
>>> regulator reference to the hd3ss3220 controller node instead of the
>>> connector like the port controllers do?
>>>
>>> That would allow us to do a simple devm_regulator_get_optional() call
>>> that's not tied to DT only.
>>
>> But we have devm_of_regulator_get_optional(), it was mentioned in the
>> previous email if I'm not mistaken. If we need, we should add
>> devm_fwnode_regulator_get(_optional).
>>
>> vbus supply is described as a part of the usb-c-connector schema, so
>> it is not that logical to describe it as a part of the Type-C
>> controller.
>>
>>
> 
> I tried the following as suggested:
> 
> hd3ss3220->vbus =  devm_of_regulator_get_optional(hd3ss3220->dev, 
> to_of_node(connector),
> if (IS_ERR(hd3ss3220->vbus))
>    hd3ss3220->vbus = NULL;
> 
> If there is a vbus supply I see its returning proper handle pointer.
> Else it returned ENODEV. (which is fine for our case as there is no vbus 
> in DT).
> 
> Can I mark the function as a void one. Instead of returning any int 
> value, would it be fine if to just mark vbus as NULL and proceed ?

You can only ignore -ENODEV. Any other error should be propagated back 
and cause an error in probing the hd3ss3220 driver.

> 
> Regards,
> Krishna,


-- 
With best wishes
Dmitry