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. > > > Yours, > -- Matti > > >