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] [day] [month] [year] [list]

Message-ID: <20250924235844.79193fd9@kemnade.info>
Date: Wed, 24 Sep 2025 23:58:44 +0200
From: Andreas Kemnade <andreas@...nade.info>
To: Guenter Roeck <linux@...ck-us.net>
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 Wed, 24 Sep 2025 14:06:57 -0700
Guenter Roeck <linux@...ck-us.net> wrote:

> 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.
> 
Yes, that is the mechanism. Well, the regulator can be marked as always
enabled via regulator-aways-on flag in devicetree so that is optional, that
would have the side effect of having temperature conversion always
working, no delay needed. The power good stuff is about the stepup
regulators, so I think (will verify that in experiments) waiting for the
conversation time (around 0.5 ms) after the on bit is set is probably enough.

And I have seen, the power good stuff is not really implemented yet,
had another incarnation of this driver in my head while writing this,
where regulator_enable waits for power good.

Regards,
Andreas