From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753678AbaGGODe (ORCPT ); Mon, 7 Jul 2014 10:03:34 -0400 Received: from mail-lb0-f182.google.com ([209.85.217.182]:35260 "EHLO mail-lb0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753654AbaGGODa (ORCPT ); Mon, 7 Jul 2014 10:03:30 -0400 MIME-Version: 1.0 In-Reply-To: <1404022428-2816-1-git-send-email-borneo.antonio@gmail.com> References: <1404022428-2816-1-git-send-email-borneo.antonio@gmail.com> Date: Mon, 7 Jul 2014 10:03:29 -0400 Message-ID: Subject: Re: [PATCH] HID: cp2112: fix gpio value in gpio_direction_output From: Benjamin Tissoires To: Antonio Borneo Cc: Jiri Kosina , linux-input , David Barksdale , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jun 29, 2014 at 2:13 AM, Antonio Borneo wrote: > CP2112 does not offer an atomic method to set both gpio > direction and value. > Also it does not permit to set gpio value before putting > gpio in output. In fact, accordingly to Silicon Labs > AN495, Rev. 0.2, cpt. 4.4, the HID report to set gpio > values "does not affect any pins that are not configured > as outputs". > > This is confirmed on evaluation board CP2112-EK. > With current driver, after execute: > echo in > /sys/class/gpio/gpio248/direction > echo low > /sys/class/gpio/gpio248/direction > gpio output is still high. Only after a following > echo low > /sys/class/gpio/gpio248/direction > gpio output gets low. > > Fix driver by changing order of operations; first set > direction then set value. > > The drawback of this new sequence is that we can have > a pulse on gpio pin when direction is changed from > input to output-low, but this cannot be avoided on > current CP2112. In this case, does keeping the first cp2112_gpio_set() before setting the output direction prevents the pulse? If so, then you can just keep the current code, and simply add the cp2112_gpio_set() at the end of cp2112_gpio_direction_output(). In both case, this is: Reviewed-by: Benjamin Tissoires Cheers, Benjamin > > Signed-off-by: Antonio Borneo > --- > drivers/hid/hid-cp2112.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c > index 56be85a..3952d90 100644 > --- a/drivers/hid/hid-cp2112.c > +++ b/drivers/hid/hid-cp2112.c > @@ -240,8 +240,6 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, > u8 buf[5]; > int ret; > > - cp2112_gpio_set(chip, offset, value); > - > ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, > sizeof(buf), HID_FEATURE_REPORT, > HID_REQ_GET_REPORT); > @@ -260,6 +258,12 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, > return ret; > } > > + /* > + * Set gpio value when output direction is already set, > + * as specified in AN495, Rev. 0.2, cpt. 4.4 > + */ > + cp2112_gpio_set(chip, offset, value); > + > return 0; > } > > -- > 2.0.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html