| Message ID | 1418691016-30681-2-git-send-email-peter@lekensteyn.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
Hi Peter, On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote: > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > these errors too to avoid 5 second delays when the device reports an > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > it has no functional difference. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > --- I'd like to have Nestor's opinion on this. I did not manage to find on the documentation that HID++ 2.0 Long report error code is 0xff, so introducing this change without Logitech's blessing would be unfortunate. I understand this will fix your qemu problem, but I am not entirely sure if we do not have to check on 0xff and 0x8f in both short and long responses. Cheers, Benjamin > drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > index 2f420c0..ae23dec 100644 > --- a/drivers/hid/hid-logitech-hidpp.c > +++ b/drivers/hid/hid-logitech-hidpp.c > @@ -105,6 +105,7 @@ struct hidpp_device { > }; > > > +/* HID++ 1.0 error codes */ > #define HIDPP_ERROR 0x8f > #define HIDPP_ERROR_SUCCESS 0x00 > #define HIDPP_ERROR_INVALID_SUBID 0x01 > @@ -119,6 +120,8 @@ struct hidpp_device { > #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a > #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b > #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c > +/* HID++ 2.0 error codes */ > +#define HIDPP20_ERROR 0xff > > static void hidpp_connect_event(struct hidpp_device *hidpp_dev); > > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, > } > > if (response->report_id == REPORT_ID_HIDPP_SHORT && > - response->fap.feature_index == HIDPP_ERROR) { > + response->rap.sub_id == HIDPP_ERROR) { > + ret = response->rap.params[1]; > + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); > + goto exit; > + } > + > + if (response->report_id == REPORT_ID_HIDPP_LONG && > + response->fap.feature_index == HIDPP20_ERROR) { > ret = response->fap.params[1]; > - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); > + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); > goto exit; > } > > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, > static inline bool hidpp_match_error(struct hidpp_report *question, > struct hidpp_report *answer) > { > - return (answer->fap.feature_index == HIDPP_ERROR) && > + return ((answer->rap.sub_id == HIDPP_ERROR) || > + (answer->fap.feature_index == HIDPP20_ERROR)) && > (answer->fap.funcindex_clientid == question->fap.feature_index) && > (answer->fap.params[0] == question->fap.funcindex_clientid); > } > -- > 2.1.3 > > -- > 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 -- 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
On Tuesday 16 December 2014 09:33:44 Benjamin Tissoires wrote: > On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote: > > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > > these errors too to avoid 5 second delays when the device reports an > > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > > it has no functional difference. > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > > --- > > I'd like to have Nestor's opinion on this. I did not manage to find on > the documentation that HID++ 2.0 Long report error code is 0xff, so > introducing this change without Logitech's blessing would be > unfortunate. > I understand this will fix your qemu problem, but I am not entirely > sure if we do not have to check on 0xff and 0x8f in both short and > long responses. > > Cheers, > Benjamin The error code was found by probing the hardware. The HID++ 2.0 spec does define some error codes, for example an OutOfRange error when GetFeatureID is called with a featureIndex greater than the available features count. The documentation also defines the valid FeatureIndex range as 1..254, so I thought it was reasonable to assume that 0xff is the HID++ 2.0 error indicator. Nestor, so far I have only seen the OutOfRange error when the arguments are invalid. Are there other cases where HID++ 2.0 are reported instead of HID++ 1.0? QEMU was not the problem though, it was just a bug in my usb-ltunify-receiver device emulation which exposed this missing check. Kind regards, Peter > > drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > > index 2f420c0..ae23dec 100644 > > --- a/drivers/hid/hid-logitech-hidpp.c > > +++ b/drivers/hid/hid-logitech-hidpp.c > > @@ -105,6 +105,7 @@ struct hidpp_device { > > }; > > > > > > +/* HID++ 1.0 error codes */ > > #define HIDPP_ERROR 0x8f > > #define HIDPP_ERROR_SUCCESS 0x00 > > #define HIDPP_ERROR_INVALID_SUBID 0x01 > > @@ -119,6 +120,8 @@ struct hidpp_device { > > #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a > > #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b > > #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c > > +/* HID++ 2.0 error codes */ > > +#define HIDPP20_ERROR 0xff > > > > static void hidpp_connect_event(struct hidpp_device *hidpp_dev); > > > > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, > > } > > > > if (response->report_id == REPORT_ID_HIDPP_SHORT && > > - response->fap.feature_index == HIDPP_ERROR) { > > + response->rap.sub_id == HIDPP_ERROR) { > > + ret = response->rap.params[1]; > > + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); > > + goto exit; > > + } > > + > > + if (response->report_id == REPORT_ID_HIDPP_LONG && > > + response->fap.feature_index == HIDPP20_ERROR) { > > ret = response->fap.params[1]; > > - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); > > + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); > > goto exit; > > } > > > > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, > > static inline bool hidpp_match_error(struct hidpp_report *question, > > struct hidpp_report *answer) > > { > > - return (answer->fap.feature_index == HIDPP_ERROR) && > > + return ((answer->rap.sub_id == HIDPP_ERROR) || > > + (answer->fap.feature_index == HIDPP20_ERROR)) && > > (answer->fap.funcindex_clientid == question->fap.feature_index) && > > (answer->fap.params[0] == question->fap.funcindex_clientid); > > } > > -- > > 2.1.3 -- 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
On Dec 16 2014 or thereabouts, Peter Wu wrote: > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > these errors too to avoid 5 second delays when the device reports an > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > it has no functional difference. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > --- Jiri, it looks like this one fall off from your radar. It's not a problem per-se, I'd like to have some feedbacks from Logitech first, but still, there is a bug and Peter fixed it :) Cheers, Benjamin > drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > index 2f420c0..ae23dec 100644 > --- a/drivers/hid/hid-logitech-hidpp.c > +++ b/drivers/hid/hid-logitech-hidpp.c > @@ -105,6 +105,7 @@ struct hidpp_device { > }; > > > +/* HID++ 1.0 error codes */ > #define HIDPP_ERROR 0x8f > #define HIDPP_ERROR_SUCCESS 0x00 > #define HIDPP_ERROR_INVALID_SUBID 0x01 > @@ -119,6 +120,8 @@ struct hidpp_device { > #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a > #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b > #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c > +/* HID++ 2.0 error codes */ > +#define HIDPP20_ERROR 0xff > > static void hidpp_connect_event(struct hidpp_device *hidpp_dev); > > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, > } > > if (response->report_id == REPORT_ID_HIDPP_SHORT && > - response->fap.feature_index == HIDPP_ERROR) { > + response->rap.sub_id == HIDPP_ERROR) { > + ret = response->rap.params[1]; > + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); > + goto exit; > + } > + > + if (response->report_id == REPORT_ID_HIDPP_LONG && > + response->fap.feature_index == HIDPP20_ERROR) { > ret = response->fap.params[1]; > - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); > + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); > goto exit; > } > > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, > static inline bool hidpp_match_error(struct hidpp_report *question, > struct hidpp_report *answer) > { > - return (answer->fap.feature_index == HIDPP_ERROR) && > + return ((answer->rap.sub_id == HIDPP_ERROR) || > + (answer->fap.feature_index == HIDPP20_ERROR)) && > (answer->fap.funcindex_clientid == question->fap.feature_index) && > (answer->fap.params[0] == question->fap.funcindex_clientid); > } > -- > 2.1.3 > -- 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
On Tuesday 16 December 2014 09:33:44 Benjamin Tissoires wrote: > Hi Peter, > > On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote: > > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > > these errors too to avoid 5 second delays when the device reports an > > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > > it has no functional difference. > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > > --- > > I'd like to have Nestor's opinion on this. I did not manage to find on > the documentation that HID++ 2.0 Long report error code is 0xff, so > introducing this change without Logitech's blessing would be > unfortunate. > I understand this will fix your qemu problem, but I am not entirely > sure if we do not have to check on 0xff and 0x8f in both short and > long responses. > > Cheers, > Benjamin Hi Benjamin, The Logitech Unifying extension for Chrome[1] is documented quite well and contains details which were not public before (including names and descriptions for all registers and subIDs!). In lib/devices/HidppFap.js you can find this logic for handling HID++ 2.0 messages: if ((reqView.getUint8(1) == rspView.getUint8(1)) // device index && (reqView.getUint8(2) == rspView.getUint8(2)) // feature index && (reqView.getUint8(3) == rspView.getUint8(3))) // function/event ID + software ID { result.matchResult = devices.MATCH_RESULT.SUCCESS; } else if ((reqView.getUint8(1) == rspView.getUint8(1)) // device index && (0xFF == rspView.getUint8(2)) // Hid++ 2.0 error && (reqView.getUint8(2) == rspView.getUint8(3)) // feature index && (reqView.getUint8(3) == rspView.getUint8(4))) // function/event ID + software ID { result.errCode = rspView.getUint8(5); // FAP_ERROR result.matchResult = devices.MATCH_RESULT.ERROR; } Looks like a sufficient proof that 0xFF is the correct number to detect HID++ 2.0 errors right? In HID++ 1.0 devices ("rap"), 0xFF is named as "SYNC" (with no further comments), so this will probably not trigger false positives either. Kind regards, Peter [1]: https://chrome.google.com/webstore/detail/logitech-unifying-for-chr/agpmgihmmmfkbhckmciedmhincdggomo > > drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > > index 2f420c0..ae23dec 100644 > > --- a/drivers/hid/hid-logitech-hidpp.c > > +++ b/drivers/hid/hid-logitech-hidpp.c > > @@ -105,6 +105,7 @@ struct hidpp_device { > > }; > > > > > > +/* HID++ 1.0 error codes */ > > #define HIDPP_ERROR 0x8f > > #define HIDPP_ERROR_SUCCESS 0x00 > > #define HIDPP_ERROR_INVALID_SUBID 0x01 > > @@ -119,6 +120,8 @@ struct hidpp_device { > > #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a > > #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b > > #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c > > +/* HID++ 2.0 error codes */ > > +#define HIDPP20_ERROR 0xff > > > > static void hidpp_connect_event(struct hidpp_device *hidpp_dev); > > > > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, > > } > > > > if (response->report_id == REPORT_ID_HIDPP_SHORT && > > - response->fap.feature_index == HIDPP_ERROR) { > > + response->rap.sub_id == HIDPP_ERROR) { > > + ret = response->rap.params[1]; > > + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); > > + goto exit; > > + } > > + > > + if (response->report_id == REPORT_ID_HIDPP_LONG && > > + response->fap.feature_index == HIDPP20_ERROR) { > > ret = response->fap.params[1]; > > - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); > > + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); > > goto exit; > > } > > > > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, > > static inline bool hidpp_match_error(struct hidpp_report *question, > > struct hidpp_report *answer) > > { > > - return (answer->fap.feature_index == HIDPP_ERROR) && > > + return ((answer->rap.sub_id == HIDPP_ERROR) || > > + (answer->fap.feature_index == HIDPP20_ERROR)) && > > (answer->fap.funcindex_clientid == question->fap.feature_index) && > > (answer->fap.params[0] == question->fap.funcindex_clientid); > > } > > -- > > 2.1.3 -- 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
On Thu, Dec 18, 2014 at 12:26 PM, Peter Wu <peter@lekensteyn.nl> wrote: > On Tuesday 16 December 2014 09:33:44 Benjamin Tissoires wrote: >> Hi Peter, >> >> On Mon, Dec 15, 2014 at 7:50 PM, Peter Wu <peter@lekensteyn.nl> wrote: >> > Devices speaking HID++ 2.0 report a different error code (0xff). Detect >> > these errors too to avoid 5 second delays when the device reports an >> > error. Caught by... well, a bug in the QEMU emulation of this receiver. >> > >> > Renamed fap to rap for HID++ 1.0 errors because it is more logical, >> > it has no functional difference. >> > >> > Signed-off-by: Peter Wu <peter@lekensteyn.nl> >> > --- >> >> I'd like to have Nestor's opinion on this. I did not manage to find on >> the documentation that HID++ 2.0 Long report error code is 0xff, so >> introducing this change without Logitech's blessing would be >> unfortunate. >> I understand this will fix your qemu problem, but I am not entirely >> sure if we do not have to check on 0xff and 0x8f in both short and >> long responses. >> >> Cheers, >> Benjamin > > Hi Benjamin, > > The Logitech Unifying extension for Chrome[1] is documented quite well > and contains details which were not public before (including names and > descriptions for all registers and subIDs!). > > In lib/devices/HidppFap.js you can find this logic for handling HID++ > 2.0 messages: > > if ((reqView.getUint8(1) == rspView.getUint8(1)) // device index > && (reqView.getUint8(2) == rspView.getUint8(2)) // feature index > && (reqView.getUint8(3) == rspView.getUint8(3))) // function/event ID + software ID > { > result.matchResult = devices.MATCH_RESULT.SUCCESS; > } else if ((reqView.getUint8(1) == rspView.getUint8(1)) // device index > && (0xFF == rspView.getUint8(2)) // Hid++ 2.0 error > && (reqView.getUint8(2) == rspView.getUint8(3)) // feature index > && (reqView.getUint8(3) == rspView.getUint8(4))) // function/event ID + software ID > { > result.errCode = rspView.getUint8(5); // FAP_ERROR > result.matchResult = devices.MATCH_RESULT.ERROR; > } > > Looks like a sufficient proof that 0xFF is the correct number to detect > HID++ 2.0 errors right? Cool :) Then the patch is: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin > > In HID++ 1.0 devices ("rap"), 0xFF is named as "SYNC" (with no further > comments), so this will probably not trigger false positives either. > > Kind regards, > Peter > > [1]: https://chrome.google.com/webstore/detail/logitech-unifying-for-chr/agpmgihmmmfkbhckmciedmhincdggomo > >> > drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- >> > 1 file changed, 14 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c >> > index 2f420c0..ae23dec 100644 >> > --- a/drivers/hid/hid-logitech-hidpp.c >> > +++ b/drivers/hid/hid-logitech-hidpp.c >> > @@ -105,6 +105,7 @@ struct hidpp_device { >> > }; >> > >> > >> > +/* HID++ 1.0 error codes */ >> > #define HIDPP_ERROR 0x8f >> > #define HIDPP_ERROR_SUCCESS 0x00 >> > #define HIDPP_ERROR_INVALID_SUBID 0x01 >> > @@ -119,6 +120,8 @@ struct hidpp_device { >> > #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a >> > #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b >> > #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c >> > +/* HID++ 2.0 error codes */ >> > +#define HIDPP20_ERROR 0xff >> > >> > static void hidpp_connect_event(struct hidpp_device *hidpp_dev); >> > >> > @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, >> > } >> > >> > if (response->report_id == REPORT_ID_HIDPP_SHORT && >> > - response->fap.feature_index == HIDPP_ERROR) { >> > + response->rap.sub_id == HIDPP_ERROR) { >> > + ret = response->rap.params[1]; >> > + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); >> > + goto exit; >> > + } >> > + >> > + if (response->report_id == REPORT_ID_HIDPP_LONG && >> > + response->fap.feature_index == HIDPP20_ERROR) { >> > ret = response->fap.params[1]; >> > - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); >> > + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); >> > goto exit; >> > } >> > >> > @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, >> > static inline bool hidpp_match_error(struct hidpp_report *question, >> > struct hidpp_report *answer) >> > { >> > - return (answer->fap.feature_index == HIDPP_ERROR) && >> > + return ((answer->rap.sub_id == HIDPP_ERROR) || >> > + (answer->fap.feature_index == HIDPP20_ERROR)) && >> > (answer->fap.funcindex_clientid == question->fap.feature_index) && >> > (answer->fap.params[0] == question->fap.funcindex_clientid); >> > } >> > -- >> > 2.1.3 > -- 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
On Wed, 17 Dec 2014, Benjamin Tissoires wrote: > > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > > these errors too to avoid 5 second delays when the device reports an > > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > > it has no functional difference. > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > > --- > > Jiri, it looks like this one fall off from your radar. > > It's not a problem per-se, I'd like to have some feedbacks from Logitech > first, but still, there is a bug and Peter fixed it :) It's actually still on my radar, but that was exactly the reason I have it on hold, because my understanding was that you are waiting for Logitech to review it. Nestor ... ?
On Tue, 16 Dec 2014, Peter Wu wrote: > Devices speaking HID++ 2.0 report a different error code (0xff). Detect > these errors too to avoid 5 second delays when the device reports an > error. Caught by... well, a bug in the QEMU emulation of this receiver. > > Renamed fap to rap for HID++ 1.0 errors because it is more logical, > it has no functional difference. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> Applied to for-3.20/logitech.
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 2f420c0..ae23dec 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -105,6 +105,7 @@ struct hidpp_device { }; +/* HID++ 1.0 error codes */ #define HIDPP_ERROR 0x8f #define HIDPP_ERROR_SUCCESS 0x00 #define HIDPP_ERROR_INVALID_SUBID 0x01 @@ -119,6 +120,8 @@ struct hidpp_device { #define HIDPP_ERROR_REQUEST_UNAVAILABLE 0x0a #define HIDPP_ERROR_INVALID_PARAM_VALUE 0x0b #define HIDPP_ERROR_WRONG_PIN_CODE 0x0c +/* HID++ 2.0 error codes */ +#define HIDPP20_ERROR 0xff static void hidpp_connect_event(struct hidpp_device *hidpp_dev); @@ -192,9 +195,16 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, } if (response->report_id == REPORT_ID_HIDPP_SHORT && - response->fap.feature_index == HIDPP_ERROR) { + response->rap.sub_id == HIDPP_ERROR) { + ret = response->rap.params[1]; + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); + goto exit; + } + + if (response->report_id == REPORT_ID_HIDPP_LONG && + response->fap.feature_index == HIDPP20_ERROR) { ret = response->fap.params[1]; - dbg_hid("__hidpp_send_report got hidpp error %02X\n", ret); + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); goto exit; } @@ -271,7 +281,8 @@ static inline bool hidpp_match_answer(struct hidpp_report *question, static inline bool hidpp_match_error(struct hidpp_report *question, struct hidpp_report *answer) { - return (answer->fap.feature_index == HIDPP_ERROR) && + return ((answer->rap.sub_id == HIDPP_ERROR) || + (answer->fap.feature_index == HIDPP20_ERROR)) && (answer->fap.funcindex_clientid == question->fap.feature_index) && (answer->fap.params[0] == question->fap.funcindex_clientid); }
Devices speaking HID++ 2.0 report a different error code (0xff). Detect these errors too to avoid 5 second delays when the device reports an error. Caught by... well, a bug in the QEMU emulation of this receiver. Renamed fap to rap for HID++ 1.0 errors because it is more logical, it has no functional difference. Signed-off-by: Peter Wu <peter@lekensteyn.nl> --- drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)