Re: [PATCH v2 2/6] iio: pressure: Simplify read_* functions

Jonathan Cameron
2024-03-14T14:38:30Z

On Wed, 13 Mar 2024 18:40:03 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add the coefficients for the IIO standard units inside the chip_info
> structure.
>
> Remove the calculations with the coefficients for the IIO compatibility
> from inside the read_(temp/press/humid) functions and move it to the
> read_raw function.
>
> Execute the calculations with the coefficients inside the read_raw
> oneshot capture functions.
>
> Also fix raw_* and comp_* values signs.

That needs some more explanation. Is this a fix that needs backporting?

>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
> drivers/iio/pressure/bmp280-core.c | 150 +++++++++++++----------------
> drivers/iio/pressure/bmp280.h | 10 +-
> 2 files changed, 74 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 871b2214121b..dfd845acfa22 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -363,8 +363,7 @@ 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 s32 bmp280_read_temp(struct bmp280_data *data)

It's returning error codes through here. If you need to force a specific type,
don't use the return code. Pass in a variable to put it in.

static int bm280_read_temp(struct bmp280_data *data, u32 *result)

> {
> s32 adc_temp, comp_temp;
> int ret;
> @@ -384,27 +383,17 @@ static int bmp280_read_temp(struct bmp280_data *data,
> }
> comp_temp = bmp280_compensate_temp(data, adc_temp);
>
> - /*
> - * val might be NULL if we're called by the read_press routine,
> - * who only cares about the carry over t_fine value.
> - */
> - if (val) {
> - *val = comp_temp * 10;
> - return IIO_VAL_INT;
> - }
> -
> - return 0;
> + return comp_temp;
> }
>
> -static int bmp280_read_press(struct bmp280_data *data,
> - int *val, int *val2)
> +static u32 bmp280_read_press(struct bmp280_data *data)
> {
> u32 comp_press;
> s32 adc_press;
> int ret;
>
> /* Read and compensate temperature so we get a reading of t_fine. */
> - ret = bmp280_read_temp(data, NULL, NULL);
> + ret = bmp280_read_temp(data);
> if (ret < 0)
> return ret;
>
> @@ -423,20 +412,17 @@ static int bmp280_read_press(struct bmp280_data *data,
> }
> comp_press = bmp280_compensate_press(data, adc_press);
>
> - *val = comp_press;
> - *val2 = 256000;
> -
> - return IIO_VAL_FRACTIONAL;
> + return comp_press;
> }
>
> -static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> +static u32 bmp280_read_humid(struct bmp280_data *data)
> {
> u32 comp_humidity;
> s32 adc_humidity;
> int ret;
>
> /* Read and compensate temperature so we get a reading of t_fine. */
> - ret = bmp280_read_temp(data, NULL, NULL);
> + ret = bmp280_read_temp(data);
> if (ret < 0)
> return ret;
>
> @@ -455,9 +441,7 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> }
> comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>
> - *val = comp_humidity * 1000 / 1024;
> -
> - return IIO_VAL_INT;
> + return comp_humidity;
> }
>
> static int bmp280_read_raw(struct iio_dev *indio_dev,
> @@ -474,13 +458,27 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_PROCESSED:
> switch (chan->type) {
> case IIO_HUMIDITYRELATIVE:
> - ret = data->chip_info->read_humid(data, val, val2);
> + ret = data->chip_info->read_humid(data);

You can still get an error code back from this and you must handle it.
Previously it was fine because both errors and IIO_VAL_ were returned by
the callback so error returns were handled, now they aren't.

> + *val = data->chip_info->humid_coeffs[0] * ret;
> + *val2 = data->chip_info->humid_coeffs[1];
> + ret = IIO_VAL_FRACTIONAL;
> break;
Same applies in all the other cases.

Jonathan