linux-kernel - Re: [PATCH 2/2] hwmon: (sy7636a) enable regulator only if needed

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: <00685721-8f34-4ad2-9452-5447eafb23b8@roeck-us.net>
Date: Wed, 24 Sep 2025 14:06:57 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Andreas Kemnade <andreas@...nade.info>
Cc: jdelvare@...e.com, lgirdwood@...il.com, broonie@...nel.org,
 linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
 Alistair Francis <alistair@...stair23.me>
Subject: Re: [PATCH 2/2] hwmon: (sy7636a) enable regulator only if needed

On 9/24/25 13:40, Andreas Kemnade wrote:
> On Wed, 24 Sep 2025 09:06:15 -0700
> Guenter Roeck <linux@...ck-us.net> wrote:
> 
>> On Sat, Sep 20, 2025 at 01:43:11PM +0200, Andreas Kemnade wrote:
>>> Avoid having all the regulators in the SY7636A enabled all the time
>>> to significantly reduce current consumption. In pratical scenarios,
>>> the regulators are only needed when a refresh is done on the epaper
>>> display powered by the SY7636A. This is can save around 10mA which
>>> is much for this kind of devices.
>>> Also fixes the asymmetrical single enable call.
>>>
>>> Signed-off-by: Andreas Kemnade <andreas@...nade.info>
>>> ---
>>>   drivers/hwmon/sy7636a-hwmon.c | 34 ++++++++++++++++++++++++----------
>>>   1 file changed, 24 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/sy7636a-hwmon.c b/drivers/hwmon/sy7636a-hwmon.c
>>> index e83d0e50c735..0fda69bea3b4 100644
>>> --- a/drivers/hwmon/sy7636a-hwmon.c
>>> +++ b/drivers/hwmon/sy7636a-hwmon.c
>>> @@ -18,14 +18,26 @@
>>>   
>>>   #include <linux/mfd/sy7636a.h>
>>>   
>>> +struct sy7636a_hwmon_data {
>>> +	struct regmap *regmap;
>>> +	struct regulator *regulator;
>>> +};
>>> +
>>> +
>>>   static int sy7636a_read(struct device *dev, enum hwmon_sensor_types type,
>>>   			u32 attr, int channel, long *temp)
>>>   {
>>> -	struct regmap *regmap = dev_get_drvdata(dev);
>>> +	struct sy7636a_hwmon_data *drvdata = dev_get_drvdata(dev);
>>>   	int ret, reg_val;
>>>   
>>> -	ret = regmap_read(regmap,
>>> +	ret = regulator_enable(drvdata->regulator);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = regmap_read(drvdata->regmap,
>>>   			  SY7636A_REG_TERMISTOR_READOUT, &reg_val);
>>
>> Does that really work without delay ? Usually it takes some time for a chip
>> to return useful data after its power supply has been enabled. calls
>>
> Hmm, enabling the onoff bit behind the regulator is just needed to turn
> ADC on here. But there is also the power good wait which should usually be enough.
> Usually... But there is no guarantee. So yes, better wait for one adc aquisition.
> 

Would that be the power good timeout (50ms) ? The conversion time on top if that
is not much, but the power good wait is a long time. That strongly suggests that
this should be made optional. Not everyone might like the additional delay for
each temperature reading.

Guenter