Re: [PATCH] iio: light: veml6040: add suspend/resume support

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 13 May 2026 14:45:36 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> The VEML6040 RGBW light sensor stays in auto-measurement mode at all
> times once probed, drawing its full active current (~200 uA per
> Vishay VEML6040 datasheet, Doc# 84276 Rev. 1.7). On system suspend
> there is no need to keep the sensor running.
> 
> Add system sleep PM callbacks that toggle the SD (shutdown) bit in
> CONF register 00H to put the chip into shutdown on suspend and back
> into normal operation on resume. The bit semantics are documented in
> the datasheet Tables 2-1 and 2-2.
> 
> The existing veml6040_shutdown_action() is unchanged; it still runs
> on module unload to leave the chip shut down.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Hi Stepan

Suspend / resume tend to be a little non trivial to add and datasheets
are sometimes less than perfect in describing powerdown modes, so can
I confirm: Do you have one of these that you are testing this with?

Thanks,

Jonathan

> ---
>  drivers/iio/light/veml6040.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/iio/light/veml6040.c b/drivers/iio/light/veml6040.c
> index f563f9f0e..ffd0a2a70 100644
> --- a/drivers/iio/light/veml6040.c
> +++ b/drivers/iio/light/veml6040.c
> @@ -13,6 +13,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/module.h>
> +#include <linux/pm.h>
>  #include <linux/regmap.h>
>  
>  /* VEML6040 Configuration Registers
> @@ -201,6 +202,32 @@ static void veml6040_shutdown_action(void *data)
>  			   VEML6040_CONF_SD_MSK, VEML6040_CONF_SD_MSK);
>  }
>  
> +/*
> + * Per Vishay VEML6040 datasheet (Doc# 84276 Rev. 1.7), Table 2-1 and
> + * Table 2-2, the SD bit in CONF register 00H controls chip shutdown:
> + * SD = 1 disables the color sensor, SD = 0 re-enables it.
> + */
> +static int veml6040_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct veml6040_data *data = iio_priv(indio_dev);
> +
> +	return regmap_update_bits(data->regmap, VEML6040_CONF_REG,
> +				  VEML6040_CONF_SD_MSK, VEML6040_CONF_SD_MSK);
> +}
> +
> +static int veml6040_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));

dev_get_drvdata() rather than going in circles.  It's get of 'implicit'
knowledge that works for i2c sequences like this so most instances of what
you have here got ripped out years ago.  I've never really like the missbalance
and would love to get rid of i2c_set_clientdata() but such is life!


> +	struct veml6040_data *data = iio_priv(indio_dev);
> +
> +	return regmap_update_bits(data->regmap, VEML6040_CONF_REG,
> +				  VEML6040_CONF_SD_MSK, 0);
Andy pointed out regmap_clear_bits() is handy here and set_bits above.

> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(veml6040_pm_ops, veml6040_suspend,
> +				veml6040_resume);
> +
>  static int veml6040_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
> @@ -271,6 +298,7 @@ static struct i2c_driver veml6040_driver = {
>  	.driver = {
>  		.name = "veml6040",
>  		.of_match_table = veml6040_of_match,
> +		.pm = pm_sleep_ptr(&veml6040_pm_ops),
>  	},
>  };
>  module_i2c_driver(veml6040_driver);