Skip to contentSkip to conversation

iio: bu27034: Ensure reset is written

PATCH Applied Matti Vaittinen 2 participants 2 messages May 8, 2023 – May 13, 2023

Subsystems (2)
IIO ROHM BU270xx light sensor DRIVERs
lore
Share & tools
lore

Matti Vaittinen

author maintainer
#0 lore raw

PATCH v3 1/1 iio: bu27034: Ensure reset is written

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 <[email protected]>
Fixes e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")

---
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(-)
drivers/iio/light/rohm-bu27034.c +11 -1
index 25c9b79574a5..f85194fda6b0 100644
@@ -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
Signature · 14 lines
2.40.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
maintainer Applied
#1 lore raw
Quoted 10 lines · On Mon, 8 May 2023 14:01:25 +0300 Matti Vaittinen <[email protected]>…

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.

Quoted 5 lines

Rebuild register cache after SWRESET and use regmap_update_bits() when performing the reset.

Signed-off-by Matti Vaittinen <[email protected]>
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

Quoted 46 lines

---
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(-)
drivers/iio/light/rohm-bu27034.c +11 -1
index 25c9b79574a5..f85194fda6b0 100644
@@ -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