Re: iio: dht11 broken

Re: iio: dht11 broken

[Harald Geyer]

Hi Richard!

Richard Weinberger writes:
> Staring with 3.12 gpiolib refuses to set an GPIO used as IRQ into output mode.
> see commit d468bf9e (gpio: add API to be strict about GPIO IRQ usage)
> The dht driver does this and is therefore not functional.
> As the dht11 driver got merged in 3.13 there is a high chance that it
> never worked.

Well, I'm developing on mxs and there it works on 3.13 and 3.14 at least.
(I have it in production using 3.14.18 ATM.)
 
> Harald, are you aware of this issue?

I got one report about this issue, but couldn't reproduce it yet and
the reporter didn't agree to quoting his messages to kernel mailing lists,
so this has been rather low priority on my todo list. Next plan is to
see if I can reproduce the issue on sunxi (A10).

However anybody able to reproduce this and willing to test patches, would
of course push this up on my todo list.

Any input from gpiolib people about what the proper fix would be in
their opinion, of course is appreciated. I don't think the dht11 driver
is doing anything unreasonable per se, so maybe they can clarify how
the API is intended to be used?

Harald

Re: iio: dht11 broken

[Richard Weinberger]

Hi!

Am 01.12.2014 um 11:49 schrieb Harald Geyer:
> Hi Richard!
> 
> Richard Weinberger writes:
>> Staring with 3.12 gpiolib refuses to set an GPIO used as IRQ into output mode.
>> see commit d468bf9e (gpio: add API to be strict about GPIO IRQ usage)
>> The dht driver does this and is therefore not functional.
>> As the dht11 driver got merged in 3.13 there is a high chance that it
>> never worked.
> 
> Well, I'm developing on mxs and there it works on 3.13 and 3.14 at least.
> (I have it in production using 3.14.18 ATM.)
>  
>> Harald, are you aware of this issue?
> 
> I got one report about this issue, but couldn't reproduce it yet and
> the reporter didn't agree to quoting his messages to kernel mailing lists,
> so this has been rather low priority on my todo list. Next plan is to
> see if I can reproduce the issue on sunxi (A10).

Not all gpio drivers use the new IRQ lock stuff.

> However anybody able to reproduce this and willing to test patches, would
> of course push this up on my todo list.
> 
> Any input from gpiolib people about what the proper fix would be in
> their opinion, of course is appreciated. I don't think the dht11 driver
> is doing anything unreasonable per se, so maybe they can clarify how
> the API is intended to be used?

Your driver sets up a GPIO as IRQ and then you switch the mode to output
to enable the DHT sensor.
This is not an API issue, it is an design issue.
I'm not a hardware expert but AFAIK setting an IRQ pin into output mode can
lead to undefined behavior.

Thanks,
//richard

Re: iio: dht11 broken

[Linus Walleij]

On Mon, Dec 1, 2014 at 11:59 AM, Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org> wrote:
> Am 01.12.2014 um 11:49 schrieb Harald Geyer:

>> Any input from gpiolib people about what the proper fix would be in
>> their opinion, of course is appreciated. I don't think the dht11 driver
>> is doing anything unreasonable per se, so maybe they can clarify how
>> the API is intended to be used?
>
> Your driver sets up a GPIO as IRQ and then you switch the mode to output
> to enable the DHT sensor.
> This is not an API issue, it is an design issue.
> I'm not a hardware expert but AFAIK setting an IRQ pin into output mode can
> lead to undefined behavior.

Well that check is there exactly to prevent madness like this.

IRQs on pins set to output *does* *not* *make* *sense*.

Remove the IRQ before setting the pin to output and vice
versa and everyone will be happier.

Yours,
Linus Walleij

Re: iio: dht11 broken

[Richard Weinberger]

Am 01.12.2014 um 15:08 schrieb Linus Walleij:
> On Mon, Dec 1, 2014 at 11:59 AM, Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org> wrote:
>> Am 01.12.2014 um 11:49 schrieb Harald Geyer:
> 
>>> Any input from gpiolib people about what the proper fix would be in
>>> their opinion, of course is appreciated. I don't think the dht11 driver
>>> is doing anything unreasonable per se, so maybe they can clarify how
>>> the API is intended to be used?
>>
>> Your driver sets up a GPIO as IRQ and then you switch the mode to output
>> to enable the DHT sensor.
>> This is not an API issue, it is an design issue.
>> I'm not a hardware expert but AFAIK setting an IRQ pin into output mode can
>> lead to undefined behavior.
> 
> Well that check is there exactly to prevent madness like this.
> 
> IRQs on pins set to output *does* *not* *make* *sense*.
> 
> Remove the IRQ before setting the pin to output and vice
> versa and everyone will be happier.

Thanks for the confirmation.
I'll cook up a patch.

Thanks,
//richard