[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2e88f394-6c84-4be7-bc41-b2eb9c5e6c41@google.com>
Date: Wed, 14 Jan 2026 18:58:35 -0800
From: Amit Sunil Dhamne <amitsd@...gle.com>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.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
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,
>
Powered by blists - more mailing lists