Re: Should irq_enable/disable be flagging/unflagging gpio lines used as irq?

[Davide Hug <d@xxxxxxxxxx>]

On Fri, 8 Sep 2017 16:22:40 +0200
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> On Sat, Aug 26, 2017 at 12:45 AM, Davide Hug <d@xxxxxxxxxx> wrote:
> > 1. Naive approach: just `msleep` long enough.
> 
> This is wrong atleastm since msleep() has no guarantees for the
> time it sleeps, it's "this amount or more".

I read Documentation/timers/timers-howto.txt and msleep seemed the right choice
since it is suggested for delays >10ms and the measurement takes 20-230ms. Also
the sensor stores the measurement till it is retrieved so I thought it wouldn't
be a problem if msleep takes some more time.

Should I use usleep_range instead?

> 
> > 2. Naive polling: `msleep` repeatedly in a loop and check for the data line to be low each time.
> 
> Same problem.
> 
> > 3. Polling with timer: use a timer for polling and `wait_event_timeout` to wait for the polling to succeed.
> 
> This seems reasonable.
> 
> > 4. Fix the irq problem in the current driver by calling `devm_request_irq` and
> > `devm_free_irq` instead of just using `disable_irq_nosync` and `enable_irq`,
> > to prevent setting as output a gpio requested as irq.
> 
> This makes me feel there is something that needs to be fixed.
> 
> If IRQs are available we should always try to use them somehow.
> I don't remember the discussuin fully, but I guess the timer in (3)
> is started in response to an IRQ? That should work fine.

At each measurement the bidirectional data line is used by the sensor as data
ready signal. So the IRQ is used only to detect the data ready signal. After that
the IRQ is disabled to avoid triggering interrupts during the serial
communication.
In (3) the IRQ is replaced by polling. So it avoids using IRQs altogether. I
posted a patch implementing this in linux-hwmon.

Thanks
Davide
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html