On Mon, 8 May 2023 14:01:25 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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@xxxxxxxxx>
> 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
