On Mon, Jul 7, 2014 at 10:03 PM, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx> wrote: > On Sun, Jun 29, 2014 at 2:13 AM, Antonio Borneo > <borneo.antonio@xxxxxxxxx> 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(). Hi Benjamin, unfortunately this would not prevent the pulse. In fact: a) if gpio is already output - the first cp2112_gpio_set() will set the value - then cp2112_gpio_direction_output() would be redundant - finally the second cp2112_gpio_set() would be redundant too. b) if gpio is input - the first cp2112_gpio_set() would be ignored, as explained in AN495 - then cp2112_gpio_direction_output() would put gpio in output. Accordingly to my experiments the value is set high - finally the second cp2112_gpio_set() would set the proper value. If value is different from what is set by cp2112_gpio_direction_output() we get a pulse. That's why I removed the first cp2112_gpio_set() or, better, I moved it after setting gpio direction. Thanks for the review. Antonio > > In both case, this is: > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > Cheers, > Benjamin > >> >> Signed-off-by: Antonio Borneo <borneo.antonio@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
