Re: [PATCH] input: gpio_keys: Don't report events on gpio failure

[Alexandre Courbot <gnurou@xxxxxxxxx>]

On Tue, Aug 18, 2015 at 4:34 AM, Bjorn Andersson
<bjorn.andersson@xxxxxxxxxxxxxx> wrote:
> On Sun 16 Aug 23:59 PDT 2015, Alexandre Courbot wrote:
>
>> On Thu, Aug 13, 2015 at 10:06 PM, Linus Walleij
>> <linus.walleij@xxxxxxxxxx> wrote:
>> > On Tue, Aug 11, 2015 at 12:41 AM, Bjorn Andersson
>> > <bjorn.andersson@xxxxxxxxxxxxxx> wrote:
>> >
>> >> But then the question first goes to Linus & co.
>> >>
>> >> gpio_chip->get() can return a negative value to indicate errors (and did
>> >> so in this case), all parts of the API seems indicates that we can get
>> >> an error (int vs bool).
>> >
>> > Ooops.
>> >
>> >> Should we change _gpiod_get_raw_value() to propagate this error?
>> >
>> > Yes for now. Can you patch it? :)
>> >
>> >>  Or
>> >> should we just ignore this issue and propagate an error as GPIO high
>> >> reading?
>> >
>> > I don't know about the future. In some sense GPIOs are so smallish
>> > resources that errorhandling every call to read/write them seem to
>> > be a royal PITA. That is why I wanted to switch them to bool and get
>> > rid of the problem, but now I also see that maybe that was not such a
>> > smart idea, if errors do occur on the set/get_value path.
>>
>> Nowadays GPIOs may reside at the other end of an i2c bus, which means
>> that even the simplest operation like reading a GPIO value can
>> potentially fail. And it will probably not get better - wait until we
>> implement GPIO-over-IP! :)
>>
>
> Now that's progress! I can't wait ;)
>
>> So I'd say it makes sense to propagate errors returned by the driver's
>> get() hook. This might contradict some of our earlier statements about
>> simplifying the GPIO API, but is preferrable to having to make a
>> decision as to which valid value to return if the driver fails...
>>
>
> Sounds good.
>
> As we're patching up _gpiod_get_raw_value(), is the lack of a get()
> implementation the same as a LOW or is that -ENOTSUPP?

I don't see any reason why it should not be -ENOTSUPP if we start to
manage errors properly.

>
>> It should then be made very clear in the documentation that the only
>> positive values ever returned by the GPIO API will be 0 and 1 (we
>> already have a clamping mechanism for that IIRC), and that negative
>> values are propagated as-is.
>>
>
> That makes sense. I'm however not able to find such clamping
> macro/mechanism and it would be very beneficial here...
>
>> Linus, does that seem reasonable to you? Does anyone has the intention
>> to address that one or should I add it to my short-term TODO list?
>
> If you have some input on above (is lack of get() an error) I can hack
> up the patch.

Excellent - since Linus gave his thumb up, I think you can go ahead.
Looking forward to seeing this finally fixed.

Thanks!
Alex.
--
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