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

[Guenter Roeck <linux@roeck-us.net>]

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@kemnade.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");