From: Adam Rizkalla <ajarizzo@gmail.com>
To: Vasileios Amoiridis <vassilisamir@gmail.com>
Cc: jic23@kernel.org, lars@metafoo.de, petre.rodan@subdimension.ro,
mazziesaccount@gmail.com, ak@it-klinger.de,
ang.iglesiasg@gmail.com, linus.walleij@linaro.org,
tgamblin@baylibre.com, phil@raspberrypi.com, 579lpy@gmail.com,
andriy.shevchenko@linux.intel.com, semen.protsenko@linaro.org,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 1/3] iio: pressure: bmp280: Generalize read_*() functions
Date: Sun, 14 Jul 2024 17:16:50 -0500 [thread overview]
Message-ID: <ZpROUow6p78VJsrO-ajarizzo@gmail.com> (raw)
In-Reply-To: <20240628171726.124852-2-vassilisamir@gmail.com>
On Fri, Jun 28, 2024 at 07:17:24PM +0200, Vasileios Amoiridis wrote:
> Add the coefficients for the IIO standard units and the IIO value
> inside the chip_info structure.
>
> Move the calculations for the IIO unit compatibility from inside the
> read_{temp,press,humid}() functions and move them to the general
> read_raw() function.
>
> In this way, all the data for the calculation of the value are
> located in the chip_info structure of the respective sensor.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
> drivers/iio/pressure/bmp280-core.c | 168 +++++++++++++++++------------
> drivers/iio/pressure/bmp280.h | 13 ++-
> 2 files changed, 108 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 49081b729618..9ff26a8e85d3 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -445,10 +445,8 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
> return (u32)p;
> }
>
> -static int bmp280_read_temp(struct bmp280_data *data,
> - int *val, int *val2)
> +static int bmp280_read_temp(struct bmp280_data *data, s32 *comp_temp)
> {
> - s32 comp_temp;
> u32 adc_temp;
> int ret;
>
> @@ -456,16 +454,15 @@ static int bmp280_read_temp(struct bmp280_data *data,
> if (ret)
> return ret;
>
> - comp_temp = bmp280_compensate_temp(data, adc_temp);
> + *comp_temp = bmp280_compensate_temp(data, adc_temp);
>
> - *val = comp_temp * 10;
> - return IIO_VAL_INT;
> + return 0;
> }
>
> -static int bmp280_read_press(struct bmp280_data *data,
> - int *val, int *val2)
> +static int bmp280_read_press(struct bmp280_data *data, u32 *comp_press)
> {
> - u32 comp_press, adc_press, t_fine;
> + u32 adc_press;
> + s32 t_fine;
> int ret;
>
> ret = bmp280_get_t_fine(data, &t_fine);
> @@ -476,17 +473,13 @@ static int bmp280_read_press(struct bmp280_data *data,
> if (ret)
> return ret;
>
> - comp_press = bmp280_compensate_press(data, adc_press, t_fine);
> -
> - *val = comp_press;
> - *val2 = 256000;
> + *comp_press = bmp280_compensate_press(data, adc_press, t_fine);
>
> - return IIO_VAL_FRACTIONAL;
> + return 0;
> }
>
> -static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> +static int bme280_read_humid(struct bmp280_data *data, u32 *comp_humidity)
> {
> - u32 comp_humidity;
> u16 adc_humidity;
> s32 t_fine;
> int ret;
> @@ -499,11 +492,9 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> if (ret)
> return ret;
>
> - comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine);
> -
> - *val = comp_humidity * 1000 / 1024;
> + *comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine);
>
> - return IIO_VAL_INT;
> + return 0;
> }
>
> static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
> @@ -511,6 +502,8 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
> int *val, int *val2, long mask)
> {
> struct bmp280_data *data = iio_priv(indio_dev);
> + int chan_value;
> + int ret;
>
> guard(mutex)(&data->lock);
>
> @@ -518,11 +511,29 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_PROCESSED:
> switch (chan->type) {
> case IIO_HUMIDITYRELATIVE:
> - return data->chip_info->read_humid(data, val, val2);
> + ret = data->chip_info->read_humid(data, &chan_value);
> + if (ret)
> + return ret;
> +
> + *val = data->chip_info->humid_coeffs[0] * chan_value;
> + *val2 = data->chip_info->humid_coeffs[1];
> + return data->chip_info->humid_coeffs_type;
> case IIO_PRESSURE:
> - return data->chip_info->read_press(data, val, val2);
> + ret = data->chip_info->read_press(data, &chan_value);
> + if (ret)
> + return ret;
> +
> + *val = data->chip_info->press_coeffs[0] * chan_value;
> + *val2 = data->chip_info->press_coeffs[1];
> + return data->chip_info->press_coeffs_type;
> case IIO_TEMP:
> - return data->chip_info->read_temp(data, val, val2);
> + ret = data->chip_info->read_temp(data, &chan_value);
> + if (ret)
> + return ret;
> +
> + *val = data->chip_info->temp_coeffs[0] * chan_value;
> + *val2 = data->chip_info->temp_coeffs[1];
> + return data->chip_info->temp_coeffs_type;
> default:
> return -EINVAL;
> }
> @@ -822,6 +833,8 @@ static int bmp280_chip_config(struct bmp280_data *data)
>
> static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
> static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
> +static const int bmp280_temp_coeffs[] = { 10, 1 };
> +static const int bmp280_press_coeffs[] = { 1, 256000 };
>
> const struct bmp280_chip_info bmp280_chip_info = {
> .id_reg = BMP280_REG_ID,
> @@ -850,6 +863,11 @@ const struct bmp280_chip_info bmp280_chip_info = {
> .num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail),
> .oversampling_press_default = BMP280_OSRS_PRESS_16X - 1,
>
> + .temp_coeffs = bmp280_temp_coeffs,
> + .temp_coeffs_type = IIO_VAL_FRACTIONAL,
> + .press_coeffs = bmp280_press_coeffs,
> + .press_coeffs_type = IIO_VAL_FRACTIONAL,
> +
> .chip_config = bmp280_chip_config,
> .read_temp = bmp280_read_temp,
> .read_press = bmp280_read_press,
> @@ -877,6 +895,7 @@ static int bme280_chip_config(struct bmp280_data *data)
> }
>
> static const u8 bme280_chip_ids[] = { BME280_CHIP_ID };
> +static const int bme280_humid_coeffs[] = { 1000, 1024 };
>
> const struct bmp280_chip_info bme280_chip_info = {
> .id_reg = BMP280_REG_ID,
> @@ -899,6 +918,13 @@ const struct bmp280_chip_info bme280_chip_info = {
> .num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail),
> .oversampling_humid_default = BME280_OSRS_HUMIDITY_16X - 1,
>
> + .temp_coeffs = bmp280_temp_coeffs,
> + .temp_coeffs_type = IIO_VAL_FRACTIONAL,
> + .press_coeffs = bmp280_press_coeffs,
> + .press_coeffs_type = IIO_VAL_FRACTIONAL,
> + .humid_coeffs = bme280_humid_coeffs,
> + .humid_coeffs_type = IIO_VAL_FRACTIONAL,
> +
> .chip_config = bme280_chip_config,
> .read_temp = bmp280_read_temp,
> .read_press = bmp280_read_press,
> @@ -1091,9 +1117,8 @@ static u32 bmp380_compensate_press(struct bmp280_data *data,
> return comp_press;
> }
>
> -static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
> +static int bmp380_read_temp(struct bmp280_data *data, s32 *comp_temp)
> {
> - s32 comp_temp;
> u32 adc_temp;
> int ret;
>
> @@ -1101,15 +1126,14 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
> if (ret)
> return ret;
>
> - comp_temp = bmp380_compensate_temp(data, adc_temp);
> + *comp_temp = bmp380_compensate_temp(data, adc_temp);
>
> - *val = comp_temp * 10;
> - return IIO_VAL_INT;
> + return 0;
> }
>
> -static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
> +static int bmp380_read_press(struct bmp280_data *data, u32 *comp_press)
> {
> - u32 adc_press, comp_press, t_fine;
> + u32 adc_press, t_fine;
> int ret;
>
> ret = bmp380_get_t_fine(data, &t_fine);
> @@ -1120,12 +1144,9 @@ static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
> if (ret)
> return ret;
>
> - comp_press = bmp380_compensate_press(data, adc_press, t_fine);
> -
> - *val = comp_press;
> - *val2 = 100000;
> + *comp_press = bmp380_compensate_press(data, adc_press, t_fine);
>
> - return IIO_VAL_FRACTIONAL;
> + return 0;
> }
>
> static int bmp380_read_calib(struct bmp280_data *data)
> @@ -1296,6 +1317,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
> static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
> static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
> static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID };
> +static const int bmp380_temp_coeffs[] = { 10, 1 };
> +static const int bmp380_press_coeffs[] = { 1, 100000 };
>
> const struct bmp280_chip_info bmp380_chip_info = {
> .id_reg = BMP380_REG_ID,
> @@ -1323,6 +1346,11 @@ const struct bmp280_chip_info bmp380_chip_info = {
> .num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail),
> .iir_filter_coeff_default = 2,
>
> + .temp_coeffs = bmp380_temp_coeffs,
> + .temp_coeffs_type = IIO_VAL_FRACTIONAL,
> + .press_coeffs = bmp380_press_coeffs,
> + .press_coeffs_type = IIO_VAL_FRACTIONAL,
> +
> .chip_config = bmp380_chip_config,
> .read_temp = bmp380_read_temp,
> .read_press = bmp380_read_press,
> @@ -1443,9 +1471,9 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> * for what is expected on IIO ABI.
> */
>
> -static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
> +static int bmp580_read_temp(struct bmp280_data *data, s32 *raw_temp)
> {
> - s32 raw_temp;
> + s32 value_temp;
> int ret;
>
> ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf,
> @@ -1455,25 +1483,19 @@ static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
> return ret;
> }
>
> - raw_temp = get_unaligned_le24(data->buf);
> - if (raw_temp == BMP580_TEMP_SKIPPED) {
> + value_temp = get_unaligned_le24(data->buf);
> + if (value_temp == BMP580_TEMP_SKIPPED) {
> dev_err(data->dev, "reading temperature skipped\n");
> return -EIO;
> }
> + *raw_temp = sign_extend32(value_temp, 23);
>
> - /*
> - * Temperature is returned in Celsius degrees in fractional
> - * form down 2^16. We rescale by x1000 to return millidegrees
> - * Celsius to respect IIO ABI.
> - */
> - raw_temp = sign_extend32(raw_temp, 23);
> - *val = ((s64)raw_temp * 1000) / (1 << 16);
> - return IIO_VAL_INT;
> + return 0;
> }
>
> -static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
> +static int bmp580_read_press(struct bmp280_data *data, u32 *raw_press)
> {
> - u32 raw_press;
> + u32 value_press;
> int ret;
>
> ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf,
> @@ -1483,18 +1505,14 @@ static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
> return ret;
> }
>
> - raw_press = get_unaligned_le24(data->buf);
> - if (raw_press == BMP580_PRESS_SKIPPED) {
> + value_press = get_unaligned_le24(data->buf);
> + if (value_press == BMP580_PRESS_SKIPPED) {
> dev_err(data->dev, "reading pressure skipped\n");
> return -EIO;
> }
> - /*
> - * Pressure is returned in Pascals in fractional form down 2^16.
> - * We rescale /1000 to convert to kilopascal to respect IIO ABI.
> - */
> - *val = raw_press;
> - *val2 = 64000; /* 2^6 * 1000 */
> - return IIO_VAL_FRACTIONAL;
> + *raw_press = value_press;
> +
> + return 0;
> }
>
> static const int bmp580_odr_table[][2] = {
> @@ -1830,6 +1848,9 @@ static int bmp580_chip_config(struct bmp280_data *data)
>
> static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
> static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT };
> +/* Instead of { 1000, 16 } we do this, to avoid overflow issues */
> +static const int bmp580_temp_coeffs[] = { 125, 13 };
I'm not really sure what we gain here from using 125/13 instead of
250/14...but I don't think it hurts either.
I don't have a way to test this with the latest kernel release
currently, but lgtm.
Acked-by: Adam Rizkalla <ajarizzo@gmail.com>
> +static const int bmp580_press_coeffs[] = { 1, 64000};
>
> const struct bmp280_chip_info bmp580_chip_info = {
> .id_reg = BMP580_REG_CHIP_ID,
> @@ -1856,6 +1877,11 @@ const struct bmp280_chip_info bmp580_chip_info = {
> .num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail),
> .iir_filter_coeff_default = 2,
>
> + .temp_coeffs = bmp580_temp_coeffs,
> + .temp_coeffs_type = IIO_VAL_FRACTIONAL_LOG2,
> + .press_coeffs = bmp580_press_coeffs,
> + .press_coeffs_type = IIO_VAL_FRACTIONAL,
> +
> .chip_config = bmp580_chip_config,
> .read_temp = bmp580_read_temp,
> .read_press = bmp580_read_press,
> @@ -2011,9 +2037,8 @@ static s32 bmp180_compensate_temp(struct bmp280_data *data, u32 adc_temp)
> return (bmp180_calc_t_fine(data, adc_temp) + 8) / 16;
> }
>
> -static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
> +static int bmp180_read_temp(struct bmp280_data *data, s32 *comp_temp)
> {
> - s32 comp_temp;
> u32 adc_temp;
> int ret;
>
> @@ -2021,10 +2046,9 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
> if (ret)
> return ret;
>
> - comp_temp = bmp180_compensate_temp(data, adc_temp);
> + *comp_temp = bmp180_compensate_temp(data, adc_temp);
>
> - *val = comp_temp * 100;
> - return IIO_VAL_INT;
> + return 0;
> }
>
> static int bmp180_read_press_adc(struct bmp280_data *data, u32 *adc_press)
> @@ -2087,9 +2111,9 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, u32 adc_press,
> return p + ((x1 + x2 + 3791) >> 4);
> }
>
> -static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
> +static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
> {
> - u32 comp_press, adc_press;
> + u32 adc_press;
> s32 t_fine;
> int ret;
>
> @@ -2101,12 +2125,9 @@ static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
> if (ret)
> return ret;
>
> - comp_press = bmp180_compensate_press(data, adc_press, t_fine);
> -
> - *val = comp_press;
> - *val2 = 1000;
> + *comp_press = bmp180_compensate_press(data, adc_press, t_fine);
>
> - return IIO_VAL_FRACTIONAL;
> + return 0;
> }
>
> static int bmp180_chip_config(struct bmp280_data *data)
> @@ -2117,6 +2138,8 @@ static int bmp180_chip_config(struct bmp280_data *data)
> static const int bmp180_oversampling_temp_avail[] = { 1 };
> static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };
> static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID };
> +static const int bmp180_temp_coeffs[] = { 100, 1 };
> +static const int bmp180_press_coeffs[] = { 1, 1000 };
>
> const struct bmp280_chip_info bmp180_chip_info = {
> .id_reg = BMP280_REG_ID,
> @@ -2137,6 +2160,11 @@ const struct bmp280_chip_info bmp180_chip_info = {
> ARRAY_SIZE(bmp180_oversampling_press_avail),
> .oversampling_press_default = BMP180_MEAS_PRESS_8X,
>
> + .temp_coeffs = bmp180_temp_coeffs,
> + .temp_coeffs_type = IIO_VAL_FRACTIONAL,
> + .press_coeffs = bmp180_press_coeffs,
> + .press_coeffs_type = IIO_VAL_FRACTIONAL,
> +
> .chip_config = bmp180_chip_config,
> .read_temp = bmp180_read_temp,
> .read_press = bmp180_read_press,
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index 7c30e4d523be..a3d2cd722760 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -446,10 +446,17 @@ struct bmp280_chip_info {
> int num_sampling_freq_avail;
> int sampling_freq_default;
>
> + const int *temp_coeffs;
> + const int temp_coeffs_type;
> + const int *press_coeffs;
> + const int press_coeffs_type;
> + const int *humid_coeffs;
> + const int humid_coeffs_type;
> +
> int (*chip_config)(struct bmp280_data *data);
> - int (*read_temp)(struct bmp280_data *data, int *val, int *val2);
> - int (*read_press)(struct bmp280_data *data, int *val, int *val2);
> - int (*read_humid)(struct bmp280_data *data, int *val, int *val2);
> + int (*read_temp)(struct bmp280_data *data, s32 *adc_temp);
> + int (*read_press)(struct bmp280_data *data, u32 *adc_press);
> + int (*read_humid)(struct bmp280_data *data, u32 *adc_humidity);
> int (*read_calib)(struct bmp280_data *data);
> int (*preinit)(struct bmp280_data *data);
> };
> --
> 2.25.1
>
next prev parent reply other threads:[~2024-07-14 22:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-28 17:17 [PATCH v9 0/3] iio: pressure: bmp280: Minor cleanup and triggered Vasileios Amoiridis
2024-06-28 17:17 ` [PATCH v9 1/3] iio: pressure: bmp280: Generalize read_*() functions Vasileios Amoiridis
2024-07-14 22:16 ` Adam Rizkalla [this message]
2024-07-16 17:08 ` Jonathan Cameron
2024-07-16 19:06 ` Vasileios Amoiridis
2024-06-28 17:17 ` [PATCH v9 2/3] iio: pressure: bmp280: Add SCALE, RAW values in channels and refactorize them Vasileios Amoiridis
2024-06-28 17:17 ` [PATCH v9 3/3] iio: pressure: bmp280: Add triggered buffer support Vasileios Amoiridis
2024-06-30 10:23 ` [PATCH v9 0/3] iio: pressure: bmp280: Minor cleanup and triggered Jonathan Cameron
2024-06-30 20:27 ` Vasileios Amoiridis
2024-07-13 10:00 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZpROUow6p78VJsrO-ajarizzo@gmail.com \
--to=ajarizzo@gmail.com \
--cc=579lpy@gmail.com \
--cc=ak@it-klinger.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=ang.iglesiasg@gmail.com \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linus.walleij@linaro.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mazziesaccount@gmail.com \
--cc=petre.rodan@subdimension.ro \
--cc=phil@raspberrypi.com \
--cc=semen.protsenko@linaro.org \
--cc=tgamblin@baylibre.com \
--cc=vassilisamir@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.