| Message ID | 20181126105218.19980-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [1/3] HID: asus: Add event handler to catch unmapped Asus Vendor UsagePage codes | expand |
Hi Hans, On Mon, Nov 26, 2018 at 11:52 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Various Asus devices generate HID events using the Asus Vendor specific > UsagePage 0xff31 and hid-asus will map these in its input_mapping for all > devices to which it binds (independent of any quirks). > > Add an event callback which check for unmapped (because sofar unknown) > usages within the Asus Vendor UsagePage and log a warning for these. > > The purpose of this patch is to help debugging / find such unmapped codes > and add them to the asus_input_mapping() function. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/hid/hid-asus.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > index a1fa2fc8c9b5..61fb5a43c1cb 100644 > --- a/drivers/hid/hid-asus.c > +++ b/drivers/hid/hid-asus.c > @@ -241,6 +241,18 @@ static int asus_report_input(struct asus_drvdata *drvdat, u8 *data, int size) > return 1; > } > > +static int asus_event(struct hid_device *hdev, struct hid_field *field, > + struct hid_usage *usage, __s32 value) > +{ > + if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 && > + (usage->hid & HID_USAGE) != 0x00 && !usage->type) { > + hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n", > + usage->hid & HID_USAGE); Are you sure you want to shout a warning each time the device receives an unmapped event? There is a high chance you end up filling the logs with those warnings in case the device uses a proprietary protocol for some random communication. Cheers, Benjamin > + } > + > + return 0; > +} > + > static int asus_raw_event(struct hid_device *hdev, > struct hid_report *report, u8 *data, int size) > { > @@ -832,6 +844,7 @@ static struct hid_driver asus_driver = { > #ifdef CONFIG_PM > .reset_resume = asus_reset_resume, > #endif > + .event = asus_event, > .raw_event = asus_raw_event > }; > module_hid_driver(asus_driver); > -- > 2.19.1 >
Hi, On 26-11-18 17:18, Benjamin Tissoires wrote: > Hi Hans, > > On Mon, Nov 26, 2018 at 11:52 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Various Asus devices generate HID events using the Asus Vendor specific >> UsagePage 0xff31 and hid-asus will map these in its input_mapping for all >> devices to which it binds (independent of any quirks). >> >> Add an event callback which check for unmapped (because sofar unknown) >> usages within the Asus Vendor UsagePage and log a warning for these. >> >> The purpose of this patch is to help debugging / find such unmapped codes >> and add them to the asus_input_mapping() function. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/hid/hid-asus.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c >> index a1fa2fc8c9b5..61fb5a43c1cb 100644 >> --- a/drivers/hid/hid-asus.c >> +++ b/drivers/hid/hid-asus.c >> @@ -241,6 +241,18 @@ static int asus_report_input(struct asus_drvdata *drvdat, u8 *data, int size) >> return 1; >> } >> >> +static int asus_event(struct hid_device *hdev, struct hid_field *field, >> + struct hid_usage *usage, __s32 value) >> +{ >> + if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 && >> + (usage->hid & HID_USAGE) != 0x00 && !usage->type) { >> + hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n", >> + usage->hid & HID_USAGE); > > Are you sure you want to shout a warning each time the device receives > an unmapped event? > There is a high chance you end up filling the logs with those warnings > in case the device uses a proprietary protocol for some random > communication. Yes, this only happens for devices which are in the usb-id table for asus-hid and then only for events inside the custom Asus vendor usage-page. I only expect these warnings to happen when adding support for new hw, this patch is how I found the 2 missing codes added in patch 2 of this set. I first gave the user a version of the hid-asus driver with this patch and just the usb-id added, this fixed 6 or 7 or so of his hotkeys not working, but 2 were still not working, thanks to this patch he could tell me the code printed in dmesg -w for each non-mapped key and adding it was easy see: https://bugzilla.redhat.com/show_bug.cgi?id=1645070 If we get complaints about this we can easily drop it, but for now this seems useful. Regards, Hans > > Cheers, > Benjamin > >> + } >> + >> + return 0; >> +} >> + >> static int asus_raw_event(struct hid_device *hdev, >> struct hid_report *report, u8 *data, int size) >> { >> @@ -832,6 +844,7 @@ static struct hid_driver asus_driver = { >> #ifdef CONFIG_PM >> .reset_resume = asus_reset_resume, >> #endif >> + .event = asus_event, >> .raw_event = asus_raw_event >> }; >> module_hid_driver(asus_driver); >> -- >> 2.19.1 >>
On Mon, Nov 26, 2018 at 5:28 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 26-11-18 17:18, Benjamin Tissoires wrote: > > Hi Hans, > > > > On Mon, Nov 26, 2018 at 11:52 AM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> Various Asus devices generate HID events using the Asus Vendor specific > >> UsagePage 0xff31 and hid-asus will map these in its input_mapping for all > >> devices to which it binds (independent of any quirks). > >> > >> Add an event callback which check for unmapped (because sofar unknown) > >> usages within the Asus Vendor UsagePage and log a warning for these. > >> > >> The purpose of this patch is to help debugging / find such unmapped codes > >> and add them to the asus_input_mapping() function. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> drivers/hid/hid-asus.c | 13 +++++++++++++ > >> 1 file changed, 13 insertions(+) > >> > >> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > >> index a1fa2fc8c9b5..61fb5a43c1cb 100644 > >> --- a/drivers/hid/hid-asus.c > >> +++ b/drivers/hid/hid-asus.c > >> @@ -241,6 +241,18 @@ static int asus_report_input(struct asus_drvdata *drvdat, u8 *data, int size) > >> return 1; > >> } > >> > >> +static int asus_event(struct hid_device *hdev, struct hid_field *field, > >> + struct hid_usage *usage, __s32 value) > >> +{ > >> + if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 && > >> + (usage->hid & HID_USAGE) != 0x00 && !usage->type) { > >> + hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n", > >> + usage->hid & HID_USAGE); > > > > Are you sure you want to shout a warning each time the device receives > > an unmapped event? > > There is a high chance you end up filling the logs with those warnings > > in case the device uses a proprietary protocol for some random > > communication. > > Yes, this only happens for devices which are in the usb-id table for > asus-hid and then only for events inside the custom Asus vendor > usage-page. > > I only expect these warnings to happen when adding support for new hw, > this patch is how I found the 2 missing codes added in patch 2 of this set. > > I first gave the user a version of the hid-asus driver with this patch > and just the usb-id added, this fixed 6 or 7 or so of his hotkeys not > working, but 2 were still not working, thanks to this patch he could tell > me the code printed in dmesg -w for each non-mapped key and adding it > was easy see: https://bugzilla.redhat.com/show_bug.cgi?id=1645070 > > If we get complaints about this we can easily drop it, but for now this > seems useful. Fair enough. I have now applied 1/3 and 2/3 to for-4.21/hid-asus. Thanks! Cheers, Benjamin > > Regards, > > Hans > > > > > > > Cheers, > > Benjamin > > > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static int asus_raw_event(struct hid_device *hdev, > >> struct hid_report *report, u8 *data, int size) > >> { > >> @@ -832,6 +844,7 @@ static struct hid_driver asus_driver = { > >> #ifdef CONFIG_PM > >> .reset_resume = asus_reset_resume, > >> #endif > >> + .event = asus_event, > >> .raw_event = asus_raw_event > >> }; > >> module_hid_driver(asus_driver); > >> -- > >> 2.19.1 > >>
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index a1fa2fc8c9b5..61fb5a43c1cb 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c @@ -241,6 +241,18 @@ static int asus_report_input(struct asus_drvdata *drvdat, u8 *data, int size) return 1; } +static int asus_event(struct hid_device *hdev, struct hid_field *field, + struct hid_usage *usage, __s32 value) +{ + if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 && + (usage->hid & HID_USAGE) != 0x00 && !usage->type) { + hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n", + usage->hid & HID_USAGE); + } + + return 0; +} + static int asus_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size) { @@ -832,6 +844,7 @@ static struct hid_driver asus_driver = { #ifdef CONFIG_PM .reset_resume = asus_reset_resume, #endif + .event = asus_event, .raw_event = asus_raw_event }; module_hid_driver(asus_driver);
Various Asus devices generate HID events using the Asus Vendor specific UsagePage 0xff31 and hid-asus will map these in its input_mapping for all devices to which it binds (independent of any quirks). Add an event callback which check for unmapped (because sofar unknown) usages within the Asus Vendor UsagePage and log a warning for these. The purpose of this patch is to help debugging / find such unmapped codes and add them to the asus_input_mapping() function. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/hid/hid-asus.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)