[PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Helge Bahmann]

Add an input driver that interprets the "operation mode" sensor offered
by the amd sfh on some laptop models.

Add a quirk to make the driver work again with the Asus VivoBook
VivoBook (turn off the "disable interrupts" flag).

Expose the intr_disable flag as a module parameter in case it turns out
to be needed on further laptop models.

Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
---
 drivers/hid/amd-sfh-hid/amd_sfh_client.c | 25 ++++++++------
 drivers/hid/amd-sfh-hid/amd_sfh_hid.c    | 43 ++++++++++++++++++++++++
 drivers/hid/amd-sfh-hid/amd_sfh_hid.h    |  6 ++++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c   |  9 +++++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.h   |  3 ++
 5 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 7017bfa59093..a24757c5a203 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -128,10 +128,16 @@ void amd_sfh_work_buffer(struct work_struct *work)
 	guard(mutex)(&mp2->lock);
 	for (i = 0; i < cli_data->num_hid_devices; i++) {
 		if (cli_data->sensor_sts[i] == SENSOR_ENABLED) {
-			report_size = mp2->mp2_ops->get_in_rep(i, cli_data->sensor_idx[i],
-							       cli_data->report_id[i], in_data);
-			hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT,
-					 in_data->input_report[i], report_size, 0);
+			if (cli_data->hid_sensor_hubs[i]) {
+				report_size = mp2->mp2_ops->get_in_rep(i, cli_data->sensor_idx[i],
+								       cli_data->report_id[i],
+								       in_data);
+				hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT,
+						 in_data->input_report[i], report_size, 0);
+			} else if (cli_data->sensor_idx[i] == op_idx &&
+				   cli_data->modeswitch_input) {
+				amdtp_modeswitch_report(i, cli_data->modeswitch_input, in_data);
+			}
 		}
 	}
 	schedule_delayed_work(&cli_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP));
@@ -327,15 +333,12 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)

 	for (i = 0; i < cl_data->num_hid_devices; i++) {
 		cl_data->cur_hid_dev = i;
-		if (cl_data->sensor_idx[i] == op_idx) {
-			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
-				cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
-				cl_data->sensor_sts[i]);
-			continue;
-		}

 		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
-			rc = amdtp_hid_probe(i, cl_data);
+			if (cl_data->sensor_idx[i] != op_idx)
+				rc = amdtp_hid_probe(i, cl_data);
+			else
+				rc = amdtp_modeswitch_probe(i, cl_data);
 			if (rc)
 				goto cleanup;
 		} else {
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
index 81f3024b7b1b..44008c02b63c 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
@@ -181,4 +181,47 @@ void amdtp_hid_remove(struct amdtp_cl_data *cli_data)
 			cli_data->hid_sensor_hubs[i] = NULL;
 		}
 	}
+
+	/* note: cli_data->modeswitch_input implicitly cleaned by devres */
+}
+
+int amdtp_modeswitch_probe(u32 cur_hid_dev, struct amdtp_cl_data *cli_data)
+{
+	struct amd_mp2_dev *mp2 = container_of(cli_data->in_data, struct amd_mp2_dev, in_data);
+	struct device *dev = &mp2->pdev->dev;
+	struct input_dev *input;
+	int rc;
+
+	input = devm_input_allocate_device(dev);
+	if (IS_ERR(input))
+		return PTR_ERR(input);
+
+	input->name = "AMD SFH tablet mode switch sensor";
+	input->id.bustype = BUS_PCI;
+
+	input_set_capability(input, EV_SW, SW_TABLET_MODE);
+
+	rc = input_register_device(input);
+	if (rc)
+		goto cleanup;
+
+	cli_data->modeswitch_input = input;
+
+	return 0;
+
+cleanup:
+	return rc;
+}
+
+void amdtp_modeswitch_report(u32 index, struct input_dev *input, struct amd_input_data *in_data)
+{
+	u32 *sensor_virt_addr = in_data->sensor_virt_addr[index];
+	u32 value = sensor_virt_addr[0];
+
+	if (value == AMD_SFH_OP_IDX_MODE_TABLET)
+		input_report_switch(input, SW_TABLET_MODE, 1);
+	else
+		input_report_switch(input, SW_TABLET_MODE, 0);
+
+	input_sync(input);
 }
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
index 7452b0302953..20aff7b75fbd 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
@@ -11,6 +11,8 @@
 #ifndef AMDSFH_HID_H
 #define AMDSFH_HID_H

+#include <linux/input.h>
+
 #define MAX_HID_DEVICES		7
 #define AMD_SFH_HID_VENDOR	0x1022
 #define AMD_SFH_HID_PRODUCT	0x0001
@@ -50,6 +52,7 @@ struct amdtp_cl_data {
 	u8 sensor_idx[MAX_HID_DEVICES];
 	u8 *feature_report[MAX_HID_DEVICES];
 	u8 request_done[MAX_HID_DEVICES];
+	struct input_dev *modeswitch_input;
 	struct amd_input_data *in_data;
 	struct delayed_work work;
 	struct delayed_work work_buffer;
@@ -78,4 +81,7 @@ void amdtp_hid_remove(struct amdtp_cl_data *cli_data);
 int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type);
 void amd_sfh_set_report(struct hid_device *hid, int report_id, int report_type);
 void amdtp_hid_wakeup(struct hid_device *hid);
+int amdtp_modeswitch_probe(u32 cur_hid_dev, struct amdtp_cl_data *cli_data);
+void amdtp_modeswitch_report(u32 index, struct input_dev *input, struct amd_input_data *in_data);
+
 #endif
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 1d9f955573aa..cd9cff75f114 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -39,6 +39,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
 MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");

 static bool intr_disable = true;
+module_param_named(intr_disable, intr_disable, bool, 0444);
+MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");

 static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts)
 {
@@ -317,6 +319,13 @@ static const struct dmi_system_id dmi_sfh_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook x360 435 G7"),
 		},
 	},
+	{
+		.callback = mp2_disable_intr,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "VivoBook_ASUSLaptop TP420UA_TM420UA"),
+		}
+	},
 	{}
 };

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
index 2eb61f4e8434..5e968894ebe4 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
@@ -83,6 +83,9 @@ enum sensor_idx {
 	als_idx = 19
 };

+#define AMD_SFH_OP_IDX_MODE_LAPTOP 1
+#define AMD_SFH_OP_IDX_MODE_TABLET 3
+
 enum mem_use_type {
 	USE_DRAM,
 	USE_C2P_REG,
--
2.47.3

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Jiri Kosina]

On Mon, 27 Apr 2026, Helge Bahmann wrote:

> Add an input driver that interprets the "operation mode" sensor offered
> by the amd sfh on some laptop models.
> 
> Add a quirk to make the driver work again with the Asus VivoBook
> VivoBook (turn off the "disable interrupts" flag).
> 
> Expose the intr_disable flag as a module parameter in case it turns out
> to be needed on further laptop models.
> 
> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>

Basavaraj, can you please review this one?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Basavaraj Natikar]

On 5/12/2026 9:36 PM, Jiri Kosina wrote:
> On Mon, 27 Apr 2026, Helge Bahmann wrote:
>
>> Add an input driver that interprets the "operation mode" sensor offered
>> by the amd sfh on some laptop models.
>>
>> Add a quirk to make the driver work again with the Asus VivoBook
>> VivoBook (turn off the "disable interrupts" flag).
>>
>> Expose the intr_disable flag as a module parameter in case it turns out
>> to be needed on further laptop models.
>>
>> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
> Basavaraj, can you please review this one?

Hi Jiri,

Sure, will review. The patch has some changes that deviate from how
other sensors are exposed in this driver, so it will take some time to
review properly. I'll respond with detailed feedback once done.

Thanks,
--
Basavaraj

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Helge Bahmann]

On Tue, 12 May 2026, Jiri Kosina wrote:
> On Mon, 27 Apr 2026, Helge Bahmann wrote:
> 
> > Add an input driver that interprets the "operation mode" sensor offered
> > by the amd sfh on some laptop models.
> > 
> > Add a quirk to make the driver work again with the Asus VivoBook
> > VivoBook (turn off the "disable interrupts" flag).
> > 
> > Expose the intr_disable flag as a module parameter in case it turns out
> > to be needed on further laptop models.
> > 
> > Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
> 
> Basavaraj, can you please review this one?

Some additional context, maybe helpful for review:

1. The numbers and behavior were extracted from the ACPI tables
(WMI driver of sorts) of the notebook; I don't have access to any 
official AMD / ASUS docs or similar.

2. I have an alternate version of this change that is more indirect:
- create a HID driver providing an "abstract table mode" message
- have an input driver attaching to this newly defined HID driver

While that is keeping "more in line" with the current driver
architecture, I am not sure this indirection really helps. Particularly,
there is no "canonical" HID tablet mode switch message defined,
so it all remains completely bespoke. I am happy to change it if 
you prefer, but would need your input.

3. Since this is based on Asus VivoBook and its ACPI tables,
there is a possibility that this "op sensor / tablet mode" behavior
is not as universal as I surmise. A point could be made to make this
entire behavior model-dependent (with a mod param to override
/ activate for other models). Happy to take input / advice.

Thanks for your work!
Helge

> 
> Thanks,
> 
> 

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Jiri Kosina]

On Mon, 27 Apr 2026, Helge Bahmann wrote:

> Add an input driver that interprets the "operation mode" sensor offered
> by the amd sfh on some laptop models.
> 
> Add a quirk to make the driver work again with the Asus VivoBook
> VivoBook (turn off the "disable interrupts" flag).
> 
> Expose the intr_disable flag as a module parameter in case it turns out
> to be needed on further laptop models.
> 
> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>

Basavaraj, could you please review this one? Thanks.

> ---
>  drivers/hid/amd-sfh-hid/amd_sfh_client.c | 25 ++++++++------
>  drivers/hid/amd-sfh-hid/amd_sfh_hid.c    | 43 ++++++++++++++++++++++++
>  drivers/hid/amd-sfh-hid/amd_sfh_hid.h    |  6 ++++
>  drivers/hid/amd-sfh-hid/amd_sfh_pcie.c   |  9 +++++
>  drivers/hid/amd-sfh-hid/amd_sfh_pcie.h   |  3 ++
>  5 files changed, 75 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 7017bfa59093..a24757c5a203 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> @@ -128,10 +128,16 @@ void amd_sfh_work_buffer(struct work_struct *work)
>  	guard(mutex)(&mp2->lock);
>  	for (i = 0; i < cli_data->num_hid_devices; i++) {
>  		if (cli_data->sensor_sts[i] == SENSOR_ENABLED) {
> -			report_size = mp2->mp2_ops->get_in_rep(i, cli_data->sensor_idx[i],
> -							       cli_data->report_id[i], in_data);
> -			hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT,
> -					 in_data->input_report[i], report_size, 0);
> +			if (cli_data->hid_sensor_hubs[i]) {
> +				report_size = mp2->mp2_ops->get_in_rep(i, cli_data->sensor_idx[i],
> +								       cli_data->report_id[i],
> +								       in_data);
> +				hid_input_report(cli_data->hid_sensor_hubs[i], HID_INPUT_REPORT,
> +						 in_data->input_report[i], report_size, 0);
> +			} else if (cli_data->sensor_idx[i] == op_idx &&
> +				   cli_data->modeswitch_input) {
> +				amdtp_modeswitch_report(i, cli_data->modeswitch_input, in_data);
> +			}
>  		}
>  	}
>  	schedule_delayed_work(&cli_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP));
> @@ -327,15 +333,12 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
> 
>  	for (i = 0; i < cl_data->num_hid_devices; i++) {
>  		cl_data->cur_hid_dev = i;
> -		if (cl_data->sensor_idx[i] == op_idx) {
> -			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> -				cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
> -				cl_data->sensor_sts[i]);
> -			continue;
> -		}
> 
>  		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> -			rc = amdtp_hid_probe(i, cl_data);
> +			if (cl_data->sensor_idx[i] != op_idx)
> +				rc = amdtp_hid_probe(i, cl_data);
> +			else
> +				rc = amdtp_modeswitch_probe(i, cl_data);
>  			if (rc)
>  				goto cleanup;
>  		} else {
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> index 81f3024b7b1b..44008c02b63c 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> @@ -181,4 +181,47 @@ void amdtp_hid_remove(struct amdtp_cl_data *cli_data)
>  			cli_data->hid_sensor_hubs[i] = NULL;
>  		}
>  	}
> +
> +	/* note: cli_data->modeswitch_input implicitly cleaned by devres */
> +}
> +
> +int amdtp_modeswitch_probe(u32 cur_hid_dev, struct amdtp_cl_data *cli_data)
> +{
> +	struct amd_mp2_dev *mp2 = container_of(cli_data->in_data, struct amd_mp2_dev, in_data);
> +	struct device *dev = &mp2->pdev->dev;
> +	struct input_dev *input;
> +	int rc;
> +
> +	input = devm_input_allocate_device(dev);
> +	if (IS_ERR(input))
> +		return PTR_ERR(input);
> +
> +	input->name = "AMD SFH tablet mode switch sensor";
> +	input->id.bustype = BUS_PCI;
> +
> +	input_set_capability(input, EV_SW, SW_TABLET_MODE);
> +
> +	rc = input_register_device(input);
> +	if (rc)
> +		goto cleanup;
> +
> +	cli_data->modeswitch_input = input;
> +
> +	return 0;
> +
> +cleanup:
> +	return rc;
> +}
> +
> +void amdtp_modeswitch_report(u32 index, struct input_dev *input, struct amd_input_data *in_data)
> +{
> +	u32 *sensor_virt_addr = in_data->sensor_virt_addr[index];
> +	u32 value = sensor_virt_addr[0];
> +
> +	if (value == AMD_SFH_OP_IDX_MODE_TABLET)
> +		input_report_switch(input, SW_TABLET_MODE, 1);
> +	else
> +		input_report_switch(input, SW_TABLET_MODE, 0);
> +
> +	input_sync(input);
>  }
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> index 7452b0302953..20aff7b75fbd 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> @@ -11,6 +11,8 @@
>  #ifndef AMDSFH_HID_H
>  #define AMDSFH_HID_H
> 
> +#include <linux/input.h>
> +
>  #define MAX_HID_DEVICES		7
>  #define AMD_SFH_HID_VENDOR	0x1022
>  #define AMD_SFH_HID_PRODUCT	0x0001
> @@ -50,6 +52,7 @@ struct amdtp_cl_data {
>  	u8 sensor_idx[MAX_HID_DEVICES];
>  	u8 *feature_report[MAX_HID_DEVICES];
>  	u8 request_done[MAX_HID_DEVICES];
> +	struct input_dev *modeswitch_input;
>  	struct amd_input_data *in_data;
>  	struct delayed_work work;
>  	struct delayed_work work_buffer;
> @@ -78,4 +81,7 @@ void amdtp_hid_remove(struct amdtp_cl_data *cli_data);
>  int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type);
>  void amd_sfh_set_report(struct hid_device *hid, int report_id, int report_type);
>  void amdtp_hid_wakeup(struct hid_device *hid);
> +int amdtp_modeswitch_probe(u32 cur_hid_dev, struct amdtp_cl_data *cli_data);
> +void amdtp_modeswitch_report(u32 index, struct input_dev *input, struct amd_input_data *in_data);
> +
>  #endif
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 1d9f955573aa..cd9cff75f114 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -39,6 +39,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
>  MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
> 
>  static bool intr_disable = true;
> +module_param_named(intr_disable, intr_disable, bool, 0444);
> +MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");
> 
>  static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts)
>  {
> @@ -317,6 +319,13 @@ static const struct dmi_system_id dmi_sfh_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook x360 435 G7"),
>  		},
>  	},
> +	{
> +		.callback = mp2_disable_intr,
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "VivoBook_ASUSLaptop TP420UA_TM420UA"),
> +		}
> +	},
>  	{}
>  };
> 
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
> index 2eb61f4e8434..5e968894ebe4 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
> @@ -83,6 +83,9 @@ enum sensor_idx {
>  	als_idx = 19
>  };
> 
> +#define AMD_SFH_OP_IDX_MODE_LAPTOP 1
> +#define AMD_SFH_OP_IDX_MODE_TABLET 3
> +
>  enum mem_use_type {
>  	USE_DRAM,
>  	USE_C2P_REG,
> --
> 2.47.3
> 
> 
> 
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Basavaraj Natikar]

On 5/14/2026 1:29 PM, Helge Bahmann wrote:

> [You don't often get email from hcb@chaoticmind.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, 12 May 2026, Jiri Kosina wrote:
>> On Mon, 27 Apr 2026, Helge Bahmann wrote:
>>
>>> Add an input driver that interprets the "operation mode" sensor offered
>>> by the amd sfh on some laptop models.
>>>
>>> Add a quirk to make the driver work again with the Asus VivoBook
>>> VivoBook (turn off the "disable interrupts" flag).
>>>
>>> Expose the intr_disable flag as a module parameter in case it turns out
>>> to be needed on further laptop models.
>>>
>>> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
>> Basavaraj, can you please review this one?
> Some additional context, maybe helpful for review:
>
> 1. The numbers and behavior were extracted from the ACPI tables
> (WMI driver of sorts) of the notebook; I don't have access to any
> official AMD / ASUS docs or similar.
>
> 2. I have an alternate version of this change that is more indirect:
> - create a HID driver providing an "abstract table mode" message
> - have an input driver attaching to this newly defined HID driver
>
> While that is keeping "more in line" with the current driver
> architecture, I am not sure this indirection really helps. Particularly,
> there is no "canonical" HID tablet mode switch message defined,
> so it all remains completely bespoke. I am happy to change it if
> you prefer, but would need your input.
>
> 3. Since this is based on Asus VivoBook and its ACPI tables,
> there is a possibility that this "op sensor / tablet mode" behavior
> is not as universal as I surmise. A point could be made to make this
> entire behavior model-dependent (with a mod param to override
> / activate for other models). Happy to take input / advice.

Thanks Helge.

I'd like to go with the dedicated input-driver approach (your option with
a standalone input driver) rather than the HID-message indirection -- it
keeps clean subsystem boundaries.

For splitting the work, either of these works for me -- whichever you
prefer:
   Option 1: I create the new input driver
   drivers/input/misc/amd_sfh_tabletmode.c and once done we will review your ASUS VivoBook
   quirk + intr_disable module-param patches on top of it.
   
   Option 2: If you'd rather keep ownership of it, you write
   drivers/input/misc/amd_sfh_tabletmode.c consuming the mode exposed by
   amd-sfh, and I'll review and help.
   

Let me know which option you'd like and I'll proceed.

Thanks,
--
Basavaraj

Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk

[Helge Bahmann]

Am Mittwoch, 10. Juni 2026, 19:12:37 Mitteleuropäische Sommerzeit schrieb Basavaraj Natikar:
> 
> On 5/14/2026 1:29 PM, Helge Bahmann wrote:
> 
> > [You don't often get email from hcb@chaoticmind.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Tue, 12 May 2026, Jiri Kosina wrote:
> >> On Mon, 27 Apr 2026, Helge Bahmann wrote:
> >>
> >>> Add an input driver that interprets the "operation mode" sensor offered
> >>> by the amd sfh on some laptop models.
> >>>
> >>> Add a quirk to make the driver work again with the Asus VivoBook
> >>> VivoBook (turn off the "disable interrupts" flag).
> >>>
> >>> Expose the intr_disable flag as a module parameter in case it turns out
> >>> to be needed on further laptop models.
> >>>
> >>> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
> >> Basavaraj, can you please review this one?
> > Some additional context, maybe helpful for review:
> >
> > 1. The numbers and behavior were extracted from the ACPI tables
> > (WMI driver of sorts) of the notebook; I don't have access to any
> > official AMD / ASUS docs or similar.
> >
> > 2. I have an alternate version of this change that is more indirect:
> > - create a HID driver providing an "abstract table mode" message
> > - have an input driver attaching to this newly defined HID driver
> >
> > While that is keeping "more in line" with the current driver
> > architecture, I am not sure this indirection really helps. Particularly,
> > there is no "canonical" HID tablet mode switch message defined,
> > so it all remains completely bespoke. I am happy to change it if
> > you prefer, but would need your input.
> >
> > 3. Since this is based on Asus VivoBook and its ACPI tables,
> > there is a possibility that this "op sensor / tablet mode" behavior
> > is not as universal as I surmise. A point could be made to make this
> > entire behavior model-dependent (with a mod param to override
> > / activate for other models). Happy to take input / advice.
> 
> Thanks Helge.
> 
> I'd like to go with the dedicated input-driver approach (your option with
> a standalone input driver) rather than the HID-message indirection -- it
> keeps clean subsystem boundaries.

Hello Basavaraj!

Thanks for your architecture input, that sounds good. Let me briefly
clean up the patches and I will send you what I have. I will also
include the input-tabletmode driver that I have, but you will likely
want to change it (use correct protocol message as I am unsure
what would be "correct" here). You can then proceed as you like
(take the driver, rewrite it, ...) -- I will leave ownership decision
entirely up to you, I care more that you are happy with the
overall structure than that.

Thank you
Helge

> 
> For splitting the work, either of these works for me -- whichever you
> prefer:
>    Option 1: I create the new input driver
>    drivers/input/misc/amd_sfh_tabletmode.c and once done we will review your ASUS VivoBook
>    quirk + intr_disable module-param patches on top of it.
>    
>    Option 2: If you'd rather keep ownership of it, you write
>    drivers/input/misc/amd_sfh_tabletmode.c consuming the mode exposed by
>    amd-sfh, and I'll review and help.
>    
> 
> Let me know which option you'd like and I'll proceed.
> 
> Thanks,
> --
> Basavaraj
> 
> 
> 

[PATCH 0/2] asus vivobook tablet mode

[Helge Bahmann]

Hi Basavaraj,

Here the two patches implementing the split hid/input
driver for the tablet mode support. I mulled over what 
exactly to use as report descriptor in the hid driver, and
there is no clean and easy solution -- I hope this is what
you had in mind? I could not figure a way how to do
a stand-alone input driver without doing any hid indirection,
as I simply could not find a way to bind to the physical
dev twice.

Please let me know in comments to the patches how you
would like to proceed.

Thanks!
Helge

[PATCH 1/2] amd-sfh-hid: tablet mode hid report and asus quirk

[Helge Bahmann]

Add an input driver that interprets the "operation mode" sensor offered
by the amd sfh as a hid driver to generate a custom hid report (tablet
mode switch).

Add a quirk to restore compatibility of the driver with the Asus
VivoBook (turn off the "disable interrupts flag). Expose the
intr_disable flag as a module parameter in case it turns out to be
needed on further laptop models.

Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
---
 drivers/hid/amd-sfh-hid/amd_sfh_client.c      | 19 ------------
 drivers/hid/amd-sfh-hid/amd_sfh_hid.h         |  1 +
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  9 ++++++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.h        |  3 ++
 .../hid_descriptor/amd_sfh_hid_desc.c         | 31 +++++++++++++++++++
 .../hid_descriptor/amd_sfh_hid_desc.h         |  9 ++++++
 .../hid_descriptor/amd_sfh_hid_report_desc.h  | 18 +++++++++++
 7 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 7017bfa59093..57ff708ca39b 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -254,19 +254,6 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
 			goto cleanup;
 		}
 
-		if (cl_data->sensor_idx[i] == op_idx) {
-			info.period = AMD_SFH_IDLE_LOOP;
-			info.sensor_idx = cl_data->sensor_idx[i];
-			info.dma_address = cl_data->sensor_dma_addr[i];
-			mp2_ops->start(privdata, info);
-			cl_data->sensor_sts[i] = amd_sfh_wait_for_response(privdata,
-									   cl_data->sensor_idx[i],
-									   SENSOR_ENABLED);
-			if (cl_data->sensor_sts[i] == SENSOR_ENABLED)
-				cl_data->is_any_sensor_enabled = true;
-			continue;
-		}
-
 		cl_data->sensor_sts[i] = SENSOR_DISABLED;
 		cl_data->sensor_requested_cnt[i] = 0;
 		cl_data->cur_hid_dev = i;
@@ -327,12 +314,6 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
 
 	for (i = 0; i < cl_data->num_hid_devices; i++) {
 		cl_data->cur_hid_dev = i;
-		if (cl_data->sensor_idx[i] == op_idx) {
-			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
-				cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
-				cl_data->sensor_sts[i]);
-			continue;
-		}
 
 		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
 			rc = amdtp_hid_probe(i, cl_data);
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
index 7452b0302953..c1b62e5df2ff 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
@@ -78,4 +78,5 @@ void amdtp_hid_remove(struct amdtp_cl_data *cli_data);
 int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type);
 void amd_sfh_set_report(struct hid_device *hid, int report_id, int report_type);
 void amdtp_hid_wakeup(struct hid_device *hid);
+
 #endif
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 1d9f955573aa..cd9cff75f114 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -39,6 +39,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
 MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
 
 static bool intr_disable = true;
+module_param_named(intr_disable, intr_disable, bool, 0444);
+MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");
 
 static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts)
 {
@@ -317,6 +319,13 @@ static const struct dmi_system_id dmi_sfh_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook x360 435 G7"),
 		},
 	},
+	{
+		.callback = mp2_disable_intr,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "VivoBook_ASUSLaptop TP420UA_TM420UA"),
+		}
+	},
 	{}
 };
 
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
index 2eb61f4e8434..5e968894ebe4 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
@@ -83,6 +83,9 @@ enum sensor_idx {
 	als_idx = 19
 };
 
+#define AMD_SFH_OP_IDX_MODE_LAPTOP 1
+#define AMD_SFH_OP_IDX_MODE_TABLET 3
+
 enum mem_use_type {
 	USE_DRAM,
 	USE_C2P_REG,
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index ef1f9be8b893..7e807b92d296 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -42,6 +42,11 @@ static int get_report_descriptor(int sensor_idx, u8 *rep_desc)
 		memcpy(rep_desc, gyro3_report_descriptor,
 		       sizeof(gyro3_report_descriptor));
 		break;
+	case op_idx: /* op */
+		memset(rep_desc, 0, sizeof(tablet_mode_report_descriptor));
+		memcpy(rep_desc, tablet_mode_report_descriptor,
+		       sizeof(tablet_mode_report_descriptor));
+		break;
 	case mag_idx: /* Magnetometer */
 		memset(rep_desc, 0, sizeof(comp3_report_descriptor));
 		memcpy(rep_desc, comp3_report_descriptor,
@@ -87,6 +92,16 @@ static u32 get_descr_sz(int sensor_idx, int descriptor_name)
 			return sizeof(struct gyro_feature_report);
 		}
 		break;
+	case op_idx:
+		switch (descriptor_name) {
+		case descr_size:
+			return sizeof(tablet_mode_report_descriptor);
+		case input_size:
+			return sizeof(struct tablet_mode_input_report);
+		case feature_size:
+			return sizeof(struct tablet_mode_feature_report);
+		}
+		break;
 	case mag_idx:
 		switch (descriptor_name) {
 		case descr_size:
@@ -139,6 +154,7 @@ static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
 {
 	struct accel3_feature_report acc_feature;
 	struct gyro_feature_report gyro_feature;
+	struct tablet_mode_feature_report tablet_mode_feature;
 	struct magno_feature_report magno_feature;
 	struct hpd_feature_report hpd_feature;
 	struct als_feature_report als_feature;
@@ -164,6 +180,11 @@ static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
 		memcpy(feature_report, &gyro_feature, sizeof(gyro_feature));
 		report_size = sizeof(gyro_feature);
 		break;
+	case op_idx: /* op */
+		get_common_features(&tablet_mode_feature.common_property, report_id);
+		memcpy(feature_report, &tablet_mode_feature, sizeof(tablet_mode_feature));
+		report_size = sizeof(tablet_mode_feature);
+		break;
 	case mag_idx: /* Magnetometer */
 		get_common_features(&magno_feature.common_property, report_id);
 		magno_feature.magno_headingchange_sensitivity = HID_DEFAULT_SENSITIVITY;
@@ -212,6 +233,7 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 	u8 supported_input = privdata->mp2_acs & GENMASK(3, 0);
 	struct magno_input_report magno_input;
 	struct accel3_input_report acc_input;
+	struct tablet_mode_input_report tablet_mode_input;
 	struct gyro_input_report gyro_input;
 	struct hpd_input_report hpd_input;
 	struct als_input_report als_input;
@@ -238,6 +260,15 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 		memcpy(input_report, &gyro_input, sizeof(gyro_input));
 		report_size = sizeof(gyro_input);
 		break;
+	case op_idx: /* op */
+		tablet_mode_input.report_id = 0x11;
+		if (sensor_virt_addr[0] == 3)
+			tablet_mode_input.tablet_state = 1;
+		else
+			tablet_mode_input.tablet_state = 0;
+		memcpy(input_report, &tablet_mode_input, sizeof(tablet_mode_input));
+		report_size = sizeof(tablet_mode_input);
+		break;
 	case mag_idx: /* Magnetometer */
 		get_common_inputs(&magno_input.common_property, report_id);
 		magno_input.in_magno_x = (int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER;
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index 882434b1501f..ff82e47eb3ca 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -114,4 +114,13 @@ struct hpd_input_report {
 	u8 human_presence;
 } __packed;
 
+struct tablet_mode_feature_report {
+	struct common_feature_property common_property;
+} __packed;
+
+struct tablet_mode_input_report {
+	u8 report_id;    /* Muss 0x11 (17) sein */
+	u8 tablet_state; /* Bit 0: Status, Bits 1-7: Padding */
+} __packed;
+
 #endif
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 67ec2d6a417d..f95368534fc6 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -776,4 +776,22 @@ static const u8 hpd_report_descriptor[] = {
 0X81, 0x02,		 /* HID Input (Data_Var_Abs) */
 0xC0			 /* HID end collection */
 };
+
+/* tablet mode switch */
+static const u8 tablet_mode_report_descriptor[] = {
+0x05, 0x0C,		/* HID usage consumer electronics */
+0x09, 0x01,		/* HID usage */
+0xA1, 0x00,		/* HID collection (Physical) */
+0x85, 0x11,		/* HID report id */
+0x0A, 0xFF, 0x02,	/* HID usage (unallocated -- (ab)used for tablet mode) */
+0x15, 0x00,		/* HID logical Min_8(0) */
+0x25, 0x01,		/* HID logical Max_8(1) */
+0x75, 0x01,		/* HID report size(1) */
+0x95, 0x01,		/* HID report count (1) */
+0X81, 0x02,		/* HID input (Data_Var_Abs) */
+0x75, 0x01,		/* HID report size (1) */
+0x95, 0x07,		/* HID report count (7) */
+0x81, 0x03,		/* HID input (Data_Var_Abs) */
+0xC0			/* HID end collection */
+};
 #endif
-- 
2.47.3

[PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report

[Helge Bahmann]

Add a driver that hooks into the tablet mode hid reports
generated by the amd-sfh-hid driver. This enables tablet
mode switching on e.g. Asus Vivobook devices.

Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
---
 drivers/input/misc/Kconfig              | 11 ++++
 drivers/input/misc/Makefile             |  1 +
 drivers/input/misc/amd_sfh_tabletmode.c | 88 +++++++++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100644 drivers/input/misc/amd_sfh_tabletmode.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 94a753fcb64f..5a1dacd6f7b9 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -1003,4 +1003,15 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.
 
+config INPUT_AMD_SFH_TABLETMODE
+	tristate "AMD sensor fusion hub tablet mode driver"
+	help
+	  Say Y to enable support of the tablet mode switch driver for
+	  convertible laptops based on the AMD sensor fusion hub. This
+	  driver is known to work on Asus VivoBook but will likely
+	  work on other laptops featuring the same chipset.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called amd_sfh_tabletmode.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 415fc4e2918b..3f5abbe39f3a 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
 obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND)	+= xen-kbdfront.o
 obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
 obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR)	+= ideapad_slidebar.o
+obj-$(CONFIG_INPUT_AMD_SFH_TABLETMODE)	+= amd_sfh_tabletmode.o
diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
new file mode 100644
index 000000000000..ee1207372207
--- /dev/null
+++ b/drivers/input/misc/amd_sfh_tabletmode.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD MP2 PCIe tablet mode hid input driver
+ * Copyright 2026 Helge Bahmann Advanced Micro Devices, Inc.
+ * Authors: Helge Bahmann <hcb@chaoticmind.net>
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#define AMD_SFH_HID_VENDOR        0x1022
+#define AMD_SFH_HID_PRODUCT       0x0001
+
+static int amd_sfh_tabletmode_probe(struct hid_device *hdev,
+				    const struct hid_device_id *id)
+{
+	int error = 0;
+
+	error = hid_parse(hdev);
+	if (error)
+		return error;
+
+	error = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static void amd_sfh_tabletmode_remove(struct hid_device *hdev)
+{
+	hid_hw_stop(hdev);
+}
+
+static int amd_sfh_tabletmode_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+					    struct hid_field *field, struct hid_usage *usage,
+					    unsigned long **bit, int *max)
+{
+	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_CONSUMER && (usage->hid & HID_USAGE) == 0x02ff) {
+		input_set_capability(hi->input, EV_SW, SW_TABLET_MODE);
+		usage->type = EV_SW;
+		usage->code = SW_TABLET_MODE;
+		return 1;
+	}
+	return 0;
+}
+
+static int amd_sfh_tabletmode_event(struct hid_device *hid, struct hid_field *field,
+				    struct hid_usage *usage, __s32 value)
+{
+	input_report_switch(field->hidinput->input, SW_TABLET_MODE, value);
+
+	return 0;
+}
+
+static const struct hid_device_id amd_sfh_tabletmode_devices[] = {
+	{ HID_DEVICE(BUS_AMD_SFH, HID_GROUP_GENERIC, AMD_SFH_HID_VENDOR, AMD_SFH_HID_PRODUCT) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, amd_sfh_tabletmode_devices);
+
+static struct hid_driver amd_sfh_tabletmode_driver = {
+	.name = "amd_sfh_tabletmode",
+	.id_table = amd_sfh_tabletmode_devices,
+	.probe = amd_sfh_tabletmode_probe,
+	.remove = amd_sfh_tabletmode_remove,
+	.input_mapping = amd_sfh_tabletmode_input_mapping,
+	.event = amd_sfh_tabletmode_event,
+};
+
+static int __init amd_sfh_tabletmode_init(void)
+{
+	int error;
+
+	error = hid_register_driver(&amd_sfh_tabletmode_driver);
+
+	return error;
+}
+module_init(amd_sfh_tabletmode_init);
+
+static void __exit amd_sfh_tabletmode_exit(void)
+{
+	hid_unregister_driver(&amd_sfh_tabletmode_driver);
+}
+module_exit(amd_sfh_tabletmode_exit);
+
+MODULE_DESCRIPTION("Input driver for tablet mode sensor on amd sfh.");
+MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional `field->hidinput` dereference and tablet mode state corruption due to unconstrained `.event` callback.
- [High] The Kconfig symbol `INPUT_AMD_SFH_TABLETMODE` is missing a dependency on `HID`.
--

--- Patch [2]: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report ---
commit 981d61514e0f554ad31b2f14f7fb391c059df340
Author: Helge Bahmann <hcb@chaoticmind.net>

amd-sfh-tabletmode: interpret sfh tablet mode hid report
    
Add a driver that hooks into the tablet mode hid reports
generated by the amd-sfh-hid driver. This enables tablet
mode switching on e.g. Asus Vivobook devices.

> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 94a753fcb64fa..5a1dacd6f7b9c 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -1003,4 +1003,15 @@ config INPUT_STPMIC1_ONKEY
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called stpmic1_onkey.
>  
> +config INPUT_AMD_SFH_TABLETMODE
> +	tristate "AMD sensor fusion hub tablet mode driver"

[Severity: High]
Is it possible this Kconfig entry is missing a depends on HID?

If the HID subsystem is disabled or configured as a module while this driver
is built-in, could this result in linking errors for HID core functions like
hid_register_driver() and hid_parse()?

> +	help
> +	  Say Y to enable support of the tablet mode switch driver for

[ ... ]

> diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
> new file mode 100644
> index 0000000000000..ee1207372207c
> --- /dev/null
> +++ b/drivers/input/misc/amd_sfh_tabletmode.c

[ ... ]

> +static int amd_sfh_tabletmode_event(struct hid_device *hid, struct hid_field *field,
> +				    struct hid_usage *usage, __s32 value)
> +{
> +	input_report_switch(field->hidinput->input, SW_TABLET_MODE, value);

[Severity: High]
Does this unconditionally dereference field->hidinput, which could be NULL
for unmapped usages?

Because the driver struct below lacks a .usage_table, hid_match_usage()
will default to true for all usages, meaning this callback intercepts every
incoming HID report field. This could lead to a NULL pointer dereference,
and will also blindly emit a SW_TABLET_MODE event for any report, causing
standard buttons or battery reports to randomly toggle tablet mode.

Since .input_mapping already maps the usage to EV_SW, allowing the generic
HID core to automatically report the event, could this callback be removed
entirely?

> +
> +	return 0;
> +}
> +
> +static const struct hid_device_id amd_sfh_tabletmode_devices[] = {
> +	{ HID_DEVICE(BUS_AMD_SFH, HID_GROUP_GENERIC, AMD_SFH_HID_VENDOR, AMD_SFH_HID_PRODUCT) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, amd_sfh_tabletmode_devices);
> +
> +static struct hid_driver amd_sfh_tabletmode_driver = {
> +	.name = "amd_sfh_tabletmode",
> +	.id_table = amd_sfh_tabletmode_devices,
> +	.probe = amd_sfh_tabletmode_probe,
> +	.remove = amd_sfh_tabletmode_remove,
> +	.input_mapping = amd_sfh_tabletmode_input_mapping,
> +	.event = amd_sfh_tabletmode_event,

[Severity: High]
Should this struct include a .usage_table?

Without it, all device usages are routed to the .event callback above.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com?part=2

Re: [PATCH 1/2] amd-sfh-hid: tablet mode hid report and asus quirk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [Medium] Missing HID Input Mapping for tablet mode switch
- [Low] Incomplete Implementation / Dead Code for Feature Report
- [Low] Unintentional Parameter Override via DMI Quirk
- [Low] Unused Macros for Tablet State Evaluated by Magic Number
- [Low] Introduction of Non-English Comments

Pre-existing issues:
- [High] Double Free / Use-After-Free in amdtp_hid_probe error path
--

--- Patch [1]: [PATCH 1/2] amd-sfh-hid: tablet mode hid report and asus quirk ---
commit 8025fc07a2e48355e8daeee6c47f30c940eb5a30
Author: Helge Bahmann <hcb@chaoticmind.net>

amd-sfh-hid: tablet mode hid report and asus quirk

Add an input driver that interprets the "operation mode" sensor offered
by the amd sfh as a hid driver to generate a custom hid report (tablet
mode switch).

Add a quirk to restore compatibility of the driver with the Asus
VivoBook (turn off the "disable interrupts flag). Expose the
intr_disable flag as a module parameter in case it turns out to be
needed on further laptop models.

> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 96ae792beeb6c..8c03279646023 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
[ ... ]
> @@ -327,12 +314,6 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
>  
>  	for (i = 0; i < cl_data->num_hid_devices; i++) {
>  		cl_data->cur_hid_dev = i;
> -		if (cl_data->sensor_idx[i] == op_idx) {
> -			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> -				cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
> -				cl_data->sensor_sts[i]);
> -			continue;
> -		}
>  
>  		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
>  			rc = amdtp_hid_probe(i, cl_data);

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing issue in the
driver's error-handling path.

If amdtp_hid_probe() fails (e.g., in hid_add_device()), it destroys the hid
device but does not clear the dangling pointer in
cli_data->hid_sensor_hubs[cur_hid_dev].

When this failure propagates up to amd_sfh_hid_client_init(), it jumps to the
cleanup label and invokes amd_sfh_hid_client_deinit() -> amdtp_hid_remove().

amdtp_hid_remove() will then iterate over the hid_sensor_hubs array and call
hid_destroy_device() again on the dangling pointer, which can cause a double
free.

Should the pointer be set to NULL in amdtp_hid_probe() on error?

> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 4b81cebdc3359..924329bfef8fe 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -39,6 +39,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
>  MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
>  
>  static bool intr_disable = true;
> +module_param_named(intr_disable, intr_disable, bool, 0444);
> +MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");

[Severity: Low]
Because intr_disable is implemented as a simple boolean, the DMI match quirk
for Asus VivoBook unconditionally invokes mp2_disable_intr(), which
overwrites intr_disable to false.

Does this prevent the driver from differentiating between the default state
and an explicit user override on the command line (e.g.,
amd_sfh.intr_disable=1)?

> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
> index ef1f9be8b8938..7e807b92d2960 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
[ ... ]
> @@ -164,6 +180,11 @@ static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
>  		memcpy(feature_report, &gyro_feature, sizeof(gyro_feature));
>  		report_size = sizeof(gyro_feature);
>  		break;
> +	case op_idx: /* op */
> +		get_common_features(&tablet_mode_feature.common_property, report_id);
> +		memcpy(feature_report, &tablet_mode_feature, sizeof(tablet_mode_feature));
> +		report_size = sizeof(tablet_mode_feature);
> +		break;

[Severity: Low]
The driver allocates and populates a Feature report
(tablet_mode_feature_report) here, but the corresponding
tablet_mode_report_descriptor lacks any Feature items (0xB1).

Since the HID core strictly relies on the parsed descriptor to create reports,
will a feature report with ID 0x11 ever be requested, or is this unused
dead code?

> @@ -238,6 +260,15 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
>  		memcpy(input_report, &gyro_input, sizeof(gyro_input));
>  		report_size = sizeof(gyro_input);
>  		break;
> +	case op_idx: /* op */
> +		tablet_mode_input.report_id = 0x11;
> +		if (sensor_virt_addr[0] == 3)
> +			tablet_mode_input.tablet_state = 1;
> +		else
> +			tablet_mode_input.tablet_state = 0;

[Severity: Low]
This code relies on a hardcoded magic number 3. Should this evaluate the
sensor payload using the newly defined AMD_SFH_OP_IDX_MODE_TABLET macro
instead?

> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> index 882434b1501ff..ff82e47eb3ca4 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> @@ -114,4 +114,13 @@ struct hpd_input_report {
>  	u8 human_presence;
>  } __packed;
>  
> +struct tablet_mode_feature_report {
> +	struct common_feature_property common_property;
> +} __packed;
> +
> +struct tablet_mode_input_report {
> +	u8 report_id;    /* Muss 0x11 (17) sein */

[Severity: Low]
Is this German comment intentional? Kernel coding style typically requires all
comments to be written in English.

> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> index 67ec2d6a417de..f95368534fc60 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> @@ -776,4 +776,22 @@ static const u8 hpd_report_descriptor[] = {
>  0X81, 0x02,		 /* HID Input (Data_Var_Abs) */
>  0xC0			 /* HID end collection */
>  };
> +
> +/* tablet mode switch */
> +static const u8 tablet_mode_report_descriptor[] = {
> +0x05, 0x0C,		/* HID usage consumer electronics */
> +0x09, 0x01,		/* HID usage */
> +0xA1, 0x00,		/* HID collection (Physical) */
> +0x85, 0x11,		/* HID report id */
> +0x0A, 0xFF, 0x02,	/* HID usage (unallocated -- (ab)used for tablet mode) */

[Severity: Medium]
Because the driver does not provide a custom .input_mapping callback to
intercept and map this unallocated Consumer usage to SW_TABLET_MODE, will the
standard hid-input core ignore it or map it to KEY_UNKNOWN (240)?

Could this result in physical changes in tablet mode emitting spurious unknown
key events instead of a standard EV_SW switch event?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com?part=1

Re: [PATCH 0/2] asus vivobook tablet mode

[Basavaraj Natikar]

On 7/21/2026 7:58 PM, Helge Bahmann wrote:
> Hi Basavaraj,
>
> Here the two patches implementing the split hid/input
> gdriver for the tablet mode support. I mulled over what
> exactly to use as report descriptor in the hid driver, and
> there is no clean and easy solution -- I hope this is what
> you had in mind? I could not figure a way how to do
> a stand-alone input driver without doing any hid indirection,

Hi Helge,
Please find below the series adding SW_TABLET_MODE support via the AMD
Sensor Fusion Hub, covering all currently supported AMD convertibles

   https://lore.kernel.org/all/20260721174422.3109166-1-Basavaraj.Natikar@amd.com/
   
Once this is merged, the ASUS VivoBook quirk can go on top of it.

Thanks,
--
Basavaraj

> as I simply could not find a way to bind to the physical
> dev twice.
>
> Please let me know in comments to the patches how you
> would like to proceed.
>
> Thanks!
> Helge
>
>