Re: [PATCH 1/2] iio: hrtimer-trigger: Mark hrtimer to expire in hard interrupt context

Jonathan Cameron
2020-09-25T12:42:09Z

On Thu, 24 Sep 2020 10:54:39 +0200
Stefan Agner <stefan@agner.ch> 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@Huawei.com>
> >> > On Mon, 21 Sep 2020 14:27:28 +0200
> >> > Sebastian Andrzej Siewior <bigeasy@linutronix.de> 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