Re: [PATCH 2/3] iio: pressure: Support ROHM BU1390

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/17/23 12:26, Jonathan Cameron wrote:
On Thu, 14 Sep 2023 14:47:44 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:

On 9/10/23 16:22, Jonathan Cameron wrote:
On Wed, 6 Sep 2023 15:37:48 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
Support for the ROHM BM1390 pressure sensor. The BM1390GLV-Z can measure
pressures ranging from 300 hPa to 1300 hPa with configurable measurement
averaging and internal FIFO. The sensor does also provide temperature
measurements.

Sensor does also contain IIR filter implemented in HW. The data-sheet
says the IIR filter can be configured to be "weak", "middle" or
"strong". Some RMS noise figures are provided in data sheet but no
accurate maths for the filter configurations is provided. Hence, the IIR
filter configuration is not supported by this driver and the filter is
configured to the "middle" setting (at least not for now).

+
+static irqreturn_t bm1390_irq_thread_handler(int irq, void *private)
+{
+	struct iio_dev *idev = private;
+	struct bm1390_data *data = iio_priv(idev);
+	int ret = IRQ_NONE;
+
+	mutex_lock(&data->mutex);
+
+	if (data->trigger_enabled) {
+		iio_trigger_poll_nested(data->trig);
+		ret = IRQ_HANDLED;
+	}
+
+	if (data->state == BM1390_STATE_FIFO) {

Can this and trigger_enabled be true?

Thanks for asking this question. Intention was that these are mutually
exclusive. However, I think that the check
if (iio_device_get_current_mode(idev) == INDIO_BUFFER_TRIGGERED)
in bm1390_buffer_postenable(), before calling the bm1390_fifo_enable()
is not 100% race free.

I, however, like the idea of having this check in the buffer-enable
function - I think it makes the design much more obvious. What I will do
is adding another check for:
	if (data->trigger_enable) {
		ret = -EBUSY;
		goto unlock_out;
	}

inside the bm1390_fifo_enable() to the section which holds the mutex.

You could make the exclusive nature obvious in the thread_handler by using an
else if () above.

Right. Thanks.




Yours,
	-- Matti





--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux