Re: [PATCH] input: gpio_keys: Don't report events on gpio failure
From: Alexandre Courbot
Date: Mon Aug 31 2015 - 00:52:11 EST
On Tue, Aug 18, 2015 at 4:34 AM, Bjorn Andersson
> 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.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/