Re: [PATCH v3 1/3] HID: asus: export asus_hid_fnlock_set() for direct fn-lock control

[Denis Benato]

On 7/27/26 13:09, Ilpo Järvinen wrote:
> On Thu, 7 May 2026, Marcus Grenängen wrote:
>
>> Some ASUS platforms cannot control fn-lock via WMI DEVS and must send a
>> HID feature report directly to the N-Key keyboard device instead.
>>
>> Add a module-level fnlock_hdev pointer (protected by a mutex) that is set
>> at probe time for devices with QUIRK_HID_FN_LOCK and cleared at remove.
>> Export asus_hid_fnlock_set(bool) so that asus-armoury can call into
>> hid-asus without a circular dependency.
>>
>> Signed-off-by: Marcus Grenängen <marcus@xxxxxxxxxxxx>
>> ---
>>  drivers/hid/hid-asus.c                     | 44 +++++++++++++++++++++-
>>  include/linux/platform_data/x86/asus-wmi.h | 15 ++++++++
>>  2 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index d34d74df3dc0..402ba9d5e982 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -584,6 +584,39 @@ static void asus_sync_fn_lock(struct work_struct *work)
>>  	asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
>>  }
>>  
>> +/*
>> + * Module-level reference to the HID device that handles fn-lock via feature
>> + * report. Set at probe and cleared at remove for QUIRK_HID_FN_LOCK devices.
>> + * Protected by fnlock_hdev_lock.
>> + */
>> +static DEFINE_MUTEX(fnlock_hdev_lock);
>> +static struct hid_device *fnlock_hdev;
>> +
>> +/**
>> + * asus_hid_fnlock_set() - Set fn-lock state directly via HID feature report.
>> + * @enabled: true to lock fn (F1-F12 primary), false to unlock.
>> + *
>> + * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
>> + * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
>> + *
>> + * Returns: 0 on success, -ENODEV if no fn-lock capable HID device is present.
>> + */
>> +int asus_hid_fnlock_set(bool enabled)
>> +{
>> +	int ret;
>> +
>> +	guard(mutex)(&fnlock_hdev_lock);
>> +	if (!fnlock_hdev)
>> +		return -ENODEV;
>> +
>> +	ret = asus_kbd_set_fn_lock(fnlock_hdev, enabled);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(asus_hid_fnlock_set);
>> +
>>  static void asus_schedule_work(struct asus_kbd_leds *led)
>>  {
>>  	unsigned long flags;
>> @@ -969,6 +1002,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>  		drvdata->fn_lock = true;
>>  		INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
>>  		asus_kbd_set_fn_lock(hdev, true);
>> +		guard(mutex)(&fnlock_hdev_lock);
>> +		fnlock_hdev = hdev;
>>  	}
>>  
>>  	if (drvdata->tp) {
>> @@ -1008,6 +1043,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>  		drvdata->fn_lock = true;
>>  		INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
>>  		asus_kbd_set_fn_lock(hdev, true);
>> +		guard(mutex)(&fnlock_hdev_lock);
>> +		fnlock_hdev = hdev;
> What's going on with this patch? Why this context appears more than once 
> on different lines???
Hi Ilpo,

I think this patch has been taken by HID already as I had to resolve another issue with it:
mutex usage in atomic context; you can read out more here:
https://lore.kernel.org/all/20260619001103.1189200-2-denis.benato@xxxxxxxxx/

The main idea as I understood it is that to activate the fn key, do things, and then
deactivate it: the problem in this logic comes from the fact that using true both time
is very likely an oversight and it should have been true -> false.

I requested clarification too but still haven't got any response yet.

Let me know if I can do something.

Thanks,
Denis
> I even wasted my time on checking what hid tree has queued, and came up 
> zero explanation (as expected).
>
> --
>  i.