Messages in this thread | | | Subject | Re: [PATCH 3/3] HID: cp2112: fix broken gpio_direction_input callback | From | Sébastien Szymanski <> | Date | Mon, 6 Nov 2017 14:58:46 +0100 |
| |
On 11/06/2017 09:11 AM, Benjamin Tissoires wrote: > On Nov 02 2017 or thereabouts, Sébastien Szymanski wrote: >> When everything goes smoothly, ret is set to 0 which makes the function >> to return EIO error. >> >> Fixes: 8e9faa15469e ("HID: cp2112: fix gpio-callback error handling") >> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com> >> --- >> drivers/hid/hid-cp2112.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c >> index 28e3c18..f7754a6 100644 >> --- a/drivers/hid/hid-cp2112.c >> +++ b/drivers/hid/hid-cp2112.c >> @@ -205,12 +205,13 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) >> ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, >> CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, >> HID_REQ_SET_REPORT); >> - if (ret < 0) { >> + if (ret != CP2112_GPIO_CONFIG_LENGTH) { > > Ack for this.
As explained in the interface specification, the device doesn't answer to set reports, so the transfer should be CP2112_GPIO_CONFIG_LENGTH (5) bytes.
> >> hid_err(hdev, "error setting GPIO config: %d\n", ret); >> goto exit; >> } >> >> - ret = 0; >> + mutex_unlock(&dev->lock); >> + return 0; > > Wouldn't it be better to just turn > - return ret < 0 ? ret : -EIO; > into > + return ret <= 0 ? ret : -EIO; > at the end of the function?
Well, the commit I mentioned in the Fixes tag, changes from
- return ret <= 0 ? ret : -EIO;
to
+ return ret < 0 ? ret : -EIO;
because ret being 0 could mean that one of the hid_hw_raw_request returned 0.
Regards,
> > I'd rather keep the same exit path in both cases, error or success. > > Cheers, > Benjamin > > >> >> exit: >> mutex_unlock(&dev->lock); >> -- >> 2.7.3 >>
-- Sébastien Szymanski Software engineer, Armadeus Systems Tel: +33 (0)9 72 29 41 44 Fax: +33 (0)9 72 28 79 26
| |