Linux Input/HID development

Linux Input/HID development
 help / color / mirror / Atom feed

* Re: [PATCH v2] HID: quirks: Add device descriptor for 4c4a:4155
From: Terry Junge @ 2025-10-24 22:28 UTC (permalink / raw)
  To: zhangheng, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <8f0155d4-72a7-45ec-a272-7892e783bbed@kylinos.cn>

Hi Zhang,

Since the manufacturer has verified that all the microphones have the 
same serial number you could add the test for that as well if you want.

The report descriptor is clearly for a keyboard/keypad and should be 
ignored.

In any case, the current v3 attached patch does resolve the conflict.

Reviewed-by: Terry Junge <linuxhid@cosmicgizmosystems.com>

On 10/23/2025 8:32 PM, zhangheng wrote:
> Hi Terry Junge，
> 
> I have made the changes as per your suggestion.
> mic.txt is the microphone report descriptor and is working properly.
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-10-24 21:20 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <0d18666a-78e1-4e69-8fd2-f15052db0cee@gmail.com>

On Fri, 24 Oct 2025 at 20:53, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/24/25 18:20, Antheas Kapenekakis wrote:
> > On Fri, 24 Oct 2025 at 01:25, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >> On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>
> >>> On 10/23/25 23:30, Antheas Kapenekakis wrote:
> >>>> On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>> On 10/23/25 20:06, Antheas Kapenekakis wrote:
> >>>>>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
> >>>>>>>> Currently, RGB initialization forks depending on whether a device is
> >>>>>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
> >>>>>>>> endpoints, and non-NKEY devices with 0x5a and then a
> >>>>>>>> backlight check, which is omitted for NKEY devices.
> >>>>>>>>
> >>>>>>>> Remove the fork, using a common initialization sequence for both,
> >>>>>>>> where they are both only initialized with 0x5a, then checked for
> >>>>>>>> backlight support. This patch should not affect existing functionality.
> >>>>>>>>
> >>>>>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
> >>>>>>>> userspace programs associated with different usages that reside under
> >>>>>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
> >>>>>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
> >>>>>>>> Neither is used currently in the driver.
> >>>>>>> What benefits do we get from removing the unused initialization?
> >>>>>>>
> >>>>>>> If this has never caused any troubles I don't see the reason for removing
> >>>>>>> them. Moreover the lighting protocol is known and I might as well add
> >>>>>>> support for it in the near future,
> >>>>>> I already have a patch that adds RGB and delay inits that endpoint. It
> >>>>>> got removed to make this easier to merge. See [1].
> >>>>>>
> >>>>>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
> >>>>> I have to main concerns about this:
> >>>>>
> >>>>> 1. taking away initialization commands in one patchset to make it
> >>>>> easier to merge another unrelated patch doesn't seem the right thing
> >>>>> to do if the other patch it's not in the same series.
> >>>>>
> >>>>> I can see [1] has been removed from the set for a later moment in time,
> >>>>> it's fine if it needs more work, just send something that function in the
> >>>>> same way and do not remove initialization commands when unnecessary,
> >>>>> especially since there will be for sure future development.
> >>>> The initialization was removed as part of general cleanup. Not to make
> >>>> it easier to merge the RGB patch. In addition, the RGB patch only runs
> >>>> the init in a lazy fashion, so if nobody uses the RGB sysfs the init
> >>>> does not run and the behavior is the same.
> >>> There are a few problems here:
> >>> 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
> >>> doesn't load hid-asus correctly has nothing to do with the initialization of anime.
> >>> The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
> >>> anime matrix initialization either.
> >>> 2. not sending the initialization can get hardware misbehave because it
> >>> is left in an uninitialized state.
> >>> 3. there are absolutely zero reasons to do that. There are even less reasons
> >>> as to do it as part of this patchset.
> >>>
> >>>>> 2. Your patchset resolves around keyboard backlight control and how
> >>>>> the keyboard device is exposed to userspace: it's fine but I do not see
> >>>>> the point in removing initialization commands that has nothing to do
> >>>>> with the issue we are trying to solve here.
> >>>>>
> >>>>> Please leave 0x5E and 0x5D initialization commands where they are now.
> >>>> I mean the second part of the patchset does that. The first part is a
> >>>> cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
> >>>> only used when initializing those endpoints to write further commands
> >>>> to them and for identification. The current driver does not write
> >>>> commands to those endpoints and identifies itself over 0x5A.
> >>> There are no bugs opened that ties initialization of devices to bugs.
> >>> Quite the opposite: I can guarantee you that removing part of the
> >>> init will introduce regressions.
> >>>
> >>> The onus is on you to provide strong evidence that the removal is
> >>> a necessary act.
> >>>
> >>> Regardless it is not in the scope of this patchset: remove it.
> >>>> I do get that it is a bit risky as some laptops might be hardcoded to
> >>>> wait for 0x5D to turn on RGB. Which is why we had the last patch until
> >>>> V4. But we have yet to find a laptop that has this problem, so I find
> >>>> it difficult to justify keeping the init.
> >>> Yes it's risky to remove initialization sequences for a device that is
> >>> in every modern ASUS laptop and is tied to the EC.
> >>>> Do note that you might need to add the 0x5D init to your userspace
> >>>> program for certain laptops if you haven't already. But that is ok,
> >>>> since in doing so you are also validating you are speaking to an Asus
> >>>> device, which is important.
> >>> This doesn't make much sense: why would anyone remove
> >>> a command from the kernel, that can be very well essential to some models
> >>> (sleep can break, for example) just to add it back in a userspace program?
> >>>
> >>> What does it mean I have to validate I am speaking to an asus device?
> >>> Software selects devices by known attribute, one of them is the vid:pid....
> >>> Beside what does this have to do with the removal of initialization commands
> >>> from the kernel?
> >>>
> >>> Even late initializing devices can lead to problems. Windows doesn't do that:
> >>> as soon as asus drivers are loaded all relevant initialization sequences are
> >>> sent; Windows is the only officially supported OS: do not introduce commands
> >>> flow divergence without strong reasons backing it up.
> >> If you think keeping 0x5D init is that important, I can spin patch [1]
> >> into this series. But then this quirk will stay in the kernel forever.
> >> I can even add 0x5E since that does not affect newer devices, which I
> >> care for simplifying the sequence.
> Fully initializing the device tied to the EC in the same windows does
> is not a "quirk". Please stop calling it that.
>
> It will stay on the kernel until we have strong evidence that it is causing
> problems, at that point we simply avoid doing it for problematic laptops.
>
> If adding other commands doesn't introduce regressions or are otherwise
> easy to bisect and makes more hardware working please do.

It is not an init sequence. It is a handshake with the userspace
program that proves to the program it is talking with a genuine asus
device and to the device with the correct program. For all devices
that I have tested it seems to NOOP.

0x5a is the only one used for a driver and it does brightness control.
0x5d/0x5e are used with userspace Windows programs. 0x5d does RGB.
Moreover, the application 0xff310076 only has a single report ID under
it, 0x5a. 0x5d and 0x5e belong to different hid applications that are
not currently checked by the driver (but when they exist they reside
under the same hid endpoint, with a multi-application collection that
bifurcates in Windows to multiple hid devices).

So it makes sense to remove the redundant handshakes. If some laptops
require 0x5d to enable shortcuts as Luke said, I have a patch that
does that and is straightforward to do. But since the shortcut
response comes from the 0x5a endpoint, I find it unlikely for it to
require a handshake over a different endpoint to init.

> >> Luke said these two pairs are the important ones to keep.
> >>
> >> I'm not sure what to do.
> > I was asked by a 2025 Asus Zenbook Duo user to add his IDs in [1]. In
> > doing so, I updated the rgb and legacy init patches for the new series
> > and added a quirk for early init of the duo keyboards.
> I will take a look when I can, but if you haven't removed anything
> that shouldn't pose any risk. None that I can think of at the moment anyway.
> > The series is 14 patches long, I don't think my email can take it :(
> linux.dev accounts for maintainers are provided free of charge
> and I had to ask for an account too. I suggest you do the same.
> > Should we merge the first part of this series with the legacy init,
> > then do the backlight refactor, and finally the new Duo stuff + rgb?
> I think so. My only doubt is about the per_app quirk. Other than
> that looks good and solves one problem while also better representing
> the hardware, so I can't think of any blockers.
> > Antheas
> >
> Thanks,
> Denis
> >> Antheas
> >>
> >> [1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/
> >>
> >>>> Antheas
> >>>>
> >>> Denis
> >>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >>>>>>>> ---
> >>>>>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
> >>>>>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >>>>>>>> index a444d41e53b6..7ea1037c3979 100644
> >>>>>>>> --- a/drivers/hid/hid-asus.c
> >>>>>>>> +++ b/drivers/hid/hid-asus.c
> >>>>>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >>>>>>>>       unsigned char kbd_func;
> >>>>>>>>       int ret;
> >>>>>>>>
> >>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> >>>>>>>> -             /* Initialize keyboard */
> >>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>> -             if (ret < 0)
> >>>>>>>> -                     return ret;
> >>>>>>>> -
> >>>>>>>> -             /* The LED endpoint is initialised in two HID */
> >>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >>>>>>>> -             if (ret < 0)
> >>>>>>>> -                     return ret;
> >>>>>>>> -
> >>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> >>>>>>>> -             if (ret < 0)
> >>>>>>>> -                     return ret;
> >>>>>>>> -
> >>>>>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>>>>> -                     ret = asus_kbd_disable_oobe(hdev);
> >>>>>>>> -                     if (ret < 0)
> >>>>>>>> -                             return ret;
> >>>>>>>> -             }
> >>>>>>>> -
> >>>>>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>>>>> -                     intf = to_usb_interface(hdev->dev.parent);
> >>>>>>>> -                     udev = interface_to_usbdev(intf);
> >>>>>>>> -                     validate_mcu_fw_version(hdev,
> >>>>>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
> >>>>>>>> -             }
> >>>>>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>> +     if (ret < 0)
> >>>>>>>> +             return ret;
> >>>>>>>>
> >>>>>>>> -     } else {
> >>>>>>>> -             /* Initialize keyboard */
> >>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>> -             if (ret < 0)
> >>>>>>>> -                     return ret;
> >>>>>>>> +     /* Get keyboard functions */
> >>>>>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>>>>> +     if (ret < 0)
> >>>>>>>> +             return ret;
> >>>>>>>>
> >>>>>>>> -             /* Get keyboard functions */
> >>>>>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>>>>> +             ret = asus_kbd_disable_oobe(hdev);
> >>>>>>>>               if (ret < 0)
> >>>>>>>>                       return ret;
> >>>>>>>> +     }
> >>>>>>>>
> >>>>>>>> -             /* Check for backlight support */
> >>>>>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>>>>> -                     return -ENODEV;
> >>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>>>>> +             intf = to_usb_interface(hdev->dev.parent);
> >>>>>>>> +             udev = interface_to_usbdev(intf);
> >>>>>>>> +             validate_mcu_fw_version(
> >>>>>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
> >>>>>>>>       }
> >>>>>>>>
> >>>>>>>> +     /* Check for backlight support */
> >>>>>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>>>>> +             return -ENODEV;
> >>>>>>>> +
> >>>>>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >>>>>>>>                                             sizeof(struct asus_kbd_leds),
> >>>>>>>>                                             GFP_KERNEL);
>


^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Denis Benato @ 2025-10-24 18:53 UTC (permalink / raw)
  To: Antheas Kapenekakis
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <CAGwozwEh32XMcGJPKMRBWd63ybYOxW1Wx4QjU-QErjQgLHwX2g@mail.gmail.com>


On 10/24/25 18:20, Antheas Kapenekakis wrote:
> On Fri, 24 Oct 2025 at 01:25, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>> On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
>>>
>>> On 10/23/25 23:30, Antheas Kapenekakis wrote:
>>>> On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>> On 10/23/25 20:06, Antheas Kapenekakis wrote:
>>>>>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
>>>>>>>> Currently, RGB initialization forks depending on whether a device is
>>>>>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
>>>>>>>> endpoints, and non-NKEY devices with 0x5a and then a
>>>>>>>> backlight check, which is omitted for NKEY devices.
>>>>>>>>
>>>>>>>> Remove the fork, using a common initialization sequence for both,
>>>>>>>> where they are both only initialized with 0x5a, then checked for
>>>>>>>> backlight support. This patch should not affect existing functionality.
>>>>>>>>
>>>>>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
>>>>>>>> userspace programs associated with different usages that reside under
>>>>>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
>>>>>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
>>>>>>>> Neither is used currently in the driver.
>>>>>>> What benefits do we get from removing the unused initialization?
>>>>>>>
>>>>>>> If this has never caused any troubles I don't see the reason for removing
>>>>>>> them. Moreover the lighting protocol is known and I might as well add
>>>>>>> support for it in the near future,
>>>>>> I already have a patch that adds RGB and delay inits that endpoint. It
>>>>>> got removed to make this easier to merge. See [1].
>>>>>>
>>>>>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
>>>>> I have to main concerns about this:
>>>>>
>>>>> 1. taking away initialization commands in one patchset to make it
>>>>> easier to merge another unrelated patch doesn't seem the right thing
>>>>> to do if the other patch it's not in the same series.
>>>>>
>>>>> I can see [1] has been removed from the set for a later moment in time,
>>>>> it's fine if it needs more work, just send something that function in the
>>>>> same way and do not remove initialization commands when unnecessary,
>>>>> especially since there will be for sure future development.
>>>> The initialization was removed as part of general cleanup. Not to make
>>>> it easier to merge the RGB patch. In addition, the RGB patch only runs
>>>> the init in a lazy fashion, so if nobody uses the RGB sysfs the init
>>>> does not run and the behavior is the same.
>>> There are a few problems here:
>>> 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
>>> doesn't load hid-asus correctly has nothing to do with the initialization of anime.
>>> The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
>>> anime matrix initialization either.
>>> 2. not sending the initialization can get hardware misbehave because it
>>> is left in an uninitialized state.
>>> 3. there are absolutely zero reasons to do that. There are even less reasons
>>> as to do it as part of this patchset.
>>>
>>>>> 2. Your patchset resolves around keyboard backlight control and how
>>>>> the keyboard device is exposed to userspace: it's fine but I do not see
>>>>> the point in removing initialization commands that has nothing to do
>>>>> with the issue we are trying to solve here.
>>>>>
>>>>> Please leave 0x5E and 0x5D initialization commands where they are now.
>>>> I mean the second part of the patchset does that. The first part is a
>>>> cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
>>>> only used when initializing those endpoints to write further commands
>>>> to them and for identification. The current driver does not write
>>>> commands to those endpoints and identifies itself over 0x5A.
>>> There are no bugs opened that ties initialization of devices to bugs.
>>> Quite the opposite: I can guarantee you that removing part of the
>>> init will introduce regressions.
>>>
>>> The onus is on you to provide strong evidence that the removal is
>>> a necessary act.
>>>
>>> Regardless it is not in the scope of this patchset: remove it.
>>>> I do get that it is a bit risky as some laptops might be hardcoded to
>>>> wait for 0x5D to turn on RGB. Which is why we had the last patch until
>>>> V4. But we have yet to find a laptop that has this problem, so I find
>>>> it difficult to justify keeping the init.
>>> Yes it's risky to remove initialization sequences for a device that is
>>> in every modern ASUS laptop and is tied to the EC.
>>>> Do note that you might need to add the 0x5D init to your userspace
>>>> program for certain laptops if you haven't already. But that is ok,
>>>> since in doing so you are also validating you are speaking to an Asus
>>>> device, which is important.
>>> This doesn't make much sense: why would anyone remove
>>> a command from the kernel, that can be very well essential to some models
>>> (sleep can break, for example) just to add it back in a userspace program?
>>>
>>> What does it mean I have to validate I am speaking to an asus device?
>>> Software selects devices by known attribute, one of them is the vid:pid....
>>> Beside what does this have to do with the removal of initialization commands
>>> from the kernel?
>>>
>>> Even late initializing devices can lead to problems. Windows doesn't do that:
>>> as soon as asus drivers are loaded all relevant initialization sequences are
>>> sent; Windows is the only officially supported OS: do not introduce commands
>>> flow divergence without strong reasons backing it up.
>> If you think keeping 0x5D init is that important, I can spin patch [1]
>> into this series. But then this quirk will stay in the kernel forever.
>> I can even add 0x5E since that does not affect newer devices, which I
>> care for simplifying the sequence.
Fully initializing the device tied to the EC in the same windows does
is not a "quirk". Please stop calling it that.

It will stay on the kernel until we have strong evidence that it is causing
problems, at that point we simply avoid doing it for problematic laptops.

If adding other commands doesn't introduce regressions or are otherwise
easy to bisect and makes more hardware working please do.
>> Luke said these two pairs are the important ones to keep.
>>
>> I'm not sure what to do.
> I was asked by a 2025 Asus Zenbook Duo user to add his IDs in [1]. In
> doing so, I updated the rgb and legacy init patches for the new series
> and added a quirk for early init of the duo keyboards.
I will take a look when I can, but if you haven't removed anything
that shouldn't pose any risk. None that I can think of at the moment anyway.
> The series is 14 patches long, I don't think my email can take it :(
linux.dev accounts for maintainers are provided free of charge
and I had to ask for an account too. I suggest you do the same.
> Should we merge the first part of this series with the legacy init,
> then do the backlight refactor, and finally the new Duo stuff + rgb?
I think so. My only doubt is about the per_app quirk. Other than
that looks good and solves one problem while also better representing
the hardware, so I can't think of any blockers.
> Antheas
>
Thanks,
Denis
>> Antheas
>>
>> [1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/
>>
>>>> Antheas
>>>>
>>> Denis
>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>>>>>> ---
>>>>>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
>>>>>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>>>>>> index a444d41e53b6..7ea1037c3979 100644
>>>>>>>> --- a/drivers/hid/hid-asus.c
>>>>>>>> +++ b/drivers/hid/hid-asus.c
>>>>>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>>>>>>       unsigned char kbd_func;
>>>>>>>>       int ret;
>>>>>>>>
>>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>>>>>>> -             /* Initialize keyboard */
>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>> -             if (ret < 0)
>>>>>>>> -                     return ret;
>>>>>>>> -
>>>>>>>> -             /* The LED endpoint is initialised in two HID */
>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>>> -             if (ret < 0)
>>>>>>>> -                     return ret;
>>>>>>>> -
>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>>> -             if (ret < 0)
>>>>>>>> -                     return ret;
>>>>>>>> -
>>>>>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>>> -                     ret = asus_kbd_disable_oobe(hdev);
>>>>>>>> -                     if (ret < 0)
>>>>>>>> -                             return ret;
>>>>>>>> -             }
>>>>>>>> -
>>>>>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>>>>> -                     intf = to_usb_interface(hdev->dev.parent);
>>>>>>>> -                     udev = interface_to_usbdev(intf);
>>>>>>>> -                     validate_mcu_fw_version(hdev,
>>>>>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
>>>>>>>> -             }
>>>>>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>> +     if (ret < 0)
>>>>>>>> +             return ret;
>>>>>>>>
>>>>>>>> -     } else {
>>>>>>>> -             /* Initialize keyboard */
>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>> -             if (ret < 0)
>>>>>>>> -                     return ret;
>>>>>>>> +     /* Get keyboard functions */
>>>>>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>>>>> +     if (ret < 0)
>>>>>>>> +             return ret;
>>>>>>>>
>>>>>>>> -             /* Get keyboard functions */
>>>>>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>>> +             ret = asus_kbd_disable_oobe(hdev);
>>>>>>>>               if (ret < 0)
>>>>>>>>                       return ret;
>>>>>>>> +     }
>>>>>>>>
>>>>>>>> -             /* Check for backlight support */
>>>>>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>> -                     return -ENODEV;
>>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>>>>> +             intf = to_usb_interface(hdev->dev.parent);
>>>>>>>> +             udev = interface_to_usbdev(intf);
>>>>>>>> +             validate_mcu_fw_version(
>>>>>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
>>>>>>>>       }
>>>>>>>>
>>>>>>>> +     /* Check for backlight support */
>>>>>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>> +             return -ENODEV;
>>>>>>>> +
>>>>>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>>>>>>>                                             sizeof(struct asus_kbd_leds),
>>>>>>>>                                             GFP_KERNEL);

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-10-24 16:20 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <CAGwozwGj-yXHXBan38_NV7G5T66bnjm7om2bz_Bha35AHhtCJQ@mail.gmail.com>

On Fri, 24 Oct 2025 at 01:25, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
> >
> >
> > On 10/23/25 23:30, Antheas Kapenekakis wrote:
> > > On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>
> > >> On 10/23/25 20:06, Antheas Kapenekakis wrote:
> > >>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
> > >>>>> Currently, RGB initialization forks depending on whether a device is
> > >>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
> > >>>>> endpoints, and non-NKEY devices with 0x5a and then a
> > >>>>> backlight check, which is omitted for NKEY devices.
> > >>>>>
> > >>>>> Remove the fork, using a common initialization sequence for both,
> > >>>>> where they are both only initialized with 0x5a, then checked for
> > >>>>> backlight support. This patch should not affect existing functionality.
> > >>>>>
> > >>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
> > >>>>> userspace programs associated with different usages that reside under
> > >>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
> > >>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
> > >>>>> Neither is used currently in the driver.
> > >>>> What benefits do we get from removing the unused initialization?
> > >>>>
> > >>>> If this has never caused any troubles I don't see the reason for removing
> > >>>> them. Moreover the lighting protocol is known and I might as well add
> > >>>> support for it in the near future,
> > >>> I already have a patch that adds RGB and delay inits that endpoint. It
> > >>> got removed to make this easier to merge. See [1].
> > >>>
> > >>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
> > >> I have to main concerns about this:
> > >>
> > >> 1. taking away initialization commands in one patchset to make it
> > >> easier to merge another unrelated patch doesn't seem the right thing
> > >> to do if the other patch it's not in the same series.
> > >>
> > >> I can see [1] has been removed from the set for a later moment in time,
> > >> it's fine if it needs more work, just send something that function in the
> > >> same way and do not remove initialization commands when unnecessary,
> > >> especially since there will be for sure future development.
> > > The initialization was removed as part of general cleanup. Not to make
> > > it easier to merge the RGB patch. In addition, the RGB patch only runs
> > > the init in a lazy fashion, so if nobody uses the RGB sysfs the init
> > > does not run and the behavior is the same.
> > There are a few problems here:
> > 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
> > doesn't load hid-asus correctly has nothing to do with the initialization of anime.
> > The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
> > anime matrix initialization either.
> > 2. not sending the initialization can get hardware misbehave because it
> > is left in an uninitialized state.
> > 3. there are absolutely zero reasons to do that. There are even less reasons
> > as to do it as part of this patchset.
> >
> > >> 2. Your patchset resolves around keyboard backlight control and how
> > >> the keyboard device is exposed to userspace: it's fine but I do not see
> > >> the point in removing initialization commands that has nothing to do
> > >> with the issue we are trying to solve here.
> > >>
> > >> Please leave 0x5E and 0x5D initialization commands where they are now.
> > > I mean the second part of the patchset does that. The first part is a
> > > cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
> > > only used when initializing those endpoints to write further commands
> > > to them and for identification. The current driver does not write
> > > commands to those endpoints and identifies itself over 0x5A.
> > There are no bugs opened that ties initialization of devices to bugs.
> > Quite the opposite: I can guarantee you that removing part of the
> > init will introduce regressions.
> >
> > The onus is on you to provide strong evidence that the removal is
> > a necessary act.
> >
> > Regardless it is not in the scope of this patchset: remove it.
> > > I do get that it is a bit risky as some laptops might be hardcoded to
> > > wait for 0x5D to turn on RGB. Which is why we had the last patch until
> > > V4. But we have yet to find a laptop that has this problem, so I find
> > > it difficult to justify keeping the init.
> > Yes it's risky to remove initialization sequences for a device that is
> > in every modern ASUS laptop and is tied to the EC.
> > > Do note that you might need to add the 0x5D init to your userspace
> > > program for certain laptops if you haven't already. But that is ok,
> > > since in doing so you are also validating you are speaking to an Asus
> > > device, which is important.
> > This doesn't make much sense: why would anyone remove
> > a command from the kernel, that can be very well essential to some models
> > (sleep can break, for example) just to add it back in a userspace program?
> >
> > What does it mean I have to validate I am speaking to an asus device?
> > Software selects devices by known attribute, one of them is the vid:pid....
> > Beside what does this have to do with the removal of initialization commands
> > from the kernel?
> >
> > Even late initializing devices can lead to problems. Windows doesn't do that:
> > as soon as asus drivers are loaded all relevant initialization sequences are
> > sent; Windows is the only officially supported OS: do not introduce commands
> > flow divergence without strong reasons backing it up.
>
> If you think keeping 0x5D init is that important, I can spin patch [1]
> into this series. But then this quirk will stay in the kernel forever.
> I can even add 0x5E since that does not affect newer devices, which I
> care for simplifying the sequence.
>
> Luke said these two pairs are the important ones to keep.
>
> I'm not sure what to do.

I was asked by a 2025 Asus Zenbook Duo user to add his IDs in [1]. In
doing so, I updated the rgb and legacy init patches for the new series
and added a quirk for early init of the duo keyboards.

The series is 14 patches long, I don't think my email can take it :(

Should we merge the first part of this series with the legacy init,
then do the backlight refactor, and finally the new Duo stuff + rgb?

Antheas

> Antheas
>
> [1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/
>
> > > Antheas
> > >
> > Denis
> > >>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > >>>>> ---
> > >>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
> > >>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
> > >>>>>
> > >>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > >>>>> index a444d41e53b6..7ea1037c3979 100644
> > >>>>> --- a/drivers/hid/hid-asus.c
> > >>>>> +++ b/drivers/hid/hid-asus.c
> > >>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > >>>>>       unsigned char kbd_func;
> > >>>>>       int ret;
> > >>>>>
> > >>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> > >>>>> -             /* Initialize keyboard */
> > >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > >>>>> -             if (ret < 0)
> > >>>>> -                     return ret;
> > >>>>> -
> > >>>>> -             /* The LED endpoint is initialised in two HID */
> > >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> > >>>>> -             if (ret < 0)
> > >>>>> -                     return ret;
> > >>>>> -
> > >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> > >>>>> -             if (ret < 0)
> > >>>>> -                     return ret;
> > >>>>> -
> > >>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > >>>>> -                     ret = asus_kbd_disable_oobe(hdev);
> > >>>>> -                     if (ret < 0)
> > >>>>> -                             return ret;
> > >>>>> -             }
> > >>>>> -
> > >>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > >>>>> -                     intf = to_usb_interface(hdev->dev.parent);
> > >>>>> -                     udev = interface_to_usbdev(intf);
> > >>>>> -                     validate_mcu_fw_version(hdev,
> > >>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
> > >>>>> -             }
> > >>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > >>>>> +     if (ret < 0)
> > >>>>> +             return ret;
> > >>>>>
> > >>>>> -     } else {
> > >>>>> -             /* Initialize keyboard */
> > >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > >>>>> -             if (ret < 0)
> > >>>>> -                     return ret;
> > >>>>> +     /* Get keyboard functions */
> > >>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > >>>>> +     if (ret < 0)
> > >>>>> +             return ret;
> > >>>>>
> > >>>>> -             /* Get keyboard functions */
> > >>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > >>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > >>>>> +             ret = asus_kbd_disable_oobe(hdev);
> > >>>>>               if (ret < 0)
> > >>>>>                       return ret;
> > >>>>> +     }
> > >>>>>
> > >>>>> -             /* Check for backlight support */
> > >>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > >>>>> -                     return -ENODEV;
> > >>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > >>>>> +             intf = to_usb_interface(hdev->dev.parent);
> > >>>>> +             udev = interface_to_usbdev(intf);
> > >>>>> +             validate_mcu_fw_version(
> > >>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
> > >>>>>       }
> > >>>>>
> > >>>>> +     /* Check for backlight support */
> > >>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > >>>>> +             return -ENODEV;
> > >>>>> +
> > >>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> > >>>>>                                             sizeof(struct asus_kbd_leds),
> > >>>>>                                             GFP_KERNEL);
> >


^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Lee Jones @ 2025-10-24 16:08 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Mathieu Poirier, Philipp Zabel,
	Olivia Mackall, Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman,
	devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On Thu, 23 Oct 2025, Rob Herring (Arm) wrote:

> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/.yamllint                  | 2 +-
>  Documentation/devicetree/bindings/arm/psci.yaml              | 1 -
>  .../bindings/clock/allwinner,sun4i-a10-gates-clk.yaml        | 1 -
>  .../devicetree/bindings/clock/renesas,cpg-mssr.yaml          | 1 -
>  .../devicetree/bindings/clock/xlnx,clocking-wizard.yaml      | 1 -
>  .../display/allwinner,sun4i-a10-display-frontend.yaml        | 1 -
>  .../devicetree/bindings/display/allwinner,sun6i-a31-drc.yaml | 1 -
>  .../bindings/display/allwinner,sun8i-a83t-dw-hdmi.yaml       | 1 -
>  .../devicetree/bindings/display/amlogic,meson-vpu.yaml       | 1 -
>  .../devicetree/bindings/display/bridge/adi,adv7511.yaml      | 1 -
>  .../devicetree/bindings/display/bridge/lvds-codec.yaml       | 1 -
>  .../devicetree/bindings/display/bridge/toshiba,tc358767.yaml | 1 -
>  .../devicetree/bindings/display/ilitek,ili9486.yaml          | 1 -
>  Documentation/devicetree/bindings/display/msm/gpu.yaml       | 1 -
>  .../devicetree/bindings/display/panel/panel-timing.yaml      | 1 -
>  .../devicetree/bindings/display/panel/tpo,tpg110.yaml        | 1 -
>  .../devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml | 1 -
>  .../devicetree/bindings/display/simple-framebuffer.yaml      | 1 -
>  .../devicetree/bindings/dma/snps,dma-spear1340.yaml          | 1 -
>  Documentation/devicetree/bindings/dma/stericsson,dma40.yaml  | 1 -
>  .../devicetree/bindings/dma/stm32/st,stm32-dma.yaml          | 1 -
>  Documentation/devicetree/bindings/edac/apm,xgene-edac.yaml   | 1 -
>  .../devicetree/bindings/firmware/qemu,fw-cfg-mmio.yaml       | 1 -
>  Documentation/devicetree/bindings/fpga/fpga-region.yaml      | 5 -----
>  .../devicetree/bindings/gpio/brcm,xgs-iproc-gpio.yaml        | 1 -
>  .../devicetree/bindings/gpio/fairchild,74hc595.yaml          | 1 -
>  Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml     | 1 -
>  Documentation/devicetree/bindings/hwmon/adi,max31827.yaml    | 1 -
>  Documentation/devicetree/bindings/hwmon/national,lm90.yaml   | 1 -
>  Documentation/devicetree/bindings/hwmon/ti,tmp513.yaml       | 1 -
>  Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml     | 1 -
>  Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml     | 1 -
>  .../devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml         | 1 -
>  Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 2 --
>  Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml   | 1 -
>  .../devicetree/bindings/iio/adc/cosmic,10001-adc.yaml        | 1 -
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml  | 1 -
>  .../devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml     | 1 -
>  .../devicetree/bindings/iio/afe/voltage-divider.yaml         | 1 -
>  .../devicetree/bindings/iio/frequency/adi,admv4420.yaml      | 1 -
>  .../devicetree/bindings/iio/pressure/murata,zpa2326.yaml     | 1 -
>  .../devicetree/bindings/iio/proximity/semtech,sx9324.yaml    | 1 -
>  .../devicetree/bindings/iio/temperature/adi,ltc2983.yaml     | 1 -
>  Documentation/devicetree/bindings/input/ti,drv266x.yaml      | 1 -
>  .../devicetree/bindings/interconnect/qcom,rpmh.yaml          | 1 -
>  .../devicetree/bindings/interrupt-controller/arm,gic-v3.yaml | 1 -
>  .../bindings/interrupt-controller/aspeed,ast2700-intc.yaml   | 1 -
>  .../bindings/interrupt-controller/fsl,vf610-mscm-ir.yaml     | 1 -
>  .../bindings/interrupt-controller/loongson,liointc.yaml      | 1 -
>  .../bindings/interrupt-controller/mediatek,mtk-cirq.yaml     | 1 -
>  .../bindings/interrupt-controller/mscc,ocelot-icpu-intr.yaml | 1 -
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml        | 4 ----
>  Documentation/devicetree/bindings/mailbox/arm,mhu.yaml       | 1 -
>  Documentation/devicetree/bindings/mailbox/arm,mhuv2.yaml     | 1 -
>  Documentation/devicetree/bindings/mailbox/mtk,adsp-mbox.yaml | 1 -
>  Documentation/devicetree/bindings/media/amphion,vpu.yaml     | 1 -
>  Documentation/devicetree/bindings/media/i2c/adi,adv7604.yaml | 2 --
>  .../devicetree/bindings/media/i2c/techwell,tw9900.yaml       | 1 -
>  Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml   | 1 -
>  .../devicetree/bindings/media/qcom,sc8280xp-camss.yaml       | 1 -
>  .../bindings/media/samsung,exynos4212-fimc-is.yaml           | 1 -
>  .../devicetree/bindings/media/samsung,s5pv210-jpeg.yaml      | 1 -
>  Documentation/devicetree/bindings/media/st,stm32-dma2d.yaml  | 1 -
>  .../devicetree/bindings/media/video-interface-devices.yaml   | 4 ----
>  .../memory-controllers/qcom,ebi2-peripheral-props.yaml       | 1 -

>  Documentation/devicetree/bindings/mfd/stericsson,ab8500.yaml | 1 -

Acked-by: Lee Jones <lee@kernel.org>

>  .../devicetree/bindings/mtd/amlogic,meson-nand.yaml          | 1 -
>  .../devicetree/bindings/mtd/marvell,nand-controller.yaml     | 1 -
>  Documentation/devicetree/bindings/mux/mux-controller.yaml    | 1 -
>  .../devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml   | 2 --
>  Documentation/devicetree/bindings/net/brcm,bcmgenet.yaml     | 1 -
>  .../devicetree/bindings/net/brcm,mdio-mux-iproc.yaml         | 1 -
>  .../devicetree/bindings/net/cortina,gemini-ethernet.yaml     | 1 -
>  Documentation/devicetree/bindings/net/fsl,gianfar.yaml       | 2 --
>  .../devicetree/bindings/net/mdio-mux-multiplexer.yaml        | 1 -
>  Documentation/devicetree/bindings/net/qcom,ipa.yaml          | 1 -
>  Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml    | 1 -
>  .../devicetree/bindings/net/wireless/ti,wlcore.yaml          | 1 -
>  .../devicetree/bindings/pci/altr,pcie-root-port.yaml         | 1 -
>  Documentation/devicetree/bindings/pci/loongson.yaml          | 1 -
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml  | 1 -
>  .../devicetree/bindings/pci/starfive,jh7110-pcie.yaml        | 1 -
>  Documentation/devicetree/bindings/pci/versatile.yaml         | 1 -
>  .../bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml         | 1 -
>  .../devicetree/bindings/pinctrl/brcm,bcm21664-pinctrl.yaml   | 1 -
>  .../devicetree/bindings/pinctrl/fsl,imx9-pinctrl.yaml        | 1 -
>  .../devicetree/bindings/pinctrl/qcom,qcs404-pinctrl.yaml     | 1 -
>  .../bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml      | 1 -
>  .../devicetree/bindings/pinctrl/qcom,sm6125-tlmm.yaml        | 1 -
>  .../devicetree/bindings/pinctrl/renesas,rza1-ports.yaml      | 3 ---
>  .../devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml | 1 -
>  .../devicetree/bindings/power/supply/mt6360_charger.yaml     | 1 -
>  .../bindings/power/supply/stericsson,ab8500-charger.yaml     | 1 -
>  .../devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml     | 1 -
>  .../bindings/regulator/richtek,rt6245-regulator.yaml         | 1 -
>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml      | 2 --
>  Documentation/devicetree/bindings/reset/ti,sci-reset.yaml    | 1 -
>  .../bindings/rng/inside-secure,safexcel-eip76.yaml           | 2 --
>  .../devicetree/bindings/soc/fsl/cpm_qe/fsl,qe-muram.yaml     | 1 -
>  .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml     | 1 -
>  .../bindings/soc/microchip/atmel,at91rm9200-tcb.yaml         | 1 -
>  Documentation/devicetree/bindings/soc/rockchip/grf.yaml      | 1 -
>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml       | 3 ---
>  Documentation/devicetree/bindings/sound/adi,adau1372.yaml    | 1 -
>  Documentation/devicetree/bindings/sound/adi,adau7118.yaml    | 1 -
>  .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml          | 1 -
>  .../devicetree/bindings/sound/rockchip,rk3328-codec.yaml     | 2 +-
>  Documentation/devicetree/bindings/sound/samsung,tm2.yaml     | 1 -
>  .../devicetree/bindings/sound/ti,tlv320dac3100.yaml          | 1 -
>  Documentation/devicetree/bindings/sound/wlf,wm8903.yaml      | 1 -
>  .../devicetree/bindings/timer/nvidia,tegra-timer.yaml        | 1 -
>  .../devicetree/bindings/timer/nvidia,tegra186-timer.yaml     | 1 -
>  Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml   | 1 -
>  116 files changed, 2 insertions(+), 136 deletions(-)

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Mathieu Poirier @ 2025-10-24 15:01 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Philipp Zabel, Olivia Mackall,
	Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman, devicetree,
	linux-kernel, linux-clk, dri-devel, linux-fbdev, dmaengine,
	linux-fpga, linux-gpio, linux-hwmon, linux-i2c, linux-iio,
	linux-input, linux-pm, iommu, linux-media, linux-mtd, netdev,
	linux-wireless, linux-pci, linux-phy, linux-pwm, linux-remoteproc,
	linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> # remoteproc

^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Georgi Djakov @ 2025-10-24 14:59 UTC (permalink / raw)
  To: Rob Herring (Arm), Krzysztof Kozlowski, Conor Dooley,
	Stephen Boyd, David Airlie, Simona Vetter, Maarten Lankhorst,
	Thomas Zimmermann, Andrzej Hajda, Robert Foss, Vinod Koul,
	Moritz Fischer, Xu Yilun, Bartosz Golaszewski, Guenter Roeck,
	Andi Shyti, Jonathan Cameron, Dmitry Torokhov, Thomas Gleixner,
	Joerg Roedel, Jassi Brar, Mauro Carvalho Chehab, Lee Jones,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Johannes Berg, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Kishon Vijay Abraham I,
	Sebastian Reichel, Uwe Kleine-König, Mark Brown,
	Mathieu Poirier, Philipp Zabel, Olivia Mackall, Herbert Xu,
	Daniel Lezcano, Greg Kroah-Hartman
  Cc: devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On 10/23/25 5:37 PM, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
[..]>   .../devicetree/bindings/interconnect/qcom,rpmh.yaml          | 1 -
Acked-by: Georgi Djakov <djakov@kernel.org>


^ permalink raw reply

* Re: [PATCH v2] HID: quirks: Add device descriptor for 4c4a:4155
From: Staffan Melin @ 2025-10-24 14:22 UTC (permalink / raw)
  To: zhangheng
  Cc: Terry Junge, jikos, bentiss, linux-input, linux-kernel, 1114557,
	stable
In-Reply-To: <8f0155d4-72a7-45ec-a272-7892e783bbed@kylinos.cn>

Thank you,

I can confirm that this latest patch fixes the issue on my GPD Duo.

Tested-by: staffan.melin@oscillator.se

Many thanks,

Staffan


On 2025-10-24 05:32, zhangheng wrote:
> Hi Terry Junge，
> 
> I have made the changes as per your suggestion.
> mic.txt is the microphone report descriptor and is working properly.

^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Vinod Koul @ 2025-10-24 14:11 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Mathieu Poirier, Philipp Zabel,
	Olivia Mackall, Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman,
	devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On 23-10-25, 09:37, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v1 1/1] HID: intel-ish-hid: Fix a build error in devm_ishtp_alloc_workqueue()
From: Jiri Kosina @ 2025-10-24 12:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Zhang Lixu, Srinivas Pandruvada, linux-input, linux-kernel, llvm,
	Benjamin Tissoires, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt
In-Reply-To: <20251024122511.1422492-1-andriy.shevchenko@linux.intel.com>

On Fri, 24 Oct 2025, Andy Shevchenko wrote:

> clang 19 is not happy about the cast and in conjunction with CONFIG_WERROR=y
> (which is default) leads to a build error:
> 
> drivers/hid/intel-ish-hid/ipc/ipc.c:935:36: error: cast from 'void (*)(struct workqueue_struct *)' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
>   935 |         if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
>       |                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fix this by providing an intermediate callback that has a correct type.
> 
> Fixes: 0d30dae38fe0 ("HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks Andy. Nathan beat you on this one:

	https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-6.19/intel-ish-v2&id=3644f4411713f52bf231574aa8759e3d8e20b341

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* [PATCH v1 1/1] HID: intel-ish-hid: Fix a build error in devm_ishtp_alloc_workqueue()
From: Andy Shevchenko @ 2025-10-24 12:25 UTC (permalink / raw)
  To: Zhang Lixu, Jiri Kosina, Srinivas Pandruvada, linux-input,
	linux-kernel, llvm
  Cc: Benjamin Tissoires, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Andy Shevchenko

clang 19 is not happy about the cast and in conjunction with CONFIG_WERROR=y
(which is default) leads to a build error:

drivers/hid/intel-ish-hid/ipc/ipc.c:935:36: error: cast from 'void (*)(struct workqueue_struct *)' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
  935 |         if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
      |                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fix this by providing an intermediate callback that has a correct type.

Fixes: 0d30dae38fe0 ("HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 59355e4a61f8..7bf4356d8ee6 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -5,10 +5,11 @@
  * Copyright (c) 2014-2016, Intel Corporation.
  */
 
+#include <linux/delay.h>
 #include <linux/devm-helpers.h>
+#include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/spinlock.h>
-#include <linux/delay.h>
 #include <linux/jiffies.h>
 #include "client.h"
 #include "hw-ish.h"
@@ -924,17 +925,23 @@ static const struct ishtp_hw_ops ish_hw_ops = {
 	.dma_no_cache_snooping = _dma_no_cache_snooping
 };
 
+static inline void devm_ishtp_destroy_workqueue(void *wq)
+{
+	destroy_workqueue(wq);
+}
+
 static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev)
 {
 	struct workqueue_struct *wq;
+	int ret;
 
 	wq = alloc_workqueue("ishtp_unbound_%d", WQ_UNBOUND, 0, dev->id);
 	if (!wq)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
-	if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
-				     wq))
-		return NULL;
+	ret = devm_add_action_or_reset(dev, devm_ishtp_destroy_workqueue, wq);
+	if (ret)
+		return ERR_PTR(ret);
 
 	return wq;
 }
@@ -950,6 +957,7 @@ static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev)
 struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
 {
 	struct ishtp_device *dev;
+	struct workqueue_struct *wq;
 	int	i;
 	int	ret;
 
@@ -959,9 +967,10 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
 	if (!dev)
 		return NULL;
 
-	dev->unbound_wq = devm_ishtp_alloc_workqueue(&pdev->dev);
-	if (!dev->unbound_wq)
+	wq = devm_ishtp_alloc_workqueue(&pdev->dev);
+	if (IS_ERR(wq))
 		return NULL;
+	dev->unbound_wq = wq;
 
 	dev->devc = &pdev->dev;
 	ishtp_device_init(dev);
-- 
2.50.1


^ permalink raw reply related

* Re: [PATCH] HID: intel-ish-hid: Fix -Wcast-function-type-strict in devm_ishtp_alloc_workqueue()
From: Jiri Kosina @ 2025-10-24 11:21 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Srinivas Pandruvada, Benjamin Tissoires, Zhang Lixu, Kees Cook,
	linux-input, llvm, patches
In-Reply-To: <20251022-ishtp-fix-function-cast-warn-v1-1-bfb06464f8ca@kernel.org>

On Wed, 22 Oct 2025, Nathan Chancellor wrote:

> Clang warns (or errors with CONFIG_WERROR=y / W=e):
> 
>   drivers/hid/intel-ish-hid/ipc/ipc.c:935:36: error: cast from 'void (*)(struct workqueue_struct *)' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
>     935 |         if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
>         |                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   include/linux/device/devres.h:168:34: note: expanded from macro 'devm_add_action_or_reset'
>     168 |         __devm_add_action_or_ireset(dev, action, data, #action)
>         |                                         ^~~~~~
> 
> This warning is pointing out a kernel control flow integrity (kCFI /
> CONFIG_CFI=y) violation will occur due to this function cast when the
> destroy_workqueue() is indirectly called via devm_action_release()
> because the prototype of destroy_workqueue() does not match the
> prototype of (*action)().
> 
> Use a local function with the correct prototype to wrap
> destroy_workqueue() to resolve the warning and CFI violation.
> 
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2139
> Fixes: 0d30dae38fe0 ("HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

I have also added

	Reported-by: kernel test robot <lkp@intel.com>
	Closes: https://lore.kernel.org/oe-kbuild-all/202510190103.qTZvfdjj-lkp@intel.com/

and applied, thanks!

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* [PATCH 2/2] Documentation: hid-alps: Format DataByte* subsection headings
From: Bagas Sanjaya @ 2025-10-24 10:39 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Linux Documentation,
	Linux Input Devices
  Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Masaki Ota,
	George Anthony Vernon, Bagas Sanjaya
In-Reply-To: <20251024103934.20019-1-bagasdotme@gmail.com>

"Command Read/Write" section has two DataByte* subsections describing
command bytes format. Add markup to these subsection heading texts.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 Documentation/hid/hid-alps.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/hid/hid-alps.rst b/Documentation/hid/hid-alps.rst
index 3a22254e43464f..4a22a357f00c02 100644
--- a/Documentation/hid/hid-alps.rst
+++ b/Documentation/hid/hid-alps.rst
@@ -69,6 +69,7 @@ To read/write to RAM, need to send a command to the device.
 The command format is as below.
 
 DataByte(SET_REPORT)
+~~~~~~~~~~~~~~~~~~~~
 
 =====	======================
 Byte1	Command Byte
@@ -89,6 +90,7 @@ Value Byte is writing data when you send the write commands.
 When you read RAM, there is no meaning.
 
 DataByte(GET_REPORT)
+~~~~~~~~~~~~~~~~~~~~
 
 =====	======================
 Byte1	Response Byte
-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply related

* [PATCH 0/2] hid-alps docs heading cleanup
From: Bagas Sanjaya @ 2025-10-24 10:39 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Linux Documentation,
	Linux Input Devices
  Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Masaki Ota,
	George Anthony Vernon, Bagas Sanjaya

Hi,

Here are two section headings cleanup patches for Alps HID documentation.
Enjoy!

Bagas Sanjaya (2):
  Documentation: hid-alps: Fix packet format section headings
  Documentation: hid-alps: Format DataByte* subsection headings

 Documentation/hid/hid-alps.rst | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: 828aeac92901c1f31b51ae0b9d792b9af5bd3e27
-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply

* [PATCH 1/2] Documentation: hid-alps: Fix packet format section headings
From: Bagas Sanjaya @ 2025-10-24 10:39 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Linux Documentation,
	Linux Input Devices
  Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Masaki Ota,
	George Anthony Vernon, Bagas Sanjaya
In-Reply-To: <20251024103934.20019-1-bagasdotme@gmail.com>

In "Packet Format" section, only "StickPointer data byte" subsection
heading is shown as such in htmldocs output; "Touchpad data byte" one
is in normal paragraph on the other hand.

Properly format the headings to be consistent.

Fixes: 2562756dde55 ("HID: add Alps I2C HID Touchpad-Stick support")
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 Documentation/hid/hid-alps.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/hid/hid-alps.rst b/Documentation/hid/hid-alps.rst
index 94382bb0ada4d8..3a22254e43464f 100644
--- a/Documentation/hid/hid-alps.rst
+++ b/Documentation/hid/hid-alps.rst
@@ -104,8 +104,10 @@ Read value is stored in Value Byte.
 
 
 Packet Format
+-------------
+
 Touchpad data byte
-------------------
+~~~~~~~~~~~~~~~~~~
 
 
 ======= ======= ======= ======= ======= ======= ======= ======= =====
@@ -156,7 +158,7 @@ Zsn_6-0(7bit):
 
 
 StickPointer data byte
-----------------------
+~~~~~~~~~~~~~~~~~~~~~~
 
 ======= ======= ======= ======= ======= ======= ======= ======= =====
 -	b7	b6	b5	b4	b3	b2	b1	b0
-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply related

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Andi Shyti @ 2025-10-24 10:30 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Mathieu Poirier, Philipp Zabel,
	Olivia Mackall, Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman,
	devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

Hi Rob,

On Thu, Oct 23, 2025 at 09:37:56AM -0500, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Acked-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi

^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Bartosz Golaszewski @ 2025-10-24  7:56 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Guenter Roeck, Andi Shyti, Jonathan Cameron, Dmitry Torokhov,
	Georgi Djakov, Thomas Gleixner, Joerg Roedel, Jassi Brar,
	Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Mathieu Poirier, Philipp Zabel,
	Olivia Mackall, Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman,
	devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On Thu, Oct 23, 2025 at 4:40 PM Rob Herring (Arm) <robh@kernel.org> wrote:
>
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

For GPIO:

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

^ permalink raw reply

* Re: [PATCH v3 03/11] Input: add INPUT_PROP_HAPTIC_TOUCHPAD
From: Peter Hutterer @ 2025-10-24  3:30 UTC (permalink / raw)
  To: Jonathan Denose
  Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Jonathan Corbet,
	Henrik Rydberg, linux-input, linux-kernel, linux-doc,
	Angela Czubak, Sean O'Brien
In-Reply-To: <20250818-support-forcepads-v3-3-e4f9ab0add84@google.com>

On Mon, Aug 18, 2025 at 11:08:44PM +0000, Jonathan Denose wrote:
> From: Angela Czubak <aczubak@google.com>
> 
> INPUT_PROP_HAPTIC_TOUCHPAD property is to be set for a device with simple
> haptic capabilities.
> 
> Signed-off-by: Angela Czubak <aczubak@google.com>
> Co-developed-by: Jonathan Denose <jdenose@google.com>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
>  Documentation/input/event-codes.rst    | 14 ++++++++++++++
>  include/uapi/linux/input-event-codes.h |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> index b4557462edd7b3fef9e9cd6c2c3cb2d05bb531ab..1ead9bb8d9c6451a81426665baab643f9e50216a 100644
> --- a/Documentation/input/event-codes.rst
> +++ b/Documentation/input/event-codes.rst
> @@ -400,6 +400,20 @@ can report through the rotational axes (absolute and/or relative rx, ry, rz).
>  All other axes retain their meaning. A device must not mix
>  regular directional axes and accelerometer axes on the same event node.
>  
> +INPUT_PROP_HAPTIC_TOUCHPAD
> +--------------------------
> +
> +The INPUT_PROP_HAPTIC_TOUCHPAD property indicates that device:
> +- supports simple haptic auto and manual triggering
> +- can differentiate between at least 5 fingers
> +- uses correct resolution for the X/Y (units and value)
> +- reports correct force per touch, and correct units for them (newtons or grams)
> +- follows the MT protocol type B
> +
> +Summing up, such devices follow the MS spec for input devices in
> +Win8 and Win8.1, and in addition support the Simple haptic controller HID table,
> +and report correct units for the pressure.

I'm a bit late to the party here but I just had to deal with a related
bug and I'm wondering if you'd be open to loosening the definition of
INPUT_PROP_HAPTIC_TOUCHPAD to include touchpads that are *physically*
haptic touchpads but do not support userspace control of the
haptics (lack of driver support, usually).

We currently lack support for those "pressurepads" - in theory they
should report pressure resolution but the number of quirks we ship in
libinput indicates none or few of them do. Those devices *should*
report HID 0x0D usage 0x55 and identify them as pressurepads [1] because
otherwise Windows probably wouldn't work (unfortunately I don't have
access to any of those devices to verify).

By using this prop for all pressurepads including those not exposing
actual haptic effects the definition would become:

INPUT_PROP_HAPTIC_TOUCHPAD
--------------------------

The INPUT_PROP_HAPTIC_TOUCHPAD property indicates that device:
- can differentiate between at least 5 fingers
- uses correct resolution for the X/Y (units and value)
- follows the MT protocol type B
- may support simple haptic auto and manual triggering and if so:
  - reports correct force per touch, and correct units for them (newtons or grams)

And then we can hook up the 0x55 usage to set that property and leave it
up to the client to check if the FF_ bits are in place to verify it's a
haptic-haptic touchpad?

This would save us from needing a separate INPUT_PROP_PRESSUREPAD.

Cheers,
  Peter

[1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report


> +
>  Guidelines
>  ==========
>  
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 3b2524e4b667d1e7cc02ff5cb674e7c2ac069a66..efe8c36d4ee9a938ffb1b0dd0ddd0ec6a3fcb8fe 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -27,6 +27,7 @@
>  #define INPUT_PROP_TOPBUTTONPAD		0x04	/* softbuttons at top of pad */
>  #define INPUT_PROP_POINTING_STICK	0x05	/* is a pointing stick */
>  #define INPUT_PROP_ACCELEROMETER	0x06	/* has accelerometer */
> +#define INPUT_PROP_HAPTIC_TOUCHPAD	0x07	/* is a haptic touchpad */
>  
>  #define INPUT_PROP_MAX			0x1f
>  #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
> 
> -- 
> 2.51.0.rc1.193.gad69d77794-goog
> 
> 

^ permalink raw reply

* Re: [PATCH v2] HID: quirks: Add device descriptor for 4c4a:4155
From: zhangheng @ 2025-10-24  3:32 UTC (permalink / raw)
  To: Terry Junge, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <365f9f8e-549e-42e1-ac8c-7ff1f42c6977@cosmicgizmosystems.com>

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

Hi Terry Junge，

I have made the changes as per your suggestion.
mic.txt is the microphone report descriptor and is working properly.




[-- Attachment #2: mic.txt --]
[-- Type: text/plain, Size: 293 bytes --]

klin@zhangheng:~/桌面$ sudo usbhid-dump 
001:008:002:DESCRIPTOR         1761275817.253121
 05 01 09 06 A1 01 05 07 19 E0 29 E7 15 00 25 01
 75 01 95 08 81 02 95 01 75 08 81 01 95 03 75 01
 05 08 19 01 29 03 91 02 95 01 75 05 91 01 95 06
 75 08 15 00 26 FF 00 05 07 19 00 2A FF 00 81 00
 C0


[-- Attachment #3: v3-0001-HID-quirks-Change-manufacturer-for-4c4a-4155.patch --]
[-- Type: text/plain, Size: 2479 bytes --]

From 4d670a785276a8fd1568c9e7e40179809b3dd5ec Mon Sep 17 00:00:00 2001
From: Zhang Heng <zhangheng@kylinos.cn>
Date: Fri, 12 Sep 2025 20:38:18 +0800
Subject: [PATCH v3] HID: quirks: Change manufacturer for 4c4a:4155

Based on available evidence, the USB ID 4c4a:4155 used by multiple
devices has been attributed to Jieli. The commit 1a8953f4f774
("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY") affected touchscreen
functionality. A manufacturer check was added to maintain microphone
compatibility, enabling both devices to function properly.

Fixes: 1a8953f4f774 ("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY")
Cc: stable@kernel.org
Tested-by: staffan.melin@oscillator.se
Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
---
 drivers/hid/hid-ids.h    |  4 ++--
 drivers/hid/hid-quirks.c | 11 ++++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5721b8414bbd..0b1ff1ec275e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1539,7 +1539,7 @@
 #define USB_VENDOR_ID_SIGNOTEC			0x2133
 #define USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011	0x0018
 
-#define USB_VENDOR_ID_SMARTLINKTECHNOLOGY              0x4c4a
-#define USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155         0x4155
+#define USB_VENDOR_ID_JIELI_SDK_DEFAULT		0x4c4a
+#define USB_DEVICE_ID_JIELI_SDK_4155		0x4155
 
 #endif
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index ffd034566e2e..0058c01aa8b3 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -913,7 +913,6 @@ static const struct hid_device_id hid_ignore_list[] = {
 #endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_HP_5MP_CAMERA_5473) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SMARTLINKTECHNOLOGY, USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155) },
 	{ }
 };
 
@@ -1062,6 +1061,16 @@ bool hid_ignore(struct hid_device *hdev)
 					     strlen(elan_acpi_id[i].id)))
 					return true;
 		break;
+	case USB_VENDOR_ID_JIELI_SDK_DEFAULT:
+		/*
+		 * Multiple USB devices with identical IDs (mic & touchscreen).
+		 * The touch screen requires hid core processing, but the
+		 * microphone does not. They can be distinguished by manufacturer.
+		 */
+		if (hdev->product == USB_DEVICE_ID_JIELI_SDK_4155 &&
+		    strncmp(hdev->name, "SmartlinkTechnology", 19) == 0)
+			return true;
+		break;
 	}
 
 	if (hdev->type == HID_TYPE_USBMOUSE &&
-- 
2.47.1


^ permalink raw reply related

* Re: [PATCH v2] HID: quirks: Add device descriptor for 4c4a:4155
From: Terry Junge @ 2025-10-24  0:39 UTC (permalink / raw)
  To: zhangheng, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557
In-Reply-To: <e605f642-c967-4d41-8145-a10e8f48fb1b@kylinos.cn>

Hi Zhang,

Sorry my earlier emails had my name mangled by Thunderbird. I am not 
Linux Hid. I am Terry Junge.

Anyway, are you planning on sending a V3 of this patch?

Also, would it be possible for you to get the report descriptor from the 
microphone.

Thanks,
Terry

On 10/13/2025 1:32 AM, zhangheng wrote:
> It happened to be the holiday, so communication was a bit troublesome.
> 
> However, after a brief discussion with the microphone manufacturer,
> 
> it was found that the serial number was still 20201111000001 on another
> 
> microphone device. So, should we add it?
> 
> 在 2025/9/29 8:42, Linux Hid 写道:
>> Hi Zhang,
>>
>> The subject doesn't reflect what the patch is doing. You are not adding
>> a device descriptor, you are fixing a regression.
>>
>> On 9/22/2025 7:24 PM, Zhang Heng wrote:
>>> Multiple USB devices have the same ID;
>>> add device descriptors to distinguish them.
>>>
>>> Fixes: 1a8953f4f774 ("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY")
>> Should have a Fixes: tag referencing the regression bug.
>> Also a CC: tag for 1114557@bugs.debian.org
>> Possibly a CC: tag for stable@vger.kernel.org as well?
>>
>>> Tested-by: staffan.melin@oscillator.se
>>> Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
>>> ---
>>>    drivers/hid/hid-quirks.c | 12 +++++++++++-
>>>    1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>>> index ffd034566e2e..d28b180abd72 100644
>>> --- a/drivers/hid/hid-quirks.c
>>> +++ b/drivers/hid/hid-quirks.c
>>> @@ -913,7 +913,6 @@ static const struct hid_device_id 
>>> hid_ignore_list[] = {
>>>    #endif
>>>        { HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, 
>>> USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
>>>        { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, 
>>> USB_DEVICE_ID_QUANTA_HP_5MP_CAMERA_5473) },
>>> -    { HID_USB_DEVICE(USB_VENDOR_ID_SMARTLINKTECHNOLOGY, 
>>> USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155) },
>>>        { }
>>>    };
>> Smartlink Technology does not own the 0x4c4a VID or the 0x4155 PID. They
>> are an artifact of the Jieli SDK they used in development so the
>> #defines should not imply ownership by Smartlink. VID 0x4c4a is
>> currently unassigned by the USBIF and is therefore 'reserved'.
>>
>> Maybe change
>> USB_VENDOR_ID_SMARTLINKTECHNOLOGY to USB_VENDOR_ID_JIELI_SDK_DEFAULT
>> and
>> USB_DEVICE_ID_SMARTLINKTRCHNOLOGY_4155 to USB_DEVICE_ID_JIELI_SDK_4155?
>>
>>> @@ -1062,6 +1061,17 @@ bool hid_ignore(struct hid_device *hdev)
>>>                             strlen(elan_acpi_id[i].id)))
>>>                        return true;
>>>            break;
>>> +    case USB_VENDOR_ID_SMARTLINKTECHNOLOGY:
>>> +        /* Multiple USB devices with identical IDs (mic & touchscreen).
>>> +         * The touch screen requires hid core processing, but the
>>> +         * microphone does not. They can be distinguished by 
>>> manufacturer
>>> +         * and serial number.
>>> +         */
>>> +        if (hdev->product == USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155 &&
>>> +            strncmp(hdev->name, "SmartlinkTechnology", 19) == 0 &&
>>> +            strncmp(hdev->uniq, "20201111000001", 14) == 0)
>> Using the serial number as a device identifier is somewhat risky. The
>> serial number is optional for a USB device but if it is used then it's
>> supposed to be unique for each device. Given how horrible the
>> configuration and HID descriptors are for this device it's unlikely that
>> they went to the trouble to give each unit a unique serial number. But
>> you should check a few of the devices (if you have more than one) to
>> verify they all have the same 20201111000001 serial number.
>>
>> It's too bad the bcdHID version test for 0x0201 didn't work. The
>> hid->version field is filled by usbhid_probe with bcdDevice before both
>> hid_lookup_quirk and hid_ignore are called and then updated with bcdHID
>> by usbhid_parse after they have been called.
>>
>>> +            return true;
>>> +        break;
>>>        }
>>>        if (hdev->type == HID_TYPE_USBMOUSE &&
>> Thanks
>> Terry

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-10-23 23:25 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <b91de7c7-74b8-4cf5-82a4-f3d4eaf418d4@gmail.com>

On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/23/25 23:30, Antheas Kapenekakis wrote:
> > On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/23/25 20:06, Antheas Kapenekakis wrote:
> >>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
> >>>>> Currently, RGB initialization forks depending on whether a device is
> >>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
> >>>>> endpoints, and non-NKEY devices with 0x5a and then a
> >>>>> backlight check, which is omitted for NKEY devices.
> >>>>>
> >>>>> Remove the fork, using a common initialization sequence for both,
> >>>>> where they are both only initialized with 0x5a, then checked for
> >>>>> backlight support. This patch should not affect existing functionality.
> >>>>>
> >>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
> >>>>> userspace programs associated with different usages that reside under
> >>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
> >>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
> >>>>> Neither is used currently in the driver.
> >>>> What benefits do we get from removing the unused initialization?
> >>>>
> >>>> If this has never caused any troubles I don't see the reason for removing
> >>>> them. Moreover the lighting protocol is known and I might as well add
> >>>> support for it in the near future,
> >>> I already have a patch that adds RGB and delay inits that endpoint. It
> >>> got removed to make this easier to merge. See [1].
> >>>
> >>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
> >> I have to main concerns about this:
> >>
> >> 1. taking away initialization commands in one patchset to make it
> >> easier to merge another unrelated patch doesn't seem the right thing
> >> to do if the other patch it's not in the same series.
> >>
> >> I can see [1] has been removed from the set for a later moment in time,
> >> it's fine if it needs more work, just send something that function in the
> >> same way and do not remove initialization commands when unnecessary,
> >> especially since there will be for sure future development.
> > The initialization was removed as part of general cleanup. Not to make
> > it easier to merge the RGB patch. In addition, the RGB patch only runs
> > the init in a lazy fashion, so if nobody uses the RGB sysfs the init
> > does not run and the behavior is the same.
> There are a few problems here:
> 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
> doesn't load hid-asus correctly has nothing to do with the initialization of anime.
> The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
> anime matrix initialization either.
> 2. not sending the initialization can get hardware misbehave because it
> is left in an uninitialized state.
> 3. there are absolutely zero reasons to do that. There are even less reasons
> as to do it as part of this patchset.
>
> >> 2. Your patchset resolves around keyboard backlight control and how
> >> the keyboard device is exposed to userspace: it's fine but I do not see
> >> the point in removing initialization commands that has nothing to do
> >> with the issue we are trying to solve here.
> >>
> >> Please leave 0x5E and 0x5D initialization commands where they are now.
> > I mean the second part of the patchset does that. The first part is a
> > cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
> > only used when initializing those endpoints to write further commands
> > to them and for identification. The current driver does not write
> > commands to those endpoints and identifies itself over 0x5A.
> There are no bugs opened that ties initialization of devices to bugs.
> Quite the opposite: I can guarantee you that removing part of the
> init will introduce regressions.
>
> The onus is on you to provide strong evidence that the removal is
> a necessary act.
>
> Regardless it is not in the scope of this patchset: remove it.
> > I do get that it is a bit risky as some laptops might be hardcoded to
> > wait for 0x5D to turn on RGB. Which is why we had the last patch until
> > V4. But we have yet to find a laptop that has this problem, so I find
> > it difficult to justify keeping the init.
> Yes it's risky to remove initialization sequences for a device that is
> in every modern ASUS laptop and is tied to the EC.
> > Do note that you might need to add the 0x5D init to your userspace
> > program for certain laptops if you haven't already. But that is ok,
> > since in doing so you are also validating you are speaking to an Asus
> > device, which is important.
> This doesn't make much sense: why would anyone remove
> a command from the kernel, that can be very well essential to some models
> (sleep can break, for example) just to add it back in a userspace program?
>
> What does it mean I have to validate I am speaking to an asus device?
> Software selects devices by known attribute, one of them is the vid:pid....
> Beside what does this have to do with the removal of initialization commands
> from the kernel?
>
> Even late initializing devices can lead to problems. Windows doesn't do that:
> as soon as asus drivers are loaded all relevant initialization sequences are
> sent; Windows is the only officially supported OS: do not introduce commands
> flow divergence without strong reasons backing it up.

If you think keeping 0x5D init is that important, I can spin patch [1]
into this series. But then this quirk will stay in the kernel forever.
I can even add 0x5E since that does not affect newer devices, which I
care for simplifying the sequence.

Luke said these two pairs are the important ones to keep.

I'm not sure what to do.

Antheas

[1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/

> > Antheas
> >
> Denis
> >>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >>>>> ---
> >>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
> >>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >>>>> index a444d41e53b6..7ea1037c3979 100644
> >>>>> --- a/drivers/hid/hid-asus.c
> >>>>> +++ b/drivers/hid/hid-asus.c
> >>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >>>>>       unsigned char kbd_func;
> >>>>>       int ret;
> >>>>>
> >>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> >>>>> -             /* Initialize keyboard */
> >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>> -             if (ret < 0)
> >>>>> -                     return ret;
> >>>>> -
> >>>>> -             /* The LED endpoint is initialised in two HID */
> >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >>>>> -             if (ret < 0)
> >>>>> -                     return ret;
> >>>>> -
> >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> >>>>> -             if (ret < 0)
> >>>>> -                     return ret;
> >>>>> -
> >>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>> -                     ret = asus_kbd_disable_oobe(hdev);
> >>>>> -                     if (ret < 0)
> >>>>> -                             return ret;
> >>>>> -             }
> >>>>> -
> >>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>> -                     intf = to_usb_interface(hdev->dev.parent);
> >>>>> -                     udev = interface_to_usbdev(intf);
> >>>>> -                     validate_mcu_fw_version(hdev,
> >>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
> >>>>> -             }
> >>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>> +     if (ret < 0)
> >>>>> +             return ret;
> >>>>>
> >>>>> -     } else {
> >>>>> -             /* Initialize keyboard */
> >>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>> -             if (ret < 0)
> >>>>> -                     return ret;
> >>>>> +     /* Get keyboard functions */
> >>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>> +     if (ret < 0)
> >>>>> +             return ret;
> >>>>>
> >>>>> -             /* Get keyboard functions */
> >>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>> +             ret = asus_kbd_disable_oobe(hdev);
> >>>>>               if (ret < 0)
> >>>>>                       return ret;
> >>>>> +     }
> >>>>>
> >>>>> -             /* Check for backlight support */
> >>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>> -                     return -ENODEV;
> >>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>> +             intf = to_usb_interface(hdev->dev.parent);
> >>>>> +             udev = interface_to_usbdev(intf);
> >>>>> +             validate_mcu_fw_version(
> >>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
> >>>>>       }
> >>>>>
> >>>>> +     /* Check for backlight support */
> >>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>> +             return -ENODEV;
> >>>>> +
> >>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >>>>>                                             sizeof(struct asus_kbd_leds),
> >>>>>                                             GFP_KERNEL);
>


^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Denis Benato @ 2025-10-23 22:53 UTC (permalink / raw)
  To: Antheas Kapenekakis
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <CAGwozwFbQWyuQB6EwLMLon5muff2WudR+oVL62DqP_MXGW+p-Q@mail.gmail.com>


On 10/23/25 23:30, Antheas Kapenekakis wrote:
> On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/23/25 20:06, Antheas Kapenekakis wrote:
>>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
>>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
>>>>> Currently, RGB initialization forks depending on whether a device is
>>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
>>>>> endpoints, and non-NKEY devices with 0x5a and then a
>>>>> backlight check, which is omitted for NKEY devices.
>>>>>
>>>>> Remove the fork, using a common initialization sequence for both,
>>>>> where they are both only initialized with 0x5a, then checked for
>>>>> backlight support. This patch should not affect existing functionality.
>>>>>
>>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
>>>>> userspace programs associated with different usages that reside under
>>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
>>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
>>>>> Neither is used currently in the driver.
>>>> What benefits do we get from removing the unused initialization?
>>>>
>>>> If this has never caused any troubles I don't see the reason for removing
>>>> them. Moreover the lighting protocol is known and I might as well add
>>>> support for it in the near future,
>>> I already have a patch that adds RGB and delay inits that endpoint. It
>>> got removed to make this easier to merge. See [1].
>>>
>>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
>> I have to main concerns about this:
>>
>> 1. taking away initialization commands in one patchset to make it
>> easier to merge another unrelated patch doesn't seem the right thing
>> to do if the other patch it's not in the same series.
>>
>> I can see [1] has been removed from the set for a later moment in time,
>> it's fine if it needs more work, just send something that function in the
>> same way and do not remove initialization commands when unnecessary,
>> especially since there will be for sure future development.
> The initialization was removed as part of general cleanup. Not to make
> it easier to merge the RGB patch. In addition, the RGB patch only runs
> the init in a lazy fashion, so if nobody uses the RGB sysfs the init
> does not run and the behavior is the same.
There are a few problems here:
1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
doesn't load hid-asus correctly has nothing to do with the initialization of anime.
The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
anime matrix initialization either.
2. not sending the initialization can get hardware misbehave because it
is left in an uninitialized state.
3. there are absolutely zero reasons to do that. There are even less reasons
as to do it as part of this patchset.

>> 2. Your patchset resolves around keyboard backlight control and how
>> the keyboard device is exposed to userspace: it's fine but I do not see
>> the point in removing initialization commands that has nothing to do
>> with the issue we are trying to solve here.
>>
>> Please leave 0x5E and 0x5D initialization commands where they are now.
> I mean the second part of the patchset does that. The first part is a
> cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
> only used when initializing those endpoints to write further commands
> to them and for identification. The current driver does not write
> commands to those endpoints and identifies itself over 0x5A.
There are no bugs opened that ties initialization of devices to bugs.
Quite the opposite: I can guarantee you that removing part of the
init will introduce regressions.

The onus is on you to provide strong evidence that the removal is
a necessary act.

Regardless it is not in the scope of this patchset: remove it.
> I do get that it is a bit risky as some laptops might be hardcoded to
> wait for 0x5D to turn on RGB. Which is why we had the last patch until
> V4. But we have yet to find a laptop that has this problem, so I find
> it difficult to justify keeping the init.
Yes it's risky to remove initialization sequences for a device that is
in every modern ASUS laptop and is tied to the EC.
> Do note that you might need to add the 0x5D init to your userspace
> program for certain laptops if you haven't already. But that is ok,
> since in doing so you are also validating you are speaking to an Asus
> device, which is important.
This doesn't make much sense: why would anyone remove
a command from the kernel, that can be very well essential to some models
(sleep can break, for example) just to add it back in a userspace program?

What does it mean I have to validate I am speaking to an asus device?
Software selects devices by known attribute, one of them is the vid:pid....
Beside what does this have to do with the removal of initialization commands
from the kernel?

Even late initializing devices can lead to problems. Windows doesn't do that:
as soon as asus drivers are loaded all relevant initialization sequences are 
sent; Windows is the only officially supported OS: do not introduce commands
flow divergence without strong reasons backing it up.

> Antheas
>
Denis
>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>>> ---
>>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
>>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>>> index a444d41e53b6..7ea1037c3979 100644
>>>>> --- a/drivers/hid/hid-asus.c
>>>>> +++ b/drivers/hid/hid-asus.c
>>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>>>       unsigned char kbd_func;
>>>>>       int ret;
>>>>>
>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>>>> -             /* Initialize keyboard */
>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>> -             if (ret < 0)
>>>>> -                     return ret;
>>>>> -
>>>>> -             /* The LED endpoint is initialised in two HID */
>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>> -             if (ret < 0)
>>>>> -                     return ret;
>>>>> -
>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>> -             if (ret < 0)
>>>>> -                     return ret;
>>>>> -
>>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>> -                     ret = asus_kbd_disable_oobe(hdev);
>>>>> -                     if (ret < 0)
>>>>> -                             return ret;
>>>>> -             }
>>>>> -
>>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>> -                     intf = to_usb_interface(hdev->dev.parent);
>>>>> -                     udev = interface_to_usbdev(intf);
>>>>> -                     validate_mcu_fw_version(hdev,
>>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
>>>>> -             }
>>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>> +     if (ret < 0)
>>>>> +             return ret;
>>>>>
>>>>> -     } else {
>>>>> -             /* Initialize keyboard */
>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>> -             if (ret < 0)
>>>>> -                     return ret;
>>>>> +     /* Get keyboard functions */
>>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>> +     if (ret < 0)
>>>>> +             return ret;
>>>>>
>>>>> -             /* Get keyboard functions */
>>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>> +             ret = asus_kbd_disable_oobe(hdev);
>>>>>               if (ret < 0)
>>>>>                       return ret;
>>>>> +     }
>>>>>
>>>>> -             /* Check for backlight support */
>>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>> -                     return -ENODEV;
>>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>> +             intf = to_usb_interface(hdev->dev.parent);
>>>>> +             udev = interface_to_usbdev(intf);
>>>>> +             validate_mcu_fw_version(
>>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
>>>>>       }
>>>>>
>>>>> +     /* Check for backlight support */
>>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>> +             return -ENODEV;
>>>>> +
>>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>>>>                                             sizeof(struct asus_kbd_leds),
>>>>>                                             GFP_KERNEL);

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-10-23 21:30 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <3947f772-691b-46a2-af68-15825e7f4939@gmail.com>

On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/23/25 20:06, Antheas Kapenekakis wrote:
> > On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/18/25 12:17, Antheas Kapenekakis wrote:
> >>> Currently, RGB initialization forks depending on whether a device is
> >>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
> >>> endpoints, and non-NKEY devices with 0x5a and then a
> >>> backlight check, which is omitted for NKEY devices.
> >>>
> >>> Remove the fork, using a common initialization sequence for both,
> >>> where they are both only initialized with 0x5a, then checked for
> >>> backlight support. This patch should not affect existing functionality.
> >>>
> >>> 0x5d and 0x5e endpoint initializations are performed by Windows
> >>> userspace programs associated with different usages that reside under
> >>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
> >>> controls RGB and 0x5e by an animation program for certain Asus laptops.
> >>> Neither is used currently in the driver.
> >> What benefits do we get from removing the unused initialization?
> >>
> >> If this has never caused any troubles I don't see the reason for removing
> >> them. Moreover the lighting protocol is known and I might as well add
> >> support for it in the near future,
> > I already have a patch that adds RGB and delay inits that endpoint. It
> > got removed to make this easier to merge. See [1].
> >
> > [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
> I have to main concerns about this:
>
> 1. taking away initialization commands in one patchset to make it
> easier to merge another unrelated patch doesn't seem the right thing
> to do if the other patch it's not in the same series.
>
> I can see [1] has been removed from the set for a later moment in time,
> it's fine if it needs more work, just send something that function in the
> same way and do not remove initialization commands when unnecessary,
> especially since there will be for sure future development.

The initialization was removed as part of general cleanup. Not to make
it easier to merge the RGB patch. In addition, the RGB patch only runs
the init in a lazy fashion, so if nobody uses the RGB sysfs the init
does not run and the behavior is the same.

> 2. Your patchset resolves around keyboard backlight control and how
> the keyboard device is exposed to userspace: it's fine but I do not see
> the point in removing initialization commands that has nothing to do
> with the issue we are trying to solve here.
>
> Please leave 0x5E and 0x5D initialization commands where they are now.

I mean the second part of the patchset does that. The first part is a
cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
only used when initializing those endpoints to write further commands
to them and for identification. The current driver does not write
commands to those endpoints and identifies itself over 0x5A.

I do get that it is a bit risky as some laptops might be hardcoded to
wait for 0x5D to turn on RGB. Which is why we had the last patch until
V4. But we have yet to find a laptop that has this problem, so I find
it difficult to justify keeping the init.

Do note that you might need to add the 0x5D init to your userspace
program for certain laptops if you haven't already. But that is ok,
since in doing so you are also validating you are speaking to an Asus
device, which is important.

Antheas

> >>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >>> ---
> >>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
> >>>  1 file changed, 19 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >>> index a444d41e53b6..7ea1037c3979 100644
> >>> --- a/drivers/hid/hid-asus.c
> >>> +++ b/drivers/hid/hid-asus.c
> >>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >>>       unsigned char kbd_func;
> >>>       int ret;
> >>>
> >>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> >>> -             /* Initialize keyboard */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> -
> >>> -             /* The LED endpoint is initialised in two HID */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> -
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> -
> >>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>> -                     ret = asus_kbd_disable_oobe(hdev);
> >>> -                     if (ret < 0)
> >>> -                             return ret;
> >>> -             }
> >>> -
> >>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>> -                     intf = to_usb_interface(hdev->dev.parent);
> >>> -                     udev = interface_to_usbdev(intf);
> >>> -                     validate_mcu_fw_version(hdev,
> >>> -                             le16_to_cpu(udev->descriptor.idProduct));
> >>> -             }
> >>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>> +     if (ret < 0)
> >>> +             return ret;
> >>>
> >>> -     } else {
> >>> -             /* Initialize keyboard */
> >>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>> -             if (ret < 0)
> >>> -                     return ret;
> >>> +     /* Get keyboard functions */
> >>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>> +     if (ret < 0)
> >>> +             return ret;
> >>>
> >>> -             /* Get keyboard functions */
> >>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>> +             ret = asus_kbd_disable_oobe(hdev);
> >>>               if (ret < 0)
> >>>                       return ret;
> >>> +     }
> >>>
> >>> -             /* Check for backlight support */
> >>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>> -                     return -ENODEV;
> >>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>> +             intf = to_usb_interface(hdev->dev.parent);
> >>> +             udev = interface_to_usbdev(intf);
> >>> +             validate_mcu_fw_version(
> >>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
> >>>       }
> >>>
> >>> +     /* Check for backlight support */
> >>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>> +             return -ENODEV;
> >>> +
> >>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >>>                                             sizeof(struct asus_kbd_leds),
> >>>                                             GFP_KERNEL);
>


^ permalink raw reply

* Touchpad needs psmouse.synaptics_intertouch
From: Luis Cebrian @ 2025-10-23 21:25 UTC (permalink / raw)
  To: linux-input

Dear linux-input,

I was having some issues with my touchpad on my Lenovo Thinkpad X390,
after reading some log messages, I noticed something among the lines
of:

psmouse serio2: synaptics: Your touchpad says it can support a
different bus. If i2c-hid and hid-rmi are not used, you might want to
try setting psmouse.synaptics_intertouch to 1.

Both of the mentioned modules were not loaded by default.
So I decided to set the intertouch option in the grub config, and
after a swift reboot I verified if the intertouch value had indeed
changed:

The touchpad is now noticeably more responsive and less jittery, hence
this email to report my findings.

As for the details:

- Hardware: Lenovo Thinkpad X390
- Distribution: Gentoo
- Kernel: 6.17.4
- Touchpad:

I: Bus=001d Vendor=06cb Product=0000 Version=0000
  N: Name="Synaptics TM3469-010"
  P: Phys=rmi4-00/input0
  S: Sysfs=/devices/pci0000:00/0000:00:1f.4/i2c-7/7-002c/rmi4-00/input/input13
  U: Uniq=
  H: Handlers=mouse2 event7
  B: PROP=5
  B: EV=b
  B: KEY=e420 10000 0 0 0 0
  B: ABS=6f3800001000003
...
  P: /devices/pci0000:00/0000:00:1f.4/i2c-7/7-002c/rmi4-00/input/input13
  M: input13
  R: 13
  U: input
  E: DEVPATH=/devices/pci0000:00/0000:00:1f.4/i2c-7/7-002c/rmi4-00/input/input13
  E: SUBSYSTEM=input
  E: PRODUCT=1d/6cb/0/0
  E: NAME="Synaptics TM3469-010"
  E: PHYS="rmi4-00/input0"
  E: PROP=5
  E: EV=b
  E: KEY=e420 10000 0 0 0 0
  E: ABS=6f3800001000003
  E: MODALIAS=input:b001Dv06CBp0000e0000-e0,1,3,k110,145,14A,14D,14E,14F,ra0,1,18,2F,30,31,34,35,36,37,39,3A,mlsfw
  E: USEC_INITIALIZED=3357289
  E: ID_INPUT=1
  E: ID_INPUT_TOUCHPAD=1
  E: ID_BUS=rmi
  E: ID_SERIAL=noserial
  E: ID_PATH=pci-0000:00:1f.4
  E: ID_PATH_TAG=pci-0000_00_1f_4
  E: ID_FOR_SEAT=input-pci-0000_00_1f_4
  E: TAGS=:seat:
  E: CURRENT_TAGS=:seat:
...
66: PS/2 00.0: 10500 PS/2 Mouse
  [Created at input.249]
  Unique ID: AH6Q.TdKqTm3XMU4
  Hardware Class: mouse
  Model: "Synaptics TM3469-010"
  Vendor: 0x06cb
  Device: "Synaptics TM3469-010"
  Compatible to: int 0x0210 0x0001
  Device File: /dev/input/mice (/dev/input/mouse2)
  Device Files: /dev/input/mice, /dev/input/mouse2, /dev/input/event7,
/dev/input/by-path/pci-0000:00:1f.4-event-mouse,
/dev/input/by-path/pci-0000:00:1f.4-mouse
  Device Number: char 13:63 (char 13:34)
  Driver Info #0:
    Buttons: 1
    Wheels: 0
    XFree86 Protocol: explorerps/2
    GPM Protocol: exps2
  Config Status: cfg=new, avail=yes, need=no, active=unknown
...

If there is more information required, feel free to ask, I am happy to help!


Kind regards,

Luis

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Denis Benato @ 2025-10-23 20:04 UTC (permalink / raw)
  To: Antheas Kapenekakis
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <CAGwozwG+gf09PQf9o9YkKFYVgVn-1w5CDVrpOe4uFavVYCNijQ@mail.gmail.com>


On 10/23/25 20:06, Antheas Kapenekakis wrote:
> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
>>> Currently, RGB initialization forks depending on whether a device is
>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
>>> endpoints, and non-NKEY devices with 0x5a and then a
>>> backlight check, which is omitted for NKEY devices.
>>>
>>> Remove the fork, using a common initialization sequence for both,
>>> where they are both only initialized with 0x5a, then checked for
>>> backlight support. This patch should not affect existing functionality.
>>>
>>> 0x5d and 0x5e endpoint initializations are performed by Windows
>>> userspace programs associated with different usages that reside under
>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
>>> Neither is used currently in the driver.
>> What benefits do we get from removing the unused initialization?
>>
>> If this has never caused any troubles I don't see the reason for removing
>> them. Moreover the lighting protocol is known and I might as well add
>> support for it in the near future,
> I already have a patch that adds RGB and delay inits that endpoint. It
> got removed to make this easier to merge. See [1].
>
> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
I have to main concerns about this:

1. taking away initialization commands in one patchset to make it
easier to merge another unrelated patch doesn't seem the right thing
to do if the other patch it's not in the same series.

I can see [1] has been removed from the set for a later moment in time,
it's fine if it needs more work, just send something that function in the
same way and do not remove initialization commands when unnecessary,
especially since there will be for sure future development.

2. Your patchset resolves around keyboard backlight control and how
the keyboard device is exposed to userspace: it's fine but I do not see
the point in removing initialization commands that has nothing to do
with the issue we are trying to solve here.

Please leave 0x5E and 0x5D initialization commands where they are now.

>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>> ---
>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
>>>  1 file changed, 19 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>> index a444d41e53b6..7ea1037c3979 100644
>>> --- a/drivers/hid/hid-asus.c
>>> +++ b/drivers/hid/hid-asus.c
>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>       unsigned char kbd_func;
>>>       int ret;
>>>
>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>> -             /* Initialize keyboard */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> -
>>> -             /* The LED endpoint is initialised in two HID */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> -
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> -
>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>> -                     ret = asus_kbd_disable_oobe(hdev);
>>> -                     if (ret < 0)
>>> -                             return ret;
>>> -             }
>>> -
>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>> -                     intf = to_usb_interface(hdev->dev.parent);
>>> -                     udev = interface_to_usbdev(intf);
>>> -                     validate_mcu_fw_version(hdev,
>>> -                             le16_to_cpu(udev->descriptor.idProduct));
>>> -             }
>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>> +     if (ret < 0)
>>> +             return ret;
>>>
>>> -     } else {
>>> -             /* Initialize keyboard */
>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>> -             if (ret < 0)
>>> -                     return ret;
>>> +     /* Get keyboard functions */
>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>> +     if (ret < 0)
>>> +             return ret;
>>>
>>> -             /* Get keyboard functions */
>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>> +             ret = asus_kbd_disable_oobe(hdev);
>>>               if (ret < 0)
>>>                       return ret;
>>> +     }
>>>
>>> -             /* Check for backlight support */
>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>> -                     return -ENODEV;
>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>> +             intf = to_usb_interface(hdev->dev.parent);
>>> +             udev = interface_to_usbdev(intf);
>>> +             validate_mcu_fw_version(
>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
>>>       }
>>>
>>> +     /* Check for backlight support */
>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>> +             return -ENODEV;
>>> +
>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>>                                             sizeof(struct asus_kbd_leds),
>>>                                             GFP_KERNEL);

^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox