Messages in this thread | | | Date | Mon, 18 Sep 2023 14:42:11 +0300 | Subject | Re: [PATCH 2/3] iio: pressure: Support ROHM BU1390 | From | Matti Vaittinen <> |
| |
On 9/17/23 12:26, Jonathan Cameron wrote: > On Thu, 14 Sep 2023 14:47:44 +0300 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> On 9/10/23 16:22, Jonathan Cameron wrote: >>> On Wed, 6 Sep 2023 15:37:48 +0300 >>> Matti Vaittinen <mazziesaccount@gmail.com> 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! ~~
| |