Received-SPF: pass (google.com: domain of linux-kernel+bounces-183130-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Sun, 19 May 2024 15:35:01 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Vasileios Amoiridis <vassilisamir@gmail.com>
Cc: lars@metafoo.de, andriy.shevchenko@linux.intel.com,
 ang.iglesiasg@gmail.com, mazziesaccount@gmail.com, ak@it-klinger.de,
 petre.rodan@subdimension.ro, phil@raspberrypi.com, 579lpy@gmail.com,
 linus.walleij@linaro.org, semen.protsenko@linaro.org,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 5/5] iio: pressure: bmp280: Add triggered buffer
 support
Message-ID: <20240519153501.7d30c7c2@jic23-huawei>
In-Reply-To: <20240512230524.53990-6-vassilisamir@gmail.com>
References: <20240512230524.53990-1-vassilisamir@gmail.com>
	<20240512230524.53990-6-vassilisamir@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 13 May 2024 01:05:24 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> BMP2xx, BME280, BMP3xx, and BMP5xx use continuous buffers for their
> temperature, pressure and humidity readings. This facilitates the
> use of burst/bulk reads in order to acquire data faster. The
> approach is different from the one used in oneshot captures.
> 
> BMP085 & BMP1xx devices use a completely different measurement
> process that is well defined and is used in their buffer_handler().
> 
> Suggested-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
>  
> +static irqreturn_t bmp280_buffer_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct bmp280_data *data = iio_priv(indio_dev);
> +	s32 adc_temp, adc_press, adc_humidity, t_fine;
> +	u8 sizeof_burst_read;
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	/*
> +	 * If humidity channel is enabled it means that we are called for the
> +	 * BME280 humidity sensor.
> +	 */
> +	if (test_bit(BME280_HUMID, indio_dev->active_scan_mask))

The only thing I though a bit about on this review was whether this
combined function really makes sense, or should we just move to two
separate handlers. It's marginal, but given you had it done this way
let us stick with this.

Definitely something to keep in mind for future changes though that may
make this more complex still.  For now it's fine.

Applied to the togreg branch of iio.git but given timing that is for now
only pushed out as testing and I'll rebase it on rc1 once available.

Thanks,

Jonathan


> +		sizeof_burst_read = BME280_BURST_READ_BYTES;
> +	else
> +		sizeof_burst_read = BMP280_BURST_READ_BYTES;
> +
> +	/* Burst read data registers */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> +			       data->buf, sizeof_burst_read);
> +	if (ret) {
> +		dev_err(data->dev, "failed to burst read sensor data\n");
> +		goto out;
> +	}
> +
> +	/* Temperature calculations */
> +	adc_temp = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[3]));
> +	if (adc_temp == BMP280_TEMP_SKIPPED) {
> +		dev_err(data->dev, "reading temperature skipped\n");
> +		goto out;
> +	}
> +
> +	data->sensor_data[1] = bmp280_compensate_temp(data, adc_temp);
> +
> +	/* Pressure calculations */
> +	adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[0]));
> +	if (adc_press == BMP280_PRESS_SKIPPED) {
> +		dev_err(data->dev, "reading pressure skipped\n");
> +		goto out;
> +	}
> +
> +	t_fine = bmp280_calc_t_fine(data, adc_temp);
> +
> +	data->sensor_data[0] = bmp280_compensate_press(data, adc_press, t_fine);
> +
> +	/* Humidity calculations */
> +	if (test_bit(BME280_HUMID, indio_dev->active_scan_mask)) {
> +		adc_humidity = get_unaligned_be16(&data->buf[6]);
> +
> +		if (adc_humidity == BMP280_HUMIDITY_SKIPPED) {
> +			dev_err(data->dev, "reading humidity skipped\n");
> +			goto out;
> +		}
> +		data->sensor_data[2] = bme280_compensate_humidity(data, adc_humidity, t_fine);
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
> +					   iio_get_time_ns(indio_dev));
> +
> +out:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>