Re: [RFC PATCH v3 6/9] iio: pressure: bmp280: enable filter settings for BMx280

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 28 Sep 2025 19:26:34 +0200
Achim Gratz <Achim.Gratz@Stromeko.DE> wrote:

> Follow the existing implementation for the BMx380 and BMx580 devices
> even though it doesn't conform to the API: The BMx280 devices were
> using a hardcoded value of 4, corresponding to the lowest corner
> frequency.  Enable filter_low_pass_3db_frequency settings to control
> the filter settings of the device.  The actual 3dB corner freqquency
> has an inverse relation to the value, which represents approximately
> the tau of the filter (for which the iio framework does not seem to
> have a suitable parameter). 
> 
> Remove a superfluous offset of -1 from
> the internal handling of the available values and use the table
> entries directly.  Keep the default value at the previous hardcoded
> value to ensure identical device behaviour after module load.
> 
> A change of the implementation to actually follow the API (breaking
> existing userspace) requires further discussion and more extensive
> changes elsewhere in the code and are left for later.

I'm not keen to introduce additional cases of the non compliant ABI
even if it is just more parts in the same driver.

So patch looks fine in so far as what it does, but that issue is a blocker.

J
> 
> ---
> 
> Filter coefficient settings in hardware: off, 2, 4, 8, 16, mapped to
> register values 0, 1, 2, 3, 4 (that's i and 2**i below), per the
> datasheet, the actual filter is:
> 
>   y(t) = ( (y(t-1) << i) - y(t-1) + x(t) ) >> i
>        = 2**-i * x(t) (2**i-1)*2**-i * y(t-1) )
> 
> That's the simplest filter that can be implemented in hardware,
> really; the canonical recursive single-pole LP with no gain has two
> coefficients, which should be 1-d and d (d because it's the sample
> decay).
> 
>    2**-i + (2**i-1)*2**-i = 2**-1 * ( 1 + 2**i - 1 ) = 1
> 
> so check that.  The time constant (rise time to 63%) is then
> 
>   tau = -1/ln(d)
> 
> Oddly enough the data sheet gives time to >75%, but that is just a
> scaling factor of ln(0.25) on the tau.  The nomalized corner frequency then is:
> 
>   fc/fs = -ln(d)/(2pi)
> 
> So lets check that:
> 
> |---+------+--------+--------+--------+--------+-----------+----------|
> | i | 2**i |    1-d |      d |    tau |  t>75% | datasheet |    fc/fs |
> |---+------+--------+--------+--------+--------+-----------+----------|
> | 0 |    1 |      1 |      0 |    --- |    --- |         1 |        1 |
> | 1 |    2 |    0.5 |    0.5 |  1.443 |  2.000 |         2 | 0.110318 |
> | 2 |    4 |   0.25 |   0.75 |  3.476 |  4.819 |         5 | 0.045786 |
> | 3 |    8 |  0.125 |  0.875 |  7.489 | 10.382 |        11 | 0.021252 |
> | 4 |   16 | 0.0625 | 0.9375 | 15.495 | 21.481 |        22 | 0.010272 |
> |---+------+--------+--------+--------+--------+-----------+----------|
> 
> Signed-off-by: Achim Gratz <Achim.Gratz@Stromeko.DE>
> ---
>  drivers/iio/pressure/bmp280-core.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 4f5c4bd89067..e72cfd4c10b9 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -159,6 +159,7 @@ static const struct iio_chan_spec bmp280_channels[] = {
>  				      BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.scan_index = 0,
>  		.scan_type = {
>  			.sign = 'u',
> @@ -174,6 +175,7 @@ static const struct iio_chan_spec bmp280_channels[] = {
>  				      BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.scan_index = 1,
>  		.scan_type = {
>  			.sign = 's',
> @@ -193,6 +195,7 @@ static const struct iio_chan_spec bme280_channels[] = {
>  				      BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.scan_index = 0,
>  		.scan_type = {
>  			.sign = 'u',
> @@ -208,6 +211,7 @@ static const struct iio_chan_spec bme280_channels[] = {
>  				      BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.scan_index = 1,
>  		.scan_type = {
>  			.sign = 's',
> @@ -223,6 +227,7 @@ static const struct iio_chan_spec bme280_channels[] = {
>  				      BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
>  		.scan_index = 2,
>  		.scan_type = {
>  			.sign = 'u',
> @@ -714,7 +719,7 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
>  		if (!data->chip_info->iir_filter_coeffs_avail)
>  			return -EINVAL;
>  
> -		*val = (1 << data->iir_filter_coeff) - 1;
> +		*val = data->chip_info->iir_filter_coeffs_avail[data->iir_filter_coeff];
>  		return IIO_VAL_INT;
>  	default:
>  		return -EINVAL;
> @@ -844,7 +849,7 @@ static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val)
>  	int i;
>  
>  	for (i = 0; i < n; i++) {
> -		if (avail[i] - 1  == val) {
> +		if (avail[i] == val) {
>  			prev = data->iir_filter_coeff;
>  			data->iir_filter_coeff = i;
>  
> @@ -1095,6 +1100,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  {
>  	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
>  		  FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1);
> +	u8 filter = FIELD_PREP(BMP280_FILTER_MASK, data->iir_filter_coeff;
>  	int ret;
>  
>  	ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> @@ -1109,7 +1115,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  
>  	ret = regmap_update_bits(data->regmap, BMP280_REG_CONFIG,
>  				 BMP280_FILTER_MASK,
> -				 BMP280_FILTER_4X);
> +				 filter);
>  	if (ret) {
>  		dev_err(data->dev, "failed to write config register\n");
>  		return ret;
> @@ -1174,6 +1180,7 @@ static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
>  static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
>  static const int bmp280_temp_coeffs[] = { 10, 1 };
>  static const int bmp280_press_coeffs[] = { 1, 256000 };
> +static const int bmp280_iir_filter_coeffs_avail[] = { 0, 2, 4, 8, 16 };
>  
>  const struct bmp280_chip_info bmp280_chip_info = {
>  	.id_reg = BMP280_REG_ID,
> @@ -1203,6 +1210,10 @@ const struct bmp280_chip_info bmp280_chip_info = {
>  	.num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail),
>  	.oversampling_press_default = BMP280_OSRS_PRESS_16X - 1,
>  
> +	.iir_filter_coeffs_avail = bmp280_iir_filter_coeffs_avail,
> +	.num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp280_iir_filter_coeffs_avail),
> +	.iir_filter_coeff_default = 2,
> +
>  	.temp_coeffs = bmp280_temp_coeffs,
>  	.temp_coeffs_type = IIO_VAL_FRACTIONAL,
>  	.press_coeffs = bmp280_press_coeffs,
> @@ -1385,6 +1396,10 @@ const struct bmp280_chip_info bme280_chip_info = {
>  	.num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail),
>  	.oversampling_humid_default = BME280_OSRS_HUMIDITY_16X - 1,
>  
> +	.iir_filter_coeffs_avail = bmp280_iir_filter_coeffs_avail,
> +	.num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp280_iir_filter_coeffs_avail),
> +	.iir_filter_coeff_default = 2,
> +
>  	.temp_coeffs = bmp280_temp_coeffs,
>  	.temp_coeffs_type = IIO_VAL_FRACTIONAL,
>  	.press_coeffs = bmp280_press_coeffs,
> @@ -1982,7 +1997,7 @@ static irqreturn_t bmp380_trigger_handler(int irq, void *p)
>  }
>  
>  static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
> -static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
> +static const int bmp380_iir_filter_coeffs_avail[] = { 0, 1, 3, 7, 15, 31, 63, 127 };
>  static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID };
>  static const int bmp380_temp_coeffs[] = { 10, 1 };
>  static const int bmp380_press_coeffs[] = { 1, 100000 };