On Thu, 24 Sep 2020 10:54:39 +0200 Stefan Agner <stefan@xxxxxxxx> wrote: > On 2020-09-24 08:41, Sanchayan Maity wrote: > > On 20-09-22 02:51:11, Andy Duan wrote: > >> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > >> > On Mon, 21 Sep 2020 14:27:28 +0200 > >> > Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote: > >> > > >> > > On 2020-09-21 10:57:03 [+0100], Jonathan Cameron wrote: > >> > > > So looking at this the other way, are there any significant risks > >> > > > associated with this change? If not I'm tempted to queue them up > >> > > > and we have the rcX time to fix anything we've missed (just like > >> > > > every other patch!) > >> > > > >> > > I've been told that it only performs IRQ-thread wake-ups in hard-IRQ > >> > > context. This is fine then. > >> > > > >> > drivers/iio/adc/vf610-adc.c > >> > > >> > However, there looks to be a lot more wrong in there than just this. > >> > So normally for a device with a data ready signal like this we would hook up as > >> > follows. > >> > > >> > Data ready #1 -> IRQ chip (trigger) -> Read sensor #1 + > >> > iio_trigger_notify_done() > >> > -> Read sensor #2 + > >> > iio_trigger_notify_done() > >> > > >> > (note that the read etc is normally in a thread - all we do in interrupt context is > >> > usually to grab a timestamp if that makes sense for a given sensor). > >> > > >> > This driver does both of. > >> > Data ready -> Read data from itself and call iio_trigger_notify_done() IRQ chip > >> > for a different trigger -> Take a timestamp and never call > >> > iio_trigger_notify_done() > >> > or read any data for that matter. > >> > > >> > Which won't do what we want at all. > >> > > >> > Andy, if I have a go at fixing this are you able to test the result? > >> > I think the simplest is probably to introduce a trigger to tie the two halves > >> > together. > >> > We can set it as the default trigger so things should keep on working for existing > >> > users. > >> > > >> > For more general case, we should probably have two functions. > >> > > >> > iio_trigger_notify_done() which is only called from places we can sleep. > >> > iio_trigger_notify_done_no_action() which only decrements the counter (or > >> > given this is only called inside industrialio-trigger.c could just replace with > >> > atomic_dec(&trig->use_count)). > >> > > >> > >> Sanchayan, can you help to verify the fixes that Jonathan will send out ? > >> > > > > Sorry for the delay in reply. Unfortunately can't as I do not access to the > > hardware having left Toradex. > > > > CCed Stefan Agner who might be able to help. > > > > @Stefan > > > > Hello Stefan :), may be you can help here? > > Hi all, > > I do have access to the hardware and can run a test if required. I guess > I would just need to test if ADC sampling still work with something like > tools/iio/iio_generic_buffer.c? Yup. I haven't written the fix yet though. Will make sure to cc you! Thanks, Jonathan > > -- > Stefan
