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

[Jonathan Cameron <jic23@kernel.org>]

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;
> +	}
> +}