[PATCH v2 1/4] iio: accel: adxl372: introduce chip_info structure

Antoniu Miclaus posted 4 patches 1 month ago
There is a newer version of this series
[PATCH v2 1/4] iio: accel: adxl372: introduce chip_info structure
Posted by Antoniu Miclaus 1 month ago
Introduce a chip_info structure to parameterize device-specific
properties such as ODR/bandwidth frequency tables, activity/inactivity
timer scale factors, and the maximum ODR value. This refactors the
driver to use chip_info lookups instead of hardcoded values, preparing
the driver to support multiple device variants.

The sampling_frequency and filter_low_pass_3db_frequency available
attributes are switched from custom sysfs callbacks to read_avail()
based handling via info_mask_shared_by_type_available. This enforces
consistent formatting through the IIO framework and makes the values
accessible to in-kernel consumers.

The SPI/I2C probe functions are updated to pass a chip_info pointer
instead of a device name string.

No functional change intended.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
Changes in v2:
 - Switch sampling_frequency and filter_low_pass_3db_frequency available
   attributes from custom sysfs callbacks to read_avail() with
   info_mask_shared_by_type_available.

 drivers/iio/accel/adxl372.c     | 125 +++++++++++++++++---------------
 drivers/iio/accel/adxl372.h     |  16 +++-
 drivers/iio/accel/adxl372_i2c.c |  12 ++-
 drivers/iio/accel/adxl372_spi.c |  12 ++-
 4 files changed, 96 insertions(+), 69 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 28a8793a53b6..6918a5834d74 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -222,6 +222,19 @@ static const int adxl372_bw_freq_tbl[5] = {
 	200, 400, 800, 1600, 3200,
 };
 
+const struct adxl372_chip_info adxl372_chip_info = {
+	.name = "adxl372",
+	.samp_freq_tbl = adxl372_samp_freq_tbl,
+	.bw_freq_tbl = adxl372_bw_freq_tbl,
+	.num_freqs = ARRAY_SIZE(adxl372_samp_freq_tbl),
+	.act_time_scale_us = 3300,
+	.act_time_scale_low_us = 6600,
+	.inact_time_scale_ms = 13,
+	.inact_time_scale_low_ms = 26,
+	.max_odr = ADXL372_ODR_6400HZ,
+};
+EXPORT_SYMBOL_NS_GPL(adxl372_chip_info, "IIO_ADXL372");
+
 struct adxl372_axis_lookup {
 	unsigned int bits;
 	enum adxl372_fifo_format fifo_format;
@@ -260,6 +273,9 @@ static const struct iio_event_spec adxl372_events[] = {
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
 				    BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
 		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),	\
+	.info_mask_shared_by_type_available =				\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ) |				\
+		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),	\
 	.scan_index = index,						\
 	.scan_type = {							\
 		.sign = 's',						\
@@ -279,6 +295,7 @@ static const struct iio_chan_spec adxl372_channels[] = {
 };
 
 struct adxl372_state {
+	const struct adxl372_chip_info	*chip_info;
 	int				irq;
 	struct device			*dev;
 	struct regmap			*regmap;
@@ -471,13 +488,14 @@ static int adxl372_set_activity_time_ms(struct adxl372_state *st,
 	int ret;
 
 	/*
-	 * 3.3 ms per code is the scale factor of the TIME_ACT register for
-	 * ODR = 6400 Hz. It is 6.6 ms per code for ODR = 3200 Hz and below.
+	 * The scale factor of the TIME_ACT register depends on the ODR.
+	 * A higher scale factor is used at the maximum ODR and a lower
+	 * one at all other rates.
 	 */
-	if (st->odr == ADXL372_ODR_6400HZ)
-		scale_factor = 3300;
+	if (st->odr == st->chip_info->max_odr)
+		scale_factor = st->chip_info->act_time_scale_us;
 	else
-		scale_factor = 6600;
+		scale_factor = st->chip_info->act_time_scale_low_us;
 
 	reg_val = DIV_ROUND_CLOSEST(act_time_ms * 1000, scale_factor);
 
@@ -501,13 +519,14 @@ static int adxl372_set_inactivity_time_ms(struct adxl372_state *st,
 	int ret;
 
 	/*
-	 * 13 ms per code is the scale factor of the TIME_INACT register for
-	 * ODR = 6400 Hz. It is 26 ms per code for ODR = 3200 Hz and below.
+	 * The scale factor of the TIME_INACT register depends on the ODR.
+	 * A higher scale factor is used at the maximum ODR and a lower
+	 * one at all other rates.
 	 */
-	if (st->odr == ADXL372_ODR_6400HZ)
-		scale_factor = 13;
+	if (st->odr == st->chip_info->max_odr)
+		scale_factor = st->chip_info->inact_time_scale_ms;
 	else
-		scale_factor = 26;
+		scale_factor = st->chip_info->inact_time_scale_low_ms;
 
 	res = DIV_ROUND_CLOSEST(inact_time_ms, scale_factor);
 	reg_val_h = (res >> 8) & 0xFF;
@@ -717,7 +736,7 @@ static int adxl372_setup(struct adxl372_state *st)
 	if (ret < 0)
 		return ret;
 
-	ret = adxl372_set_odr(st, ADXL372_ODR_6400HZ);
+	ret = adxl372_set_odr(st, st->chip_info->max_odr);
 	if (ret < 0)
 		return ret;
 
@@ -777,10 +796,10 @@ static int adxl372_read_raw(struct iio_dev *indio_dev,
 		*val2 = ADXL372_USCALE;
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		*val = adxl372_samp_freq_tbl[st->odr];
+		*val = st->chip_info->samp_freq_tbl[st->odr];
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		*val = adxl372_bw_freq_tbl[st->bw];
+		*val = st->chip_info->bw_freq_tbl[st->bw];
 		return IIO_VAL_INT;
 	}
 
@@ -796,23 +815,17 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
 
 	switch (info) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		odr_index = adxl372_find_closest_match(adxl372_samp_freq_tbl,
-					ARRAY_SIZE(adxl372_samp_freq_tbl),
-					val);
+		odr_index = adxl372_find_closest_match(st->chip_info->samp_freq_tbl,
+						       st->chip_info->num_freqs,
+						       val);
 		ret = adxl372_set_odr(st, odr_index);
 		if (ret < 0)
 			return ret;
-		/*
-		 * The timer period depends on the ODR selected.
-		 * At 3200 Hz and below, it is 6.6 ms; at 6400 Hz, it is 3.3 ms
-		 */
+		/* Recalculate activity time as the timer period depends on ODR */
 		ret = adxl372_set_activity_time_ms(st, st->act_time_ms);
 		if (ret < 0)
 			return ret;
-		/*
-		 * The timer period depends on the ODR selected.
-		 * At 3200 Hz and below, it is 26 ms; at 6400 Hz, it is 13 ms
-		 */
+		/* Recalculate inactivity time as the timer period depends on ODR */
 		ret = adxl372_set_inactivity_time_ms(st, st->inact_time_ms);
 		if (ret < 0)
 			return ret;
@@ -825,9 +838,9 @@ static int adxl372_write_raw(struct iio_dev *indio_dev,
 
 		return ret;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		bw_index = adxl372_find_closest_match(adxl372_bw_freq_tbl,
-					ARRAY_SIZE(adxl372_bw_freq_tbl),
-					val);
+		bw_index = adxl372_find_closest_match(st->chip_info->bw_freq_tbl,
+						      st->chip_info->num_freqs,
+						      val);
 		return adxl372_set_bandwidth(st, bw_index);
 	default:
 		return -EINVAL;
@@ -957,24 +970,6 @@ static int adxl372_write_event_config(struct iio_dev *indio_dev, const struct ii
 	return adxl372_set_interrupts(st, st->int1_bitmask, 0);
 }
 
-static ssize_t adxl372_show_filter_freq_avail(struct device *dev,
-					      struct device_attribute *attr,
-					      char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct adxl372_state *st = iio_priv(indio_dev);
-	int i;
-	size_t len = 0;
-
-	for (i = 0; i <= st->odr; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len,
-				 "%d ", adxl372_bw_freq_tbl[i]);
-
-	buf[len - 1] = '\n';
-
-	return len;
-}
-
 static ssize_t adxl372_get_fifo_enabled(struct device *dev,
 					  struct device_attribute *attr,
 					  char *buf)
@@ -1142,25 +1137,34 @@ static const struct iio_trigger_ops adxl372_peak_data_trigger_ops = {
 	.set_trigger_state = adxl372_peak_dready_trig_set_state,
 };
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("400 800 1600 3200 6400");
-static IIO_DEVICE_ATTR(in_accel_filter_low_pass_3db_frequency_available,
-		       0444, adxl372_show_filter_freq_avail, NULL, 0);
-
-static struct attribute *adxl372_attributes[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
-	&iio_dev_attr_in_accel_filter_low_pass_3db_frequency_available.dev_attr.attr,
-	NULL,
-};
+static int adxl372_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct adxl372_state *st = iio_priv(indio_dev);
 
-static const struct attribute_group adxl372_attrs_group = {
-	.attrs = adxl372_attributes,
-};
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = st->chip_info->samp_freq_tbl;
+		*type = IIO_VAL_INT;
+		*length = st->chip_info->num_freqs;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = st->chip_info->bw_freq_tbl;
+		*type = IIO_VAL_INT;
+		*length = st->odr + 1;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
 
 static const struct iio_info adxl372_info = {
 	.validate_trigger = &adxl372_validate_trigger,
-	.attrs = &adxl372_attrs_group,
 	.read_raw = adxl372_read_raw,
 	.write_raw = adxl372_write_raw,
+	.read_avail = adxl372_read_avail,
 	.read_event_config = adxl372_read_event_config,
 	.write_event_config = adxl372_write_event_config,
 	.read_event_value = adxl372_read_event_value,
@@ -1176,7 +1180,7 @@ bool adxl372_readable_noinc_reg(struct device *dev, unsigned int reg)
 EXPORT_SYMBOL_NS_GPL(adxl372_readable_noinc_reg, "IIO_ADXL372");
 
 int adxl372_probe(struct device *dev, struct regmap *regmap,
-		  int irq, const char *name)
+		  int irq, const struct adxl372_chip_info *chip_info)
 {
 	struct iio_dev *indio_dev;
 	struct adxl372_state *st;
@@ -1192,13 +1196,14 @@ int adxl372_probe(struct device *dev, struct regmap *regmap,
 	st->dev = dev;
 	st->regmap = regmap;
 	st->irq = irq;
+	st->chip_info = chip_info;
 
 	mutex_init(&st->threshold_m);
 
 	indio_dev->channels = adxl372_channels;
 	indio_dev->num_channels = ARRAY_SIZE(adxl372_channels);
 	indio_dev->available_scan_masks = adxl372_channel_masks;
-	indio_dev->name = name;
+	indio_dev->name = chip_info->name;
 	indio_dev->info = &adxl372_info;
 	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
 
diff --git a/drivers/iio/accel/adxl372.h b/drivers/iio/accel/adxl372.h
index 80a0aa9714fc..3ce06609446c 100644
--- a/drivers/iio/accel/adxl372.h
+++ b/drivers/iio/accel/adxl372.h
@@ -10,8 +10,22 @@
 
 #define ADXL372_REVID	0x03
 
+struct adxl372_chip_info {
+	const char *name;
+	const int *samp_freq_tbl;
+	const int *bw_freq_tbl;
+	unsigned int num_freqs;
+	unsigned int act_time_scale_us;
+	unsigned int act_time_scale_low_us;
+	unsigned int inact_time_scale_ms;
+	unsigned int inact_time_scale_low_ms;
+	unsigned int max_odr;
+};
+
+extern const struct adxl372_chip_info adxl372_chip_info;
+
 int adxl372_probe(struct device *dev, struct regmap *regmap,
-		  int irq, const char *name);
+		  int irq, const struct adxl372_chip_info *chip_info);
 bool adxl372_readable_noinc_reg(struct device *dev, unsigned int reg);
 
 #endif /* _ADXL372_H_ */
diff --git a/drivers/iio/accel/adxl372_i2c.c b/drivers/iio/accel/adxl372_i2c.c
index 186d4fe9a556..3f97126a87a1 100644
--- a/drivers/iio/accel/adxl372_i2c.c
+++ b/drivers/iio/accel/adxl372_i2c.c
@@ -20,11 +20,15 @@ static const struct regmap_config adxl372_regmap_config = {
 
 static int adxl372_i2c_probe(struct i2c_client *client)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	const struct adxl372_chip_info *chip_info;
 	struct regmap *regmap;
 	unsigned int regval;
 	int ret;
 
+	chip_info = i2c_get_match_data(client);
+	if (!chip_info)
+		return -ENODEV;
+
 	regmap = devm_regmap_init_i2c(client, &adxl372_regmap_config);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
@@ -38,17 +42,17 @@ static int adxl372_i2c_probe(struct i2c_client *client)
 		dev_warn(&client->dev,
 		"I2C might not work properly with other devices on the bus");
 
-	return adxl372_probe(&client->dev, regmap, client->irq, id->name);
+	return adxl372_probe(&client->dev, regmap, client->irq, chip_info);
 }
 
 static const struct i2c_device_id adxl372_i2c_id[] = {
-	{ "adxl372" },
+	{ "adxl372", (kernel_ulong_t)&adxl372_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, adxl372_i2c_id);
 
 static const struct of_device_id adxl372_of_match[] = {
-	{ .compatible = "adi,adxl372" },
+	{ .compatible = "adi,adxl372", .data = &adxl372_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adxl372_of_match);
diff --git a/drivers/iio/accel/adxl372_spi.c b/drivers/iio/accel/adxl372_spi.c
index 39941b519c3b..0e199feb405e 100644
--- a/drivers/iio/accel/adxl372_spi.c
+++ b/drivers/iio/accel/adxl372_spi.c
@@ -22,24 +22,28 @@ static const struct regmap_config adxl372_spi_regmap_config = {
 
 static int adxl372_spi_probe(struct spi_device *spi)
 {
-	const struct spi_device_id *id = spi_get_device_id(spi);
+	const struct adxl372_chip_info *chip_info;
 	struct regmap *regmap;
 
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
+
 	regmap = devm_regmap_init_spi(spi, &adxl372_spi_regmap_config);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	return adxl372_probe(&spi->dev, regmap, spi->irq, id->name);
+	return adxl372_probe(&spi->dev, regmap, spi->irq, chip_info);
 }
 
 static const struct spi_device_id adxl372_spi_id[] = {
-	{ "adxl372", 0 },
+	{ "adxl372", (kernel_ulong_t)&adxl372_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, adxl372_spi_id);
 
 static const struct of_device_id adxl372_of_match[] = {
-	{ .compatible = "adi,adxl372" },
+	{ .compatible = "adi,adxl372", .data = &adxl372_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adxl372_of_match);
-- 
2.43.0
Re: [PATCH v2 1/4] iio: accel: adxl372: introduce chip_info structure
Posted by Jonathan Cameron 1 month ago
On Fri, 6 Mar 2026 17:18:21 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Introduce a chip_info structure to parameterize device-specific
> properties such as ODR/bandwidth frequency tables, activity/inactivity
> timer scale factors, and the maximum ODR value. This refactors the
> driver to use chip_info lookups instead of hardcoded values, preparing
> the driver to support multiple device variants.
> 
> The sampling_frequency and filter_low_pass_3db_frequency available
> attributes are switched from custom sysfs callbacks to read_avail()
> based handling via info_mask_shared_by_type_available. This enforces
> consistent formatting through the IIO framework and makes the values
> accessible to in-kernel consumers.
> 
> The SPI/I2C probe functions are updated to pass a chip_info pointer
> instead of a device name string.
> 
> No functional change intended.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
One really minor request for a comment on where the 3db frequency
number of entries comes from + some musing on whether it would have
been sensible to do the read_avail stuff as a precursor.
Otherwise LGTM.

> ---
> Changes in v2:
>  - Switch sampling_frequency and filter_low_pass_3db_frequency available
>    attributes from custom sysfs callbacks to read_avail() with
>    info_mask_shared_by_type_available.
> 
>  drivers/iio/accel/adxl372.c     | 125 +++++++++++++++++---------------
>  drivers/iio/accel/adxl372.h     |  16 +++-
>  drivers/iio/accel/adxl372_i2c.c |  12 ++-
>  drivers/iio/accel/adxl372_spi.c |  12 ++-
>  4 files changed, 96 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 28a8793a53b6..6918a5834d74 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c


> +static int adxl372_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	struct adxl372_state *st = iio_priv(indio_dev);
>  
> -static const struct attribute_group adxl372_attrs_group = {

In ideal patch break up I'd have liked to have seen the switch to the
read_avail as a precursor patch but given there would be a fair bit
of churn as a result maybe it wasn't worth it.

> -	.attrs = adxl372_attributes,
> -};
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = st->chip_info->samp_freq_tbl;
> +		*type = IIO_VAL_INT;
> +		*length = st->chip_info->num_freqs;
> +		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*vals = st->chip_info->bw_freq_tbl;
> +		*type = IIO_VAL_INT;
> +		*length = st->odr + 1;
I know there wasn't one in the original code, but this length is odd enough
that I'd like to see a brief comment saying why.  My assumption is because
you can't filter above half the sampling frequency (Nyquist and all that
would mean it was at best pointless) but good to state that.

> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}