Re: [PATCH v3 5/5] usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode

[Amit Sunil Dhamne]

On 1/12/26 5:20 AM, Heikki Krogerus wrote:
> Fri, Jan 09, 2026 at 06:16:57PM -0800, Amit Sunil Dhamne kirjoitti:
> > Hi Heikki,
> >
> > Thanks for the review!
> >
> > On 1/9/26 5:14 AM, Heikki Krogerus wrote:
> > > Hi,
> > >
> > > > +	if (source) {
> > > > +		if (!regulator_is_enabled(chip->vbus_reg))
> > > > +			ret = regulator_enable(chip->vbus_reg);
> > > > +	} else {
> > > > +		if (regulator_is_enabled(chip->vbus_reg))
> > > > +			ret = regulator_disable(chip->vbus_reg);
> > > > +	}
> > > It looks like you have to do one more round, so can drop the
> > > regulator_is_enabled() checks and just always enable/disable it
> > > unconditionally.
> > >
> > >          if (source)
> > > 		ret = regulator_enable(chip->vbus_reg);
> > > 	else
> > > 		ret = regulator_disable(chip->vbus_reg);
> > The regulator framework uses refcounting on the number of enables. If
> > the number of times regulator is disabled > enabled, a warning will be
> > thrown. Also, I don't want to call regulator_enable more than once for
> > the same refcounting reason (will have to call disable those many number
> > of times to actually disable).
> >
> > > I don't think you need the check in any case, but if I've understood
> > > this correctly, you should not use that check when the regulator does
> > > not support that check because then the API claims it's always
> > > enabled. So I guess in that case "if (!regulator_is_enabled())" may
> > > not work as expected, and you may actually be left with a disabled
> > > regulator. This may not be a problem on current platforms, but who
> > > knows what happens in the future.
> > I don't think this should be an issue in the future as this driver is
> > specifically meant for max77759_tcpci device and should only be used
> > with max77759 charger (they both exist only in the same package). And
> > that the max77759_charger driver does implement the callback. However,
> > if you think that regulator_is_enabled() is unreliable, I could track
> > the state within the tcpci driver instead of calling
> > regulator_is_enabled() and call enable/disable regulator accordingly.
> >
> > Let me know wdyt and I'll update the next revision accordingly.
> Let's go with this then as is.

Sounds good. Thanks!


> thanks,