HID: input: support Microsoft wireless radio control hotkey
PATCH 1/1 HID: input: support Microsoft wireless radio control hotkey
The ASUS laptops start to support the airplane mode radio management to replace the original machanism of airplane mode toggle hotkey. On the ASUS P5440FF, it presents as a HID device connecting via I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller send hid report up via I2C and switch the airplane mode indicator LED based on the status.
However, it's not working because it fails to be identified as a hidinput device. It fails in hidinput_connect() due to the macro IS_INPUT_APPLICATION doesn't identify HID_GD_WIRELESS_RADIO_CTLS as a legit application code.
It's easy to add the HID I2C vendor and product id to the quirk list and apply HID_QUIRK_HIDINPUT_FORCE to make it work. But it can be more generic to support such kind of application on PC.
--- include/linux/hid.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/hid.h +1 -1
@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,/* Applications from HID Usage Tables 4/8/99 Version 1.1 */ /* We ignore a few input applications that are not widely used */ -#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006))) +#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))/* HID core API */
Signature · 1 line
2.11.0
kbuild test robot
build botThank you for the patch! Yet something to improve:
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc4 next-20181130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-Chiu/HID-input-support-Microsoft-wireless-radio-control-hotkey/20181130-150723
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
All error/warnings (new ones prefixed by >>):
In file included from drivers/hid/hid-input.c:32:0:
drivers/hid/hid-input.c: In function 'hidinput_connect':
>> include/linux/hid.h:839:99: error: expected expression before '||' token
#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
^
>> drivers/hid/hid-input.c:1737:9: note: in expansion of macro 'IS_INPUT_APPLICATION'
if (IS_INPUT_APPLICATION(col->usage))
^~~~~~~~~~~~~~~~~~~~
--
In file included from drivers//hid/hid-input.c:32:0:
drivers//hid/hid-input.c: In function 'hidinput_connect':
>> include/linux/hid.h:839:99: error: expected expression before '||' token
#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
^
drivers//hid/hid-input.c:1737:9: note: in expansion of macro 'IS_INPUT_APPLICATION'
if (IS_INPUT_APPLICATION(col->usage))
^~~~~~~~~~~~~~~~~~~~vim +839 include/linux/hid.h
836
837 /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
838 /* We ignore a few input applications that are not widely used */
> 839 #define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
840
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Quoted 8 lines · Chris Chiu
The ASUS laptops start to support the airplane mode radio management to replace the original machanism of airplane mode toggle hotkey. On the ASUS P5440FF, it presents as a HID device connecting via I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller send hid report up via I2C and switch the airplane mode indicator LED based on the status.
Quoted 8 lines
However, it's not working because it fails to be identified as a hidinput device. It fails in
hidinput_connect()due to the macroIS_INPUT_APPLICATIONdoesn't identifyHID_GD_WIRELESS_RADIO_CTLSas a legit application code.It's easy to add the HID I2C vendor and product id to the quirk list and apply
HID_QUIRK_HIDINPUT_FORCEto make it work. But it can be more generic to support such kind of application on PC.
Sounds good, but while you are at it, can you please:
- fix the kbuild warning
- rewrite the whole line to use macros,
- make the macro prettier by inserting new lines were required
- and define the missing macro:
0x000d0006 -> HID_DG_WHITEBOARD
Maybe we should keep the ranges definitions with raw values and put a comment on the side with the names.
Cheers,
Benjamin
Quoted 22 lines
Signed-off-by Chris Chiu <[email protected]>--- include/linux/hid.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/hid.h +1
-2@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,/* Applications from HID Usage Tables 4/8/99 Version 1.1 */ /* We ignore a few input applications that are not widely used */ -#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006))) +#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))/* HID core API */ --2.11.0
Quoted 26 lines · Benjamin Tissoires
Quoted 8 lines · Chris Chiu
The ASUS laptops start to support the airplane mode radio management to replace the original machanism of airplane mode toggle hotkey. On the ASUS P5440FF, it presents as a HID device connecting via I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller send hid report up via I2C and switch the airplane mode indicator LED based on the status.
Quoted 8 lines
However, it's not working because it fails to be identified as a hidinput device. It fails in
hidinput_connect()due to the macroIS_INPUT_APPLICATIONdoesn't identifyHID_GD_WIRELESS_RADIO_CTLSas a legit application code.It's easy to add the HID I2C vendor and product id to the quirk list and apply
HID_QUIRK_HIDINPUT_FORCEto make it work. But it can be more generic to support such kind of application on PC.Sounds good, but while you are at it, can you please:
- fix the kbuild warning
- rewrite the whole line to use macros,
- make the macro prettier by inserting new lines were required
- and define the missing macro:
0x000d0006->HID_DG_WHITEBOARD
Quoted 6 lines
Maybe we should keep the ranges definitions with raw values and put a comment on the side with the names.
Cheers,
Benjamin
Hi Bejamin,
Thanks for your comment, I've submitted other patches which may
fulfill your suggestion.
https://lore.kernel.org/patchwork/patch/1020373/
https://lore.kernel.org/patchwork/patch/1020374/
Please help me review and correct me if there's any modification
required. Thanks & Regards.
Chris
Quoted 22 lines
Quoted 22 lines
Signed-off-by Chris Chiu <[email protected]>--- include/linux/hid.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/hid.h +1
-2@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,/* Applications from HID Usage Tables 4/8/99 Version 1.1 */ /* We ignore a few input applications that are not widely used */ -#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006))) +#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))/* HID core API */ --2.11.0
Quoted 42 lines · Chris Chiu
Quoted 26 lines · Benjamin Tissoires
Quoted 8 lines · Chris Chiu
The ASUS laptops start to support the airplane mode radio management to replace the original machanism of airplane mode toggle hotkey. On the ASUS P5440FF, it presents as a HID device connecting via I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller send hid report up via I2C and switch the airplane mode indicator LED based on the status.
Quoted 8 lines
However, it's not working because it fails to be identified as a hidinput device. It fails in
hidinput_connect()due to the macroIS_INPUT_APPLICATIONdoesn't identifyHID_GD_WIRELESS_RADIO_CTLSas a legit application code.It's easy to add the HID I2C vendor and product id to the quirk list and apply
HID_QUIRK_HIDINPUT_FORCEto make it work. But it can be more generic to support such kind of application on PC.Sounds good, but while you are at it, can you please:
- fix the kbuild warning
- rewrite the whole line to use macros,
- make the macro prettier by inserting new lines were required
- and define the missing macro:
0x000d0006->HID_DG_WHITEBOARDQuoted 6 lines
Maybe we should keep the ranges definitions with raw values and put a comment on the side with the names.
Cheers,
BenjaminHi Bejamin,
Thanks for your comment, I've submitted other patches which may
fulfill your suggestion.
https://lore.kernel.org/patchwork/patch/1020373/
https://lore.kernel.org/patchwork/patch/1020374/
Quoted 5 lines
Please help me review and correct me if there's any modification
required. Thanks & Regards.Chris
Gentle ping. Any comment for the new patch? Thanks and Regards.
Chris
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Signed-off-by Chris Chiu <[email protected]>--- include/linux/hid.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/hid.h +1
-2@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,/* Applications from HID Usage Tables 4/8/99 Version 1.1 */ /* We ignore a few input applications that are not widely used */ -#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006))) +#define IS_INPUT_APPLICATION (a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))/* HID core API */ --2.11.0
Quoted 44 lines · Chris Chiu
Quoted 42 lines · Chris Chiu
Quoted 26 lines · Benjamin Tissoires
Quoted 8 lines · Chris Chiu
The ASUS laptops start to support the airplane mode radio management to replace the original machanism of airplane mode toggle hotkey. On the ASUS P5440FF, it presents as a HID device connecting via I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller send hid report up via I2C and switch the airplane mode indicator LED based on the status.
Quoted 8 lines
However, it's not working because it fails to be identified as a hidinput device. It fails in
hidinput_connect()due to the macroIS_INPUT_APPLICATIONdoesn't identifyHID_GD_WIRELESS_RADIO_CTLSas a legit application code.It's easy to add the HID I2C vendor and product id to the quirk list and apply
HID_QUIRK_HIDINPUT_FORCEto make it work. But it can be more generic to support such kind of application on PC.Sounds good, but while you are at it, can you please:
- fix the kbuild warning
- rewrite the whole line to use macros,
- make the macro prettier by inserting new lines were required
- and define the missing macro:
0x000d0006->HID_DG_WHITEBOARDQuoted 6 lines
Maybe we should keep the ranges definitions with raw values and put a comment on the side with the names.
Cheers,
BenjaminHi Bejamin,
Thanks for your comment, I've submitted other patches which may
fulfill your suggestion.
https://lore.kernel.org/patchwork/patch/1020373/
https://lore.kernel.org/patchwork/patch/1020374/
Quoted 8 lines
Quoted 5 lines
Please help me review and correct me if there's any modification
required. Thanks & Regards.Chris
Gentle ping. Any comment for the new patch? Thanks and Regards.
Sorry for the delay. I broke my system and it took me a few days to get it back on its feet.
The v2 is now scheduled for 4.21.
Cheers,
Benjamin
kbuild test robot
build botThank you for the patch! Perhaps something to improve:
Hi Chris,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc4 next-20181130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-Chiu/HID-input-support-Microsoft-wireless-radio-control-hotkey/20181130-150723
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce:
# save the attached .config to linux build tree
make ARCH=i386 All warnings (new ones prefixed by >>):
In file included from drivers/hid/usbhid/hiddev.c:35:0:
drivers/hid/usbhid/hiddev.c: In function 'hiddev_connect':
include/linux/hid.h:839:99: error: expected expression before '||' token
#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
^
>> drivers/hid/usbhid/hiddev.c:906:9: note: in expansion of macro 'IS_INPUT_APPLICATION'
!IS_INPUT_APPLICATION(hid->collection[i].usage))
^~~~~~~~~~~~~~~~~~~~vim +/IS_INPUT_APPLICATION +906 drivers/hid/usbhid/hiddev.c
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 891
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 892 /*
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 893 * This is where hid.c calls us to connect a hid device to the hiddev driver
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 894 */
93c10132 drivers/hid/usbhid/hiddev.c Jiri Slaby 2008-06-27 895 int hiddev_connect(struct hid_device *hid, unsigned int force)
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 896 {
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 897 struct hiddev *hiddev;
4916b3a5 drivers/usb/input/hiddev.c Jiri Kosina 2006-12-08 898 struct usbhid_device *usbhid = hid->driver_data;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 899 int retval;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 900
93c10132 drivers/hid/usbhid/hiddev.c Jiri Slaby 2008-06-27 901 if (!force) {
93c10132 drivers/hid/usbhid/hiddev.c Jiri Slaby 2008-06-27 902 unsigned int i;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 903 for (i = 0; i < hid->maxcollection; i++)
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 904 if (hid->collection[i].type ==
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 905 HID_COLLECTION_APPLICATION &&
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 @906 !IS_INPUT_APPLICATION(hid->collection[i].usage))
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 907 break;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 908
93c10132 drivers/hid/usbhid/hiddev.c Jiri Slaby 2008-06-27 909 if (i == hid->maxcollection)
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 910 return -1;
93c10132 drivers/hid/usbhid/hiddev.c Jiri Slaby 2008-06-27 911 }
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 912
bbdb7daf drivers/usb/input/hiddev.c Oliver Neukum 2006-01-06 913 if (!(hiddev = kzalloc(sizeof(struct hiddev), GFP_KERNEL)))
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 914 return -1;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 915
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 916 init_waitqueue_head(&hiddev->wait);
826d5982 drivers/usb/input/hiddev.c Dmitry Torokhov 2006-07-19 917 INIT_LIST_HEAD(&hiddev->list);
cdcb44e8 drivers/hid/usbhid/hiddev.c Jiri Kosina 2007-05-10 918 spin_lock_init(&hiddev->list_lock);
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 919 mutex_init(&hiddev->existancelock);
76052749 drivers/hid/usbhid/hiddev.c Jiri Kosina 2009-01-07 920 hid->hiddev = hiddev;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 921 hiddev->hid = hid;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 922 hiddev->exist = 1;
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 923 retval = usb_register_dev(usbhid->intf, &hiddev_class);
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 924 if (retval) {
4291ee30 drivers/hid/usbhid/hiddev.c Joe Perches 2010-12-09 925 hid_err(hid, "Not able to get a minor for this device\n");
76052749 drivers/hid/usbhid/hiddev.c Jiri Kosina 2009-01-07 926 hid->hiddev = NULL;
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 927 kfree(hiddev);
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 928 return -1;
07903407 drivers/hid/usbhid/hiddev.c Oliver Neukum 2008-12-16 929 }
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 930
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 931 /*
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 932 * If HID_QUIRK_NO_INIT_REPORTS is set, make sure we don't initialize
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 933 * the reports.
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 934 */
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 935 hiddev->initialized = hid->quirks & HID_QUIRK_NO_INIT_REPORTS;
9143059f drivers/hid/usbhid/hiddev.c Benjamin Tissoires 2017-03-08 936
733aca90 drivers/hid/usbhid/hiddev.c Jaejoong Kim 2017-03-03 937 hiddev->minor = usbhid->intf->minor;
733aca90 drivers/hid/usbhid/hiddev.c Jaejoong Kim 2017-03-03 938
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 939 return 0;
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 940 }
^1da177e drivers/usb/input/hiddev.c Linus Torvalds 2005-04-16 941 :::::: The code at line 906 was first introduced by commit :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <[email protected]> :::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation