Use regmap cache to replace maintaining the member variable intio
for the interrupt mapping state. The interrupt mapping is initialized
when the driver is probed, and it is perfectly cacheable.
The patch will still leave the function set_interrupts(). A follow up
patch takes care of it, when cleaning up the INT enable register
variable.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl345.h | 4 ++
drivers/iio/accel/adxl345_core.c | 63 ++++++++++++++++++++------------
drivers/iio/accel/adxl345_i2c.c | 2 +
drivers/iio/accel/adxl345_spi.c | 2 +
4 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index bc6d634bd85c..a2a81caa292a 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -8,6 +8,8 @@
#ifndef _ADXL345_H_
#define _ADXL345_H_
+#include <linux/regmap.h>
+
#define ADXL345_REG_DEVID 0x00
#define ADXL345_REG_THRESH_TAP 0x1D
#define ADXL345_REG_OFSX 0x1E
@@ -111,6 +113,8 @@
*/
#define ADXL375_USCALE 480000
+bool adxl345_is_volatile_reg(struct device *dev, unsigned int reg);
+
struct adxl345_chip_info {
const char *name;
int uscale;
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 22c5a9c08459..c4cff74a5d10 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -36,7 +36,6 @@ struct adxl345_state {
struct regmap *regmap;
bool fifo_delay; /* delay: delay is needed for SPI */
int irq;
- u8 intio;
u8 int_map;
u8 watermark;
u8 fifo_mode;
@@ -76,6 +75,24 @@ static const unsigned long adxl345_scan_masks[] = {
0
};
+bool adxl345_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ADXL345_REG_DATA_AXIS(0):
+ case ADXL345_REG_DATA_AXIS(1):
+ case ADXL345_REG_DATA_AXIS(2):
+ case ADXL345_REG_DATA_AXIS(3):
+ case ADXL345_REG_DATA_AXIS(4):
+ case ADXL345_REG_DATA_AXIS(5):
+ case ADXL345_REG_FIFO_STATUS:
+ case ADXL345_REG_INT_SOURCE:
+ return true;
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL_NS_GPL(adxl345_is_volatile_reg, IIO_ADXL345);
+
/**
* adxl345_set_measure_en() - Enable and disable measuring.
*
@@ -98,22 +115,7 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
static int adxl345_set_interrupts(struct adxl345_state *st)
{
- int ret;
- unsigned int int_enable = st->int_map;
- unsigned int int_map;
-
- /*
- * Any bits set to 0 in the INT map register send their respective
- * interrupts to the INT1 pin, whereas bits set to 1 send their respective
- * interrupts to the INT2 pin. The intio shall convert this accordingly.
- */
- int_map = st->intio ? st->int_map : ~st->int_map;
-
- ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
- if (ret)
- return ret;
-
- return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, int_enable);
+ return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
}
static int adxl345_read_raw(struct iio_dev *indio_dev,
@@ -265,6 +267,7 @@ static const struct attribute_group adxl345_attrs_group = {
static int adxl345_set_fifo(struct adxl345_state *st)
{
+ unsigned int intio;
int ret;
/* FIFO should only be configured while in standby mode */
@@ -272,11 +275,14 @@ static int adxl345_set_fifo(struct adxl345_state *st)
if (ret < 0)
return ret;
+ ret = regmap_read(st->regmap, ADXL345_REG_INT_MAP, &intio);
+ if (ret)
+ return ret;
+
ret = regmap_write(st->regmap, ADXL345_REG_FIFO_CTL,
FIELD_PREP(ADXL345_FIFO_CTL_SAMPLES_MSK,
st->watermark) |
- FIELD_PREP(ADXL345_FIFO_CTL_TRIGGER_MSK,
- st->intio) |
+ FIELD_PREP(ADXL345_FIFO_CTL_TRIGGER_MSK, intio) |
FIELD_PREP(ADXL345_FIFO_CTL_MODE_MSK,
st->fifo_mode));
if (ret < 0)
@@ -492,6 +498,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
struct adxl345_state *st;
struct iio_dev *indio_dev;
u32 regval;
+ u8 intio = ADXL345_INT1;
unsigned int data_format_mask = (ADXL345_DATA_FORMAT_RANGE |
ADXL345_DATA_FORMAT_JUSTIFY |
ADXL345_DATA_FORMAT_FULL_RES |
@@ -556,16 +563,26 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
if (ret < 0)
return ret;
- st->intio = ADXL345_INT1;
st->irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
if (st->irq < 0) {
- st->intio = ADXL345_INT2;
+ intio = ADXL345_INT2;
st->irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
if (st->irq < 0)
- st->intio = ADXL345_INT_NONE;
+ intio = ADXL345_INT_NONE;
}
- if (st->intio != ADXL345_INT_NONE) {
+ if (intio != ADXL345_INT_NONE) {
+ /*
+ * Any bits set to 0 in the INT map register send their respective
+ * interrupts to the INT1 pin, whereas bits set to 1 send their respective
+ * interrupts to the INT2 pin. The intio shall convert this accordingly.
+ */
+ regval = intio ? 0xff : 0;
+
+ ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, regval);
+ if (ret)
+ return ret;
+
/* FIFO_STREAM mode is going to be activated later */
ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &adxl345_buffer_ops);
if (ret)
diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
index eb3e0aadf51d..6ce567fd3ba6 100644
--- a/drivers/iio/accel/adxl345_i2c.c
+++ b/drivers/iio/accel/adxl345_i2c.c
@@ -17,6 +17,8 @@
static const struct regmap_config adxl345_i2c_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
+ .volatile_reg = adxl345_is_volatile_reg,
+ .cache_type = REGCACHE_MAPLE,
};
static int adxl345_i2c_probe(struct i2c_client *client)
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index e03915ece8b6..200bdf975518 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -19,6 +19,8 @@ static const struct regmap_config adxl345_spi_regmap_config = {
.val_bits = 8,
/* Setting bits 7 and 6 enables multiple-byte read */
.read_flag_mask = BIT(7) | BIT(6),
+ .volatile_reg = adxl345_is_volatile_reg,
+ .cache_type = REGCACHE_MAPLE,
};
static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
--
2.39.5On Thu, 20 Feb 2025 10:42:23 +0000 Lothar Rubusch <l.rubusch@gmail.com> wrote: > Use regmap cache to replace maintaining the member variable intio > for the interrupt mapping state. The interrupt mapping is initialized > when the driver is probed, and it is perfectly cacheable. > > The patch will still leave the function set_interrupts(). A follow up > patch takes care of it, when cleaning up the INT enable register > variable. > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com> > --- > drivers/iio/accel/adxl345.h | 4 ++ > drivers/iio/accel/adxl345_core.c | 63 ++++++++++++++++++++------------ > drivers/iio/accel/adxl345_i2c.c | 2 + > drivers/iio/accel/adxl345_spi.c | 2 + > 4 files changed, 48 insertions(+), 23 deletions(-) > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > index bc6d634bd85c..a2a81caa292a 100644 > --- a/drivers/iio/accel/adxl345.h > +++ b/drivers/iio/accel/adxl345.h > @@ -8,6 +8,8 @@ > #ifndef _ADXL345_H_ > #define _ADXL345_H_ > > +#include <linux/regmap.h> Why add this include? The file should have a forwards def of struct regmap; which is currently missing. If you clean that up in this patch that is fine (mention it in the patch description though as it isn't directly related) but I don't see a reason to include regmap.h here. Given rest if fine I'll tweak this whilst applying. Applied to the togreg branch of iio.git, pushed out for now as testing for 0-day to poke at it. Also move to a newer kernel tree. The changes in export symbol should be causing you build errors for this path. I'll fix that up. Quotes now needed around IIO_ADXL345 in the EXPORT_SYMBOL_NS_GPL() calls. I fixed that up. Jonathan
On Sun, Mar 2, 2025 at 12:45 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Thu, 20 Feb 2025 10:42:23 +0000 > Lothar Rubusch <l.rubusch@gmail.com> wrote: > > > Use regmap cache to replace maintaining the member variable intio > > for the interrupt mapping state. The interrupt mapping is initialized > > when the driver is probed, and it is perfectly cacheable. > > > > The patch will still leave the function set_interrupts(). A follow up > > patch takes care of it, when cleaning up the INT enable register > > variable. > > > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com> > > --- > > drivers/iio/accel/adxl345.h | 4 ++ > > drivers/iio/accel/adxl345_core.c | 63 ++++++++++++++++++++------------ > > drivers/iio/accel/adxl345_i2c.c | 2 + > > drivers/iio/accel/adxl345_spi.c | 2 + > > 4 files changed, 48 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > > index bc6d634bd85c..a2a81caa292a 100644 > > --- a/drivers/iio/accel/adxl345.h > > +++ b/drivers/iio/accel/adxl345.h > > @@ -8,6 +8,8 @@ > > #ifndef _ADXL345_H_ > > #define _ADXL345_H_ > > > > +#include <linux/regmap.h> > > Why add this include? > > The file should have a forwards def of > struct regmap; Sure. > which is currently missing. If you clean that up in this patch that > is fine (mention it in the patch description though as it isn't > directly related) but I don't see a reason to include regmap.h here. > > Given rest if fine I'll tweak this whilst applying. Applied to the > togreg branch of iio.git, pushed out for now as testing for 0-day > to poke at it. > > Also move to a newer kernel tree. The changes in export symbol > should be causing you build errors for this path. I'll fix that up. > Quotes now needed around IIO_ADXL345 in the EXPORT_SYMBOL_NS_GPL() > calls. I fixed that up. Yes, this gives/gave errors. I'm sorry, I left this in the patch. FYI, I'm always patching against recent kernel source of your "testing" branch of the linux iio repo. Anyway, I'm testing/verifying on a semi-automized setup here, involving the particular sensor hardware connected to an RPI. Although the self-compiled pi-kernel is +/- recent, it usually needs additional tweaks. My pi-kernel still uses unquoted symbols here, which I already cover by an auxiliary patch (not supposed to go upstream). I missed to update this patch here for the added function. Thank you for the hint. Anyway probably a good idea to update and rebuild my setup pi-kernel. TY > > Jonathan
On Sun, 2 Mar 2025 11:45:03 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Thu, 20 Feb 2025 10:42:23 +0000 > Lothar Rubusch <l.rubusch@gmail.com> wrote: > > > Use regmap cache to replace maintaining the member variable intio > > for the interrupt mapping state. The interrupt mapping is initialized > > when the driver is probed, and it is perfectly cacheable. > > > > The patch will still leave the function set_interrupts(). A follow up > > patch takes care of it, when cleaning up the INT enable register > > variable. > > > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com> > > --- > > drivers/iio/accel/adxl345.h | 4 ++ > > drivers/iio/accel/adxl345_core.c | 63 ++++++++++++++++++++------------ > > drivers/iio/accel/adxl345_i2c.c | 2 + > > drivers/iio/accel/adxl345_spi.c | 2 + > > 4 files changed, 48 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > > index bc6d634bd85c..a2a81caa292a 100644 > > --- a/drivers/iio/accel/adxl345.h > > +++ b/drivers/iio/accel/adxl345.h > > @@ -8,6 +8,8 @@ > > #ifndef _ADXL345_H_ > > #define _ADXL345_H_ > > > > +#include <linux/regmap.h> > > Why add this include? > > The file should have a forwards def of > struct regmap; > which is currently missing. If you clean that up in this patch that > is fine (mention it in the patch description though as it isn't > directly related) but I don't see a reason to include regmap.h here. > > Given rest if fine I'll tweak this whilst applying. Applied to the > togreg branch of iio.git, pushed out for now as testing for 0-day > to poke at it. > > Also move to a newer kernel tree. The changes in export symbol > should be causing you build errors for this path. I'll fix that up. > Quotes now needed around IIO_ADXL345 in the EXPORT_SYMBOL_NS_GPL() > calls. I fixed that up. Dropped again after reviewing later patch. I think the volatile stuff should specify all registers that are volatile from the start not add them as we go. > > Jonathan
© 2016 - 2025 Red Hat, Inc.