LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH] HID: input: support Microsoft wireless radio control hotkey
@ 2018-11-30  6:46 Chris Chiu
  2018-11-30 10:46 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chris Chiu @ 2018-11-30  6:46 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, rydberg
  Cc: linux-input, linux-kernel, linux, 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.

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.

Signed-off-by: Chris Chiu <chiu@endlessm.com>
---
 include/linux/hid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d44a78362942..f4805f605fed 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-11-30  6:46 [PATCH] HID: input: support Microsoft wireless radio control hotkey Chris Chiu
@ 2018-11-30 10:46 ` kbuild test robot
  2018-11-30 11:18 ` Benjamin Tissoires
  2018-11-30 15:03 ` kbuild test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-11-30 10:46 UTC (permalink / raw)
  To: Chris Chiu
  Cc: kbuild-all, jikos, benjamin.tissoires, rydberg, linux-input,
	linux-kernel, linux, Chris Chiu

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12152 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-11-30  6:46 [PATCH] HID: input: support Microsoft wireless radio control hotkey Chris Chiu
  2018-11-30 10:46 ` kbuild test robot
@ 2018-11-30 11:18 ` Benjamin Tissoires
  2018-12-05  0:56   ` Chris Chiu
  2018-11-30 15:03 ` kbuild test robot
  2 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2018-11-30 11:18 UTC (permalink / raw)
  To: chiu; +Cc: Jiri Kosina, Henrik Rydberg, open list:HID CORE LAYER, lkml, linux

On Fri, Nov 30, 2018 at 7:46 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> 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.

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

>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>  include/linux/hid.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d44a78362942..f4805f605fed 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-11-30  6:46 [PATCH] HID: input: support Microsoft wireless radio control hotkey Chris Chiu
  2018-11-30 10:46 ` kbuild test robot
  2018-11-30 11:18 ` Benjamin Tissoires
@ 2018-11-30 15:03 ` kbuild test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-11-30 15:03 UTC (permalink / raw)
  To: Chris Chiu
  Cc: kbuild-all, jikos, benjamin.tissoires, rydberg, linux-input,
	linux-kernel, linux, Chris Chiu

[-- Attachment #1: Type: text/plain, Size: 6650 bytes --]

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 <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26902 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-11-30 11:18 ` Benjamin Tissoires
@ 2018-12-05  0:56   ` Chris Chiu
  2018-12-07  4:07     ` Chris Chiu
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Chiu @ 2018-12-05  0:56 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, rydberg, linux-input, Linux Kernel, Linux Upstreaming Team

On Fri, Nov 30, 2018 at 7:18 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Fri, Nov 30, 2018 at 7:46 AM Chris Chiu <chiu@endlessm.com> wrote:
> >
> > 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.
>
> 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
>

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

> >
> > Signed-off-by: Chris Chiu <chiu@endlessm.com>
> > ---
> >  include/linux/hid.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index d44a78362942..f4805f605fed 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -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
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-12-05  0:56   ` Chris Chiu
@ 2018-12-07  4:07     ` Chris Chiu
  2018-12-07 13:02       ` Benjamin Tissoires
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Chiu @ 2018-12-07  4:07 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, rydberg, linux-input, Linux Kernel, Linux Upstreaming Team

On Wed, Dec 5, 2018 at 8:56 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> On Fri, Nov 30, 2018 at 7:18 PM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > On Fri, Nov 30, 2018 at 7:46 AM Chris Chiu <chiu@endlessm.com> wrote:
> > >
> > > 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.
> >
> > 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
> >
>
> 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
>

Gentle ping. Any comment for the new patch? Thanks and Regards.

Chris

> > >
> > > Signed-off-by: Chris Chiu <chiu@endlessm.com>
> > > ---
> > >  include/linux/hid.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > > index d44a78362942..f4805f605fed 100644
> > > --- a/include/linux/hid.h
> > > +++ b/include/linux/hid.h
> > > @@ -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
> > >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
  2018-12-07  4:07     ` Chris Chiu
@ 2018-12-07 13:02       ` Benjamin Tissoires
  0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2018-12-07 13:02 UTC (permalink / raw)
  To: chiu
  Cc: Jiri Kosina, Henrik Rydberg, open list:HID CORE LAYER, lkml,
	Linux Upstreaming Team

On Fri, Dec 7, 2018 at 5:07 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> On Wed, Dec 5, 2018 at 8:56 AM Chris Chiu <chiu@endlessm.com> wrote:
> >
> > On Fri, Nov 30, 2018 at 7:18 PM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > > On Fri, Nov 30, 2018 at 7:46 AM Chris Chiu <chiu@endlessm.com> wrote:
> > > >
> > > > 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.
> > >
> > > 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
> > >
> >
> > 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
> >
>
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30  6:46 [PATCH] HID: input: support Microsoft wireless radio control hotkey Chris Chiu
2018-11-30 10:46 ` kbuild test robot
2018-11-30 11:18 ` Benjamin Tissoires
2018-12-05  0:56   ` Chris Chiu
2018-12-07  4:07     ` Chris Chiu
2018-12-07 13:02       ` Benjamin Tissoires
2018-11-30 15:03 ` kbuild test robot

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox