linux-kernel - Re: [PATCH 06/10] iio: accel: BMA220 add LPF cut-off frequency mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cf18ede2-2077-41f4-a49c-adb3c13c4c44@baylibre.com>
Date: Fri, 5 Sep 2025 14:59:27 -0500
From: David Lechner <dlechner@...libre.com>
To: Petre Rodan <petre.rodan@...dimension.ro>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>
Subject: Re: [PATCH 06/10] iio: accel: BMA220 add LPF cut-off frequency
 mapping

On 9/1/25 2:47 PM, Petre Rodan wrote:
>  - add mapping for the low pass filter cut-off frequency.
>  - make valid values visible for both the cut-off frequency and the scale.
> 
> Signed-off-by: Petre Rodan <petre.rodan@...dimension.ro>
> ---
>  drivers/iio/accel/bma220_core.c | 60 ++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 86347cf8ab1e..e60acd62cf96 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -4,6 +4,15 @@
>   *
>   * Copyright (c) 2016,2020 Intel Corporation.
>   * Copyright (c) 2025 Petre Rodan  <petre.rodan@...dimension.ro>
> + *
> + *
> + * Device register to IIO ABI mapping:
> + *
> + * Register                  | IIO ABI (sysfs)                | valid values
> + * --------------------------+--------------------------------+---------------
> + * scale (range)             | in_accel_scale                 | see _available
> + * cut-off freq (filt_config)| in_accel_filter_low_pass_...   | see _available
> + * ---------------------------------------------------------------------------
>   */
> 
>  #include <linux/bits.h>
> @@ -135,13 +144,23 @@
> 
>  #define BMA220_DEVICE_NAME			"bma220"
> 
> +#define BMA220_COF_1000HZ			0x0
> +#define BMA220_COF_500HZ			0x1
> +#define BMA220_COF_250HZ			0x2
> +#define BMA220_COF_125HZ			0x3
> +#define BMA220_COF_64HZ				0x4
> +#define BMA220_COF_32HZ				0x5
> +
>  #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
>  	.type = IIO_ACCEL,						\
>  	.address = reg,							\
>  	.modified = 1,							\
>  	.channel2 = IIO_MOD_##axis,					\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
> +	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) |\
> +	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
>  	.scan_index = index,						\
>  	.scan_type = {							\
>  		.sign = 's',						\
> @@ -172,6 +191,7 @@ struct bma220_data {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct mutex lock;
> +	u8 lpf_3db_freq_idx;
>  	u8 range_idx;
>  	struct iio_trigger *trig;
>  	struct {
> @@ -188,6 +208,18 @@ static const struct iio_chan_spec bma220_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
> 
> +/*
> + * Available cut-off frequencies of the low pass filter in Hz.
> + */
> +static const int bma220_lpf_3db_freq_hz_table[][2] = {
> +	[BMA220_COF_1000HZ] = {1000, 0},
> +	[BMA220_COF_500HZ] = {500, 0},
> +	[BMA220_COF_250HZ] = {250, 0},
> +	[BMA220_COF_125HZ] = {125, 0},
> +	[BMA220_COF_64HZ] = {64, 0},
> +	[BMA220_COF_32HZ] = {32, 0},

If all of these are integer values, why do we need 2-D table?

> +};
> +
>  static const unsigned long bma220_accel_scan_masks[] = {
>  	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
>  	0
> @@ -309,6 +341,11 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
>  		*val = bma220_scale_table[index][0];
>  		*val2 = bma220_scale_table[index][1];
>  		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		index = data->lpf_3db_freq_idx;
> +		*val = bma220_lpf_3db_freq_hz_table[index][0];
> +		*val2 = bma220_lpf_3db_freq_hz_table[index][1];
> +		return IIO_VAL_INT_PLUS_MICRO;

Why not IIO_VAL_INT?

>  	}
> 
>  	return -EINVAL;
> @@ -353,6 +390,22 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
>  				"failed to set measurement range\n");
>  		data->range_idx = index;
> 
> +		return 0;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		index = bma220_find_match(bma220_lpf_3db_freq_hz_table,
> +					  ARRAY_SIZE(bma220_lpf_3db_freq_hz_table),
> +					  val, val2);
> +		if (index < 0)
> +			return -EINVAL;
> +
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_FILTER,
> +					 BMA220_FILTER_MASK,
> +					 FIELD_PREP(BMA220_FILTER_MASK, index));
> +		if (ret < 0)
> +			dev_err(data->dev,
> +				"failed to set low pass filter\n");

Should `return ret;` here rather than logging error.

> +		data->lpf_3db_freq_idx = index;
> +
>  		return 0;
>  	}
> 
> @@ -370,6 +423,11 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
>  		*type = IIO_VAL_INT_PLUS_MICRO;
>  		*length = ARRAY_SIZE(bma220_scale_table) * 2;
>  		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*vals = (const int *)bma220_lpf_3db_freq_hz_table;
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = ARRAY_SIZE(bma220_lpf_3db_freq_hz_table) * 2;
> +		return IIO_AVAIL_LIST;
>  	default:
>  		return -EINVAL;
>  	}
> --
> 2.49.1
>