Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933245AbbHJWlR (ORCPT ); Mon, 10 Aug 2015 18:41:17 -0400 Received: from seldrel01.sonyericsson.com ([37.139.156.2]:2757 "EHLO seldrel01.sonyericsson.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752341AbbHJWlP (ORCPT ); Mon, 10 Aug 2015 18:41:15 -0400 Date: Mon, 10 Aug 2015 15:41:09 -0700 From: Bjorn Andersson To: Dmitry Torokhov , Linus Walleij CC: "linux-input@vger.kernel.org" , "linux-kernel@vger.kernel.org" , John Stultz , Subject: Re: [PATCH] input: gpio_keys: Don't report events on gpio failure Message-ID: <20150810224109.GN6519@usrtlx11787.corpusers.net> References: <1438048204-632-1-git-send-email-bjorn.andersson@sonymobile.com> <20150728210040.GE19610@dtor-ws> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20150728210040.GE19610@dtor-ws> User-Agent: Mutt/1.5.22 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2622 Lines: 74 On Tue 28 Jul 14:00 PDT 2015, Dmitry Torokhov wrote: > Hi Bjorn, > > On Mon, Jul 27, 2015 at 06:50:04PM -0700, Bjorn Andersson wrote: > > In the cases where the gpio chip fails to acquire the current state an > > error is reported back to gpio_keys. This is currently interpreted as if > > the line went high, which just confuses the developer. > > > > This patch introduces an error print in this case and skipps the > > reporting of a input event; to aid in debugging this issue. > > > > Reported-by: John Stultz > > Signed-off-by: Bjorn Andersson > > --- > > drivers/input/keyboard/gpio_keys.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c > > index ddf4045de084..3ce3298ac09e 100644 > > --- a/drivers/input/keyboard/gpio_keys.c > > +++ b/drivers/input/keyboard/gpio_keys.c > > @@ -336,8 +336,14 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata) > > const struct gpio_keys_button *button = bdata->button; > > struct input_dev *input = bdata->input; > > unsigned int type = button->type ?: EV_KEY; > > - int state = (gpio_get_value_cansleep(button->gpio) ? 1 : 0) ^ button->active_low; > > + int state = gpio_get_value_cansleep(button->gpio); > > > > + if (state < 0) { > > + dev_err(input->dev.parent, "failed to get gpio state\n"); > > As far as I can see: > > static inline int gpio_get_value_cansleep(unsigned gpio) > { > return gpiod_get_raw_value_cansleep(gpio_to_desc(gpio)); > } > > int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc) > { > might_sleep_if(extra_checks); > if (!desc) > return 0; > return _gpiod_get_raw_value(desc); > } > > static bool _gpiod_get_raw_value(const struct gpio_desc *desc) > { > ... > } > > So how exactly do we get negative here? I'm sorry, I obviously didn't pay enough attention when running through that callstack... 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). Should we change _gpiod_get_raw_value() to propagate this error? Or should we just ignore this issue and propagate an error as GPIO high reading? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/