Re: [PATCH 2/2] platform/x86: hp-wmi: Add support for Fn+P hotkey

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Sun, 3 Aug 2025, edip@medip.dev wrote:

> From: Edip Hazuri <edip@medip.dev>
> 
> Newer Hp Victus (And probably newer omen) e.g. Victus 16-s1011nt that i
> own have a Fn+P shortcut intended to use with omen gaming hub for

Thanks for the patch.

It's irrelevant to record in the changelog that you own one. Just 
state what it has. :-)

> changing the performance profile. see [1]

Why isn't the key made to cycle platform profiles then?

> > hp_wmi: Unknown event_id - 27 - 0x7

Add empty line here.

> Based on the dmesg warning. I found that the key have 27 as event_id and
> 0x7 as event_data (keycode). I implemented it in the driver and now
> libinput can capture the event.

Please write changelogs in imperative tone. Don't write 'I did x' style 
sentences.

> Tested On Victus 16-s1011nt (9Z791EA) with MB 8C9C

Tested on

Missing .

> [1]: https://jpcdn.it/img/adadf6c927ffeb75afd8038f95db400a.png
> 
> Signed-off-by: Edip Hazuri <edip@medip.dev>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index db5fdee2109..6467ca27db7 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -122,6 +122,7 @@ enum hp_wmi_event_ids {
>  	HPWMI_BATTERY_CHARGE_PERIOD	= 0x10,
>  	HPWMI_SANITIZATION_MODE		= 0x17,
>  	HPWMI_CAMERA_TOGGLE		= 0x1A,
> +	HPWMI_FN_P_HOTKEY		= 0x1B,
>  	HPWMI_OMEN_KEY			= 0x1D,
>  	HPWMI_SMART_EXPERIENCE_APP	= 0x21,
>  };
> @@ -280,6 +281,7 @@ struct bios_rfkill2_state {
>  static const struct key_entry hp_wmi_keymap[] = {
>  	{ KE_KEY, 0x02,    { KEY_BRIGHTNESSUP } },
>  	{ KE_KEY, 0x03,    { KEY_BRIGHTNESSDOWN } },
> +	{ KE_KEY, 0x07,    { KEY_FN_P } },
>  	{ KE_KEY, 0x270,   { KEY_MICMUTE } },
>  	{ KE_KEY, 0x20e6,  { KEY_PROG1 } },
>  	{ KE_KEY, 0x20e8,  { KEY_MEDIA } },
> @@ -981,6 +983,15 @@ static void hp_wmi_notify(union acpi_object *obj, void *context)
>  						key_code, 1, true))
>  			pr_info("Unknown key code - 0x%x\n", key_code);
>  		break;
> +	case HPWMI_FN_P_HOTKEY:
> +		key_code = hp_wmi_read_int(HPWMI_HOTKEY_QUERY);
> +		if (key_code < 0)
> +			break;
> +
> +		if (!sparse_keymap_report_event(hp_wmi_input_dev,
> +			key_code, 1, true))

This is misaligned.

> +			pr_info("Unknown key code - 0x%x\n", key_code);
> +		break;

The code looks 100% identical to that of in HPWMI_BEZEL_BUTTON. But please 
consider what I noted above about cycling platform profiles.

>  	case HPWMI_OMEN_KEY:
>  		if (event_data) /* Only should be true for HP Omen */
>  			key_code = event_data;
> 

-- 
 i.