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: <25ef8eef-a4e6-436b-87cd-c77506b4b215@roeck-us.net>
Date: Wed, 24 Sep 2025 09:06:15 -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 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.

Guenter

> +	regulator_disable(drvdata->regulator);
> +
>  	if (ret)
>  		return ret;
>  
> @@ -66,23 +78,24 @@ static const struct hwmon_chip_info sy7636a_chip_info = {
>  static int sy7636a_sensor_probe(struct platform_device *pdev)
>  {
>  	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	struct regulator *regulator;
> +	struct sy7636a_hwmon_data *drvdata;
>  	struct device *hwmon_dev;
>  	int err;
>  
>  	if (!regmap)
>  		return -EPROBE_DEFER;
>  
> -	regulator = devm_regulator_get_optional(&pdev->dev, "vcom");
> -	if (IS_ERR_OR_NULL(regulator))
> -		return -EPROBE_DEFER;
> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
>  
> -	err = regulator_enable(regulator);
> -	if (err)
> -		return err;
> +	drvdata->regmap = regmap;
> +	drvdata->regulator = devm_regulator_get_optional(&pdev->dev, "vcom");
> +	if (IS_ERR_OR_NULL(drvdata->regulator))
> +		return -EPROBE_DEFER;
>  
>  	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> -							 "sy7636a_temperature", regmap,
> +							 "sy7636a_temperature", drvdata,
>  							 &sy7636a_chip_info, NULL);
>  
>  	if (IS_ERR(hwmon_dev)) {
> @@ -102,5 +115,6 @@ static struct platform_driver sy7636a_sensor_driver = {
>  };
>  module_platform_driver(sy7636a_sensor_driver);
>  
> +MODULE_ALIAS("platform:sy7636a-temperature");
>  MODULE_DESCRIPTION("SY7636A sensor driver");
>  MODULE_LICENSE("GPL");