Re: [PATCH v3] iio: bu27034: Ensure reset is written

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 8 May 2023 14:01:25 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The reset bit must be always written to the hardware no matter what value
> is in a cache or register. Ensure this by using regmap_write_bits()
> instead of the regmap_update_bits(). Furthermore, the SWRESET bit may be
> self-clearing, so mark the SYSTEM_CONTROL register volatile to guarantee
> we do also read the right state - should we ever need to read it.
> 
> Finally, writing the SWRESET bit will restore the default register
> values. This can cause register cache to be outdated if there are any
> register values cached.
> 
> Rebuild register cache after SWRESET and use regmap_update_bits() when
> performing the reset.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Fixes: e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")

Still unclear to me if we should treat the register as volatile as hitting
relevant bits causes a reset of the whole device anyway, but meh, it doesn't
cause any problems and it's your driver.

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> 
> ---
> v3: Combined patches:
> ("iio: bu27034: Ensure reset is written")
> https://lore.kernel.org/lkml/ZFIw%2FKdApZe1euN8@fedora/
> ("iio: bu27034: Reinit regmap cache after reset")
> https://lore.kernel.org/lkml/ZFM7lE4ZuDrUTspH@fedora/
> ---
>  drivers/iio/light/rohm-bu27034.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
> index 25c9b79574a5..f85194fda6b0 100644
> --- a/drivers/iio/light/rohm-bu27034.c
> +++ b/drivers/iio/light/rohm-bu27034.c
> @@ -231,6 +231,9 @@ struct bu27034_result {
>  
>  static const struct regmap_range bu27034_volatile_ranges[] = {
>  	{
> +		.range_min = BU27034_REG_SYSTEM_CONTROL,
> +		.range_max = BU27034_REG_SYSTEM_CONTROL,
> +	}, {
>  		.range_min = BU27034_REG_MODE_CONTROL4,
>  		.range_max = BU27034_REG_MODE_CONTROL4,
>  	}, {
> @@ -1272,12 +1275,19 @@ static int bu27034_chip_init(struct bu27034_data *data)
>  	int ret, sel;
>  
>  	/* Reset */
> -	ret = regmap_update_bits(data->regmap, BU27034_REG_SYSTEM_CONTROL,
> +	ret = regmap_write_bits(data->regmap, BU27034_REG_SYSTEM_CONTROL,
>  			   BU27034_MASK_SW_RESET, BU27034_MASK_SW_RESET);
>  	if (ret)
>  		return dev_err_probe(data->dev, ret, "Sensor reset failed\n");
>  
>  	msleep(1);
> +
> +	ret = regmap_reinit_cache(data->regmap, &bu27034_regmap);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to reinit reg cache\n");
> +		return ret;
> +	}
> +
>  	/*
>  	 * Read integration time here to ensure it is in regmap cache. We do
>  	 * this to speed-up the int-time acquisition in the start of the buffer