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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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