[PATCH v2 09/14] iio: accel: adxl345: extend sample frequency adjustments

Lothar Rubusch posted 14 patches 12 months ago
There is a newer version of this series
[PATCH v2 09/14] iio: accel: adxl345: extend sample frequency adjustments
Posted by Lothar Rubusch 12 months ago
Introduce enums and functions to work with the sample frequency
adjustments. Let the sample frequency adjust via IIO and configure
a reasonable default.

Replace the old static sample frequency handling. The patch is in
preparation for activity/inactivity handling.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      |   2 +-
 drivers/iio/accel/adxl345_core.c | 163 ++++++++++++++++++++++++-------
 2 files changed, 127 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index bc6d634bd85c..0a549d5898c2 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -69,7 +69,7 @@
  * BW_RATE bits - Bandwidth and output data rate. The default value is
  * 0x0A, which translates to a 100 Hz output data rate
  */
-#define ADXL345_BW_RATE			GENMASK(3, 0)
+#define ADXL345_BW_RATE_MSK		GENMASK(3, 0)
 #define ADXL345_BW_LOW_POWER		BIT(4)
 #define ADXL345_BASE_RATE_NANO_HZ	97656250LL
 
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 56c5a4d85d71..08ad71875c5e 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -67,6 +67,45 @@ static const unsigned int adxl345_tap_time_reg[3] = {
 	[ADXL345_TAP_TIME_DUR] = ADXL345_REG_DUR,
 };
 
+enum adxl345_odr {
+	ADXL345_ODR_0P10HZ = 0,
+	ADXL345_ODR_0P20HZ,
+	ADXL345_ODR_0P39HZ,
+	ADXL345_ODR_0P78HZ,
+	ADXL345_ODR_1P56HZ,
+	ADXL345_ODR_3P13HZ,
+	ADXL345_ODR_6P25HZ,
+	ADXL345_ODR_12P50HZ,
+	ADXL345_ODR_25HZ,
+	ADXL345_ODR_50HZ,
+	ADXL345_ODR_100HZ,
+	ADXL345_ODR_200HZ,
+	ADXL345_ODR_400HZ,
+	ADXL345_ODR_800HZ,
+	ADXL345_ODR_1600HZ,
+	ADXL345_ODR_3200HZ,
+};
+
+/* Certain features recommend 12.5 Hz - 400 Hz ODR */
+static const int adxl345_odr_tbl[][2] = {
+	[ADXL345_ODR_0P10HZ]	= {   0,  97000},
+	[ADXL345_ODR_0P20HZ]	= {   0, 195000},
+	[ADXL345_ODR_0P39HZ]	= {   0, 390000},
+	[ADXL345_ODR_0P78HZ]	= {   0, 781000},
+	[ADXL345_ODR_1P56HZ]	= {   1, 562000},
+	[ADXL345_ODR_3P13HZ]	= {   3, 125000},
+	[ADXL345_ODR_6P25HZ]	= {   6, 250000},
+	[ADXL345_ODR_12P50HZ]	= {  12, 500000},
+	[ADXL345_ODR_25HZ]	= {  25, 0},
+	[ADXL345_ODR_50HZ]	= {  50, 0},
+	[ADXL345_ODR_100HZ]	= { 100, 0},
+	[ADXL345_ODR_200HZ]	= { 200, 0},
+	[ADXL345_ODR_400HZ]	= { 400, 0},
+	[ADXL345_ODR_800HZ]	= { 800, 0},
+	[ADXL345_ODR_1600HZ]	= {1600, 0},
+	[ADXL345_ODR_3200HZ]	= {3200, 0},
+};
+
 struct adxl345_state {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
@@ -77,6 +116,8 @@ struct adxl345_state {
 	u8 watermark;
 	u8 fifo_mode;
 
+	enum adxl345_odr odr;
+
 	u32 tap_axis_ctrl;
 	u8 tap_threshold;
 	u32 tap_duration_us;
@@ -126,6 +167,7 @@ static struct iio_event_spec adxl345_events[] = {
 		BIT(IIO_CHAN_INFO_CALIBBIAS),				\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
 	.scan_index = (index),				\
 	.scan_type = {					\
 		.sign = 's',				\
@@ -427,13 +469,61 @@ static int adxl345_set_ff_time(struct adxl345_state *st, u32 val_int,
 	return regmap_write(st->regmap, ADXL345_REG_TIME_FF, min(regval, 0xff));
 }
 
+static int adxl345_find_odr(struct adxl345_state *st, int val,
+			    int val2, enum adxl345_odr *odr)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(adxl345_odr_tbl); i++)
+		if (val == adxl345_odr_tbl[i][0] &&
+		    val2 == adxl345_odr_tbl[i][1])
+			break;
+
+	if (i == ARRAY_SIZE(adxl345_odr_tbl))
+		return -EINVAL;
+
+	*odr = i;
+
+	return 0;
+}
+
+static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
+{
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
+				 ADXL345_BW_RATE_MSK,
+				 FIELD_PREP(ADXL345_BW_RATE_MSK, odr));
+	if (ret)
+		return ret;
+
+	st->odr = odr;
+
+	return 0;
+}
+
+static int adxl345_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type,
+			      int *length, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (int *)adxl345_odr_tbl;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(adxl345_odr_tbl) * 2;
+		return IIO_AVAIL_LIST;
+	}
+
+	return -EINVAL;
+}
+
 static int adxl345_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
 	__le16 accel;
-	long long samp_freq_nhz;
 	unsigned int regval;
 	int ret;
 
@@ -469,15 +559,9 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		ret = regmap_read(st->regmap, ADXL345_REG_BW_RATE, &regval);
-		if (ret < 0)
-			return ret;
-
-		samp_freq_nhz = ADXL345_BASE_RATE_NANO_HZ <<
-				(regval & ADXL345_BW_RATE);
-		*val = div_s64_rem(samp_freq_nhz, NANOHZ_PER_HZ, val2);
-
-		return IIO_VAL_INT_PLUS_NANO;
+		*val = adxl345_odr_tbl[st->odr][0];
+		*val2 = adxl345_odr_tbl[st->odr][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 
 	return -EINVAL;
@@ -488,7 +572,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct adxl345_state *st = iio_priv(indio_dev);
-	s64 n;
+	enum adxl345_odr odr;
+	int ret;
+
+	ret = adxl345_set_measure_en(st, false);
+	if (ret)
+		return ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_CALIBBIAS:
@@ -496,20 +585,24 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
 		 * 8-bit resolution at +/- 2g, that is 4x accel data scale
 		 * factor
 		 */
-		return regmap_write(st->regmap,
-				    ADXL345_REG_OFS_AXIS(chan->address),
-				    val / 4);
+		ret = regmap_write(st->regmap,
+				   ADXL345_REG_OFS_AXIS(chan->address),
+				   val / 4);
+		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		n = div_s64(val * NANOHZ_PER_HZ + val2,
-			    ADXL345_BASE_RATE_NANO_HZ);
-
-		return regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
-					  ADXL345_BW_RATE,
-					  clamp_val(ilog2(n), 0,
-						    ADXL345_BW_RATE));
+		ret = adxl345_find_odr(st, val, val2, &odr);
+		if (ret)
+			return ret;
+		ret = adxl345_set_odr(st, odr);
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	return -EINVAL;
+	if (ret)
+		return ret;
+
+	return adxl345_set_measure_en(st, true);
 }
 
 static int adxl345_read_event_config(struct iio_dev *indio_dev,
@@ -741,7 +834,7 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_CALIBBIAS:
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		return IIO_VAL_INT_PLUS_NANO;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -808,19 +901,6 @@ static void adxl345_powerdown(void *ptr)
 	adxl345_set_measure_en(st, false);
 }
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
-"0.09765625 0.1953125 0.390625 0.78125 1.5625 3.125 6.25 12.5 25 50 100 200 400 800 1600 3200"
-);
-
-static struct attribute *adxl345_attrs[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
-	NULL
-};
-
-static const struct attribute_group adxl345_attrs_group = {
-	.attrs = adxl345_attrs,
-};
-
 static int adxl345_set_fifo(struct adxl345_state *st)
 {
 	int ret;
@@ -1094,10 +1174,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
 }
 
 static const struct iio_info adxl345_info = {
-	.attrs		= &adxl345_attrs_group,
 	.event_attrs	= &adxl345_event_attrs_group,
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
+	.read_avail	= adxl345_read_avail,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
 	.read_event_config = adxl345_read_event_config,
 	.write_event_config = adxl345_write_event_config,
@@ -1166,6 +1246,15 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
 	indio_dev->available_scan_masks = adxl345_scan_masks;
 
+	/*
+	 * Using I2C at 100kHz would limit the maximum ODR to 200Hz, operation
+	 * at an output rate above the recommended maximum may result in
+	 * undesired behavior.
+	 */
+	ret = adxl345_set_odr(st, ADXL345_ODR_200HZ);
+	if (ret)
+		return ret;
+
 	/* Reset interrupts at start up */
 	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
 	if (ret)
-- 
2.39.5
Re: [PATCH v2 09/14] iio: accel: adxl345: extend sample frequency adjustments
Posted by Jonathan Cameron 11 months, 3 weeks ago
On Mon, 10 Feb 2025 11:01:14 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Introduce enums and functions to work with the sample frequency
> adjustments. Let the sample frequency adjust via IIO and configure
> a reasonable default.
> 
> Replace the old static sample frequency handling. The patch is in
> preparation for activity/inactivity handling.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

I noticed now that we don't actually have regmap caching enabled.
I think it will make things sufficiently simpler that it is worth
doing even though you have to specify which registers are volatile etc.


> +
> +static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
> +				 ADXL345_BW_RATE_MSK,
> +				 FIELD_PREP(ADXL345_BW_RATE_MSK, odr));
> +	if (ret)
> +		return ret;
> +
> +	st->odr = odr;
Why do we need to keep a copy cached? Seems like we can get it from
regmap cache easily enough?

> +
> +	return 0;
> +}

> @@ -488,7 +572,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct adxl345_state *st = iio_priv(indio_dev);
> -	s64 n;
> +	enum adxl345_odr odr;
> +	int ret;
> +
> +	ret = adxl345_set_measure_en(st, false);

Please add some more on why this is now necessary to the patch description.

> +	if (ret)
> +		return ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_CALIBBIAS:
> @@ -496,20 +585,24 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  		 * 8-bit resolution at +/- 2g, that is 4x accel data scale
>  		 * factor
>  		 */
> -		return regmap_write(st->regmap,
> -				    ADXL345_REG_OFS_AXIS(chan->address),
> -				    val / 4);
> +		ret = regmap_write(st->regmap,
> +				   ADXL345_REG_OFS_AXIS(chan->address),
> +				   val / 4);
> +		break;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		n = div_s64(val * NANOHZ_PER_HZ + val2,
> -			    ADXL345_BASE_RATE_NANO_HZ);
> -
> -		return regmap_update_bits(st->regmap, ADXL345_REG_BW_RATE,
> -					  ADXL345_BW_RATE,
> -					  clamp_val(ilog2(n), 0,
> -						    ADXL345_BW_RATE));
> +		ret = adxl345_find_odr(st, val, val2, &odr);
> +		if (ret)
> +			return ret;
> +		ret = adxl345_set_odr(st, odr);
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
>  
> -	return -EINVAL;
> +	if (ret)
> +		return ret;
> +
> +	return adxl345_set_measure_en(st, true);
>  }