* [PATCH 0/2] bme680 cleanup @ 2018-08-07 20:07 David Frey 2018-08-07 20:07 ` [PATCH 1/2] iio: bme680: perform cosmetic cleanup David Frey 2018-08-07 20:07 ` [PATCH 2/2] iio: bme680: simplify oversampling handling David Frey 0 siblings, 2 replies; 5+ messages in thread From: David Frey @ 2018-08-07 20:07 UTC (permalink / raw) To: linux-iio, himanshujha199640; +Cc: David Frey I have performed some minor cleanup on the bme680 driver. I don't think there's anything controversial in my changes. Let me know what you think David Frey (2): iio: bme680: perform cosmetic cleanup iio: bme680: simplify oversampling handling drivers/iio/chemical/bme680.h | 17 +++--- drivers/iio/chemical/bme680_core.c | 110 ++++++++++++------------------------- 2 files changed, 42 insertions(+), 85 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] iio: bme680: perform cosmetic cleanup 2018-08-07 20:07 [PATCH 0/2] bme680 cleanup David Frey @ 2018-08-07 20:07 ` David Frey 2018-08-08 10:54 ` Himanshu Jha 2018-08-07 20:07 ` [PATCH 2/2] iio: bme680: simplify oversampling handling David Frey 1 sibling, 1 reply; 5+ messages in thread From: David Frey @ 2018-08-07 20:07 UTC (permalink / raw) To: linux-iio, himanshujha199640; +Cc: David Frey * use consistent naming for masks. _MSK -> _MASK * use GEN_MASK to define masks rather than hex constants * consistently indent masks and values under the corresponding register define * use FIELD_GET instead of explicit mask and shift * Join split lines in bme680_read_calib() which can fit in 80 chars * Make use clamp macro Signed-off-by: David Frey <dpfrey@gmail.com> --- drivers/iio/chemical/bme680.h | 17 +++++++------- drivers/iio/chemical/bme680_core.c | 45 +++++++++++++------------------------- 2 files changed, 23 insertions(+), 39 deletions(-) diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h index e049323f209a..a9f2a9a6abc5 100644 --- a/drivers/iio/chemical/bme680.h +++ b/drivers/iio/chemical/bme680.h @@ -4,10 +4,10 @@ #define BME680_REG_CHIP_I2C_ID 0xD0 #define BME680_REG_CHIP_SPI_ID 0x50 -#define BME680_CHIP_ID_VAL 0x61 +#define BME680_CHIP_ID_VAL 0x61 #define BME680_REG_SOFT_RESET_I2C 0xE0 #define BME680_REG_SOFT_RESET_SPI 0x60 -#define BME680_CMD_SOFTRESET 0xB6 +#define BME680_CMD_SOFTRESET 0xB6 #define BME680_REG_STATUS 0x73 #define BME680_SPI_MEM_PAGE_BIT BIT(4) #define BME680_SPI_MEM_PAGE_1_VAL 1 @@ -18,6 +18,7 @@ #define BME680_REG_GAS_MSB 0x2A #define BME680_REG_GAS_R_LSB 0x2B #define BME680_GAS_STAB_BIT BIT(4) +#define BME680_GAS_RANGE_MASK GENMASK(3, 0) #define BME680_REG_CTRL_HUMIDITY 0x72 #define BME680_OSRS_HUMIDITY_MASK GENMASK(2, 0) @@ -26,9 +27,8 @@ #define BME680_OSRS_TEMP_MASK GENMASK(7, 5) #define BME680_OSRS_PRESS_MASK GENMASK(4, 2) #define BME680_MODE_MASK GENMASK(1, 0) - -#define BME680_MODE_FORCED 1 -#define BME680_MODE_SLEEP 0 +#define BME680_MODE_FORCED 1 +#define BME680_MODE_SLEEP 0 #define BME680_REG_CONFIG 0x75 #define BME680_FILTER_MASK GENMASK(4, 2) @@ -39,16 +39,15 @@ #define BME680_MAX_OVERFLOW_VAL 0x40000000 #define BME680_HUM_REG_SHIFT_VAL 4 -#define BME680_BIT_H1_DATA_MSK 0x0F +#define BME680_BIT_H1_DATA_MASK GENMASK(3, 0) #define BME680_REG_RES_HEAT_RANGE 0x02 -#define BME680_RHRANGE_MSK 0x30 +#define BME680_RHRANGE_MASK GENMASK(5, 4) #define BME680_REG_RES_HEAT_VAL 0x00 #define BME680_REG_RANGE_SW_ERR 0x04 -#define BME680_RSERROR_MSK 0xF0 +#define BME680_RSERROR_MASK GENMASK(7, 4) #define BME680_REG_RES_HEAT_0 0x5A #define BME680_REG_GAS_WAIT_0 0x64 -#define BME680_GAS_RANGE_MASK 0x0F #define BME680_ADC_GAS_RES_SHIFT 6 #define BME680_AMB_TEMP 25 diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c index 7d9bb62baa3f..0e79d03ecc40 100644 --- a/drivers/iio/chemical/bme680_core.c +++ b/drivers/iio/chemical/bme680_core.c @@ -102,16 +102,14 @@ static int bme680_read_calib(struct bme680_data *data, __le16 buf; /* Temperature related coefficients */ - ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_T1_LSB_REG\n"); return ret; } calib->par_t1 = le16_to_cpu(buf); - ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_T2_LSB_REG\n"); return ret; @@ -126,16 +124,14 @@ static int bme680_read_calib(struct bme680_data *data, calib->par_t3 = tmp; /* Pressure related coefficients */ - ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P1_LSB_REG\n"); return ret; } calib->par_p1 = le16_to_cpu(buf); - ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P2_LSB_REG\n"); return ret; @@ -149,16 +145,14 @@ static int bme680_read_calib(struct bme680_data *data, } calib->par_p3 = tmp; - ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P4_LSB_REG\n"); return ret; } calib->par_p4 = le16_to_cpu(buf); - ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P5_LSB_REG\n"); return ret; @@ -179,16 +173,14 @@ static int bme680_read_calib(struct bme680_data *data, } calib->par_p7 = tmp; - ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P8_LSB_REG\n"); return ret; } calib->par_p8 = le16_to_cpu(buf); - ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG, (u8 *) &buf, 2); if (ret < 0) { dev_err(dev, "failed to read BME680_P9_LSB_REG\n"); return ret; @@ -208,30 +200,26 @@ static int bme680_read_calib(struct bme680_data *data, dev_err(dev, "failed to read BME680_H1_MSB_REG\n"); return ret; } - ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb); if (ret < 0) { dev_err(dev, "failed to read BME680_H1_LSB_REG\n"); return ret; } - calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | - (tmp_lsb & BME680_BIT_H1_DATA_MSK); + (tmp_lsb & BME680_BIT_H1_DATA_MASK); ret = regmap_read(data->regmap, BME680_H2_MSB_REG, &tmp_msb); if (ret < 0) { dev_err(dev, "failed to read BME680_H2_MSB_REG\n"); return ret; } - ret = regmap_read(data->regmap, BME680_H2_LSB_REG, &tmp_lsb); if (ret < 0) { dev_err(dev, "failed to read BME680_H2_LSB_REG\n"); return ret; } - calib->par_h2 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | - (tmp_lsb >> BME680_HUM_REG_SHIFT_VAL); + (tmp_lsb >> BME680_HUM_REG_SHIFT_VAL); ret = regmap_read(data->regmap, BME680_H3_REG, &tmp); if (ret < 0) { @@ -276,8 +264,8 @@ static int bme680_read_calib(struct bme680_data *data, } calib->par_gh1 = tmp; - ret = regmap_bulk_read(data->regmap, BME680_GH2_LSB_REG, - (u8 *) &buf, 2); + ret = regmap_bulk_read(data->regmap, BME680_GH2_LSB_REG, (u8 *) &buf, + 2); if (ret < 0) { dev_err(dev, "failed to read BME680_GH2_LSB_REG\n"); return ret; @@ -297,7 +285,7 @@ static int bme680_read_calib(struct bme680_data *data, dev_err(dev, "failed to read resistance heat range\n"); return ret; } - calib->res_heat_range = (tmp & BME680_RHRANGE_MSK) / 16; + calib->res_heat_range = FIELD_GET(BME680_RHRANGE_MASK, tmp); ret = regmap_read(data->regmap, BME680_REG_RES_HEAT_VAL, &tmp); if (ret < 0) { @@ -311,7 +299,7 @@ static int bme680_read_calib(struct bme680_data *data, dev_err(dev, "failed to read range software error\n"); return ret; } - calib->range_sw_err = (tmp & BME680_RSERROR_MSK) / 16; + calib->range_sw_err = FIELD_GET(BME680_RSERROR_MASK, tmp); return 0; } @@ -408,10 +396,7 @@ static u32 bme680_compensate_humid(struct bme680_data *data, var6 = (var4 * var5) >> 1; calc_hum = (((var3 + var6) >> 10) * 1000) >> 12; - if (calc_hum > 100000) /* Cap at 100%rH */ - calc_hum = 100000; - else if (calc_hum < 0) - calc_hum = 0; + calc_hum = clamp(calc_hum, 0, 100000); /* clamp between 0-100 %rH */ return calc_hum; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] iio: bme680: perform cosmetic cleanup 2018-08-07 20:07 ` [PATCH 1/2] iio: bme680: perform cosmetic cleanup David Frey @ 2018-08-08 10:54 ` Himanshu Jha 0 siblings, 0 replies; 5+ messages in thread From: Himanshu Jha @ 2018-08-08 10:54 UTC (permalink / raw) To: David Frey; +Cc: linux-iio On Tue, Aug 07, 2018 at 01:07:20PM -0700, David Frey wrote: > * use consistent naming for masks. _MSK -> _MASK > * use GEN_MASK to define masks rather than hex constants > * consistently indent masks and values under the corresponding register > define > * use FIELD_GET instead of explicit mask and shift > * Join split lines in bme680_read_calib() which can fit in 80 chars > * Make use clamp macro > > Signed-off-by: David Frey <dpfrey@gmail.com> > --- This patch is fine but ideally it should be split for each change. "One patch per change policy!" Dan is on vacation I guess otherwise he must have poked you before me ;) Also, please consider making subject: "iio: chemical: bme680: .... .. " Once you roll a v2, I will get it tested on my sensor. And yes, please also CC Jonathan <jic23@kernel.org> so that it doesn't get lost. Thanks for making the effort, David! -- Himanshu Jha Undergraduate Student Department of Electronics & Communication Guru Tegh Bahadur Institute of Technology ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] iio: bme680: simplify oversampling handling 2018-08-07 20:07 [PATCH 0/2] bme680 cleanup David Frey 2018-08-07 20:07 ` [PATCH 1/2] iio: bme680: perform cosmetic cleanup David Frey @ 2018-08-07 20:07 ` David Frey 2018-08-08 10:47 ` Himanshu Jha 1 sibling, 1 reply; 5+ messages in thread From: David Frey @ 2018-08-07 20:07 UTC (permalink / raw) To: linux-iio, himanshujha199640; +Cc: David Frey Temperature, pressure and humidity all expose and oversampling setting that works in the same way. Provide common handling for the oversampling sysfs attributes. Signed-off-by: David Frey <dpfrey@gmail.com> --- drivers/iio/chemical/bme680_core.c | 65 +++++++++++--------------------------- 1 file changed, 19 insertions(+), 46 deletions(-) diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c index 0e79d03ecc40..446cf1cbef23 100644 --- a/drivers/iio/chemical/bme680_core.c +++ b/drivers/iio/chemical/bme680_core.c @@ -91,8 +91,6 @@ static const struct iio_chan_spec bme680_channels[] = { }, }; -static const int bme680_oversampling_avail[] = { 1, 2, 4, 8, 16 }; - static int bme680_read_calib(struct bme680_data *data, struct bme680_calib *calib) { @@ -783,49 +781,14 @@ static int bme680_read_raw(struct iio_dev *indio_dev, } } -static int bme680_write_oversampling_ratio_temp(struct bme680_data *data, - int val) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { - if (bme680_oversampling_avail[i] == val) { - data->oversampling_temp = ilog2(val); - - return bme680_chip_config(data); - } - } - - return -EINVAL; -} - -static int bme680_write_oversampling_ratio_press(struct bme680_data *data, - int val) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { - if (bme680_oversampling_avail[i] == val) { - data->oversampling_press = ilog2(val); - - return bme680_chip_config(data); - } - } - - return -EINVAL; -} - -static int bme680_write_oversampling_ratio_humid(struct bme680_data *data, - int val) +static int bme680_oversampling_value_to_setting(int value) { int i; - - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { - if (bme680_oversampling_avail[i] == val) { - data->oversampling_humid = ilog2(val); - - return bme680_chip_config(data); - } + /* valid values are 2^n where n >=0 && n <= 4 */ + for (i = 0; i <= 4; i++) { + u8 setting = (1 << i); + if (setting == value) + return setting; } return -EINVAL; @@ -839,16 +802,26 @@ static int bme680_write_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: + { + int os_setting = bme680_oversampling_value_to_setting(val); + if (os_setting < 0) + return -EINVAL; + switch (chan->type) { case IIO_TEMP: - return bme680_write_oversampling_ratio_temp(data, val); + data->oversampling_temp = os_setting; + break; case IIO_PRESSURE: - return bme680_write_oversampling_ratio_press(data, val); + data->oversampling_press = os_setting; + break; case IIO_HUMIDITYRELATIVE: - return bme680_write_oversampling_ratio_humid(data, val); + data->oversampling_humid = os_setting; + break; default: return -EINVAL; } + return bme680_chip_config(data); + } default: return -EINVAL; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] iio: bme680: simplify oversampling handling 2018-08-07 20:07 ` [PATCH 2/2] iio: bme680: simplify oversampling handling David Frey @ 2018-08-08 10:47 ` Himanshu Jha 0 siblings, 0 replies; 5+ messages in thread From: Himanshu Jha @ 2018-08-08 10:47 UTC (permalink / raw) To: David Frey; +Cc: linux-iio On Tue, Aug 07, 2018 at 01:07:21PM -0700, David Frey wrote: > Temperature, pressure and humidity all expose and oversampling setting > that works in the same way. Provide common handling for the > oversampling sysfs attributes. > > Signed-off-by: David Frey <dpfrey@gmail.com> > --- > drivers/iio/chemical/bme680_core.c | 65 +++++++++++--------------------------- > 1 file changed, 19 insertions(+), 46 deletions(-) > > diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c > index 0e79d03ecc40..446cf1cbef23 100644 > --- a/drivers/iio/chemical/bme680_core.c > +++ b/drivers/iio/chemical/bme680_core.c > @@ -91,8 +91,6 @@ static const struct iio_chan_spec bme680_channels[] = { > }, > }; > > -static const int bme680_oversampling_avail[] = { 1, 2, 4, 8, 16 }; > - > static int bme680_read_calib(struct bme680_data *data, > struct bme680_calib *calib) > { > @@ -783,49 +781,14 @@ static int bme680_read_raw(struct iio_dev *indio_dev, > } > } > > -static int bme680_write_oversampling_ratio_temp(struct bme680_data *data, > - int val) > -{ > - int i; > - > - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { > - if (bme680_oversampling_avail[i] == val) { > - data->oversampling_temp = ilog2(val); > - > - return bme680_chip_config(data); > - } > - } > - > - return -EINVAL; > -} > - > -static int bme680_write_oversampling_ratio_press(struct bme680_data *data, > - int val) > -{ > - int i; > - > - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { > - if (bme680_oversampling_avail[i] == val) { > - data->oversampling_press = ilog2(val); > - > - return bme680_chip_config(data); > - } > - } > - > - return -EINVAL; > -} > - > -static int bme680_write_oversampling_ratio_humid(struct bme680_data *data, > - int val) > +static int bme680_oversampling_value_to_setting(int value) > { > int i; > - > - for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) { > - if (bme680_oversampling_avail[i] == val) { > - data->oversampling_humid = ilog2(val); > - > - return bme680_chip_config(data); > - } > + /* valid values are 2^n where n >=0 && n <= 4 */ > + for (i = 0; i <= 4; i++) { > + u8 setting = (1 << i); > + if (setting == value) > + return setting; > } No, this is not correct! Take example of: osrs_h<2:0> Humidity oversampling 011 oversampling * 4 And you will understand the ilog2() magic and also look bme680_chip_config() how that value is written. In your case, u8 osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK, data->oversampling_humid + 1); 4 + 1 = 5(101) will be written instead of 4(101). But I like the refactoring of using a generic oversampling function. Just use: while making a generic function bme680_write_oversampling_ratio() and everything is fine as is using the ilog2 magic. ... switch (chan->type) { case IIO_TEMP: return bme680_write_oversampling_ratio(data, val); case IIO_PRESSURE: return bme680_write_oversampling_ratio(data, val); case IIO_HUMIDITYRELATIVE: return bme680_write_oversampling_ratio(data, val); default: return -EINVAL; ... Thanks. -- Himanshu Jha Undergraduate Student Department of Electronics & Communication Guru Tegh Bahadur Institute of Technology ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-08 13:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-07 20:07 [PATCH 0/2] bme680 cleanup David Frey 2018-08-07 20:07 ` [PATCH 1/2] iio: bme680: perform cosmetic cleanup David Frey 2018-08-08 10:54 ` Himanshu Jha 2018-08-07 20:07 ` [PATCH 2/2] iio: bme680: simplify oversampling handling David Frey 2018-08-08 10:47 ` Himanshu Jha
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.