Re: [PATCH 2/2] usb: typec: hd3ss3220: Add support for supply regulators

[Alexey Charkov]

Hi Biju,

On Fri, Aug 21, 2026 at 9:03 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
>
>
> Hi Alexey Charkov,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: Alexey Charkov <alchark@xxxxxxxxxxx>
> > Sent: 21 August 2026 17:17
> > Subject: [PATCH 2/2] usb: typec: hd3ss3220: Add support for supply regulators
> >
> > HD3SS3220 requires VDD5 input to be present 2ms before VCC33 is applied, or else it gets backpowered via
> > the 3.3V rail in a non-functional state and wedges the I2C bus, bringing down all devices on it.
> >
> > Enable both regulators in the datasheet prescribed sequence if provided.
> >
> > Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
> > ---
> >  drivers/usb/typec/hd3ss3220.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c index
> > d0de5a2488f9..b53455abc048 100644
> > --- a/drivers/usb/typec/hd3ss3220.c
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -49,6 +49,9 @@
> >  #define HD3SS3220_REG_GEN_CTRL_MODE_SELECT_UFP               BIT(4)
> >  #define HD3SS3220_REG_GEN_CTRL_MODE_SELECT_DRP               (BIT(5) | BIT(4))
> >
> > +/* Minimum time VDD5 has to be stable before VCC33 starts ramping up */
> > +#define HD3SS3220_TVDD5V_PG_US                               2000
> > +
> >  struct hd3ss3220 {
> >       struct device *dev;
> >       struct regmap *regmap;
> > @@ -358,6 +361,31 @@ static irqreturn_t hd3ss3220_id_isr(int irq, void *dev_id)
> >       return IRQ_HANDLED;
> >  }
> >
> > +/*
> > + * Bring both supplies up in the order the datasheet asks for. Powering
> > +VCC33
> > + * first can back-power the device in a non-functioning state, which
> > +grounds
> > + * the I2C bus and takes both this device and any others on the same
> > +bus down  */ static int hd3ss3220_power_up(struct device *dev) {
> > +     int ret;
> > +
> > +     ret = devm_regulator_get_enable(dev, "vdd5");
>
> Maybe use optional API as existing dt users don't have this property defined in DT.

The optional API is explicitly for devices that can function without
the respective supply entirely, which is not the case here (both lines
have to be wired for the chip to work). If a board doesn't define this
supply in its DT the "normal" API will auto-assign a dummy regulator
and proceed.

I got called out by Mark once [1] trying to shoehorn the _optional
functions where they don't belong :)

[1] https://lore.kernel.org/all/agUoq2N_nE_Sz0Z_@xxxxxxxxxxxx/

> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "failed to enable VDD5\n");
> > +
> > +     /* Nothing to stagger against unless the board describes both rails */
> > +     if (device_property_present(dev, "vdd5-supply") &&
> > +         device_property_present(dev, "vcc33-supply"))
> > +             fsleep(HD3SS3220_TVDD5V_PG_US);
>
> No need for fsleep for the consumers that does not have vdd5 and vcc33.

Correct, hence the conditional.

> > +     ret = devm_regulator_get_enable(dev, "vcc33");
>
> Same here.

Best regards,
Alexey