Re: [PATCH v5 08/15] iio: adc: ad4134: Add support for digital filter type selection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: adc: ad4134: undocumented sinc6 filter_type and sysfs ABI attributes
--

commit f7364f2a84e232ee97a6956a54b434e35fd7f79d
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

iio: adc: ad4134: Add support for digital filter type selection

This commit adds per-channel digital filter type configuration for the AD4134
ADC by exposing filter_type and filter_type_available sysfs attributes via
IIO_ENUM.

> diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> index 91a7c21077709..b245001d45517 100644
> --- a/drivers/iio/adc/ad4134.c
> +++ b/drivers/iio/adc/ad4134.c
[ ... ]
> @@ -80,19 +83,18 @@
[ ... ]
> +enum ad4134_filter_type {
> +	AD4134_WIDEBAND,
> +	AD4134_SINC6,
> +	AD4134_SINC3,
> +	AD4134_SINC3_REJ60,
> +};
>  
> -static const struct iio_chan_spec ad4134_chan_set[] = {
> -	AD4134_CHANNEL(0),
> -	AD4134_CHANNEL(1),
> -	AD4134_CHANNEL(2),
> -	AD4134_CHANNEL(3),
> +static const char * const ad4134_filt_names[] = {
> +	[AD4134_WIDEBAND] = "wideband",
> +	[AD4134_SINC6] = "sinc6",
> +	[AD4134_SINC3] = "sinc3",
> +	[AD4134_SINC3_REJ60] = "sinc3+rej60",
>  };
>  
>  struct ad4134_state {
> @@ -115,6 +117,73 @@ struct ad4134_state {
>  	u8 tx_buf[AD4134_SPI_MAX_XFER_LEN];
>  };
>  
[ ... ]
> +static const struct iio_enum ad4134_filter_type_enum = {
> +	.items = ad4134_filt_names,
> +	.num_items = ARRAY_SIZE(ad4134_filt_names),
> +	.get = ad4134_get_filter_type,
> +	.set = ad4134_set_filter_type,
> +};
> +
> +static const struct iio_chan_spec_ext_info ad4134_filter_type_ext_info[] = {
> +	IIO_ENUM("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
> +	{ }
> +};
> +
> +#define AD4134_CHANNEL(_index) {						\
> +	.type = IIO_VOLTAGE,							\
> +	.indexed = 1,								\
> +	.channel = (_index),							\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
> +	.ext_info = ad4134_filter_type_ext_info,				\
> +}

[Severity: Low]
In drivers/iio/adc/ad4134.c, ad4134_filt_names[] introduces "sinc6" as a new
filter_type string option when the driver probes and registers
ad4134_chan_set[].

In addition, attaching ad4134_filter_type_ext_info[] with IIO_SEPARATE to
indexed non-differential IIO_VOLTAGE channels in AD4134_CHANNEL() exposes
/sys/bus/iio/devices/iio:deviceX/in_voltageY_filter_type and
/sys/bus/iio/devices/iio:deviceX/in_voltageY_filter_type_available to
userspace.

Currently, Documentation/ABI/testing/sysfs-bus-iio only lists What entries
for filter_type, in_voltageY-voltageZ_filter_type, filter_type_available, and
in_voltage-voltage_filter_type_available, and its list of valid filter types
does not include "sinc6".

Should Documentation/ABI/testing/sysfs-bus-iio be updated to document the
"sinc6" filter option and the in_voltageY_filter_type and
in_voltageY_filter_type_available attributes?

> +
> +static const struct iio_chan_spec ad4134_chan_set[] = {
> +	AD4134_CHANNEL(0),
> +	AD4134_CHANNEL(1),
> +	AD4134_CHANNEL(2),
> +	AD4134_CHANNEL(3),
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792200021.git.marcelo.schmitt@analog.com?part=8