Re: [PATCH v1 12/13] iio: chemical: bme680: Add triggered buffer support

Andy Shevchenko
2024-10-11T10:38:03Z

On Thu, Oct 10, 2024 at 11:00:29PM +0200, vamoirid wrote:
> From: Vasileios Amoiridis <vassilisamir@gmail.com>
>
> Add triggered buffer and soft timestamp support. The available scan mask
> enables all the channels of the sensor in order to follow the operation of
> the sensor. The sensor basically starts to capture from all channels
> as long as it enters into FORCED mode.

...

> struct regulator_bulk_data supplies[BME680_NUM_SUPPLIES];
> int ambient_temp;
>
> + u8 buffer[ALIGN(sizeof(s32) * BME680_NUM_CHANNELS, sizeof(s64))
> + + sizeof(s64)] __aligned(sizeof(s64));

Can it be represented as a structure?
We also have aligned_s64 for the timestamp.

> union {
> - u8 buf[3];
> + u8 buf[15];
> unsigned int check;
> __be16 be16;
> u8 bme680_cal_buf_1[BME680_CALIB_RANGE_1_LEN];

...

> +static irqreturn_t bme680_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct bme680_data *data = iio_priv(indio_dev);
> + u32 adc_temp, adc_press, adc_humid, comp_press, comp_humid;

> + s32 *chans = (s32 *)data->buffer;

With the structure in place this becomes much more readable.

> + u16 adc_gas_res, gas_regs_val;
> + s32 t_fine, comp_gas_res;
> + s16 comp_temp;
> + u8 gas_range;
> + int ret;
> +
> + guard(mutex)(&data->lock);
> +
> + ret = bme680_set_mode(data, BME680_FORCED);
> + if (ret < 0)
> + goto out;
> +
> + ret = bme680_wait_for_eoc(data);
> + if (ret)
> + goto out;
> +
> + /* Burst read data regs */
> + ret = regmap_bulk_read(data->regmap, BME680_REG_MEAS_STAT_0,
> + data->buf, sizeof(data->buf));
> + if (ret) {
> + dev_err(data->dev, "failed to burst read sensor data\n");
> + goto out;
> + }
> + if (data->buf[0] & BME680_GAS_MEAS_BIT) {
> + dev_err(data->dev, "gas measurement incomplete\n");
> + goto out;
> + }
> +
> + /* Temperature calculations */
> + adc_temp = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[5]));
> + if (adc_temp == BME680_MEAS_SKIPPED) {
> + dev_err(data->dev, "reading temperature skipped\n");
> + goto out;
> + }
> + comp_temp = bme680_compensate_temp(data, adc_temp);
> + t_fine = bme680_calc_t_fine(data, adc_temp);
> +
> + /* Pressure calculations */
> + adc_press = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[2]));
> + if (adc_press == BME680_MEAS_SKIPPED) {
> + dev_err(data->dev, "reading pressure skipped\n");
> + goto out;
> + }
> + comp_press = bme680_compensate_press(data, adc_press, t_fine);

> + pr_info("comp_press: %d\n", comp_press);

No debugging in the production code. Or if you really need that, it should
use dev_dbg().

> + /* Humidity calculations */
> + adc_humid = get_unaligned_be16(&data->buf[8]);
> + if (adc_humid == BME680_MEAS_SKIPPED) {
> + dev_err(data->dev, "reading humidity skipped\n");
> + goto out;
> + }
> + comp_humid = bme680_compensate_humid(data, adc_humid, t_fine);

> + pr_info("comp_humid: %d\n", comp_humid);

Ditto.

> +
> + /* Gas calculations */
> + gas_regs_val = get_unaligned_be16(&data->buf[13]);
> + adc_gas_res = FIELD_GET(BME680_ADC_GAS_RES, gas_regs_val);
> + if ((gas_regs_val & BME680_GAS_STAB_BIT) == 0) {
> + dev_err(data->dev, "heater failed to reach the target temperature\n");
> + goto out;
> + }
> + gas_range = FIELD_GET(BME680_GAS_RANGE_MASK, gas_regs_val);
> + comp_gas_res = bme680_compensate_gas(data, adc_gas_res, gas_range);
> + pr_info("comp_gas_res: %d\n", comp_gas_res);
> +
> + chans[0] = comp_temp;
> + chans[1] = comp_press;
> + chans[2] = comp_humid;
> + chans[3] = comp_gas_res;
> +
> + /* Push to buffer */
> + iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> + iio_get_time_ns(indio_dev));
> +out:
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}

--
With Best Regards,
Andy Shevchenko