Re: [PATCH v9 3/4] iio: accel: adxl345: add FIFO with watermark events

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 28 Dec 2024 23:29:48 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add a basic setup for FIFO with configurable watermark. Add a handler
> for watermark interrupt events and extend the channel for the
> scan_index needed for the iio channel. The sensor is configurable to use
> a FIFO_BYPASSED mode or a FIFO_STREAM mode. For the FIFO_STREAM mode now
> a watermark can be configured, or disabled by setting 0. Further features
> require a working FIFO setup.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Hi Lothar,

Applied with a tweak. See below and please check my testing branch
to see if I messed this mechanical change up (wouldn't be the first time!)

Thanks,

Jonathan

>  
> +#define ADXL345_FIFO_CTL_SAMPLES(x)	FIELD_PREP(GENMASK(4, 0), x)
> +/* 0: INT1, 1: INT2 */
> +#define ADXL345_FIFO_CTL_TRIGGER(x)	FIELD_PREP(BIT(5), x)
> +#define ADXL345_FIFO_CTL_MODE(x)	FIELD_PREP(GENMASK(7, 6), x)

Ah.  I now realize I subtly misread your reply to v8.

What I want to see the masks defined with the rest of the fields and
the FIELD_PREP used with those masks inline.

Rather than go around again, I've applied the following tweak.
diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index b78b4973a4d4..9fcf6756768e 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -25,6 +25,10 @@
 	(ADXL345_REG_XYZ_BASE + (index) * sizeof(__le16))
 
 #define ADXL345_REG_FIFO_CTL		0x38
+#define ADXL345_FIFO_CTL_SAMPLES_MSK	GENMASK(4, 0)
+/* 0: INT1, 1: INT2 */
+#define ADXL345_FIFO_CTL_TRIGGER_MSK	BIT(5)
+#define ADXL345_FIFO_CTL_MODE_MSK	GENMASK(7, 6)
 #define ADXL345_REG_FIFO_STATUS	0x39
 #define ADXL345_REG_FIFO_STATUS_MSK	0x3F
 
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index d33e3c6528a9..d1b2d3985a40 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -31,11 +31,6 @@
 #define ADXL345_INT1			0
 #define ADXL345_INT2			1
 
-#define ADXL345_FIFO_CTL_SAMPLES(x)	FIELD_PREP(GENMASK(4, 0), x)
-/* 0: INT1, 1: INT2 */
-#define ADXL345_FIFO_CTL_TRIGGER(x)	FIELD_PREP(BIT(5), x)
-#define ADXL345_FIFO_CTL_MODE(x)	FIELD_PREP(GENMASK(7, 6), x)
-
 struct adxl345_state {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
@@ -269,9 +264,12 @@ static int adxl345_set_fifo(struct adxl345_state *st)
 		return ret;
 
 	ret = regmap_write(st->regmap, ADXL345_REG_FIFO_CTL,
-			   ADXL345_FIFO_CTL_SAMPLES(st->watermark) |
-			   ADXL345_FIFO_CTL_TRIGGER(st->intio) |
-			   ADXL345_FIFO_CTL_MODE(st->fifo_mode));
+			   FIELD_PREP(ADXL345_FIFO_CTL_SAMPLES_MSK,
+				      st->watermark) |
+			   FIELD_PREP(ADXL345_FIFO_CTL_TRIGGER_MSK,
+				      st->intio) |
+			   FIELD_PREP(ADXL345_FIFO_CTL_MODE_MSK,
+				      st->fifo_mode));
 	if (ret < 0)
 		return ret;
 
@@ -500,7 +498,6 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 					 ADXL345_DATA_FORMAT_JUSTIFY |
 					 ADXL345_DATA_FORMAT_FULL_RES |
 					 ADXL345_DATA_FORMAT_SELF_TEST);
-	u8 fifo_ctl;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
@@ -583,9 +580,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 		if (ret)
 			return ret;
 	} else {
-		/* FIFO_BYPASS mode */
-		fifo_ctl = ADXL345_FIFO_CTL_MODE(ADXL345_FIFO_BYPASS);
-		ret = regmap_write(st->regmap, ADXL345_REG_FIFO_CTL, fifo_ctl);
+		ret = regmap_write(st->regmap, ADXL345_REG_FIFO_CTL,
+				   FIELD_PREP(ADXL345_FIFO_CTL_MODE_MSK,
+					      ADXL345_FIFO_BYPASS));
 		if (ret < 0)
 			return ret;
 	}