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 | 79 +++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 3ecc688dda82..970c0ca8f8d4 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,49 @@ 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_get_vbus_supply(struct hd3ss3220 *hd3ss3220)
+{
+ struct device_node *hd3ss3220_node = hd3ss3220->dev->of_node;
+ struct device_node *np;
+ int ret = 0;
+
+ 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 ret;
+}
+
static int hd3ss3220_probe(struct i2c_client *client)
{
struct typec_capability typec_cap = { };
@@ -354,6 +405,34 @@ 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;
+ }
+ }
+
+ ret = hd3ss3220_get_vbus_supply(hd3ss3220);
+ if (ret)
+ 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.1Le 25/10/2025 à 14:28, Krishna Kurapati a écrit :
> 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-5oFBVzJwu8Ry9aJCnZT0Uw@public.gmane.org>
> ---
> drivers/usb/typec/hd3ss3220.c | 79 +++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index 3ecc688dda82..970c0ca8f8d4 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,49 @@ 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))
I don't think it can be ERR. hd3ss3220_get_vbus_supply() forces it to
NULL in such a case.
> + 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;
> + int ret = 0;
> +
> + 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 ret;
return 0 and avoid 'ret'?
> +}
> +
> static int hd3ss3220_probe(struct i2c_client *client)
> {
> struct typec_capability typec_cap = { };
> @@ -354,6 +405,34 @@ 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");
Maybe return dev_err_probe() to log the error and simplify code?
> + 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");
Maybe return dev_err_probe() to log the error and simplify code?
Above, you use "ID IRQ" and here "id irq". Maybe keep the case case? Or
change the 2nd message that looks a copy'n'paste error to me.
> + return ret;
> + }
> + }
> +
> + ret = hd3ss3220_get_vbus_supply(hd3ss3220);
> + if (ret)
> + return dev_err_probe(hd3ss3220->dev,
> + PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
Why PTR_ERR(hd3ss3220->vbus)? Should this be 'ret'?
If hd3ss3220_get_vbus_supply() fails, vbus will be NULL in all cases.
In hd3ss3220_get_vbus_supply(), if -ENODEV is returned, it is not
initialized yet, and if of_regulator_get_optional() fails, it is set to
NULL.
> +
> if (IS_ERR(hd3ss3220->role_sw)) {
> ret = PTR_ERR(hd3ss3220->role_sw);
> goto err_put_fwnode;
CJ
On 10/25/2025 7:44 PM, Christophe JAILLET wrote:
> Le 25/10/2025 à 14:28, Krishna Kurapati a écrit :
>> 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.
>>
[...]
>> +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))
>
> I don't think it can be ERR. hd3ss3220_get_vbus_supply() forces it to
> NULL in such a case.
>
ACK. Will make it (!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;
>> + int ret = 0;
>> +
>> + 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 ret;
>
> return 0 and avoid 'ret'?
ACK.
>
>> +}
>> +
>> static int hd3ss3220_probe(struct i2c_client *client)
>> {
>> struct typec_capability typec_cap = { };
>> @@ -354,6 +405,34 @@ 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");
>
> Maybe return dev_err_probe() to log the error and simplify code?
>
ACK.
>> + return hd3
ss3220->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");
>
> Maybe return dev_err_probe() to log the error and simplify code?
>
> Above, you use "ID IRQ" and here "id irq". Maybe keep the case case? Or
> change the 2nd message that looks a copy'n'paste error to me.
>
ACK. Will correct it.
>> + return ret;
>> + }
>> + }
>> +
>> + ret = hd3ss3220_get_vbus_supply(hd3ss3220);
>> + if (ret)
>> + return dev_err_probe(hd3ss3220->dev,
>> + PTR_ERR(hd3ss3220->vbus), "failed to get vbus\n");
>
> Why PTR_ERR(hd3ss3220->vbus)? Should this be 'ret'?
>
> If hd3ss3220_get_vbus_supply() fails, vbus will be NULL in all cases.
>
> In hd3ss3220_get_vbus_supply(), if -ENODEV is returned, it is not
> initialized yet, and if of_regulator_get_optional() fails, it is set to
> NULL.
>
Yes, ret is better. Will change it accordingly.
Regards,
Krishna,
© 2016 - 2026 Red Hat, Inc.