devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Implement vbus support for HD3SS3220 port controller
@ 2025-10-02 17:25 Krishna Kurapati
  2025-10-02 17:25 ` [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state Krishna Kurapati
  2025-10-02 17:25 ` [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state Krishna Kurapati
  0 siblings, 2 replies; 8+ messages in thread
From: Krishna Kurapati @ 2025-10-02 17:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heikki Krogerus, Liam Girdwood, Mark Brown,
	Biju Das
  Cc: linux-usb, devicetree, linux-kernel, 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.

Krishna Kurapati (3):
  dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
  usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  arm64: dts: qcom: lemans-evk: Add OTG support for primary USB
    controller

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 | 13 +++++
 drivers/usb/typec/hd3ss3220.c                 | 58 +++++++++++++++++++
 2 files changed, 71 insertions(+)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
  2025-10-02 17:25 [PATCH 0/2] Implement vbus support for HD3SS3220 port controller Krishna Kurapati
@ 2025-10-02 17:25 ` Krishna Kurapati
  2025-10-08 20:53   ` Conor Dooley
  2025-10-02 17:25 ` [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state Krishna Kurapati
  1 sibling, 1 reply; 8+ messages in thread
From: Krishna Kurapati @ 2025-10-02 17:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heikki Krogerus, Liam Girdwood, Mark Brown,
	Biju Das
  Cc: linux-usb, devicetree, linux-kernel, 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 and vbus-
supply property to describe the regulator for USB VBUS.

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

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
index bec1c8047bc0..c869eece39a7 100644
--- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
@@ -25,6 +25,19 @@ 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
+
+  vbus-supply:
+    description: A phandle to the regulator for USB VBUS if needed when host
+      mode or dual role mode is supported.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
     description: OF graph bindings (specified in bindings/graph.txt) that model
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  2025-10-02 17:25 [PATCH 0/2] Implement vbus support for HD3SS3220 port controller Krishna Kurapati
  2025-10-02 17:25 ` [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state Krishna Kurapati
@ 2025-10-02 17:25 ` Krishna Kurapati
  2025-10-03  5:56   ` kernel test robot
  2025-10-08 11:09   ` Heikki Krogerus
  1 sibling, 2 replies; 8+ messages in thread
From: Krishna Kurapati @ 2025-10-02 17:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heikki Krogerus, Liam Girdwood, Mark Brown,
	Biju Das
  Cc: linux-usb, devicetree, linux-kernel, Krishna Kurapati

Enable VBUS on HD3SS3220 when the ID pin is low, as required by the Type-C
specification. The ID pin stays high when VBUS is not at VSafe0V, and goes
low when VBUS is at VSafe0V.

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 | 58 +++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 3ecc688dda82..44ee0be27644 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -54,6 +54,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 +324,28 @@ 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 (IS_ERR_OR_NULL(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_probe(struct i2c_client *client)
 {
 	struct typec_capability typec_cap = { };
@@ -354,6 +381,37 @@ 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) {
+			dev_err(hd3ss3220->dev, "failed to get ID IRQ\n");
+			return hd3ss3220->id_irq;
+		}
+
+		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) {
+			dev_err(hd3ss3220->dev, "failed to get id irq\n");
+			return ret;
+		}
+	}
+
+	hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");
+	if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
+		hd3ss3220->vbus = NULL;
+
+	if (IS_ERR(hd3ss3220->vbus))
+		return dev_err_probe(hd3ss3220->dev,
+				     PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
+
 	if (IS_ERR(hd3ss3220->role_sw)) {
 		ret = PTR_ERR(hd3ss3220->role_sw);
 		goto err_put_fwnode;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  2025-10-02 17:25 ` [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state Krishna Kurapati
@ 2025-10-03  5:56   ` kernel test robot
  2025-10-08 11:09   ` Heikki Krogerus
  1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-10-03  5:56 UTC (permalink / raw)
  To: Krishna Kurapati, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus, Liam Girdwood,
	Mark Brown, Biju Das
  Cc: llvm, oe-kbuild-all, linux-usb, devicetree, linux-kernel,
	Krishna Kurapati

Hi Krishna,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next krzk-dt/for-next linus/master v6.17 next-20251002]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Krishna-Kurapati/dt-bindings-usb-ti-hd3ss3220-Add-support-for-VBUS-based-on-ID-state/20251003-012933
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20251002172539.586538-3-krishna.kurapati%40oss.qualcomm.com
patch subject: [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
config: x86_64-buildonly-randconfig-001-20251003 (https://download.01.org/0day-ci/archive/20251003/202510031333.zZYEFOH0-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510031333.zZYEFOH0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510031333.zZYEFOH0-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/typec/hd3ss3220.c:336:29: error: call to undeclared function 'gpiod_get_value_cansleep'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     336 |         id = hd3ss3220->id_gpiod ? gpiod_get_value_cansleep(hd3ss3220->id_gpiod) : 1;
         |                                    ^
>> drivers/usb/typec/hd3ss3220.c:384:24: error: call to undeclared function 'devm_gpiod_get_optional'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     384 |         hd3ss3220->id_gpiod = devm_gpiod_get_optional(hd3ss3220->dev, "id", GPIOD_IN);
         |                               ^
   drivers/usb/typec/hd3ss3220.c:384:24: note: did you mean 'devm_regulator_get_optional'?
   include/linux/regulator/consumer.h:163:32: note: 'devm_regulator_get_optional' declared here
     163 | struct regulator *__must_check devm_regulator_get_optional(struct device *dev,
         |                                ^
>> drivers/usb/typec/hd3ss3220.c:384:70: error: use of undeclared identifier 'GPIOD_IN'
     384 |         hd3ss3220->id_gpiod = devm_gpiod_get_optional(hd3ss3220->dev, "id", GPIOD_IN);
         |                                                                             ^
>> drivers/usb/typec/hd3ss3220.c:389:23: error: call to undeclared function 'gpiod_to_irq'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     389 |                 hd3ss3220->id_irq = gpiod_to_irq(hd3ss3220->id_gpiod);
         |                                     ^
   4 errors generated.


vim +/gpiod_get_value_cansleep +336 drivers/usb/typec/hd3ss3220.c

   326	
   327	static irqreturn_t hd3ss3220_id_isr(int irq, void *dev_id)
   328	{
   329		struct hd3ss3220 *hd3ss3220 = dev_id;
   330		int ret;
   331		int id;
   332	
   333		if (IS_ERR_OR_NULL(hd3ss3220->vbus))
   334			return IRQ_HANDLED;
   335	
 > 336		id = hd3ss3220->id_gpiod ? gpiod_get_value_cansleep(hd3ss3220->id_gpiod) : 1;
   337	
   338		if (!id) {
   339			ret = regulator_enable(hd3ss3220->vbus);
   340			if (ret)
   341				dev_err(hd3ss3220->dev, "enable vbus regulator failed\n");
   342		} else {
   343			regulator_disable(hd3ss3220->vbus);
   344		}
   345	
   346		return IRQ_HANDLED;
   347	}
   348	
   349	static int hd3ss3220_probe(struct i2c_client *client)
   350	{
   351		struct typec_capability typec_cap = { };
   352		struct hd3ss3220 *hd3ss3220;
   353		struct fwnode_handle *connector, *ep;
   354		int ret;
   355		unsigned int data;
   356	
   357		hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
   358					 GFP_KERNEL);
   359		if (!hd3ss3220)
   360			return -ENOMEM;
   361	
   362		i2c_set_clientdata(client, hd3ss3220);
   363	
   364		hd3ss3220->dev = &client->dev;
   365		hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
   366		if (IS_ERR(hd3ss3220->regmap))
   367			return PTR_ERR(hd3ss3220->regmap);
   368	
   369		/* For backward compatibility check the connector child node first */
   370		connector = device_get_named_child_node(hd3ss3220->dev, "connector");
   371		if (connector) {
   372			hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
   373		} else {
   374			ep = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev), NULL);
   375			if (!ep)
   376				return -ENODEV;
   377			connector = fwnode_graph_get_remote_port_parent(ep);
   378			fwnode_handle_put(ep);
   379			if (!connector)
   380				return -ENODEV;
   381			hd3ss3220->role_sw = usb_role_switch_get(hd3ss3220->dev);
   382		}
   383	
 > 384		hd3ss3220->id_gpiod = devm_gpiod_get_optional(hd3ss3220->dev, "id", GPIOD_IN);
   385		if (IS_ERR(hd3ss3220->id_gpiod))
   386			return PTR_ERR(hd3ss3220->id_gpiod);
   387	
   388		if (hd3ss3220->id_gpiod) {
 > 389			hd3ss3220->id_irq = gpiod_to_irq(hd3ss3220->id_gpiod);
   390			if (hd3ss3220->id_irq < 0) {
   391				dev_err(hd3ss3220->dev, "failed to get ID IRQ\n");
   392				return hd3ss3220->id_irq;
   393			}
   394	
   395			ret = devm_request_threaded_irq(hd3ss3220->dev,
   396							hd3ss3220->id_irq, NULL,
   397							hd3ss3220_id_isr,
   398							IRQF_TRIGGER_RISING |
   399							IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
   400							dev_name(hd3ss3220->dev), hd3ss3220);
   401			if (ret < 0) {
   402				dev_err(hd3ss3220->dev, "failed to get id irq\n");
   403				return ret;
   404			}
   405		}
   406	
   407		hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");
   408		if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
   409			hd3ss3220->vbus = NULL;
   410	
   411		if (IS_ERR(hd3ss3220->vbus))
   412			return dev_err_probe(hd3ss3220->dev,
   413					     PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
   414	
   415		if (IS_ERR(hd3ss3220->role_sw)) {
   416			ret = PTR_ERR(hd3ss3220->role_sw);
   417			goto err_put_fwnode;
   418		}
   419	
   420		typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
   421		typec_cap.driver_data = hd3ss3220;
   422		typec_cap.type = TYPEC_PORT_DRP;
   423		typec_cap.data = TYPEC_PORT_DRD;
   424		typec_cap.ops = &hd3ss3220_ops;
   425		typec_cap.fwnode = connector;
   426	
   427		ret = hd3ss3220_configure_source_pref(hd3ss3220, connector, &typec_cap);
   428		if (ret < 0)
   429			goto err_put_role;
   430	
   431		ret = hd3ss3220_configure_port_type(hd3ss3220, connector, &typec_cap);
   432		if (ret < 0)
   433			goto err_put_role;
   434	
   435		hd3ss3220->port = typec_register_port(&client->dev, &typec_cap);
   436		if (IS_ERR(hd3ss3220->port)) {
   437			ret = PTR_ERR(hd3ss3220->port);
   438			goto err_put_role;
   439		}
   440	
   441		ret = hd3ss3220_configure_power_opmode(hd3ss3220, connector);
   442		if (ret < 0)
   443			goto err_unreg_port;
   444	
   445		hd3ss3220_set_role(hd3ss3220);
   446		ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
   447		if (ret < 0)
   448			goto err_unreg_port;
   449	
   450		if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
   451			ret = regmap_write(hd3ss3220->regmap,
   452					HD3SS3220_REG_CN_STAT_CTRL,
   453					data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
   454			if (ret < 0)
   455				goto err_unreg_port;
   456		}
   457	
   458		if (client->irq > 0) {
   459			ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
   460						hd3ss3220_irq_handler,
   461						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
   462						"hd3ss3220", &client->dev);
   463			if (ret)
   464				goto err_unreg_port;
   465		} else {
   466			INIT_DELAYED_WORK(&hd3ss3220->output_poll_work, output_poll_execute);
   467			hd3ss3220->poll = true;
   468		}
   469	
   470		ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
   471		if (ret < 0)
   472			goto err_unreg_port;
   473	
   474		fwnode_handle_put(connector);
   475	
   476		if (hd3ss3220->poll)
   477			schedule_delayed_work(&hd3ss3220->output_poll_work, HZ);
   478	
   479		dev_info(&client->dev, "probed revision=0x%x\n", ret);
   480	
   481		return 0;
   482	err_unreg_port:
   483		typec_unregister_port(hd3ss3220->port);
   484	err_put_role:
   485		usb_role_switch_put(hd3ss3220->role_sw);
   486	err_put_fwnode:
   487		fwnode_handle_put(connector);
   488	
   489		return ret;
   490	}
   491	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  2025-10-02 17:25 ` [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state Krishna Kurapati
  2025-10-03  5:56   ` kernel test robot
@ 2025-10-08 11:09   ` Heikki Krogerus
  2025-10-08 11:15     ` Krishna Kurapati PSSNV
  1 sibling, 1 reply; 8+ messages in thread
From: Heikki Krogerus @ 2025-10-08 11:09 UTC (permalink / raw)
  To: Krishna Kurapati
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Liam Girdwood, Mark Brown, Biju Das, linux-usb,
	devicetree, linux-kernel

On Thu, Oct 02, 2025 at 10:55:39PM +0530, Krishna Kurapati wrote:
> Enable VBUS on HD3SS3220 when the ID pin is low, as required by the Type-C
> specification.

There is not ID pin on Type-C connector.

> The ID pin stays high when VBUS is not at VSafe0V, and goes
> low when VBUS is at VSafe0V.
> 
> Add support to read the ID pin state and enable VBUS accordingly.

I'm a bit confused about this... Why can't you just check the attached
state, and if it's DFP, then you drive VBUS?

thanks,

> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/typec/hd3ss3220.c | 58 +++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index 3ecc688dda82..44ee0be27644 100644
> --- a/drivers/usb/typec/hd3ss3220.c
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -54,6 +54,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 +324,28 @@ 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 (IS_ERR_OR_NULL(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_probe(struct i2c_client *client)
>  {
>  	struct typec_capability typec_cap = { };
> @@ -354,6 +381,37 @@ 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) {
> +			dev_err(hd3ss3220->dev, "failed to get ID IRQ\n");
> +			return hd3ss3220->id_irq;
> +		}
> +
> +		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) {
> +			dev_err(hd3ss3220->dev, "failed to get id irq\n");
> +			return ret;
> +		}
> +	}
> +
> +	hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");
> +	if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
> +		hd3ss3220->vbus = NULL;
> +
> +	if (IS_ERR(hd3ss3220->vbus))
> +		return dev_err_probe(hd3ss3220->dev,
> +				     PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
> +
>  	if (IS_ERR(hd3ss3220->role_sw)) {
>  		ret = PTR_ERR(hd3ss3220->role_sw);
>  		goto err_put_fwnode;
> -- 
> 2.34.1

-- 
heikki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  2025-10-08 11:09   ` Heikki Krogerus
@ 2025-10-08 11:15     ` Krishna Kurapati PSSNV
  2025-10-08 13:30       ` Heikki Krogerus
  0 siblings, 1 reply; 8+ messages in thread
From: Krishna Kurapati PSSNV @ 2025-10-08 11:15 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Liam Girdwood, Mark Brown, Biju Das, linux-usb,
	devicetree, linux-kernel



On 10/8/2025 4:39 PM, Heikki Krogerus wrote:
> On Thu, Oct 02, 2025 at 10:55:39PM +0530, Krishna Kurapati wrote:
>> Enable VBUS on HD3SS3220 when the ID pin is low, as required by the Type-C
>> specification.
> 
> There is not ID pin on Type-C connector.

There is an ID pin coming out from HD3SS3220 controller to SoC that is 
being referred to here.

> 
>> The ID pin stays high when VBUS is not at VSafe0V, and goes
>> low when VBUS is at VSafe0V.
>>
>> Add support to read the ID pin state and enable VBUS accordingly.
> 
> I'm a bit confused about this... Why can't you just check the attached
> state, and if it's DFP, then you drive VBUS?
> 

We could, but checking for DFP doesn't ensure VBUS is at VSafe0V as per 
the datasheet. So using the ID pin to enable vbus.

Regards,
Krishna,

> thanks,
> 
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/usb/typec/hd3ss3220.c | 58 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
>> index 3ecc688dda82..44ee0be27644 100644
>> --- a/drivers/usb/typec/hd3ss3220.c
>> +++ b/drivers/usb/typec/hd3ss3220.c
>> @@ -54,6 +54,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 +324,28 @@ 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 (IS_ERR_OR_NULL(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_probe(struct i2c_client *client)
>>   {
>>   	struct typec_capability typec_cap = { };
>> @@ -354,6 +381,37 @@ 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) {
>> +			dev_err(hd3ss3220->dev, "failed to get ID IRQ\n");
>> +			return hd3ss3220->id_irq;
>> +		}
>> +
>> +		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) {
>> +			dev_err(hd3ss3220->dev, "failed to get id irq\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");
>> +	if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
>> +		hd3ss3220->vbus = NULL;
>> +
>> +	if (IS_ERR(hd3ss3220->vbus))
>> +		return dev_err_probe(hd3ss3220->dev,
>> +				     PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
>> +
>>   	if (IS_ERR(hd3ss3220->role_sw)) {
>>   		ret = PTR_ERR(hd3ss3220->role_sw);
>>   		goto err_put_fwnode;
>> -- 
>> 2.34.1
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state
  2025-10-08 11:15     ` Krishna Kurapati PSSNV
@ 2025-10-08 13:30       ` Heikki Krogerus
  0 siblings, 0 replies; 8+ messages in thread
From: Heikki Krogerus @ 2025-10-08 13:30 UTC (permalink / raw)
  To: Krishna Kurapati PSSNV
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Liam Girdwood, Mark Brown, Biju Das, linux-usb,
	devicetree, linux-kernel

On Wed, Oct 08, 2025 at 04:45:06PM +0530, Krishna Kurapati PSSNV wrote:
> On 10/8/2025 4:39 PM, Heikki Krogerus wrote:
> > On Thu, Oct 02, 2025 at 10:55:39PM +0530, Krishna Kurapati wrote:
> > > Enable VBUS on HD3SS3220 when the ID pin is low, as required by the Type-C
> > > specification.
> > 
> > There is not ID pin on Type-C connector.
> 
> There is an ID pin coming out from HD3SS3220 controller to SoC that is being
> referred to here.

So please fix the statement. The Type-C specification does not place
any requirements on the ID pin.

> > > The ID pin stays high when VBUS is not at VSafe0V, and goes
> > > low when VBUS is at VSafe0V.
> > > 
> > > Add support to read the ID pin state and enable VBUS accordingly.
> > 
> > I'm a bit confused about this... Why can't you just check the attached
> > state, and if it's DFP, then you drive VBUS?
> > 
> 
> We could, but checking for DFP doesn't ensure VBUS is at VSafe0V as per the
> datasheet. So using the ID pin to enable vbus.

Fair enough.

thanks,

> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >   drivers/usb/typec/hd3ss3220.c | 58 +++++++++++++++++++++++++++++++++++
> > >   1 file changed, 58 insertions(+)
> > > 
> > > diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> > > index 3ecc688dda82..44ee0be27644 100644
> > > --- a/drivers/usb/typec/hd3ss3220.c
> > > +++ b/drivers/usb/typec/hd3ss3220.c
> > > @@ -54,6 +54,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 +324,28 @@ 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 (IS_ERR_OR_NULL(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_probe(struct i2c_client *client)
> > >   {
> > >   	struct typec_capability typec_cap = { };
> > > @@ -354,6 +381,37 @@ 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) {
> > > +			dev_err(hd3ss3220->dev, "failed to get ID IRQ\n");
> > > +			return hd3ss3220->id_irq;
> > > +		}
> > > +
> > > +		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) {
> > > +			dev_err(hd3ss3220->dev, "failed to get id irq\n");
> > > +			return ret;
> > > +		}
> > > +	}
> > > +
> > > +	hd3ss3220->vbus = devm_regulator_get_optional(hd3ss3220->dev, "vbus");
> > > +	if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
> > > +		hd3ss3220->vbus = NULL;
> > > +
> > > +	if (IS_ERR(hd3ss3220->vbus))
> > > +		return dev_err_probe(hd3ss3220->dev,
> > > +				     PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
> > > +
> > >   	if (IS_ERR(hd3ss3220->role_sw)) {
> > >   		ret = PTR_ERR(hd3ss3220->role_sw);
> > >   		goto err_put_fwnode;
> > > -- 
> > > 2.34.1
> > 

-- 
heikki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state
  2025-10-02 17:25 ` [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state Krishna Kurapati
@ 2025-10-08 20:53   ` Conor Dooley
  0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2025-10-08 20:53 UTC (permalink / raw)
  To: Krishna Kurapati
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Heikki Krogerus, Liam Girdwood, Mark Brown,
	Biju Das, linux-usb, devicetree, linux-kernel

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

On Thu, Oct 02, 2025 at 10:55:38PM +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 and vbus-
> supply property to describe the regulator for USB VBUS.

IDK anything about this class of USB device, but I think the binding
patch is acceptable.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/usb/ti,hd3ss3220.yaml       | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> index bec1c8047bc0..c869eece39a7 100644
> --- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> @@ -25,6 +25,19 @@ 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
> +
> +  vbus-supply:
> +    description: A phandle to the regulator for USB VBUS if needed when host
> +      mode or dual role mode is supported.
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>      description: OF graph bindings (specified in bindings/graph.txt) that model
> -- 
> 2.34.1
> 

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-08 20:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-02 17:25 [PATCH 0/2] Implement vbus support for HD3SS3220 port controller Krishna Kurapati
2025-10-02 17:25 ` [PATCH 1/2] dt-bindings: usb: ti,hd3ss3220: Add support for VBUS based on ID state Krishna Kurapati
2025-10-08 20:53   ` Conor Dooley
2025-10-02 17:25 ` [PATCH 2/2] usb: typec: hd3ss3220: Enable VBUS based on ID pin state Krishna Kurapati
2025-10-03  5:56   ` kernel test robot
2025-10-08 11:09   ` Heikki Krogerus
2025-10-08 11:15     ` Krishna Kurapati PSSNV
2025-10-08 13:30       ` Heikki Krogerus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).