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 | 72 +++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 3ecc688dda82..75fbda42eaf4 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,44 @@ 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 fwnode_handle *connector)
+{
+ int ret = 0;
+
+ hd3ss3220->vbus = devm_of_regulator_get_optional(hd3ss3220->dev,
+ to_of_node(connector),
+ "vbus");
+ if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
+ hd3ss3220->vbus = NULL;
+ else if (IS_ERR(hd3ss3220->vbus))
+ ret = PTR_ERR(hd3ss3220->vbus);
+
+ return ret;
+}
+
static int hd3ss3220_probe(struct i2c_client *client)
{
struct typec_capability typec_cap = { };
@@ -354,11 +400,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)
+ 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");
+ }
+
if (IS_ERR(hd3ss3220->role_sw)) {
ret = PTR_ERR(hd3ss3220->role_sw);
goto err_put_fwnode;
}
+ ret = hd3ss3220_get_vbus_supply(hd3ss3220, connector);
+ if (ret) {
+ dev_err(hd3ss3220->dev, "failed to get vbus: %d\n", ret);
+ goto err_put_fwnode;
+ }
+
typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
typec_cap.driver_data = hd3ss3220;
typec_cap.type = TYPEC_PORT_DRP;
--
2.34.1Hi Krishna,
Sun, Nov 02, 2025 at 10:18:19PM +0530, Krishna Kurapati kirjoitti:
> 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 | 72 +++++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index 3ecc688dda82..75fbda42eaf4 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,44 @@ 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;
If you don't need this routine unless there is a vbus regulator, then
don't register it at all if there is no vbus regulator.
> + id = hd3ss3220->id_gpiod ? gpiod_get_value_cansleep(hd3ss3220->id_gpiod) : 1;
You still don't need to check for hd3ss3220->id_gpiod - this function
will not get called unless it's there.
if (gpiod_get_value_cansleep(hd3ss3220->id_gpiod))
ret = regulator_disable(hd3ss3220->vbus);
else
ret = regulator_enable(hd3ss3220->vbus);
Note:
If you are concerned that the reference to the id_gpiod may be
released before this routine is unregistered, then that condition will
not help. The hd3ss3220->id_gpiod member is _not_ NULL after the
reference is released.
If you need a specific order in which the references are released,
then you can't use the resource management (devm_*) to automate things
for you.
> + 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 fwnode_handle *connector)
> +{
> + int ret = 0;
> +
> + hd3ss3220->vbus = devm_of_regulator_get_optional(hd3ss3220->dev,
> + to_of_node(connector),
> + "vbus");
> + if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
> + hd3ss3220->vbus = NULL;
> + else if (IS_ERR(hd3ss3220->vbus))
> + ret = PTR_ERR(hd3ss3220->vbus);
So the regulator API's optional functions return -ENODEV instead of NULL :(
In any case, don't double assign the member. Use local variable.
struct regulator *vbus;
vbus = devm_of_regulator_get_optional(...
if (IS_ERR(vbus) && vbus != ERR_PTR(-ENODEV))
return PTR_ERR(vbus);
hd3ss3220->vbus = vbus;
return 0;
I don't think you need this function - just do that in the probe function.
> + return ret;
> +}
> +
> static int hd3ss3220_probe(struct i2c_client *client)
> {
> struct typec_capability typec_cap = { };
> @@ -354,11 +400,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)
> + 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");
> + }
> +
> if (IS_ERR(hd3ss3220->role_sw)) {
> ret = PTR_ERR(hd3ss3220->role_sw);
> goto err_put_fwnode;
> }
>
> + ret = hd3ss3220_get_vbus_supply(hd3ss3220, connector);
> + if (ret) {
> + dev_err(hd3ss3220->dev, "failed to get vbus: %d\n", ret);
> + goto err_put_fwnode;
> + }
> +
> typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> typec_cap.driver_data = hd3ss3220;
> typec_cap.type = TYPEC_PORT_DRP;
> --
> 2.34.1
thanks,
--
heikki
On 11/6/2025 3:35 PM, Heikki Krogerus wrote:
> Hi Krishna,
>
> Sun, Nov 02, 2025 at 10:18:19PM +0530, Krishna Kurapati kirjoitti:
>> 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 | 72 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 72 insertions(+)
>>
>> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
>> index 3ecc688dda82..75fbda42eaf4 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,44 @@ 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;
>
> If you don't need this routine unless there is a vbus regulator, then
> don't register it at all if there is no vbus regulator.
>
Will move vbus check before id retrieval in probe and ignore retrieval
of ID if vbus is absent.
>> + id = hd3ss3220->id_gpiod ? gpiod_get_value_cansleep(hd3ss3220->id_gpiod) : 1;
>
> You still don't need to check for hd3ss3220->id_gpiod - this function
> will not get called unless it's there.
>
> if (gpiod_get_value_cansleep(hd3ss3220->id_gpiod))
> ret = regulator_disable(hd3ss3220->vbus);
> else
> ret = regulator_enable(hd3ss3220->vbus);
>
ACK.
> Note:
>
> If you are concerned that the reference to the id_gpiod may be
> released before this routine is unregistered, then that condition will
> not help. The hd3ss3220->id_gpiod member is _not_ NULL after the
> reference is released.
>
> If you need a specific order in which the references are released,
> then you can't use the resource management (devm_*) to automate things
> for you.
There is no specific order. So the id part I can keep it intact except
for checking presence of ID pin in interrupt handler.
>
>> + 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 fwnode_handle *connector)
>> +{
>> + int ret = 0;
>> +
>> + hd3ss3220->vbus = devm_of_regulator_get_optional(hd3ss3220->dev,
>> + to_of_node(connector),
>> + "vbus");
>> + if (PTR_ERR(hd3ss3220->vbus) == -ENODEV)
>> + hd3ss3220->vbus = NULL;
>> + else if (IS_ERR(hd3ss3220->vbus))
>> + ret = PTR_ERR(hd3ss3220->vbus);
>
> So the regulator API's optional functions return -ENODEV instead of NULL :(
> In any case, don't double assign the member. Use local variable.
>
> struct regulator *vbus;
>
> vbus = devm_of_regulator_get_optional(...
> if (IS_ERR(vbus) && vbus != ERR_PTR(-ENODEV))
> return PTR_ERR(vbus);
>
> hd3ss3220->vbus = vbus;
> return 0;
>
> I don't think you need this function - just do that in the probe function.
>
ACK.
Regards,
Krishna,
> struct regulator *vbus;
>
> vbus = devm_of_regulator_get_optional(...
> if (IS_ERR(vbus) && vbus != ERR_PTR(-ENODEV))
> return PTR_ERR(vbus);
>
> hd3ss3220->vbus = vbus;
Sorry, that has to be:
hd3ss3220->vbus = vbus == ERR_PTR(-ENODEV) ? NULL : vbus;
--
heikki
On 11/6/2025 3:46 PM, Heikki Krogerus wrote: >> struct regulator *vbus; >> >> vbus = devm_of_regulator_get_optional(... >> if (IS_ERR(vbus) && vbus != ERR_PTR(-ENODEV)) >> return PTR_ERR(vbus); >> >> hd3ss3220->vbus = vbus; > > Sorry, that has to be: > > hd3ss3220->vbus = vbus == ERR_PTR(-ENODEV) ? NULL : vbus; > > Thanks for the review Heikki. Will update the suggestions in next revision. Regards, Krishna,
© 2016 - 2026 Red Hat, Inc.