linux-kernel - Re: [PATCH v1 2/2] hid-asus: change the report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <ac50e89e-b0e9-48f8-a4e5-074c19f684e6@app.fastmail.com>
Date: Wed, 29 May 2024 21:27:52 +1200
From: "Luke Jones" <luke@...nes.dev>
To: "Hans de Goede" <hdegoede@...hat.com>, "Jiri Kosina" <jikos@...nel.org>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 corentin.chary@...il.com, platform-driver-x86@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, bentiss@...nel.org
Subject: Re: [PATCH v1 2/2] hid-asus: change the report_id used for HID LED control



On Wed, 29 May 2024, at 8:25 PM, Hans de Goede wrote:
> Hi,
> 
> On 5/29/24 3:28 AM, Luke D. Jones wrote:
> > On some laptops the report_id used for LED brightness control must be
> > 0x5D instead of 0x5A.
> > 
> > Signed-off-by: Luke D. Jones <luke@...nes.dev>
> > ---
> >  drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 4cba8e143031..ec3556cc4eef 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >  
> >  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
> >  
> > +static const char * const use_alt_led_report_id[] = { "GU605", "GA403" };
> > +
> >  struct asus_kbd_leds {
> >  struct led_classdev cdev;
> >  struct hid_device *hdev;
> > @@ -101,6 +103,7 @@ struct asus_kbd_leds {
> >  unsigned int brightness;
> >  spinlock_t lock;
> >  bool removed;
> > + int report_id;
> >  };
> >  
> >  struct asus_touchpad_info {
> > @@ -473,7 +476,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> >  static void asus_kbd_backlight_work(struct work_struct *work)
> >  {
> >  struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> > - u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> > + u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
> >  int ret;
> >  unsigned long flags;
> >  
> > @@ -513,6 +516,23 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> >  return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> >  }
> >  
> > +static bool asus_kbd_is_input_led(void)
> > +{
> > + const char *product;
> > + int i;
> > +
> > + product = dmi_get_system_info(DMI_PRODUCT_NAME);
> > + if (!product)
> > + return false;
> > +
> > + for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) {
> > + if (strstr(product, use_alt_led_report_id[i]))
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> 
> 
> This again feels like you are re-inventing dmi_check_system() please
> change use_alt_led_report_id to a dmi_system_id array and drop this
> function (you can directly call dmi_check_system(use_alt_led_report_id)
> instead).

I wasn't actually aware of that, so i guess I've learned a whole lot of new stuff today :)

Thanks mate, I'll get round to a v2 in the next few days.

> 
> >  static int asus_kbd_register_leds(struct hid_device *hdev)
> >  {
> >  struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > @@ -555,6 +575,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >  if (!drvdata->kbd_backlight)
> >  return -ENOMEM;
> >  
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
> > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
> > +
> >  drvdata->kbd_backlight->removed = false;
> >  drvdata->kbd_backlight->brightness = 0;
> >  drvdata->kbd_backlight->hdev = hdev;
> 
> Regards,
> 
> Hans
> 
> 
>