Add support for pressure sensor Bosch BMP380
Subsystems (2)
v4 cover Add support for pressure sensor Bosch BMP380
This patchset adds BMP380 variant to the already existing drivers for the Bosch BMP180/280 pressure sensors.
Patch 1 adds references and sensor id to the devicetree bindings docs. Patch 2 is a minor refactor simplifying driver initialization logic to facilitate the integration of the new sensor variant extending the information stored in the "chip_info" struct.
Patch 3 fixes DMA unsafe regmap_bulk_* calls reported by Jonathan Cameron <[email protected]>.
Patch 4 adds the basic logic to initialize and read measurements from the sensor.
Patch 5 adds advanced configurable features such as sampling frequency and IIR filter through the IIO sysfs ABI.
Changes in v4:
- Patch 4 and 2: Merged v3 patch 2 (Kconfig refs update) into this patch.
- Patch 3: Added patch fixing unsafe DMA regmap_bulk_* calls reported by Jonathan Cameron <[email protected]>.
- Patch 4: Fixed DMA unsafe buffers used on regmap_bulk_* calls reported by Jonathan Cameron <[email protected]>.
Changes in v3:
- Patch 2: Fixed incorrect abbreviation.
- Patch 3: use dev_err_probe helper to handle error initializing sensor.
- Patch 4: Fixed kernel test robot warning provoked by missing include.
- Patch 4: Fixed bug reported by Dan Carpenter <[email protected]>.
- Patch 5: Fixed formatting and typos on multiple comments.
- Patch 5: Fixed missing boolean initialization reported by Andy Shevchenko <[email protected]>.
- Patch 5: Replaced duplicated comments with a single comment containing a brief explantation in a shared location.
- Patch 5: Dropped incorrect use of unlikely macro.
Changes in v2:
- Added patch 2 updating Kconfig with references to new sensor.
- Patch 3 adds changes proposed by Jonathan Cameron <[email protected]> to declutter and unify configuration logic for the different sensors extending "chip_info" struct with default configuration parameters.
- Patch 4: store temperature and pressure adc values on 3 byte array instead of using the type __le32. Uses function get_unaligned_le24 to convert the little-endian encoded 3 byte value to an integer.
- Patch 4: drops custom macro le16_from_bytes and use get_unaligned_le16.
- Patch 4: generate masks using GENMASK macro.
- Patch 4: use FIELD_PREP to generate bitfields for registries.
- Patch 4: dropped stray formatting change.
- Patch 5: adds sanity checks in bmp280_read_raw for channel properties only available in the BMP380.
- Patch 5: on bmp280_write_* checks if a problem ocurred committing new configuration and tries to restore previous working configuration to keep the sensor in a previous working state.
- Patch 5: refactored bmp380_chip_config to only check for configuration errors when a configuration change is detected.
- Patch 5: improved invalid configuration detection on BMP380 restarting measurement loop to force a new measurement after the configuration is updated.
Angel Iglesias (5):
dt-bindings: iio: pressure: bmp085: Add BMP380 compatible string
iio: pressure: bmp280: simplify driver initialization logic
iio: pressure: bmp280: Fix alignment for DMA safety
iio: pressure: bmp280: Add support for BMP380 sensor family
iio: pressure: bmp280: Add more tunable config parameters for BMP380
.../bindings/iio/pressure/bmp085.yaml | 4 +- drivers/iio/pressure/Kconfig | 6 +- drivers/iio/pressure/bmp280-core.c | 884 ++++++++++++++++-- drivers/iio/pressure/bmp280-i2c.c | 5 + drivers/iio/pressure/bmp280-regmap.c | 55 ++ drivers/iio/pressure/bmp280-spi.c | 5 + drivers/iio/pressure/bmp280.h | 83 ++ 7 files changed, 937 insertions(+), 105 deletions(-)
base-commit: 180c6cb6b9b79c55b79e8414f4c0208f2463af7d
Signature · 1 line
2.37.1
PATCH v4 1/5 dt-bindings: iio: pressure: bmp085: Add BMP380 compatible string
Add bosch,bmp380 compatible string for the new family of sensors. This family includes the BMP380, BMP384 and BMP388. The register map in this family changes substantially and introduces new features but core concepts and operations carryover from the previous iterations
--- Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/iio/pressure/bmp085.yaml +3 -1
@@ -4,7 +4,7 @@ $id: http://devicetree.org/schemas/iio/pressure/bmp085.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: BMP085/BMP180/BMP280/BME280 pressure iio sensors +title: BMP085/BMP180/BMP280/BME280/BMP380 pressure iio sensors maintainers: - Andreas Klinger <[email protected]>
@@ -16,6 +16,7 @@ description: | https://www.bosch-sensortec.com/bst/products/all_products/bmp180 https://www.bosch-sensortec.com/bst/products/all_products/bmp280 https://www.bosch-sensortec.com/bst/products/all_products/bme280 + https://www.bosch-sensortec.com/bst/products/all_products/bmp380 properties: compatible:
@@ -24,6 +25,7 @@ properties: - bosch,bmp180 - bosch,bmp280 - bosch,bme280 + - bosch,bmp380 reg: maxItems: 1
Signature · 1 line
2.37.1
PATCH v4 2/5 iio: pressure: bmp280: simplify driver initialization logic
In reply to Angel IglesiasSimplified common initialization logic of different sensor types unifying calibration and initial configuration recovery.
Default config param values of each sensor type are stored inside chip_info structure and used to initialize sensor data struct instance.
The auxiliar functions for read each sensor type calibration are converted to a callback available on the chip_info struct.
--- drivers/iio/pressure/bmp280-core.c | 91 ++++++++++++++++++------------ 1 file changed, 54 insertions(+), 37 deletions(-)
drivers/iio/pressure/bmp280-core.c +54 -37
@@ -107,19 +107,28 @@ struct bmp280_data { };struct bmp280_chip_info { +unsigned int id_reg; + +int num_channels; +unsigned int start_up_time; +const int *oversampling_temp_avail;int num_oversampling_temp_avail; +int oversampling_temp_default;const int *oversampling_press_avail;int num_oversampling_press_avail; +int oversampling_press_default;const int *oversampling_humid_avail;int num_oversampling_humid_avail; +int oversampling_humid_default;int (*chip_config)(struct bmp280_data *);int (*read_temp)(struct bmp280_data *,int *);int (*read_press)(struct bmp280_data *,int *,int *);int (*read_humid)(struct bmp280_data *,int *,int *); +int (*read_calib)(struct bmp280_data *,unsigned int ); };/*
@@ -147,15 +156,14 @@ static const struct iio_chan_spec bmp280_channels[] = { }, }; -static int bmp280_read_calib (struct bmp280_data *data, -struct bmp280_calib *calib, -unsigned int chip) +static int bmp280_read_calib (struct bmp280_data *data,unsigned int chip) {int ret;unsigned int tmp;__le16 l16;__be16 b16;struct device *dev = data->dev; +struct bmp280_calib *calib = &data->calib.bmp280;__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT /2 ];__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT /2 ];
@@ -640,15 +648,22 @@ static int bmp280_chip_config(struct bmp280_data *data)static const int bmp280_oversampling_avail[] = {1 ,2 ,4 ,8 ,16 };static const struct bmp280_chip_info bmp280_chip_info = { + .id_reg =BMP280_REG_ID , + .start_up_time =2000 , + .num_channels =2 , + .oversampling_temp_avail = bmp280_oversampling_avail, .num_oversampling_temp_avail =ARRAY_SIZE (bmp280_oversampling_avail), + .oversampling_temp_default =ilog2 (2 ), .oversampling_press_avail = bmp280_oversampling_avail, .num_oversampling_press_avail =ARRAY_SIZE (bmp280_oversampling_avail), + .oversampling_press_default =ilog2 (16 ), .chip_config = bmp280_chip_config, .read_temp = bmp280_read_temp, .read_press = bmp280_read_press, + .read_calib = bmp280_read_calib, };static int bme280_chip_config (struct bmp280_data *data)
@@ -670,19 +685,27 @@ static int bme280_chip_config(struct bmp280_data *data) }static const struct bmp280_chip_info bme280_chip_info = { + .id_reg =BMP280_REG_ID , + .start_up_time =2000 , + .num_channels =3 , + .oversampling_temp_avail = bmp280_oversampling_avail, .num_oversampling_temp_avail =ARRAY_SIZE (bmp280_oversampling_avail), + .oversampling_temp_default =ilog2 (2 ), .oversampling_press_avail = bmp280_oversampling_avail, .num_oversampling_press_avail =ARRAY_SIZE (bmp280_oversampling_avail), + .oversampling_press_default =ilog2 (16 ), .oversampling_humid_avail = bmp280_oversampling_avail, .num_oversampling_humid_avail =ARRAY_SIZE (bmp280_oversampling_avail), + .oversampling_humid_default =ilog2 (16 ), .chip_config = bme280_chip_config, .read_temp = bmp280_read_temp, .read_press = bmp280_read_press, .read_humid = bmp280_read_humid, + .read_calib = bmp280_read_calib, };static int bmp180_measure (struct bmp280_data *data,u8 ctrl_meas)
@@ -748,11 +771,11 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)return 0 ; } -static int bmp180_read_calib (struct bmp280_data *data, -struct bmp180_calib *calib) +static int bmp180_read_calib (struct bmp280_data *data,unsigned int chip) {int ret;int i; +struct bmp180_calib *calib = &data->calib.bmp180;__be16 buf[BMP180_REG_CALIB_COUNT /2 ]; ret = regmap_bulk_read(data->regmap, BMP180_REG_CALIB_START, buf,
@@ -913,17 +936,24 @@ static const int bmp180_oversampling_temp_avail[] = { 1 };static const int bmp180_oversampling_press_avail[] = {1 ,2 ,4 ,8 };static const struct bmp280_chip_info bmp180_chip_info = { + .id_reg =BMP280_REG_ID , + .start_up_time =2000 , + .num_channels =2 , + .oversampling_temp_avail = bmp180_oversampling_temp_avail, .num_oversampling_temp_avail =ARRAY_SIZE (bmp180_oversampling_temp_avail), + .oversampling_temp_default =ilog2 (1 ), .oversampling_press_avail = bmp180_oversampling_press_avail, .num_oversampling_press_avail =ARRAY_SIZE (bmp180_oversampling_press_avail), + .oversampling_press_default =ilog2 (8 ), .chip_config = bmp180_chip_config, .read_temp = bmp180_read_temp, .read_press = bmp180_read_press, + .read_calib = bmp180_read_calib, };static irqreturn_t bmp085_eoc_irq (int irq,void *d)
@@ -993,6 +1023,7 @@ int bmp280_common_probe(struct device *dev,int ret;struct iio_dev *indio_dev;struct bmp280_data *data; +const struct bmp280_chip_info *chip_info;unsigned int chip_id;struct gpio_desc *gpiod;
@@ -1011,30 +1042,25 @@ int bmp280_common_probe(struct device *dev,switch (chip) {case BMP180_CHIP_ID : - indio_dev->num_channels =2 ; - data->chip_info = &bmp180_chip_info; - data->oversampling_press =ilog2 (8 ); - data->oversampling_temp =ilog2 (1 ); - data->start_up_time =10000 ; + chip_info = &bmp180_chip_info;break ;case BMP280_CHIP_ID : - indio_dev->num_channels =2 ; - data->chip_info = &bmp280_chip_info; - data->oversampling_press =ilog2 (16 ); - data->oversampling_temp =ilog2 (2 ); - data->start_up_time =2000 ; + chip_info = &bmp280_chip_info;break ;case BME280_CHIP_ID : - indio_dev->num_channels =3 ; - data->chip_info = &bme280_chip_info; - data->oversampling_press =ilog2 (16 ); - data->oversampling_humid =ilog2 (16 ); - data->oversampling_temp =ilog2 (2 ); - data->start_up_time =2000 ; + chip_info = &bme280_chip_info;break ;default :return -EINVAL ; } + data->chip_info = chip_info; + +/* apply initial values from chip info structure */ + indio_dev->num_channels = chip_info->num_channels; + data->oversampling_press = chip_info->oversampling_press_default; + data->oversampling_humid = chip_info->oversampling_humid_default; + data->oversampling_temp = chip_info->oversampling_temp_default; + data->start_up_time = chip_info->start_up_time;/* Bring up regulators */ regulator_bulk_set_supply_names(data->supplies,
@@ -1071,7 +1097,8 @@ int bmp280_common_probe(struct device *dev, } data->regmap = regmap; - ret =regmap_read (regmap,BMP280_REG_ID , &chip_id); + + ret =regmap_read (regmap, data->chip_info->id_reg, &chip_id);if (ret <0 )return ret;if (chip_id != chip) {
@@ -1091,21 +1118,11 @@ int bmp280_common_probe(struct device *dev,* non-volatile memory during production". Let's read them out at probe * time once. They will not change. */ -if (chip_id ==BMP180_CHIP_ID ) { - ret =bmp180_read_calib (data, &data->calib.bmp180); -if (ret <0 ) { -dev_err (data->dev, -"failed to read calibration coefficients\n" ); -return ret; - } - }else if (chip_id ==BMP280_CHIP_ID || chip_id ==BME280_CHIP_ID ) { - ret =bmp280_read_calib (data, &data->calib.bmp280, chip_id); -if (ret <0 ) { -dev_err (data->dev, -"failed to read calibration coefficients\n" ); -return ret; - } - } + + ret = data->chip_info->read_calib (data, chip_id); +if (ret <0 ) +return dev_err_probe (data->dev, ret, +"failed to read calibration coefficients\n" );/* * Attempt to grab an optional EOC IRQ - only the BMP085 has this
Signature · 1 line
2.37.1
Re: [PATCH v4 2/5] iio: pressure: bmp280: simplify driver initialization logic
Quoted 9 lines · Angel Iglesias
Simplified common initialization logic of different sensor types unifying calibration and initial configuration recovery.
Default config param values of each sensor type are stored inside
chip_infostructure and used to initialize sensor data struct instance.The auxiliar functions for read each sensor type calibration are converted to a callback available on the
chip_infostruct.
...
drivers/iio/pressure/bmp280-core.c static int bme280_chip_config(struct bmp280_data *data)
Quoted 5 lines
drivers/iio/pressure/bmp280-core.c +5
-0+ .oversampling_temp_default = ilog2(2), + .oversampling_press_default = ilog2(16), + .oversampling_humid_default = ilog2(16), + .oversampling_temp_default = ilog2(1), + .oversampling_press_default = ilog2(8),
It's a very interesting way of writing BIT(x)...
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v4 3/5 iio: pressure: bmp280: Fix alignment for DMA safety
In reply to Angel IglesiasAdds a DMA-safe buffer to driver data struct to store raw data from sensor
Updated calibration read functions to use DMA-safe buffers as well. In this case, as calibration data is only read at sensor initialization, the buffers are allocated with kmalloc and freed after being processed.
--- drivers/iio/pressure/bmp280-core.c | 164 ++++++++++++++++++----------- 1 file changed, 103 insertions(+), 61 deletions(-)
drivers/iio/pressure/bmp280-core.c +103 -61
@@ -29,6 +29,7 @@#include <linux/completion.h> #include <linux/pm_runtime.h> #include <linux/random.h> +#include <asm/unaligned.h> #include "bmp280.h"
@@ -36,7 +37,19 @@* These enums are used for indexing into the array of calibration * coefficients for BMP180. */ -enum {AC1 ,AC2 ,AC3 ,AC4 ,AC5 ,AC6 ,B1 ,B2 ,MB ,MC ,MD }; +enum { +AC1 =0 , +AC2 =2 , +AC3 =4 , +AC4 =6 , +AC5 =8 , +AC6 =10 , +B1 =12 , +B2 =14 , +MB =16 , +MC =18 , +MD =20 , +};struct bmp180_calib {s16 AC1;
@@ -104,6 +117,12 @@ struct bmp280_data {* calculation. */ s32 t_fine; + +/* +* DMA (thus cache coherency maintenance) may require the +* transfer buffers to live in their own cache lines. +*/ +u8 buf[3 ]__aligned(IIO_DMA_MINALIGN) ; };struct bmp280_chip_info {
@@ -135,8 +154,18 @@ struct bmp280_chip_info {* These enums are used for indexing into the array of compensation * parameters for BMP280. */ -enum {T1 ,T2 ,T3 }; -enum {P1 ,P2 ,P3 ,P4 ,P5 ,P6 ,P7 ,P8 ,P9 }; +enum {T1 =0 ,T2 =2 ,T3 =4 }; +enum { +P1 =0 , +P2 =2 , +P3 =4 , +P4 =6 , +P5 =8 , +P6 =10 , +P7 =12 , +P8 =14 , +P9 =16 , +};static const struct iio_chan_spec bmp280_channels[] = { {
@@ -159,51 +188,59 @@ static const struct iio_chan_spec bmp280_channels[] = {static int bmp280_read_calib (struct bmp280_data *data,unsigned int chip) {int ret; +u8 *t_buf; +u8 *p_buf;unsigned int tmp; -__le16 l16; -__be16 b16;struct device *dev = data->dev;struct bmp280_calib *calib = &data->calib.bmp280; -__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT /2 ]; -__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT /2 ]; + t_buf =kmalloc (BMP280_COMP_TEMP_REG_COUNT ,GFP_KERNEL ); +if (!t_buf) +return -ENOMEM ;/* Read temperature calibration values. */ ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_TEMP_START , t_buf,BMP280_COMP_TEMP_REG_COUNT );if (ret <0 ) {dev_err (data->dev,"failed to read temperature calibration parameters\n" ); +kfree (t_buf);return ret; }/* Toss the temperature calibration data into the entropy pool */ add_device_randomness (t_buf,sizeof (t_buf)); - calib->T1 =le16_to_cpu (t_buf[T1 ]); - calib->T2 =le16_to_cpu (t_buf[T2 ]); - calib->T3 =le16_to_cpu (t_buf[T3 ]); + calib->T1 =get_unaligned_le16 (&t_buf[T1 ]); + calib->T2 =get_unaligned_le16 (&t_buf[T2 ]); + calib->T3 =get_unaligned_le16 (&t_buf[T3 ]); +kfree (t_buf); + p_buf =kmalloc (BMP280_COMP_PRESS_REG_COUNT ,GFP_KERNEL ); +if (!p_buf) +return -ENOMEM ;/* Read pressure calibration values. */ ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_PRESS_START , p_buf,BMP280_COMP_PRESS_REG_COUNT );if (ret <0 ) {dev_err (data->dev,"failed to read pressure calibration parameters\n" ); +kfree (p_buf);return ret; }/* Toss the pressure calibration data into the entropy pool */ add_device_randomness (p_buf,sizeof (p_buf)); - calib->P1 =le16_to_cpu (p_buf[P1 ]); - calib->P2 =le16_to_cpu (p_buf[P2 ]); - calib->P3 =le16_to_cpu (p_buf[P3 ]); - calib->P4 =le16_to_cpu (p_buf[P4 ]); - calib->P5 =le16_to_cpu (p_buf[P5 ]); - calib->P6 =le16_to_cpu (p_buf[P6 ]); - calib->P7 =le16_to_cpu (p_buf[P7 ]); - calib->P8 =le16_to_cpu (p_buf[P8 ]); - calib->P9 =le16_to_cpu (p_buf[P9 ]); + calib->P1 =get_unaligned_le16 (&p_buf[P1 ]); + calib->P2 =get_unaligned_le16 (&p_buf[P2 ]); + calib->P3 =get_unaligned_le16 (&p_buf[P3 ]); + calib->P4 =get_unaligned_le16 (&p_buf[P4 ]); + calib->P5 =get_unaligned_le16 (&p_buf[P5 ]); + calib->P6 =get_unaligned_le16 (&p_buf[P6 ]); + calib->P7 =get_unaligned_le16 (&p_buf[P7 ]); + calib->P8 =get_unaligned_le16 (&p_buf[P8 ]); + calib->P9 =get_unaligned_le16 (&p_buf[P9 ]); +kfree (p_buf);/* * Read humidity calibration values.
@@ -222,12 +259,12 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H1 = tmp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H2 , &l16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H2 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H2 comp value\n" );return ret; } - calib->H2 =sign_extend32 (le16_to_cpu (l16),15 ); + calib->H2 =get_unaligned_le16 (data->buf); ret =regmap_read (data->regmap,BMP280_REG_COMP_H3 , &tmp);if (ret <0 ) {
@@ -236,20 +273,20 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H3 = tmp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H4 , &b16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H4 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H4 comp value\n" );return ret; } - calib->H4 =sign_extend32 (((be16_to_cpu (b16) >>4 ) &0xff0 ) | - (be16_to_cpu (b16) &0xf ),11 ); + calib->H4 =sign_extend32 (((get_unaligned_be16 (data->buf) >>4 ) &0xff0 ) | + (get_unaligned_be16 (data->buf) &0xf ),11 ); - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H5 , &l16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H5 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H5 comp value\n" );return ret; } - calib->H5 =sign_extend32 (((le16_to_cpu (l16) >>4 ) &0xfff ),11 ); + calib->H5 =sign_extend32 (((get_unaligned_le16 (data->buf) >>4 ) &0xfff ),11 ); ret =regmap_read (data->regmap,BMP280_REG_COMP_H6 , &tmp);if (ret <0 ) {
@@ -344,16 +381,15 @@ static int bmp280_read_temp(struct bmp280_data *data, int *val) {int ret; -__be32 tmp =0 ;s32 adc_temp, comp_temp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_TEMP_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_TEMP_MSB , data->buf,3 );if (ret <0 ) {dev_err (data->dev,"failed to read temperature\n" );return ret; } - adc_temp =be32_to_cpu (tmp) >>12 ; + adc_temp =get_unaligned_be24 (data->buf) >>4 ;if (adc_temp ==BMP280_TEMP_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading temperature skipped\n" );
@@ -377,7 +413,6 @@ static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2) { int ret; -__be32 tmp =0 ;s32 adc_press;u32 comp_press;
@@ -386,13 +421,13 @@ static int bmp280_read_press(struct bmp280_data *data,if (ret <0 )return ret; - ret =regmap_bulk_read (data->regmap,BMP280_REG_PRESS_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_PRESS_MSB , data->buf,3 );if (ret <0 ) {dev_err (data->dev,"failed to read pressure\n" );return ret; } - adc_press =be32_to_cpu (tmp) >>12 ; + adc_press =get_unaligned_be24 (data->buf) >>4 ;if (adc_press ==BMP280_PRESS_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading pressure skipped\n" );
@@ -408,7 +443,6 @@ static int bmp280_read_press(struct bmp280_data *data,static int bmp280_read_humid (struct bmp280_data *data,int *val,int *val2) { -__be16 tmp;int ret;s32 adc_humidity;u32 comp_humidity;
@@ -418,13 +452,13 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)if (ret <0 )return ret; - ret =regmap_bulk_read (data->regmap,BMP280_REG_HUMIDITY_MSB , &tmp,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_HUMIDITY_MSB , data->buf,2 );if (ret <0 ) {dev_err (data->dev,"failed to read humidity\n" );return ret; } - adc_humidity =be16_to_cpu (tmp); + adc_humidity =get_unaligned_be16 (data->buf);if (adc_humidity ==BMP280_HUMIDITY_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading humidity skipped\n" );
@@ -755,56 +789,65 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)static int bmp180_read_adc_temp (struct bmp280_data *data,int *val) { -__be16 tmp;int ret; ret =bmp180_measure (data,BMP180_MEAS_TEMP );if (ret)return ret; - ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , &tmp,2 ); + ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , data->buf,2 );if (ret)return ret; - *val =be16_to_cpu (tmp); + *val =get_unaligned_be16 (data->buf);return 0 ; }static int bmp180_read_calib (struct bmp280_data *data,unsigned int chip) { +struct bmp180_calib *calib = &data->calib.bmp180; +u8 *buf;int ret;int i; -struct bmp180_calib *calib = &data->calib.bmp180; -__be16 buf[BMP180_REG_CALIB_COUNT /2 ]; + + buf =kmalloc (BMP180_REG_CALIB_COUNT ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; ret =regmap_bulk_read (data->regmap,BMP180_REG_CALIB_START , buf, -sizeof (buf)); +BMP180_REG_CALIB_COUNT ); -if (ret <0 ) +if (ret <0 ) { +kfree (buf);return ret; + }/* None of the words has the value 0 or 0xFFFF */ -for (i =0 ; i <ARRAY_SIZE (buf); i++) { -if (buf[i] ==cpu_to_be16 (0 ) || buf[i] ==cpu_to_be16 (0xffff )) +for (i =0 ; i <BMP180_REG_CALIB_COUNT ; i +=2 ) { +if (get_unaligned_be16 (&buf[i]) ==cpu_to_be16 (0 ) || +get_unaligned_be16 (&buf[i]) ==cpu_to_be16 (0xffff )) { +kfree (buf);return -EIO ; + } }/* Toss the calibration data into the entropy pool */ -add_device_randomness (buf,sizeof (buf)); - - calib->AC1 =be16_to_cpu (buf[AC1 ]); - calib->AC2 =be16_to_cpu (buf[AC2 ]); - calib->AC3 =be16_to_cpu (buf[AC3 ]); - calib->AC4 =be16_to_cpu (buf[AC4 ]); - calib->AC5 =be16_to_cpu (buf[AC5 ]); - calib->AC6 =be16_to_cpu (buf[AC6 ]); - calib->B1 =be16_to_cpu (buf[B1 ]); - calib->B2 =be16_to_cpu (buf[B2 ]); - calib->MB =be16_to_cpu (buf[MB ]); - calib->MC =be16_to_cpu (buf[MC ]); - calib->MD =be16_to_cpu (buf[MD ]); - +add_device_randomness (buf,BMP180_REG_CALIB_COUNT ); + + calib->AC1 =get_unaligned_be16 (&buf[AC1 ]); + calib->AC2 =get_unaligned_be16 (&buf[AC2 ]); + calib->AC3 =get_unaligned_be16 (&buf[AC3 ]); + calib->AC4 =get_unaligned_be16 (&buf[AC4 ]); + calib->AC5 =get_unaligned_be16 (&buf[AC5 ]); + calib->AC6 =get_unaligned_be16 (&buf[AC6 ]); + calib->B1 =get_unaligned_be16 (&buf[B1 ]); + calib->B2 =get_unaligned_be16 (&buf[B2 ]); + calib->MB =get_unaligned_be16 (&buf[MB ]); + calib->MC =get_unaligned_be16 (&buf[MC ]); + calib->MD =get_unaligned_be16 (&buf[MD ]); + +kfree (buf);return 0 ; }
@@ -852,18 +895,17 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val)static int bmp180_read_adc_press (struct bmp280_data *data,int *val) {int ret; -__be32 tmp =0 ;u8 oss = data->oversampling_press; ret =bmp180_measure (data,BMP180_MEAS_PRESS_X (oss));if (ret)return ret; - ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , data->buf,3 );if (ret)return ret; - *val = (be32_to_cpu (tmp) >>8 ) >> (8 - oss); + *val =get_unaligned_be24 (data->buf) >> (8 - oss);return 0 ; }
Signature · 1 line
2.37.1
Re: [PATCH v4 3/5] iio: pressure: bmp280: Fix alignment for DMA safety
Quoted 8 lines · Angel Iglesias
Adds a DMA-safe buffer to driver data struct to store raw data from sensor
Updated calibration read functions to use DMA-safe buffers as well. In this case, as calibration data is only read at sensor initialization, the buffers are allocated with kmalloc and freed after being processed.
Suggested-by Jonathan Cameron <[email protected]>
Jonathan, maybe it's easier to fix in regmap once for all (something like a new flag there dma_safe)? Also a side note: i2c doesn't need that, since it has its own implementation.
P.S. Use sizeof() in regmap calls where it's appropriate.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerRe: [PATCH v4 3/5] iio: pressure: bmp280: Fix alignment for DMA safety
Quoted 8 lines · On Mon, 25 Jul 2022 23:08:47 +0200 Andy Shevchenko <[email protected]>…
Quoted 6 lines · Angel Iglesias
Adds a DMA-safe buffer to driver data struct to store raw data from sensor
Updated calibration read functions to use DMA-safe buffers as well. In this case, as calibration data is only read at sensor initialization, the buffers are allocated with kmalloc and freed after being processed.
Quoted 5 lines
Quoted 1 line
Suggested-by Jonathan Cameron <[email protected]>Jonathan, maybe it's easier to fix in regmap once for all (something like a new flag there
dma_safe)? Also a side note: i2c doesn't need that, since it has its own implementation.
Possibly... Not something I want to tie up with an individual driver though as it might take a while and be (somewhat) controversial. Nice idea though as, if a bounce buffer is needed for a particular regmap operation it can implement one and when not, it can elide that step and just use the provided buffer directly.
Jonathan
Quoted 3 lines
P.S. Usesizeof()in regmap calls where it's appropriate.
Jonathan Cameron
maintainerRe: [PATCH v4 3/5] iio: pressure: bmp280: Fix alignment for DMA safety
In reply to Angel IglesiasQuoted 8 lines · On Sat, 23 Jul 2022 19:39:10 +0200 Angel Iglesias <[email protected]>…
Adds a DMA-safe buffer to driver data struct to store raw data from sensor
Updated calibration read functions to use DMA-safe buffers as well. In this case, as calibration data is only read at sensor initialization, the buffers are allocated with kmalloc and freed after being processed.
Suggested-by Jonathan Cameron <[email protected]>Signed-off-by Angel Iglesias <[email protected]>
A few comments inline. This is more complex than it needs to be because you can still use __be16 *buf; and index in the right steps. Side effect being it's aligned, so conversions will be cheaper as well.
Quoted 23 lines
--- drivers/iio/pressure/bmp280-core.c | 164 ++++++++++++++++++----------- 1 file changed, 103 insertions(+), 61 deletions(-)drivers/iio/pressure/bmp280-core.c +3
-1@@ -29,6 +29,7 @@#include <linux/completion.h> #include <linux/pm_runtime.h> #include <linux/random.h> +#include <asm/unaligned.h> #include "bmp280.h" @@ -36,7 +37,19 @@* These enums are used for indexing into the array of calibration * coefficients for BMP180. */ -enum {AC1 ,AC2 ,AC3 ,AC4 ,AC5 ,AC6 ,B1 ,B2 ,MB ,MC ,MD }; +enum { +AC1 =0 ,
drivers/iio/pressure/bmp280-core.c line 51
Quoted 10 lines
drivers/iio/pressure/bmp280-core.c +10
-0+AC2 =2 , +AC3 =4 , +AC4 =6 , +AC5 =8 , +AC6 =10 , +B1 =12 , +B2 =14 , +MB =16 , +MC =18 , + MD = 20,
Looks like we are specifying a size wrong somewhere as these are all multiples of 2.
Quoted 5 lines
drivers/iio/pressure/bmp280-core.c +1
-0+};struct bmp180_calib {s16 AC1;
drivers/iio/pressure/bmp280-core.c struct bmp280_data { · line 125
Quoted 9 lines
drivers/iio/pressure/bmp280-core.c +6
-0@@ -104,6 +117,12 @@ struct bmp280_data {* calculation. */ s32 t_fine; + +/* +* DMA (thus cache coherency maintenance) may require the +* transfer buffers to live in their own cache lines. +*/ +u8 buf[3 ]__aligned(IIO_DMA_MINALIGN) ;
So, on many platforms of interest (anything arm64) we are going to be padding this struture out after this to 128 bytes anyway. So no advantage to keeping this short. I'd just use this buffer for the calibration cases as well and drop the extra allocations. You'll waste space on x86/x86_64 but meh, who cares about those legacy architectures :)
Quoted 230 lines
drivers/iio/pressure/bmp280-core.c +51
-37};struct bmp280_chip_info {@@ -135,8 +154,18 @@ struct bmp280_chip_info {* These enums are used for indexing into the array of compensation * parameters for BMP280. */ -enum {T1 ,T2 ,T3 }; -enum {P1 ,P2 ,P3 ,P4 ,P5 ,P6 ,P7 ,P8 ,P9 }; +enum {T1 =0 ,T2 =2 ,T3 =4 }; +enum { +P1 =0 , +P2 =2 , +P3 =4 , +P4 =6 , +P5 =8 , +P6 =10 , +P7 =12 , +P8 =14 , +P9 =16 , +};static const struct iio_chan_spec bmp280_channels[] = { {@@ -159,51 +188,59 @@ static const struct iio_chan_spec bmp280_channels[] = {static int bmp280_read_calib (struct bmp280_data *data,unsigned int chip) {int ret; +u8 *t_buf; +u8 *p_buf;unsigned int tmp; -__le16 l16; -__be16 b16;struct device *dev = data->dev;struct bmp280_calib *calib = &data->calib.bmp280; -__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT /2 ]; -__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT /2 ]; + t_buf =kmalloc (BMP280_COMP_TEMP_REG_COUNT ,GFP_KERNEL ); +if (!t_buf) +return -ENOMEM ;/* Read temperature calibration values. */ ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_TEMP_START , t_buf,BMP280_COMP_TEMP_REG_COUNT );if (ret <0 ) {dev_err (data->dev,"failed to read temperature calibration parameters\n" ); +kfree (t_buf);return ret; }/* Toss the temperature calibration data into the entropy pool */ add_device_randomness (t_buf,sizeof (t_buf)); - calib->T1 =le16_to_cpu (t_buf[T1 ]); - calib->T2 =le16_to_cpu (t_buf[T2 ]); - calib->T3 =le16_to_cpu (t_buf[T3 ]); + calib->T1 =get_unaligned_le16 (&t_buf[T1 ]); + calib->T2 =get_unaligned_le16 (&t_buf[T2 ]); + calib->T3 =get_unaligned_le16 (&t_buf[T3 ]); +kfree (t_buf); + p_buf =kmalloc (BMP280_COMP_PRESS_REG_COUNT ,GFP_KERNEL ); +if (!p_buf) +return -ENOMEM ;/* Read pressure calibration values. */ ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_PRESS_START , p_buf,BMP280_COMP_PRESS_REG_COUNT );if (ret <0 ) {dev_err (data->dev,"failed to read pressure calibration parameters\n" ); +kfree (p_buf);return ret; }/* Toss the pressure calibration data into the entropy pool */ add_device_randomness (p_buf,sizeof (p_buf)); - calib->P1 =le16_to_cpu (p_buf[P1 ]); - calib->P2 =le16_to_cpu (p_buf[P2 ]); - calib->P3 =le16_to_cpu (p_buf[P3 ]); - calib->P4 =le16_to_cpu (p_buf[P4 ]); - calib->P5 =le16_to_cpu (p_buf[P5 ]); - calib->P6 =le16_to_cpu (p_buf[P6 ]); - calib->P7 =le16_to_cpu (p_buf[P7 ]); - calib->P8 =le16_to_cpu (p_buf[P8 ]); - calib->P9 =le16_to_cpu (p_buf[P9 ]); + calib->P1 =get_unaligned_le16 (&p_buf[P1 ]); + calib->P2 =get_unaligned_le16 (&p_buf[P2 ]); + calib->P3 =get_unaligned_le16 (&p_buf[P3 ]); + calib->P4 =get_unaligned_le16 (&p_buf[P4 ]); + calib->P5 =get_unaligned_le16 (&p_buf[P5 ]); + calib->P6 =get_unaligned_le16 (&p_buf[P6 ]); + calib->P7 =get_unaligned_le16 (&p_buf[P7 ]); + calib->P8 =get_unaligned_le16 (&p_buf[P8 ]); + calib->P9 =get_unaligned_le16 (&p_buf[P9 ]); +kfree (p_buf);/* * Read humidity calibration values. @@ -222,12 +259,12 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H1 = tmp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H2 , &l16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H2 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H2 comp value\n" );return ret; } - calib->H2 =sign_extend32 (le16_to_cpu (l16),15 ); + calib->H2 =get_unaligned_le16 (data->buf); ret =regmap_read (data->regmap,BMP280_REG_COMP_H3 , &tmp);if (ret <0 ) {@@ -236,20 +273,20 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H3 = tmp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H4 , &b16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H4 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H4 comp value\n" );return ret; } - calib->H4 =sign_extend32 (((be16_to_cpu (b16) >>4 ) &0xff0 ) | - (be16_to_cpu (b16) &0xf ),11 ); + calib->H4 =sign_extend32 (((get_unaligned_be16 (data->buf) >>4 ) &0xff0 ) | + (get_unaligned_be16 (data->buf) &0xf ),11 ); - ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H5 , &l16,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_COMP_H5 , data->buf,2 );if (ret <0 ) {dev_err (dev,"failed to read H5 comp value\n" );return ret; } - calib->H5 =sign_extend32 (((le16_to_cpu (l16) >>4 ) &0xfff ),11 ); + calib->H5 =sign_extend32 (((get_unaligned_le16 (data->buf) >>4 ) &0xfff ),11 ); ret =regmap_read (data->regmap,BMP280_REG_COMP_H6 , &tmp);if (ret <0 ) {@@ -344,16 +381,15 @@ static int bmp280_read_temp(struct bmp280_data *data, int *val) {int ret; -__be32 tmp =0 ;s32 adc_temp, comp_temp; - ret =regmap_bulk_read (data->regmap,BMP280_REG_TEMP_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_TEMP_MSB , data->buf,3 );if (ret <0 ) {dev_err (data->dev,"failed to read temperature\n" );return ret; } - adc_temp =be32_to_cpu (tmp) >>12 ; + adc_temp =get_unaligned_be24 (data->buf) >>4 ;if (adc_temp ==BMP280_TEMP_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading temperature skipped\n" );@@ -377,7 +413,6 @@ static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2) { int ret; -__be32 tmp =0 ;s32 adc_press;u32 comp_press;@@ -386,13 +421,13 @@ static int bmp280_read_press(struct bmp280_data *data,if (ret <0 )return ret; - ret =regmap_bulk_read (data->regmap,BMP280_REG_PRESS_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_PRESS_MSB , data->buf,3 );if (ret <0 ) {dev_err (data->dev,"failed to read pressure\n" );return ret; } - adc_press =be32_to_cpu (tmp) >>12 ; + adc_press =get_unaligned_be24 (data->buf) >>4 ;if (adc_press ==BMP280_PRESS_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading pressure skipped\n" );@@ -408,7 +443,6 @@ static int bmp280_read_press(struct bmp280_data *data,static int bmp280_read_humid (struct bmp280_data *data,int *val,int *val2) { -__be16 tmp;int ret;s32 adc_humidity;u32 comp_humidity;@@ -418,13 +452,13 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)if (ret <0 )return ret; - ret =regmap_bulk_read (data->regmap,BMP280_REG_HUMIDITY_MSB , &tmp,2 ); + ret =regmap_bulk_read (data->regmap,BMP280_REG_HUMIDITY_MSB , data->buf,2 );if (ret <0 ) {dev_err (data->dev,"failed to read humidity\n" );return ret; } - adc_humidity =be16_to_cpu (tmp); + adc_humidity =get_unaligned_be16 (data->buf);if (adc_humidity ==BMP280_HUMIDITY_SKIPPED ) {/* reading was skipped */ dev_err (data->dev,"reading humidity skipped\n" );@@ -755,56 +789,65 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)static int bmp180_read_adc_temp (struct bmp280_data *data,int *val) { -__be16 tmp;int ret; ret =bmp180_measure (data,BMP180_MEAS_TEMP );if (ret)return ret; - ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , &tmp,2 ); + ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , data->buf,2 );if (ret)return ret; - *val =be16_to_cpu (tmp); + *val =get_unaligned_be16 (data->buf);return 0 ; }
drivers/iio/pressure/bmp280-core.c static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas) · line 814
Quoted 10 lines
drivers/iio/pressure/bmp280-core.c +4
-2static int bmp180_read_calib (struct bmp280_data *data,unsigned int chip) { +struct bmp180_calib *calib = &data->calib.bmp180; +u8 *buf;int ret;int i; -struct bmp180_calib *calib = &data->calib.bmp180; -__be16 buf[BMP180_REG_CALIB_COUNT /2 ]; + + buf =kmalloc (BMP180_REG_CALIB_COUNT ,GFP_KERNEL );
No need to change the type of buf. Keep it as __be16 *buf = kmalloc()... That way your indexing still works nicely without needing to scatter *2 and += 2 throughout. Note that if you embed this as I suggest above, just use a union so you can overlap it with the u8 buffer.
Quoted 78 lines
drivers/iio/pressure/bmp280-core.c +28
-21+if (!buf) +return -ENOMEM ; ret =regmap_bulk_read (data->regmap,BMP180_REG_CALIB_START , buf, -sizeof (buf)); +BMP180_REG_CALIB_COUNT ); -if (ret <0 ) +if (ret <0 ) { +kfree (buf);return ret; + }/* None of the words has the value 0 or 0xFFFF */ -for (i =0 ; i <ARRAY_SIZE (buf); i++) { -if (buf[i] ==cpu_to_be16 (0 ) || buf[i] ==cpu_to_be16 (0xffff )) +for (i =0 ; i <BMP180_REG_CALIB_COUNT ; i +=2 ) { +if (get_unaligned_be16 (&buf[i]) ==cpu_to_be16 (0 ) || +get_unaligned_be16 (&buf[i]) ==cpu_to_be16 (0xffff )) { +kfree (buf);return -EIO ; + } }/* Toss the calibration data into the entropy pool */ -add_device_randomness (buf,sizeof (buf)); - - calib->AC1 =be16_to_cpu (buf[AC1 ]); - calib->AC2 =be16_to_cpu (buf[AC2 ]); - calib->AC3 =be16_to_cpu (buf[AC3 ]); - calib->AC4 =be16_to_cpu (buf[AC4 ]); - calib->AC5 =be16_to_cpu (buf[AC5 ]); - calib->AC6 =be16_to_cpu (buf[AC6 ]); - calib->B1 =be16_to_cpu (buf[B1 ]); - calib->B2 =be16_to_cpu (buf[B2 ]); - calib->MB =be16_to_cpu (buf[MB ]); - calib->MC =be16_to_cpu (buf[MC ]); - calib->MD =be16_to_cpu (buf[MD ]); - +add_device_randomness (buf,BMP180_REG_CALIB_COUNT ); + + calib->AC1 =get_unaligned_be16 (&buf[AC1 ]); + calib->AC2 =get_unaligned_be16 (&buf[AC2 ]); + calib->AC3 =get_unaligned_be16 (&buf[AC3 ]); + calib->AC4 =get_unaligned_be16 (&buf[AC4 ]); + calib->AC5 =get_unaligned_be16 (&buf[AC5 ]); + calib->AC6 =get_unaligned_be16 (&buf[AC6 ]); + calib->B1 =get_unaligned_be16 (&buf[B1 ]); + calib->B2 =get_unaligned_be16 (&buf[B2 ]); + calib->MB =get_unaligned_be16 (&buf[MB ]); + calib->MC =get_unaligned_be16 (&buf[MC ]); + calib->MD =get_unaligned_be16 (&buf[MD ]); + +kfree (buf);return 0 ; }@@ -852,18 +895,17 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val)static int bmp180_read_adc_press (struct bmp280_data *data,int *val) {int ret; -__be32 tmp =0 ;u8 oss = data->oversampling_press; ret =bmp180_measure (data,BMP180_MEAS_PRESS_X (oss));if (ret)return ret; - ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , &tmp,3 ); + ret =regmap_bulk_read (data->regmap,BMP180_REG_OUT_MSB , data->buf,3 );if (ret)return ret; - *val = (be32_to_cpu (tmp) >>8 ) >> (8 - oss); + *val =get_unaligned_be24 (data->buf) >> (8 - oss);return 0 ; }
Re: [PATCH v4 3/5] iio: pressure: bmp280: Fix alignment for DMA safety
Quoted 63 lines · Jonathan Cameron
Quoted 8 lines · On Sat, 23 Jul 2022 19:39:10 +0200 Angel Iglesias <[email protected]>…
Adds a DMA-safe buffer to driver data struct to store raw data from sensor
Updated calibration read functions to use DMA-safe buffers as well. In this case, as calibration data is only read at sensor initialization, the buffers are allocated with kmalloc and freed after being processed.
Suggested-by Jonathan Cameron <[email protected]>Signed-off-by Angel Iglesias <[email protected]>A few comments inline. This is more complex than it needs to be because you can still use
__be16*buf; and index in the right steps. Side effect being it's aligned, so conversions will be cheaper as well.Quoted 24 lines
---
drivers/iio/pressure/bmp280-core.c | 164 ++++++++++++++++++-----------1 file changed, 103 insertions(+), 61 deletions(-)drivers/iio/pressure/bmp280-core.c +1
-0@@ -29,6 +29,7 @@ #include <linux/completion.h> #include <linux/pm_runtime.h> #include <linux/random.h> +#include <asm/unaligned.h>
#include "bmp280.h"drivers/iio/pressure/bmp280-core.c +2
-1@@ -36,7 +37,19 @@* These enums are used for indexing into the array of calibration * coefficients for BMP180. */ -enum {AC1 ,AC2 ,AC3 ,AC4 ,AC5 ,AC6 ,B1 ,B2 ,MB ,MC ,MD }; +enum { + AC1 = 0,drivers/iio/pressure/bmp280-core.c line 51
Quoted 10 lines
drivers/iio/pressure/bmp280-core.c +10
-0+ AC2 = 2, + AC3 = 4, + AC4 = 6, + AC5 = 8, + AC6 = 10, + B1 = 12, + B2 = 14, + MB = 16, + MC = 18, + MD = 20,Looks like we are specifying a size wrong somewhere as these are all multiples of 2.
Quoted 11 lines
drivers/iio/pressure/bmp280-core.c +1
-0+};
structbmp180_calib{
s16 AC1;drivers/iio/pressure/bmp280-core.c +3
-0@@ -104,6 +117,12 @@ struct bmp280_data {* calculation. */ s32 t_fine; + + /* +* DMA (thus cache coherency maintenance) may require the
Quoted 8 lines
drivers/iio/pressure/bmp280-core.c struct bmp280_data { · line 125
Quoted 3 lines
drivers/iio/pressure/bmp280-core.c +3
-0+* transfer buffers to live in their own cache lines. +*/ + u8 buf[3] __aligned(IIO_DMA_MINALIGN);So, on many platforms of interest (anything arm64) we are going to be padding this struture out after this to 128 bytes anyway. So no advantage to keeping this short. I'd just use this buffer for the calibration cases as well and drop the extra allocations. You'll waste space on x86/x86_64 but meh, who cares about those legacy architectures :)
Oh I see, I tried to save memory only using this 3 bytes to read the adcs raw values from the sensors and allocating dynamically the buffers to read calibration values and ended wasting more memory :(
Thanks for the insight, I'll do my homework and study in more detail DMA and memory allocation in this platforms.
Yes, this will probably end running on fancier archs like arm64 or risc-v :) (I'm using a couple of arm64 boards to test the sensors)
Quoted 337 lines
Quoted 249 lines
drivers/iio/pressure/bmp280-core.c +0
-0};
structbmp280_chip_info{drivers/iio/pressure/bmp280-core.c +12
-2@@ -135,8 +154,18 @@ struct bmp280_chip_info {* These enums are used for indexing into the array of compensation * parameters for BMP280. */ -enum {T1 ,T2 ,T3 }; -enum {P1 ,P2 ,P3 ,P4 ,P5 ,P6 ,P7 ,P8 ,P9 }; +enum {T1 =0 ,T2 =2 ,T3 =4 }; +enum { + P1 = 0, + P2 = 2, + P3 = 4, + P4 = 6, + P5 = 8, + P6 = 10, + P7 = 12, + P8 = 14, + P9 = 16, +};
static const structiio_chan_specbmp280_channels[] = { {drivers/iio/pressure/bmp280-core.c +2
-4@@ -159,51 +188,59 @@ static const struct iio_chan_spec bmp280_channels[] = { static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) { int ret; + u8 *t_buf; + u8 *p_buf; unsigned int tmp; - __le16 l16; - __be16 b16; struct device *dev = data->dev; struct bmp280_calib *calib = &data->calib.bmp280; - __le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2]; - __le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
+ t_buf =kmalloc(BMP280_COMP_TEMP_REG_COUNT, GFP_KERNEL); + if (!t_buf)
+ return-ENOMEM;
/* Read temperature calibration values. */
ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START, t_buf, BMP280_COMP_TEMP_REG_COUNT); if (ret < 0) {
dev_err(data->dev,
"failed to read temperature calibration parameters\n");
+ kfree(t_buf);
return ret;
}
/* Toss the temperature calibration data into the entropy pool */add_device_randomness(t_buf, sizeof(t_buf));
- calib->T1 =le16_to_cpu(t_buf[T1]);
- calib->T2 =le16_to_cpu(t_buf[T2]);
- calib->T3 =le16_to_cpu(t_buf[T3]);
+ calib->T1 =get_unaligned_le16(&t_buf[T1]);
+ calib->T2 =get_unaligned_le16(&t_buf[T2]);
+ calib->T3 =get_unaligned_le16(&t_buf[T3]);
+ kfree(t_buf);
+ p_buf =kmalloc(BMP280_COMP_PRESS_REG_COUNT, GFP_KERNEL); + if (!p_buf)
+ return-ENOMEM;
/* Read pressure calibration values. */
ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START, p_buf, BMP280_COMP_PRESS_REG_COUNT); if (ret < 0) {
dev_err(data->dev,
"failed to read pressure calibration parameters\n"); + kfree(p_buf);
return ret;
}
/* Toss the pressure calibration data into the entropy pool */add_device_randomness(p_buf, sizeof(p_buf));
- calib->P1 =le16_to_cpu(p_buf[P1]);
- calib->P2 =le16_to_cpu(p_buf[P2]);
- calib->P3 =le16_to_cpu(p_buf[P3]);
- calib->P4 =le16_to_cpu(p_buf[P4]);
- calib->P5 =le16_to_cpu(p_buf[P5]);
- calib->P6 =le16_to_cpu(p_buf[P6]);
- calib->P7 =le16_to_cpu(p_buf[P7]);
- calib->P8 =le16_to_cpu(p_buf[P8]);
- calib->P9 =le16_to_cpu(p_buf[P9]);
+ calib->P1 =get_unaligned_le16(&p_buf[P1]);
+ calib->P2 =get_unaligned_le16(&p_buf[P2]);
+ calib->P3 =get_unaligned_le16(&p_buf[P3]);
+ calib->P4 =get_unaligned_le16(&p_buf[P4]);
+ calib->P5 =get_unaligned_le16(&p_buf[P5]);
+ calib->P6 =get_unaligned_le16(&p_buf[P6]);
+ calib->P7 =get_unaligned_le16(&p_buf[P7]);
+ calib->P8 =get_unaligned_le16(&p_buf[P8]);
+ calib->P9 =get_unaligned_le16(&p_buf[P9]);
+ kfree(p_buf);
/*
* Read humidity calibration values.drivers/iio/pressure/bmp280-core.c +0
-0@@ -222,12 +259,12 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H1 = tmp;
- ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &l16, 2); + ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, data->buf, 2);
if (ret < 0) {
dev_err(dev, "failed to read H2 comp value\n"); return ret;
}
- calib->H2 =sign_extend32(le16_to_cpu(l16), 15);
+ calib->H2 =get_unaligned_le16(data->buf);
ret =regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp); if (ret < 0) {drivers/iio/pressure/bmp280-core.c +0
-0@@ -236,20 +273,20 @@ static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip) } calib->H3 = tmp;
- ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &b16, 2); + ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, data->buf, 2);
if (ret < 0) {
dev_err(dev, "failed to read H4 comp value\n"); return ret;
}
- calib->H4 = sign_extend32(((be16_to_cpu(b16) >> 4) &0xff0) | - (be16_to_cpu(b16)&0xf), 11); + calib->H4 = sign_extend32(((get_unaligned_be16(data->buf) >> 4) &0xff0) |
+ (get_unaligned_be16(data->buf)&0xf), 11);
- ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &l16, 2); + ret =regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, data->buf, 2);
if (ret < 0) {
dev_err(dev, "failed to read H5 comp value\n"); return ret;
}
- calib->H5 =sign_extend32(((le16_to_cpu(l16) >> 4) & 0xfff), 11); + calib->H5 =sign_extend32(((get_unaligned_le16(data->buf) >> 4) & 0xfff), 11);
ret =regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp); if (ret < 0) {drivers/iio/pressure/bmp280-core.c +0
-1@@ -344,16 +381,15 @@ static int bmp280_read_temp(struct bmp280_data *data, int *val) { int ret; - __be32 tmp = 0; s32 adc_temp, comp_temp;
- ret =regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB, &tmp, 3); + ret =regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB, data->buf, 3);
if (ret < 0) {
dev_err(data->dev, "failed to read temperature\n"); return ret;
}
- adc_temp =be32_to_cpu(tmp)>> 12;
+ adc_temp =get_unaligned_be24(data->buf)>> 4;
if (adc_temp==BMP280_TEMP_SKIPPED) {
/* reading was skipped */
dev_err(data->dev, "reading temperature skipped\n");drivers/iio/pressure/bmp280-core.c +0
-1@@ -377,7 +413,6 @@ static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2) { int ret; - __be32 tmp = 0; s32 adc_press; u32 comp_press;drivers/iio/pressure/bmp280-core.c +0
-0@@ -386,13 +421,13 @@ static int bmp280_read_press(struct bmp280_data *data, if (ret < 0) return ret;
- ret =regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB, &tmp, 3); + ret = regmap_bulk_read(data->regmap,BMP280_REG_PRESS_MSB, data-Quoted 1 line
buf, 3);
if (ret < 0) {
dev_err(data->dev, "failed to read pressure\n"); return ret;
}
- adc_press =be32_to_cpu(tmp)>> 12;
+ adc_press =get_unaligned_be24(data->buf)>> 4; if (adc_press==BMP280_PRESS_SKIPPED) {
/* reading was skipped */dev_err(data->dev, "reading pressure skipped\n");drivers/iio/pressure/bmp280-core.c +0
-0@@ -408,7 +443,6 @@ static int bmp280_read_press(struct bmp280_data *data,
static intbmp280_read_humid(struct bmp280_data *data, int *val, int *val2){
- __be16 tmp;
int ret;
s32adc_humidity;
u32comp_humidity;drivers/iio/pressure/bmp280-core.c +0
-0@@ -418,13 +452,13 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2) if (ret < 0) return ret;
- ret =regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB, &tmp, 2);
+ ret = regmap_bulk_read(data->regmap,BMP280_REG_HUMIDITY_MSB, data-Quoted 1 line
buf, 2);
if (ret < 0) {
dev_err(data->dev, "failed to read humidity\n"); return ret;
}
- adc_humidity =be16_to_cpu(tmp);
+ adc_humidity =get_unaligned_be16(data->buf); if (adc_humidity==BMP280_HUMIDITY_SKIPPED) { /* reading was skipped */dev_err(data->dev, "reading humidity skipped\n");drivers/iio/pressure/bmp280-core.c +0
-0@@ -755,56 +789,65 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
static intbmp180_read_adc_temp(struct bmp280_data *data, int *val){
- __be16 tmp;
int ret;
ret =bmp180_measure(data, BMP180_MEAS_TEMP);
if (ret)
return ret;
- ret =regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB, &tmp, 2); + ret =regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB, data->buf, 2);
if (ret)
return ret;
- *val =be16_to_cpu(tmp);
+ *val =get_unaligned_be16(data->buf);
return 0;
}
static intbmp180_read_calib(struct bmp280_data *data, unsigned int chip){Quoted 8 lines
+ struct
bmp180_calib*calib =&data->calib.bmp180; + u8 *buf;
int ret;
int i;
- structbmp180_calib*calib =&data->calib.bmp180; - __be16 buf[BMP180_REG_CALIB_COUNT / 2];
+
+ buf =kmalloc(BMP180_REG_CALIB_COUNT, GFP_KERNEL);No need to change the type of buf. Keep it as
__be16*buf =kmalloc()... That way your indexing still works nicely without needing to scatter *2 and += 2
throughout. Note that if you embed this as I suggest above, just use a union so
you can overlap it with the u8 buffer.Quoted 73 lines
+ if (!buf)
+ return-ENOMEM;
ret = regmap_bulk_read(data->regmap,BMP180_REG_CALIB_START, buf, - sizeof(buf));
+BMP180_REG_CALIB_COUNT);
- if (ret < 0)
+ if (ret < 0) {
+ kfree(buf);
return ret;
+ }
/* None of the words has the value 0 or0xFFFF*/
- for (i = 0; i <ARRAY_SIZE(buf); i++) {
- if (buf[i] ==cpu_to_be16(0)|| buf[i] ==cpu_to_be16(0xffff))
+ for (i = 0; i <BMP180_REG_CALIB_COUNT; i += 2) {
+ if (get_unaligned_be16(&buf[i])==cpu_to_be16(0)|| +get_unaligned_be16(&buf[i])==cpu_to_be16(0xffff)) { + kfree(buf);
return-EIO;
+ }
}
/* Toss the calibration data into the entropy pool */ - add_device_randomness(buf, sizeof(buf));drivers/iio/pressure/bmp280-core.c +15
-13- - calib->AC1 = be16_to_cpu(buf[AC1]); - calib->AC2 = be16_to_cpu(buf[AC2]); - calib->AC3 = be16_to_cpu(buf[AC3]); - calib->AC4 = be16_to_cpu(buf[AC4]); - calib->AC5 = be16_to_cpu(buf[AC5]); - calib->AC6 = be16_to_cpu(buf[AC6]); - calib->B1 = be16_to_cpu(buf[B1]); - calib->B2 = be16_to_cpu(buf[B2]); - calib->MB = be16_to_cpu(buf[MB]); - calib->MC = be16_to_cpu(buf[MC]); - calib->MD = be16_to_cpu(buf[MD]); - + add_device_randomness(buf, BMP180_REG_CALIB_COUNT); + + calib->AC1 = get_unaligned_be16(&buf[AC1]); + calib->AC2 = get_unaligned_be16(&buf[AC2]); + calib->AC3 = get_unaligned_be16(&buf[AC3]); + calib->AC4 = get_unaligned_be16(&buf[AC4]); + calib->AC5 = get_unaligned_be16(&buf[AC5]); + calib->AC6 = get_unaligned_be16(&buf[AC6]); + calib->B1 = get_unaligned_be16(&buf[B1]); + calib->B2 = get_unaligned_be16(&buf[B2]); + calib->MB = get_unaligned_be16(&buf[MB]); + calib->MC = get_unaligned_be16(&buf[MC]); + calib->MD = get_unaligned_be16(&buf[MD]); + + kfree(buf);return 0;
}drivers/iio/pressure/bmp280-core.c +0
-1@@ -852,18 +895,17 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val) static int bmp180_read_adc_press(struct bmp280_data *data, int *val) { int ret; - __be32 tmp = 0; u8 oss = data->oversampling_press;
ret =bmp180_measure(data, BMP180_MEAS_PRESS_X(oss)); if (ret)
return ret;
- ret =regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB, &tmp, 3); + ret =regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB, data->buf, 3);
Quoted 9 lines
Quoted 8 lines
if (ret)
return ret;
- *val = (be32_to_cpu(tmp)>> 8) >> (8 - oss); + *val =get_unaligned_be24(data->buf)>> (8 - oss);
return 0;
}
As always, thanks for your comments,
Kind regards,
Angel
PATCH v4 4/5 iio: pressure: bmp280: Add support for BMP380 sensor family
In reply to Angel IglesiasAdds compatibility with the new generation of this sensor, the BMP380
Includes basic sensor initialization to do pressure and temp measurements and allows tuning oversampling settings for each channel.
The compensation algorithms are adapted from the device datasheet and the repository https://github.com/BoschSensortec/BMP3-Sensor-API
--- drivers/iio/pressure/Kconfig | 6 +- drivers/iio/pressure/bmp280-core.c | 359 +++++++++++++++++++++++++++ drivers/iio/pressure/bmp280-i2c.c | 5 + drivers/iio/pressure/bmp280-regmap.c | 55 ++++ drivers/iio/pressure/bmp280-spi.c | 5 + drivers/iio/pressure/bmp280.h | 101 ++++++++ 6 files changed, 528 insertions(+), 3 deletions(-)
drivers/iio/pressure/Kconfig +3 -3
@@ -17,14 +17,14 @@ config ABP060MG will be called abp060mg. config BMP280 - tristate "Bosch Sensortec BMP180/BMP280 pressure sensor I2C driver" + tristate "Bosch Sensortec BMP180/BMP280/BMP380 pressure sensor I2C driver" depends on (I2C || SPI_MASTER) select REGMAP select BMP280_I2C if (I2C) select BMP280_SPI if (SPI_MASTER) help - Say yes here to build support for Bosch Sensortec BMP180 and BMP280 - pressure and temperature sensors. Also supports the BME280 with + Say yes here to build support for Bosch Sensortec BMP180, BMP280 and + BMP380 pressure and temperature sensors. Also supports the BME280 with an additional humidity sensor channel. To compile this driver as a module, choose M here: the core module
drivers/iio/pressure/bmp280-core.c +359 -0
405 lines not on this page · raw message
drivers/iio/pressure/bmp280-i2c.c +5 -0
@@ -19,6 +19,9 @@ static int bmp280_i2c_probe(struct i2c_client *client,case BME280_CHIP_ID : regmap_config = &bmp280_regmap_config;break ; +case BMP380_CHIP_ID : + regmap_config = &bmp380_regmap_config; +break ;default :return -EINVAL ; }
@@ -37,6 +40,7 @@ static int bmp280_i2c_probe(struct i2c_client *client, }static const struct of_device_id bmp280_of_i2c_match[] = { + { .compatible ="bosch,bmp380" , .data = (void *)BMP380_CHIP_ID }, { .compatible ="bosch,bme280" , .data = (void *)BME280_CHIP_ID }, { .compatible ="bosch,bmp280" , .data = (void *)BMP280_CHIP_ID }, { .compatible ="bosch,bmp180" , .data = (void *)BMP180_CHIP_ID },
@@ -46,6 +50,7 @@ static const struct of_device_id bmp280_of_i2c_match[] = {MODULE_DEVICE_TABLE (of, bmp280_of_i2c_match);static const struct i2c_device_id bmp280_i2c_id[] = { + {"bmp380" ,BMP380_CHIP_ID }, {"bmp280" ,BMP280_CHIP_ID }, {"bmp180" ,BMP180_CHIP_ID }, {"bmp085" ,BMP180_CHIP_ID },
drivers/iio/pressure/bmp280-regmap.c +55 -0
64 lines not on this page · raw message
drivers/iio/pressure/bmp280-spi.c +5 -0
@@ -66,6 +66,9 @@ static int bmp280_spi_probe(struct spi_device *spi)case BME280_CHIP_ID : regmap_config = &bmp280_regmap_config;break ; +case BMP380_CHIP_ID : + regmap_config = &bmp380_regmap_config; +break ;default :return -EINVAL ; }
@@ -92,6 +95,7 @@ static const struct of_device_id bmp280_of_spi_match[] = { { .compatible = "bosch,bmp181", }, { .compatible = "bosch,bmp280", }, { .compatible = "bosch,bme280", }, + { .compatible = "bosch,bmp380", }, { }, }; MODULE_DEVICE_TABLE(of, bmp280_of_spi_match);
@@ -101,6 +105,7 @@ static const struct spi_device_id bmp280_spi_id[] = { { "bmp181", BMP180_CHIP_ID }, { "bmp280", BMP280_CHIP_ID }, { "bme280", BME280_CHIP_ID }, + { "bmp380", BMP380_CHIP_ID }, { } };MODULE_DEVICE_TABLE (spi, bmp280_spi_id);
drivers/iio/pressure/bmp280.h +101 -0
119 lines not on this page · raw message
Signature · 1 line
2.37.1
kernel test robot
build botRe: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
Thank you for the patch! Yet something to improve:
Hi Angel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on 180c6cb6b9b79c55b79e8414f4c0208f2463af7d]
url: https://github.com/intel-lab-lkp/linux/commits/Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 base: 180c6cb6b9b79c55b79e8414f4c0208f2463af7d config: arm-buildonly-randconfig-r003-20220724 (https://download.01.org/0day-ci/archive/20220724/[email protected]/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 12fbd2d377e396ad61bce56d71c98a1eb1bebfa9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/eb385b5ff634acc29a3d9f20847e62ec73484fcb
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 git checkout eb385b5ff634acc29a3d9f20847e62ec73484fcb
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __aeabi_ldivmod >>> referenced by bmp280-core.c:887 (drivers/iio/pressure/bmp280-core.c:887) >>> iio/pressure/bmp280-core.o:(bmp380_read_press) in archive drivers/built-in.a >>> did you mean: __aeabi_idivmod >>> defined in: arch/arm/lib/lib.a(lib1funcs.o)
pahole: .tmp_vmlinux.btf: No such file or directory ld.lld: error: .btf.vmlinux.bin.o: unknown file type
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
kernel test robot
build botRe: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
In reply to Angel IglesiasThank you for the patch! Yet something to improve:
Hi Angel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on 180c6cb6b9b79c55b79e8414f4c0208f2463af7d]
url: https://github.com/intel-lab-lkp/linux/commits/Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 base: 180c6cb6b9b79c55b79e8414f4c0208f2463af7d config: m68k-randconfig-r025-20220724 (https://download.01.org/0day-ci/archive/20220724/[email protected]/config) compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/eb385b5ff634acc29a3d9f20847e62ec73484fcb
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 git checkout eb385b5ff634acc29a3d9f20847e62ec73484fcb
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__divdi3" [drivers/iio/pressure/bmp280.ko] undefined!
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
kernel test robot
build botRe: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
In reply to Angel IglesiasThank you for the patch! Yet something to improve:
Hi Angel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on 180c6cb6b9b79c55b79e8414f4c0208f2463af7d]
url: https://github.com/intel-lab-lkp/linux/commits/Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 base: 180c6cb6b9b79c55b79e8414f4c0208f2463af7d config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220724/[email protected]/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 12fbd2d377e396ad61bce56d71c98a1eb1bebfa9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/eb385b5ff634acc29a3d9f20847e62ec73484fcb
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Angel-Iglesias/Add-support-for-pressure-sensor-Bosch-BMP380/20220724-014146 git checkout eb385b5ff634acc29a3d9f20847e62ec73484fcb
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__divdi3" [drivers/iio/pressure/bmp280.ko] undefined!
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Re: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
In reply to Angel IglesiasQuoted 8 lines · Angel Iglesias
Adds compatibility with the new generation of this sensor, the BMP380
Includes basic sensor initialization to do pressure and temp measurements and allows tuning oversampling settings for each channel.
The compensation algorithms are adapted from the device datasheet and the repository https://github.com/BoschSensortec/BMP3-Sensor-API
...
Quoted 1 line
+ /* wait for 2ms for command to be proccessed */
processed
Quoted 1 line
+
dev_dbg(data->dev, "Command 0x%X proccessed successfully\n", cmd);
Ditto. Can you run a spell checker? Kernel has a little one called codespell.
Quoted 3 lines
drivers/iio/pressure/bmp280-core.c +3
-0+ +return 0 ; +}
...
Quoted 17 lines
drivers/iio/pressure/bmp280-core.c +17
-0+static s32 bmp380_compensate_temp (struct bmp280_data *data,u32 adc_temp) +{ +s64 var1, var2, var3, var4, var5, var6, comp_temp; +struct bmp380_calib *calib = &data->calib.bmp380; + + var1 = ((s64 ) adc_temp) - (((s64 ) calib->T1) <<8 ); + var2 = var1 * ((s64 ) calib->T2); + var3 = var1 * var1; + var4 = var3 * ((s64 ) calib->T3); + var5 = (var2 <<18 ) + var4; + var6 = var5 >>32 ; + data->t_fine = (s32 ) var6; + comp_temp = (var6 *25 ) >>14 ; + + comp_temp =clamp_val (comp_temp,BMP380_MIN_TEMP ,BMP380_MAX_TEMP ); +return (s32 ) comp_temp; +}
...
Quoted 23 lines
drivers/iio/pressure/bmp280-core.c +23
-0+s64 var1, var2, var3, var4, var5, var6, offset, sensitivity; +u64 comp_press; +struct bmp380_calib *calib = &data->calib.bmp380; + + var1 = ((s64 )data->t_fine) * ((s64 )data->t_fine); + var2 = var1 >>6 ; + var3 = (var2 * ((s64 ) data->t_fine)) >>8 ; + var4 = (((s64 )calib->P8) * var3) >>5 ; + var5 = (((s64 ) calib->P7) * var1) <<4 ; + var6 = (((s64 ) calib->P6) * ((s64 )data->t_fine)) <<22 ; + offset = (((s64 )calib->P5) <<47 ) + var4 + var5 + var6; + var2 = (((s64 )calib->P4) * var3) >>5 ; + var4 = (((s64 ) calib->P3) * var1) <<2 ; + var5 = (((s64 ) calib->P2) - ((s64 )1 <<14 )) * + (((s64 )data->t_fine) <<21 ); + sensitivity = ((((s64 ) calib->P1) - ((s64 )1 <<14 )) <<46 ) + + var2 + var4 + var5; + var1 = (sensitivity >>24 ) * ((s64 )adc_press); + var2 = ((s64 )calib->P10) * ((s64 ) data->t_fine); + var3 = var2 + (((s64 ) calib->P9) <<16 ); + var4 = (var3 * ((s64 )adc_press)) >>13 ; + +/*
drivers/iio/pressure/bmp280-core.c static const struct bmp280_chip_info bme280_chip_info = { · line 893
Quoted 10 lines
drivers/iio/pressure/bmp280-core.c +10
-0+* Dividing by 10 followed by multiplying by 10 to avoid +* possible overflow caused by (uncomp_data->pressure * partial_data4) +*/ + var5 = (((s64 )adc_press) * (var4 /10 )) >>9 ; + var5 *=10 ; + var6 = ((s64 )adc_press) * ((s64 )adc_press); + var2 = (((s64 )calib->P11) * var6) >>16 ; + var3 = (var2 * ((s64 )adc_press)) >>7 ; + var4 = (offset >>2 ) + var1 + var5 + var3; + comp_press = ((u64 )var4 *25 ) >>40 ;
Kbuild bot is right, you forgot to (compile-)test for a 32-bit machine.
...
Quoted 1 line
+ ret =
regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB, data->buf, 3);
sizeof() ?
...
Quoted 1 line
+ /* Read and compensate temperature so we get a reading of
t_fine. */
for temperature
...
Quoted 1 line
+ ret =
regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB, data->buf, 3);
sizeof() ?
...
drivers/iio/pressure/bmp280-core.c static const struct bmp280_chip_info bme280_chip_info = { · line 1086
Quoted 3 lines
drivers/iio/pressure/bmp280-core.c +2
-0+ .oversampling_temp_default = ilog2(1), + .oversampling_press_default = ilog2(4),
BIT()
...
Quoted 24 lines
drivers/iio/pressure/bmp280.h +24
-0+#define BMP380_REG_CMD 0x7E +#define BMP380_REG_CONFIG 0x1F +#define BMP380_REG_ODR 0X1D +#define BMP380_REG_OSR 0X1C +#define BMP380_REG_POWER_CONTROL 0X1B +#define BMP380_REG_IF_CONFIG 0X1A +#define BMP380_REG_INT_CONTROL 0X19 +#define BMP380_REG_INT_STATUS 0X11 +#define BMP380_REG_EVENT 0X10 +#define BMP380_REG_STATUS 0X03 +#define BMP380_REG_ERROR 0X02 +#define BMP380_REG_ID 0X00 + +#define BMP380_REG_FIFO_CONFIG_1 0X18 +#define BMP380_REG_FIFO_CONFIG_2 0X17 +#define BMP380_REG_FIFO_WATERMARK_MSB 0X16 +#define BMP380_REG_FIFO_WATERMARK_LSB 0X15 +#define BMP380_REG_FIFO_DATA 0X14 +#define BMP380_REG_FIFO_LENGTH_MSB 0X13 +#define BMP380_REG_FIFO_LENGTH_LSB 0X12 + +#define BMP380_REG_SENSOR_TIME_MSB 0X0E +#define BMP380_REG_SENSOR_TIME_LSB 0X0D +#define BMP380_REG_SENSOR_TIME_XLSB 0X0C
drivers/iio/pressure/bmp280.h line 40
Quoted 10 lines
drivers/iio/pressure/bmp280.h +10
-0+ +#define BMP380_REG_TEMP_MSB 0X09 +#define BMP380_REG_TEMP_LSB 0X08 +#define BMP380_REG_TEMP_XLSB 0X07 + +#define BMP380_REG_PRESS_MSB 0X06 +#define BMP380_REG_PRESS_LSB 0X05 +#define BMP380_REG_PRESS_XLSB 0X04 + +#define BMP380_REG_CALIB_TEMP_START 0x31
Be consistent x vs X (we prefer x).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
Quoted 13 lines · Andy Shevchenko
Quoted 8 lines · Angel Iglesias
Adds compatibility with the new generation of this sensor, the BMP380
Includes basic sensor initialization to do pressure and temp measurements and allows tuning oversampling settings for each channel.
The compensation algorithms are adapted from the device datasheet and the repository https://github.com/BoschSensortec/BMP3-Sensor-API
...
Quoted 7 lines
Quoted 1 line
+ /* wait for 2ms for command to be proccessed */
processed
Quoted 1 line
+
dev_dbg(data->dev, "Command 0x%X proccessed successfully\n", cmd);Ditto. Can you run a spell checker? Kernel has a little one called codespell.
Once again, my typos strike back! I didn't know of this tool, I only checked format issues using the script check_patch.pl
Quoted 55 lines
Quoted 3 lines
drivers/iio/pressure/bmp280-core.c +3
-0+ + return 0; +}...
Quoted 17 lines
drivers/iio/pressure/bmp280-core.c +17
-0+static s32 bmp380_compensate_temp (struct bmp280_data *data,u32 adc_temp) +{ + s64 var1, var2, var3, var4, var5, var6, comp_temp; + struct bmp380_calib *calib = &data->calib.bmp380; + + var1 = ((s64) adc_temp) - (((s64) calib->T1) << 8); + var2 = var1 * ((s64) calib->T2); + var3 = var1 * var1; + var4 = var3 * ((s64) calib->T3); + var5 = (var2 << 18) + var4; + var6 = var5 >> 32; + data->t_fine = (s32) var6; + comp_temp = (var6 * 25) >> 14; + + comp_temp = clamp_val(comp_temp, BMP380_MIN_TEMP, BMP380_MAX_TEMP); + return (s32) comp_temp; +}...
Quoted 29 lines
+ s64 var1, var2, var3, var4, var5, var6, offset, sensitivity; + u64
comp_press;
+ structbmp380_calib*calib =&data->calib.bmp380;drivers/iio/pressure/bmp280-core.c +22
-0+ + var1 = ((s64)data->t_fine) * ((s64)data->t_fine); + var2 = var1 >> 6; + var3 = (var2 * ((s64) data->t_fine)) >> 8; + var4 = (((s64)calib->P8) * var3) >> 5; + var5 = (((s64) calib->P7) * var1) << 4; + var6 = (((s64) calib->P6) * ((s64)data->t_fine)) << 22; + offset = (((s64)calib->P5) << 47) + var4 + var5 + var6; + var2 = (((s64)calib->P4) * var3) >> 5; + var4 = (((s64) calib->P3) * var1) << 2; + var5 = (((s64) calib->P2) - ((s64) 1<<14)) * + (((s64)data->t_fine) << 21); + sensitivity = ((((s64) calib->P1) - ((s64) 1 << 14)) << 46) + + var2 + var4 + var5; + var1 = (sensitivity >> 24) * ((s64)adc_press); + var2 = ((s64)calib->P10) * ((s64) data->t_fine); + var3 = var2 + (((s64) calib->P9) << 16); + var4 = (var3 * ((s64)adc_press)) >> 13; + + /* +* Dividing by 10 followed by multiplying by 10 to avoid +* possible overflow caused by (uncomp_data->pressure *
partial_data4)
+ */
+ var5 = (((s64)adc_press) * (var4 / 10)) >> 9; + var5 *= 10;
Quoted 8 lines
Quoted 5 lines
+ var6 = ((s64)adc_press) * ((s64)adc_press); + var2 = (((s64)calib->P11) * var6) >> 16; + var3 = (var2 * ((s64)adc_press)) >> 7; + var4 = (offset >> 2) + var1 + var5 + var3; +
comp_press= ((u64)var4 * 25) >> 40;
Kbuildbot is right, you forgot to (compile-)test for a 32-bit machine.
Oh I see. Thanks for heads up, I only tested arm64 and x86_64 archs.
Quoted 20 lines
...
Quoted 2 lines
+ ret = regmap_bulk_read(data->regmap,
BMP380_REG_TEMP_XLSB, data-Quoted 1 line
buf, 3);
sizeof()?...
Quoted 1 line
+ /* Read and compensate temperature so we get a reading of
t_fine. */for temperature
...
Quoted 2 lines
+ ret = regmap_bulk_read(data->regmap,
BMP380_REG_PRESS_XLSB, data-Quoted 1 line
buf, 3);
sizeof()?
Quoted 7 lines
...
Quoted 3 lines
+ .oversampling_temp_default = ilog2(1),
+ .oversampling_press_default = ilog2(4),
BIT()
I preserved previous committer use of ilog2() to recover the index value as it is done on bmp280_write_oversampling_* functions, but you're right, is better to initialize using BIT() macros.
Quoted 30 lines
...
´Quoted 28 lines
drivers/iio/pressure/bmp280.h +28
-0+#define BMP380_REG_CMD 0x7E +#define BMP380_REG_CONFIG 0x1F +#define BMP380_REG_ODR 0X1D +#define BMP380_REG_OSR 0X1C +#define BMP380_REG_POWER_CONTROL 0X1B +#define BMP380_REG_IF_CONFIG 0X1A +#define BMP380_REG_INT_CONTROL 0X19 +#define BMP380_REG_INT_STATUS 0X11 +#define BMP380_REG_EVENT 0X10 +#define BMP380_REG_STATUS 0X03 +#define BMP380_REG_ERROR 0X02 +#define BMP380_REG_ID 0X00 + +#define BMP380_REG_FIFO_CONFIG_1 0X18 +#define BMP380_REG_FIFO_CONFIG_2 0X17 +#define BMP380_REG_FIFO_WATERMARK_MSB 0X16 +#define BMP380_REG_FIFO_WATERMARK_LSB 0X15 +#define BMP380_REG_FIFO_DATA 0X14 +#define BMP380_REG_FIFO_LENGTH_MSB 0X13 +#define BMP380_REG_FIFO_LENGTH_LSB 0X12 + +#define BMP380_REG_SENSOR_TIME_MSB 0X0E +#define BMP380_REG_SENSOR_TIME_LSB 0X0D +#define BMP380_REG_SENSOR_TIME_XLSB 0X0C + +#define BMP380_REG_TEMP_MSB 0X09 +#define BMP380_REG_TEMP_LSB 0X08 +#define BMP380_REG_TEMP_XLSB 0X07
Quoted 9 lines
drivers/iio/pressure/bmp280.h line 40
Quoted 6 lines
drivers/iio/pressure/bmp280.h +6
-0+ +#define BMP380_REG_PRESS_MSB 0X06 +#define BMP380_REG_PRESS_LSB 0X05 +#define BMP380_REG_PRESS_XLSB 0X04 + +#define BMP380_REG_CALIB_TEMP_START 0x31Be consistent x vs X (we prefer x).
Sorry about that
As always, thank you for your comments,
Kind regards,
Angel
Jonathan Cameron
maintainerRe: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
In reply to Angel IglesiasQuoted 9 lines · On Sat, 23 Jul 2022 19:39:44 +0200 Angel Iglesias <[email protected]>…
Adds compatibility with the new generation of this sensor, the BMP380
Includes basic sensor initialization to do pressure and temp measurements and allows tuning oversampling settings for each channel.
The compensation algorithms are adapted from the device datasheet and the repository https://github.com/BoschSensortec/BMP3-Sensor-API
Signed-off-by Angel Iglesias <[email protected]>
Hi Angel,
A comment below. Follows on from comment on previous patch rather than being a suggestion to change anything in here (beyond what has already been raised by others!)
Jonathan
drivers/iio/pressure/bmp280-core.c static const struct bmp280_chip_info bme280_chip_info = { · line 970
Quoted 7 lines
drivers/iio/pressure/bmp280-core.c +7
-0+static int bmp380_read_calib (struct bmp280_data *data,unsigned int chip) +{ +struct bmp380_calib *calib = &data->calib.bmp380; +int ret; +u8 *buf; + + buf =kmalloc (BMP380_CALIB_REG_COUNT ,GFP_KERNEL );
Ah. The complexity in here explains somewhat why you did it with u8 in the previous patch. Probably still better to have a __be16 buffer for that one though even though we can't do that here.
Quoted 36 lines
drivers/iio/pressure/bmp280-core.c +36
-0+if (!buf) +return -ENOMEM ; + +/* Read temperature calibration values. */ + ret =regmap_bulk_read (data->regmap,BMP380_REG_CALIB_TEMP_START , buf, +BMP380_CALIB_REG_COUNT ); +if (ret <0 ) { +dev_err (data->dev, +"failed to read temperature calibration parameters\n" ); +kfree (buf); +return ret; + } + +/* Toss the temperature calibration data into the entropy pool */ +add_device_randomness (buf,BMP380_CALIB_REG_COUNT ); + +/* Parse calibration data */ + calib->T1 =get_unaligned_le16 (&buf[BMP380_T1 ]); + calib->T2 =get_unaligned_le16 (&buf[BMP380_T2 ]); + calib->T3 = buf[BMP380_T3 ]; + calib->P1 =get_unaligned_le16 (&buf[BMP380_P1 ]); + calib->P2 =get_unaligned_le16 (&buf[BMP380_P2 ]); + calib->P3 = buf[BMP380_P3 ]; + calib->P4 = buf[BMP380_P4 ]; + calib->P5 =get_unaligned_le16 (&buf[BMP380_P5 ]); + calib->P6 =get_unaligned_le16 (&buf[BMP380_P6 ]); + calib->P7 = buf[BMP380_P7 ]; + calib->P8 = buf[BMP380_P8 ]; + calib->P9 =get_unaligned_le16 (&buf[BMP380_P9 ]); + calib->P10 = buf[BMP380_P10 ]; + calib->P11 = buf[BMP380_P11 ]; + +kfree (buf); +return 0 ; +} +
Re: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380 sensor family
Quoted 21 lines · Jonathan Cameron
Quoted 9 lines · On Sat, 23 Jul 2022 19:39:44 +0200 Angel Iglesias <[email protected]>…
Adds compatibility with the new generation of this sensor, the BMP380
Includes basic sensor initialization to do pressure and temp measurements and allows tuning oversampling settings for each channel.
The compensation algorithms are adapted from the device datasheet and the repository https://github.com/BoschSensortec/BMP3-Sensor-API
Signed-off-by Angel Iglesias <[email protected]>Hi Angel,
A comment below. Follows on from comment on previous patch rather than being a suggestion to change anything in here (beyond what has already been raised by others!)
Jonathan
Quoted 12 lines
drivers/iio/pressure/bmp280-core.c static const struct bmp280_chip_info bme280_chip_info = { · line 970
Quoted 7 lines
drivers/iio/pressure/bmp280-core.c +7
-0+static int bmp380_read_calib (struct bmp280_data *data,unsigned int chip) +{ + struct bmp380_calib *calib = &data->calib.bmp380; + int ret; + u8 *buf; + + buf = kmalloc(BMP380_CALIB_REG_COUNT, GFP_KERNEL);Ah. The complexity in here explains somewhat why you did it with u8 in the previous
patch. Probably still better to have a__be16buffer for that one though even though we can't do that here.
Yes, I thought that, although I had to make more cumbersome the conversions and do the +2 increment on the enum with the index, it would be better to have a somewhat more uniform structure in each calib reading function.
Quoted 30 lines
Quoted 30 lines
+ if (!buf)
+ return-ENOMEM;
+
+ /* Read temperature calibration values. */
+ ret = regmap_bulk_read(data->regmap,BMP380_REG_CALIB_TEMP_START, buf,
+BMP380_CALIB_REG_COUNT);
+ if (ret < 0) {
+ dev_err(data->dev,
+ "failed to read temperature calibration parameters\n");
+ kfree(buf);
+ return ret;
+ }drivers/iio/pressure/bmp280-core.c +16
-0+ + /* Toss the temperature calibration data into the entropy pool */ + add_device_randomness(buf, BMP380_CALIB_REG_COUNT); + + /* Parse calibration data */ + calib->T1 = get_unaligned_le16(&buf[BMP380_T1]); + calib->T2 = get_unaligned_le16(&buf[BMP380_T2]); + calib->T3 = buf[BMP380_T3]; + calib->P1 = get_unaligned_le16(&buf[BMP380_P1]); + calib->P2 = get_unaligned_le16(&buf[BMP380_P2]); + calib->P3 = buf[BMP380_P3]; + calib->P4 = buf[BMP380_P4]; + calib->P5 = get_unaligned_le16(&buf[BMP380_P5]); + calib->P6 = get_unaligned_le16(&buf[BMP380_P6]); + calib->P7 = buf[BMP380_P7]; + calib->P8 = buf[BMP380_P8];
Quoted 8 lines
drivers/iio/pressure/bmp280-core.c static const struct bmp280_chip_info bme280_chip_info = { · line 1006
Quoted 8 lines
+ calib->P9 =
get_unaligned_le16(&buf[BMP380_P9]); + calib->P10 = buf[BMP380_P10];
+ calib->P11 = buf[BMP380_P11];drivers/iio/pressure/bmp280-core.c +5
-0+ + kfree(buf); + return 0; +} +
Thanks for your comments,
Kind regards,
Angel
PATCH v4 5/5 iio: pressure: bmp280: Add more tunable config parameters for BMP380
In reply to Angel IglesiasAllows sampling frequency and IIR filter coefficients configuration using sysfs ABI.
The IIR filter coefficient is configurable using the sysfs attribute "filter_low_pass_3db_frequency".
--- drivers/iio/pressure/bmp280-core.c | 324 ++++++++++++++++++++++++++--- drivers/iio/pressure/bmp280.h | 18 -- 2 files changed, 294 insertions(+), 48 deletions(-)
drivers/iio/pressure/bmp280-core.c +294 -30
@@ -112,6 +112,27 @@ static const char *const bmp280_supply_names[] = {"vddd" ,"vdda" }; +enum bmp380_odr { +BMP380_ODR_200HZ , +BMP380_ODR_100HZ , +BMP380_ODR_50HZ , +BMP380_ODR_25HZ , +BMP380_ODR_12_5HZ , +BMP380_ODR_6_25HZ , +BMP380_ODR_3_125HZ , +BMP380_ODR_1_5625HZ , +BMP380_ODR_0_78HZ , +BMP380_ODR_0_39HZ , +BMP380_ODR_0_2HZ , +BMP380_ODR_0_1HZ , +BMP380_ODR_0_05HZ , +BMP380_ODR_0_02HZ , +BMP380_ODR_0_01HZ , +BMP380_ODR_0_006HZ , +BMP380_ODR_0_003HZ , +BMP380_ODR_0_0015HZ , +}; +#define BMP280_NUM_SUPPLIES ARRAY_SIZE(bmp280_supply_names) struct bmp280_data {
@@ -133,6 +154,17 @@ struct bmp280_data {u8 oversampling_press;u8 oversampling_temp;u8 oversampling_humid; +u8 iir_filter_coeff; + +/* +* BMP380 devices introduce sampling frequency configuration. See +* datasheet sections 3.3.3. and 4.3.19 for more details. +* +* BMx280 devices allowed indirect configuration of sampling frequency +* changing the t_standby duration between measurements, as detailed on +* section 3.6.3 of the datasheet. +*/ +int sampling_freq;/* * Carryover value from temperature conversion, used in pressure
@@ -150,6 +182,7 @@ struct bmp280_data {struct bmp280_chip_info {unsigned int id_reg; +const struct iio_chan_spec *channels;int num_channels;unsigned int start_up_time;
@@ -165,6 +198,14 @@ struct bmp280_chip_info {int num_oversampling_humid_avail;int oversampling_humid_default; +const int *iir_filter_coeffs_avail; +int num_iir_filter_coeffs_avail; +int iir_filter_coeff_default; + +const int (*sampling_freq_avail)[2 ]; +int num_sampling_freq_avail; +int sampling_freq_default; +int (*chip_config)(struct bmp280_data *);int (*read_temp)(struct bmp280_data *,int *);int (*read_press)(struct bmp280_data *,int *,int *);
@@ -226,6 +267,30 @@ static const struct iio_chan_spec bmp280_channels[] = { }, }; +static const struct iio_chan_spec bmp380_channels[] = { + { + .type =IIO_PRESSURE , + .info_mask_separate =BIT (IIO_CHAN_INFO_PROCESSED ) | +BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), + .info_mask_shared_by_all =BIT (IIO_CHAN_INFO_SAMP_FREQ ) | +BIT (IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY ), + }, + { + .type =IIO_TEMP , + .info_mask_separate =BIT (IIO_CHAN_INFO_PROCESSED ) | +BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), + .info_mask_shared_by_all =BIT (IIO_CHAN_INFO_SAMP_FREQ ) | +BIT (IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY ), + }, + { + .type =IIO_HUMIDITYRELATIVE , + .info_mask_separate =BIT (IIO_CHAN_INFO_PROCESSED ) | +BIT (IIO_CHAN_INFO_OVERSAMPLING_RATIO ), + .info_mask_shared_by_all =BIT (IIO_CHAN_INFO_SAMP_FREQ ) | +BIT (IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY ), + }, +}; +static int bmp280_read_calib (struct bmp280_data *data,unsigned int chip) {int ret;
@@ -558,6 +623,25 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,break ; }break ; +case IIO_CHAN_INFO_SAMP_FREQ : +if (!data->chip_info->sampling_freq_avail) { + ret = -EINVAL ; +break ; + } + + *val = data->chip_info->sampling_freq_avail[data->sampling_freq][0 ]; + *val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1 ]; + ret =IIO_VAL_INT_PLUS_MICRO ; +break ; +case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY : +if (!data->chip_info->iir_filter_coeffs_avail) { + ret = -EINVAL ; +break ; + } + + *val = data->chip_info->iir_filter_coeffs_avail[data->iir_filter_coeff]; + ret =IIO_VAL_INT ; +break ;default : ret = -EINVAL ;break ;
@@ -574,14 +658,22 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data, int val) {int i; +int ret, prev;const int *avail = data->chip_info->oversampling_humid_avail;const int n = data->chip_info->num_oversampling_humid_avail;for (i =0 ; i < n; i++) {if (avail[i] == val) { + prev = data->oversampling_humid; data->oversampling_humid =ilog2 (val); -return data->chip_info->chip_config (data); + ret = data->chip_info->chip_config (data); +if (ret) { + data->oversampling_humid = prev; + data->chip_info->chip_config (data); +return ret; + } +return 0 ; } }return -EINVAL ;
@@ -591,14 +683,22 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data, int val) {int i; +int ret, prev;const int *avail = data->chip_info->oversampling_temp_avail;const int n = data->chip_info->num_oversampling_temp_avail;for (i =0 ; i < n; i++) {if (avail[i] == val) { + prev = data->oversampling_temp; data->oversampling_temp =ilog2 (val); -return data->chip_info->chip_config (data); + ret = data->chip_info->chip_config (data); +if (ret) { + data->oversampling_temp = prev; + data->chip_info->chip_config (data); +return ret; + } +return 0 ; } }return -EINVAL ;
@@ -608,14 +708,72 @@ static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data, int val) {int i; +int ret, prev;const int *avail = data->chip_info->oversampling_press_avail;const int n = data->chip_info->num_oversampling_press_avail;for (i =0 ; i < n; i++) {if (avail[i] == val) { + prev = data->oversampling_press; data->oversampling_press =ilog2 (val); -return data->chip_info->chip_config (data); + ret = data->chip_info->chip_config (data); +if (ret) { + data->oversampling_press = prev; + data->chip_info->chip_config (data); +return ret; + } +return 0 ; + } + } +return -EINVAL ; +} + +static int bmp280_write_sampling_frequency (struct bmp280_data *data, +int val,int val2) +{ +int i; +int ret, prev; +const int (*avail)[2 ] = data->chip_info->sampling_freq_avail; +const int n = data->chip_info->num_sampling_freq_avail; + +for (i =0 ; i < n; i++) { +if (avail[i][0 ] == val && avail[i][1 ] == val2) { + prev = data->sampling_freq; + data->sampling_freq = i; + + ret = data->chip_info->chip_config (data); +if (ret) { + data->sampling_freq = prev; + data->chip_info->chip_config (data); +return ret; + } +return 0 ; + } + } +return -EINVAL ; +} + +static int bmp280_write_iir_filter_coeffs (struct bmp280_data *data,int val) +{ +int i; +int ret, prev; +const int *avail = data->chip_info->iir_filter_coeffs_avail; +const int n = data->chip_info->num_iir_filter_coeffs_avail; + +for (i =0 ; i < n; i++) { +if (avail[i] == val) { + prev = data->iir_filter_coeff; + data->iir_filter_coeff = i; + + ret = data->chip_info->chip_config (data); +if (ret) { + data->iir_filter_coeff = prev; + data->chip_info->chip_config (data); +return ret; + + } +return 0 ; } }return -EINVAL ;
@@ -628,6 +786,12 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,int ret =0 ;struct bmp280_data *data =iio_priv (indio_dev); +/* +* Auxiliar functions to update sensor running configuration. +* If an error happens applying new settings, will try restore +* previous parameters to ensure the sensor is left in a known +* working configuration. +*/ switch (mask) {case IIO_CHAN_INFO_OVERSAMPLING_RATIO :pm_runtime_get_sync (data->dev);
@@ -650,6 +814,22 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,pm_runtime_mark_last_busy (data->dev);pm_runtime_put_autosuspend (data->dev);break ; +case IIO_CHAN_INFO_SAMP_FREQ : +pm_runtime_get_sync (data->dev); +mutex_lock (&data->lock); + ret =bmp280_write_sampling_frequency (data, val, val2); +mutex_unlock (&data->lock); +pm_runtime_mark_last_busy (data->dev); +pm_runtime_put_autosuspend (data->dev); +break ; +case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY : +pm_runtime_get_sync (data->dev); +mutex_lock (&data->lock); + ret =bmp280_write_iir_filter_coeffs (data, val); +mutex_unlock (&data->lock); +pm_runtime_mark_last_busy (data->dev); +pm_runtime_put_autosuspend (data->dev); +break ;default :return -EINVAL ; }
@@ -680,6 +860,17 @@ static int bmp280_read_avail(struct iio_dev *indio_dev, } *type =IIO_VAL_INT ;return IIO_AVAIL_LIST ; +case IIO_CHAN_INFO_SAMP_FREQ : + *vals = (const int *)data->chip_info->sampling_freq_avail; + *type =IIO_VAL_INT_PLUS_MICRO ; +/* Values are stored in a 2D matrix */ + *length = data->chip_info->num_sampling_freq_avail; +return IIO_AVAIL_LIST ; +case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY : + *vals = data->chip_info->iir_filter_coeffs_avail; + *type =IIO_VAL_INT ; + *length = data->chip_info->num_iir_filter_coeffs_avail; +return IIO_AVAIL_LIST ;default :return -EINVAL ; }
@@ -725,6 +916,7 @@ static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };static const struct bmp280_chip_info bmp280_chip_info = { .id_reg =BMP280_REG_ID , .start_up_time =2000 , + .channels = bmp280_channels, .num_channels =2 , .oversampling_temp_avail = bmp280_oversampling_avail,
@@ -762,6 +954,7 @@ static int bme280_chip_config(struct bmp280_data *data)static const struct bmp280_chip_info bme280_chip_info = { .id_reg =BMP280_REG_ID , .start_up_time =2000 , + .channels = bmp280_channels, .num_channels =3 , .oversampling_temp_avail = bmp280_oversampling_avail,
@@ -1004,18 +1197,39 @@ static int bmp380_read_calib(struct bmp280_data *data, unsigned int chip)return 0 ; } +static const int bmp380_odr_table[][2 ] = { + [BMP380_ODR_200HZ ] = {200 ,0 }, + [BMP380_ODR_100HZ ] = {100 ,0 }, + [BMP380_ODR_50HZ ] = {50 ,0 }, + [BMP380_ODR_25HZ ] = {25 ,0 }, + [BMP380_ODR_12_5HZ ] = {12 ,500000 }, + [BMP380_ODR_6_25HZ ] = {6 ,250000 }, + [BMP380_ODR_3_125HZ ] = {3 ,125000 }, + [BMP380_ODR_1_5625HZ ] = {1 ,562500 }, + [BMP380_ODR_0_78HZ ] = {0 ,781250 }, + [BMP380_ODR_0_39HZ ] = {0 ,390625 }, + [BMP380_ODR_0_2HZ ] = {0 ,195313 }, + [BMP380_ODR_0_1HZ ] = {0 ,97656 }, + [BMP380_ODR_0_05HZ ] = {0 ,48828 }, + [BMP380_ODR_0_02HZ ] = {0 ,24414 }, + [BMP380_ODR_0_01HZ ] = {0 ,12207 }, + [BMP380_ODR_0_006HZ ] = {0 ,6104 }, + [BMP380_ODR_0_003HZ ] = {0 ,3052 }, + [BMP380_ODR_0_0015HZ ] = {0 ,1526 }, +}; +static int bmp380_chip_config (struct bmp280_data *data) { +bool change = false, aux;unsigned int tmp;u8 osrs;int ret;/* configure power control register */ - ret =regmap_write_bits (data->regmap,BMP380_REG_POWER_CONTROL , -BMP380_CTRL_SENSORS_MASK |BMP380_MODE_MASK , -BMP380_CTRL_SENSORS_PRESS_EN | -BMP380_CTRL_SENSORS_TEMP_EN | -FIELD_PREP (BMP380_MODE_MASK ,BMP380_MODE_NORMAL )); + ret =regmap_update_bits (data->regmap,BMP380_REG_POWER_CONTROL , +BMP380_CTRL_SENSORS_MASK , +BMP380_CTRL_SENSORS_PRESS_EN | +BMP380_CTRL_SENSORS_TEMP_EN );if (ret <0 ) {dev_err (data->dev,"failed to write operation control register\n" );
@@ -1026,55 +1240,94 @@ static int bmp380_chip_config(struct bmp280_data *data) osrs =FIELD_PREP (BMP380_OSRS_TEMP_MASK , data->oversampling_temp) |FIELD_PREP (BMP380_OSRS_PRESS_MASK , data->oversampling_press); - ret =regmap_write_bits (data->regmap,BMP380_REG_OSR , -BMP380_OSRS_TEMP_MASK |BMP380_OSRS_PRESS_MASK , - osrs); + ret =regmap_update_bits_check (data->regmap,BMP380_REG_OSR , +BMP380_OSRS_TEMP_MASK | +BMP380_OSRS_PRESS_MASK , + osrs, &aux);if (ret <0 ) {dev_err (data->dev,"failed to write oversampling register\n" );return ret; } + change = change || aux;/* configure output data rate */ - ret =regmap_write_bits (data->regmap,BMP380_REG_ODR , -BMP380_ODRS_MASK ,BMP380_ODRS_50HZ ); + ret =regmap_update_bits_check (data->regmap,BMP380_REG_ODR , +BMP380_ODRS_MASK , data->sampling_freq, + &aux);if (ret <0 ) {dev_err (data->dev,"failed to write ODR selection register\n" );return ret; } + change = change || aux;/* set filter data */ - ret =regmap_update_bits (data->regmap,BMP380_REG_CONFIG , -BMP380_FILTER_MASK , -FIELD_PREP (BMP380_FILTER_MASK ,BMP380_FILTER_3X )); + ret =regmap_update_bits_check (data->regmap,BMP380_REG_CONFIG , +BMP380_FILTER_MASK , +FIELD_PREP (BMP380_FILTER_MASK , data->iir_filter_coeff), + &aux);if (ret <0 ) {dev_err (data->dev,"failed to write config register\n" );return ret; } + change = change || aux; -/* wait startup_time before verifying config changes */ -usleep_range (data->start_up_time, data->start_up_time +100 ); +if (change) { +/* +* Configuration errors are detected on the fly during a measurement +* cycle. If the sampling frequency is too low, it's faster to reset +* measurement loop than wait until next measurement is due. +* +* Resets sensor measurement loop toggling between sleep and normal +* operating modes. +*/ + ret =regmap_write_bits (data->regmap,BMP380_REG_POWER_CONTROL , +BMP380_MODE_MASK , +FIELD_PREP (BMP380_MODE_MASK , +BMP380_MODE_SLEEP )); +if (ret <0 ) { +dev_err (data->dev,"failed to set sleep mode\n" ); +return ret; + } +usleep_range (2000 ,2500 ); + ret =regmap_write_bits (data->regmap,BMP380_REG_POWER_CONTROL , +BMP380_MODE_MASK , +FIELD_PREP (BMP380_MODE_MASK , +BMP380_MODE_NORMAL )); +if (ret <0 ) { +dev_err (data->dev,"failed to set normal mode\n" ); +return ret; + } +/* +* Waits for measurement before checking configuration error flag. +* Selected longest measure time indicated in section 3.9.1 +* in the datasheet. +*/ +msleep (80 ); -/* check config error flag */ - ret =regmap_read (data->regmap,BMP380_REG_ERROR , &tmp); -if (ret <0 ) { -dev_err (data->dev, -"failed to read error register\n" ); -return ret; - } -if (tmp &BMP380_ERR_CONF_MASK ) { -dev_warn (data->dev, -"sensor flagged configuration as incompatible\n" ); -return -EINVAL ; +/* check config error flag */ + ret =regmap_read (data->regmap,BMP380_REG_ERROR , &tmp); +if (ret <0 ) { +dev_err (data->dev, +"failed to read error register\n" ); +return ret; + } +if (tmp &BMP380_ERR_CONF_MASK ) { +dev_warn (data->dev, +"sensor flagged configuration as incompatible\n" ); +return -EINVAL ; + } }return 0 ; }static const int bmp380_oversampling_avail[] = {1 ,2 ,4 ,8 ,16 ,32 }; +static const int bmp380_iir_filter_coeffs_avail[] = {0 ,1 ,3 ,7 ,15 ,31 ,63 ,127 };static const struct bmp280_chip_info bmp380_chip_info = { .id_reg =BMP380_REG_ID , .start_up_time =2000 , + .channels = bmp380_channels, .num_channels =2 , .oversampling_temp_avail = bmp380_oversampling_avail,
@@ -1085,6 +1338,14 @@ static const struct bmp280_chip_info bmp380_chip_info = { .num_oversampling_press_avail = ARRAY_SIZE(bmp380_oversampling_avail), .oversampling_press_default = ilog2(4), + .sampling_freq_avail = bmp380_odr_table, + .num_sampling_freq_avail = ARRAY_SIZE(bmp380_odr_table) * 2, + .sampling_freq_default = BMP380_ODR_50HZ, + + .iir_filter_coeffs_avail = bmp380_iir_filter_coeffs_avail, + .num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail), + .iir_filter_coeff_default = 2, + .chip_config = bmp380_chip_config, .read_temp = bmp380_read_temp, .read_press = bmp380_read_press,
@@ -1329,6 +1590,7 @@ static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };static const struct bmp280_chip_info bmp180_chip_info = { .id_reg =BMP280_REG_ID , .start_up_time =2000 , + .channels = bmp280_channels, .num_channels =2 , .oversampling_temp_avail = bmp180_oversampling_temp_avail,
@@ -1427,7 +1689,6 @@ int bmp280_common_probe(struct device *dev, data->dev = dev; indio_dev->name = name; - indio_dev->channels = bmp280_channels; indio_dev->info = &bmp280_info; indio_dev->modes =INDIO_DIRECT_MODE ;
@@ -1450,10 +1711,13 @@ int bmp280_common_probe(struct device *dev, data->chip_info = chip_info;/* apply initial values from chip info structure */ + indio_dev->channels = chip_info->channels; indio_dev->num_channels = chip_info->num_channels; data->oversampling_press = chip_info->oversampling_press_default; data->oversampling_humid = chip_info->oversampling_humid_default; data->oversampling_temp = chip_info->oversampling_temp_default; + data->iir_filter_coeff = chip_info->iir_filter_coeff_default; + data->sampling_freq = chip_info->sampling_freq_default; data->start_up_time = chip_info->start_up_time;/* Bring up regulators */
drivers/iio/pressure/bmp280.h +0 -18
@@ -54,24 +54,6 @@#define BMP380_OSRS_PRESS_MASK GENMASK(2, 0)#define BMP380_ODRS_MASK GENMASK(4, 0) -#define BMP380_ODRS_200HZ 0x00 -#define BMP380_ODRS_100HZ 0x01 -#define BMP380_ODRS_50HZ 0x02 -#define BMP380_ODRS_25HZ 0x03 -#define BMP380_ODRS_12_5HZ 0x04 -#define BMP380_ODRS_6_25HZ 0x05 -#define BMP380_ODRS_3_1HZ 0x06 -#define BMP380_ODRS_1_5HZ 0x07 -#define BMP380_ODRS_0_78HZ 0x08 -#define BMP380_ODRS_0_39HZ 0x09 -#define BMP380_ODRS_0_2HZ 0x0A -#define BMP380_ODRS_0_1HZ 0x0B -#define BMP380_ODRS_0_05HZ 0x0C -#define BMP380_ODRS_0_02HZ 0x0D -#define BMP380_ODRS_0_01HZ 0x0E -#define BMP380_ODRS_0_006HZ 0x0F -#define BMP380_ODRS_0_003HZ 0x10 -#define BMP380_ODRS_0_0015HZ 0x11#define BMP380_CTRL_SENSORS_MASK GENMASK(1, 0)#define BMP380_CTRL_SENSORS_PRESS_EN BIT(0)
Signature · 1 line
2.37.1
Re: [PATCH v4 5/5] iio: pressure: bmp280: Add more tunable config parameters for BMP380
Quoted 10 lines · Angel Iglesias
Allows sampling frequency and IIR filter coefficients configuration using sysfs ABI.
The IIR filter coefficient is configurable using the sysfs attribute "
filter_low_pass_3db_frequency".Signed-off-by Angel Iglesias <[email protected]>Reported-by Andy Shevchenko <[email protected]> # uninit var
Bugs in new features can't have Reported-by tag.
If you wish to give credit, use a cover letter / changelog.
...
drivers/iio/pressure/bmp280-core.c static int bmp380_chip_config(struct bmp280_data *data) · line 1276
Quoted 2 lines
drivers/iio/pressure/bmp280-core.c +2
-0+/* +* Configuration errors are detected on the fly during a measurement
The configurations
drivers/iio/pressure/bmp280-core.c static int bmp380_chip_config(struct bmp280_data *data) · line 1278
Quoted 2 lines
drivers/iio/pressure/bmp280-core.c +2
-0+* cycle. If the sampling frequency is too low, it's faster to reset +* measurement loop than wait until next measurement is due.
the measurement
the next
Quoted 3 lines
drivers/iio/pressure/bmp280-core.c +3
-0+* Resets sensor measurement loop toggling between sleep and normal +* operating modes. +*/
Signature · 2 lines
With Best Regards, Andy Shevchenko