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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aWT1NgxDSaU7LL2g@kuha>
Date: Mon, 12 Jan 2026 15:20:54 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Amit Sunil Dhamne <amitsd@...gle.com>
Cc: Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	André Draszik <andre.draszik@...aro.org>,
	Lee Jones <lee@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Badhri Jagan Sridharan <badhri@...gle.com>,
	Peter Griffin <peter.griffin@...aro.org>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Alim Akhtar <alim.akhtar@...sung.com>, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
	linux-usb@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org,
	RD Babiera <rdbabiera@...gle.com>, Kyle Tso <kyletso@...gle.com>
Subject: Re: [PATCH v3 5/5] usb: typec: tcpm/tcpci_maxim: deprecate WAR for
 setting charger mode

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.

thanks,

-- 
heikki