* [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor
2026-07-21 17:44 [PATCH 0/3] Add SW_TABLET_MODE support for AMD SFH convertibles Basavaraj Natikar
@ 2026-07-21 17:44 ` Basavaraj Natikar
2026-07-21 18:04 ` sashiko-bot
2026-07-21 17:44 ` [PATCH 2/3] HID: amd_sfh: Register tablet-mode auxiliary device Basavaraj Natikar
2026-07-21 17:44 ` [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver Basavaraj Natikar
2 siblings, 1 reply; 6+ messages in thread
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov; +Cc: linux-input, Basavaraj Natikar
Allow other drivers to query the operating mode (laptop or tablet)
reported by the Sensor Fusion Hub. This is the interface used by the
tablet-mode switch driver to report the device posture to userspace.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 16 +++++++++
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 +++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 12 +++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 33 +++++++++++++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 -
include/linux/amd-pmf-io.h | 14 ++++++++
6 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 96ae792beeb6..0f5346d2174c 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -383,3 +383,19 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
return 0;
}
+
+bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2)
+{
+ struct amdtp_cl_data *cl = mp2->cl_data;
+ int i;
+
+ if (!cl)
+ return false;
+
+ for (i = 0; i < cl->num_hid_devices; i++)
+ if (cl->sensor_idx[i] == op_idx &&
+ cl->sensor_sts[i] == SENSOR_ENABLED)
+ return true;
+
+ return false;
+}
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 78f830c133e5..cad7d5f8fd4d 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -100,4 +100,9 @@ static inline u64 amd_get_p2c_val(struct amd_mp2_dev *mp2, u32 idx)
{
return mp2->rver == 1 ? AMD_P2C_MSG_V1(idx) : AMD_P2C_MSG(idx);
}
+
+bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2);
+void sfh_set_emp2(struct amd_mp2_dev *mp2);
+void sfh_deinit_emp2(void);
+
#endif
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 4b81cebdc335..a3672a9302c5 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -248,6 +248,11 @@ int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 *sensor_id)
return num_of_sensors;
}
+static void amd_sfh_deinit_emp2_action(void *unused)
+{
+ sfh_deinit_emp2();
+}
+
static void amd_mp2_pci_remove(void *privdata)
{
struct amd_mp2_dev *mp2 = privdata;
@@ -447,6 +452,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
privdata->pdev = pdev;
dev_set_drvdata(&pdev->dev, privdata);
+
rc = pcim_enable_device(pdev);
if (rc)
return rc;
@@ -471,6 +477,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (rc)
return rc;
+ rc = devm_add_action_or_reset(&pdev->dev, amd_sfh_deinit_emp2_action, NULL);
+ if (rc)
+ return rc;
+
privdata->sfh1_1_ops = (const struct amd_sfh1_1_ops *)id->driver_data;
if (privdata->sfh1_1_ops) {
if (boot_cpu_data.x86 >= 0x1A)
@@ -480,6 +490,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (rc)
return rc;
+ sfh_set_emp2(privdata);
schedule_work(&privdata->work);
return 0;
}
@@ -498,6 +509,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
return rc;
}
+ sfh_set_emp2(privdata);
schedule_work(&privdata->work);
return 0;
}
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 837d59e7a661..74b3d3382a82 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -81,12 +81,43 @@ void sfh_deinit_emp2(void)
emp2 = NULL;
}
+void sfh_set_emp2(struct amd_mp2_dev *mp2)
+{
+ emp2 = mp2;
+}
+
void sfh_interface_init(struct amd_mp2_dev *mp2)
{
mp2->mp2_ops = &amd_sfh_ops;
emp2 = mp2;
}
+static int amd_sfh_op_mode_info(u32 *op_mode)
+{
+ struct sfh_op_mode mode;
+ bool present;
+
+ if (!op_mode)
+ return -EINVAL;
+ if (!emp2)
+ return -ENODEV;
+
+ present = emp2->sfh1_1_ops ? emp2->dev_en.is_sra_present
+ : amd_sfh_op_idx_enabled(emp2);
+ if (!present)
+ return -ENODEV;
+
+ mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
+ dev_dbg(&emp2->pdev->dev,
+ "op-mode: raw=0x%08x mode=%u lid=%u angle=%u ontable=%u devmode=%u\n",
+ mode.val, mode.op_mode.mode, mode.op_mode.lidstatus,
+ mode.op_mode.angle, mode.op_mode.ontablestate,
+ mode.op_mode.devicemode);
+ *op_mode = mode.op_mode.mode;
+
+ return 0;
+}
+
static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
{
struct sfh_op_mode mode;
@@ -169,6 +200,8 @@ int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
case MT_SRA:
return amd_sfh_mode_info(&sfh_info->platform_type,
&sfh_info->laptop_placement);
+ case MT_OP_MODE:
+ return amd_sfh_op_mode_info(&sfh_info->op_mode);
}
}
return -EINVAL;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 665c99ad779f..56258c4d1b3a 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -185,7 +185,6 @@ struct sfh_op_mode {
};
void sfh_interface_init(struct amd_mp2_dev *mp2);
-void sfh_deinit_emp2(void);
void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
int amd_sfh_float_to_int(u32 flt32_val);
#endif
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 55198d2875cc..dc59c43bd8f2 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -19,11 +19,13 @@
* @MT_HPD: Message ID to know the Human presence info from MP2 FW
* @MT_ALS: Message ID to know the Ambient light info from MP2 FW
* @MT_SRA: Message ID to know the SRA data from MP2 FW
+ * @MT_OP_MODE: Message ID to know the operating-mode (tablet/laptop) info
*/
enum sfh_message_type {
MT_HPD,
MT_ALS,
MT_SRA,
+ MT_OP_MODE,
};
/**
@@ -44,12 +46,24 @@ enum sfh_hpd_info {
* @user_present: Populates the user presence information
* @platform_type: Operating modes (clamshell, flat, tent, etc.)
* @laptop_placement: Device states (ontable, onlap, outbag)
+ * @op_mode: Operating-mode field (see enum sfh_dev_mode); used for tablet detection
*/
struct amd_sfh_info {
u32 ambient_light;
u8 user_present;
u32 platform_type;
u32 laptop_placement;
+ u32 op_mode;
+};
+
+/**
+ * enum sfh_dev_mode - SFH operating-mode field (sfh_op_mode.mode, bits 0-2)
+ * @SFH_MODE_LAPTOP: Device is in laptop/clamshell posture
+ * @SFH_MODE_TABLET: Device is in tablet posture
+ */
+enum sfh_dev_mode {
+ SFH_MODE_LAPTOP = 1,
+ SFH_MODE_TABLET = 3,
};
enum laptop_placement {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor
2026-07-21 17:44 ` [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor Basavaraj Natikar
@ 2026-07-21 18:04 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-21 18:04 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The auxiliary device created for the tablet-mode switch is never destroyed, leaking memory and leaving dangling device registrations.
- [High] Reading the operating mode on a v1.0 device accesses a register that is corrupted by DMA addresses, returning invalid data.
- [Medium] The `amd_sfh_deinit_emp2_action` devres action is registered prematurely and unconditionally clears the global `emp2` pointer, potentially corrupting global state.
- [Medium] The global `emp2` pointer is published before asynchronous initialization completes, causing data races on dynamically populated structures.
Pre-existing issues:
- [High] Exported APIs accessing the global `emp2` pointer lack synchronization, leading to a potential use-after-free and unmapped MMIO access if the device is concurrently unbound.
--
--- Patch [1]: [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor ---
commit 1243ba22a0e430d7c20b9d71ce0f64ee46332ff2
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
HID: amd_sfh: Add accessor to read the operating-mode sensor
Allow other drivers to query the operating mode (laptop or tablet)
reported by the Sensor Fusion Hub. This is the interface used by the
tablet-mode switch driver to report the device posture to userspace.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 96ae792beeb6c..0f5346d2174c2 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
[ ... ]
> @@ -383,3 +383,19 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
>
> return 0;
> }
> +
> +bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2)
> +{
> + struct amdtp_cl_data *cl = mp2->cl_data;
> + int i;
> +
> + if (!cl)
> + return false;
> +
> + for (i = 0; i < cl->num_hid_devices; i++)
> + if (cl->sensor_idx[i] == op_idx &&
> + cl->sensor_sts[i] == SENSOR_ENABLED)
> + return true;
> +
> + return false;
> +}
[Severity: High]
Does enabling this operating mode index leak the auxiliary device?
When amd_sfh_op_idx_enabled() returns true, it appears to trigger the
creation of mp2->tm_auxdev in amd_sfh_maybe_register_tm():
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:amd_sfh_maybe_register_tm() {
...
present = mp2->sfh1_1_ops ? mp2->dev_en.is_sra_present
: amd_sfh_op_idx_enabled(mp2);
if (!present)
return;
mp2->tm_auxdev = auxiliary_device_create(&mp2->pdev->dev, KBUILD_MODNAME,
"tabletmode", NULL, 0);
...
}
However, amd_mp2_pci_remove() does not seem to destroy this auxiliary
device, which could leave a dangling sysfs registration and leak memory
if the device is unbound.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 4b81cebdc3359..a3672a9302c5d 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
[ ... ]
> @@ -471,6 +477,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
> if (rc)
> return rc;
>
> + rc = devm_add_action_or_reset(&pdev->dev, amd_sfh_deinit_emp2_action, NULL);
> + if (rc)
> + return rc;
> +
[Severity: Medium]
Could this unconditionally clear the global emp2 pointer on a probe failure?
If devm_add_action_or_reset() is called before the device successfully
assigns its privdata to emp2 (which happens later in probe), and a
subsequent probe step fails, the devres cleanup will fire.
Will this wipe the emp2 pointer via sfh_deinit_emp2() even if it is
currently owned by another active device in a multi-device scenario?
> privdata->sfh1_1_ops = (const struct amd_sfh1_1_ops *)id->driver_data;
> if (privdata->sfh1_1_ops) {
> if (boot_cpu_data.x86 >= 0x1A)
> @@ -480,6 +490,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
> if (rc)
> return rc;
>
> + sfh_set_emp2(privdata);
> schedule_work(&privdata->work);
> return 0;
> }
[Severity: Medium]
Is there a data race when publishing the device globally before the workqueue
finishes?
sfh_set_emp2() exposes the device to exported APIs like amd_get_sfh_info().
If that API is called concurrently, amd_sfh_op_idx_enabled() might locklessly
iterate over cl->num_hid_devices and cl->sensor_idx[] while they are still
being populated by the workqueue in amd_sfh_hid_client_init().
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 837d59e7a6610..74b3d3382a82c 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
[ ... ]
> @@ -81,12 +81,43 @@ void sfh_deinit_emp2(void)
> emp2 = NULL;
> }
>
> +void sfh_set_emp2(struct amd_mp2_dev *mp2)
> +{
> + emp2 = mp2;
> +}
> +
> +static int amd_sfh_op_mode_info(u32 *op_mode)
> +{
> + struct sfh_op_mode mode;
> + bool present;
> +
> + if (!op_mode)
> + return -EINVAL;
> + if (!emp2)
> + return -ENODEV;
[Severity: High]
This is a pre-existing issue, but does this lockless check of emp2 create a
race condition with device unbind?
If a privileged user unbinds the SFH PCI driver concurrently, the teardown
path unmaps the MMIO, frees privdata, and the devres action clears emp2.
Could the querying thread dereference emp2 after it has been freed if the
unbind happens immediately after this !emp2 check?
> +
> + present = emp2->sfh1_1_ops ? emp2->dev_en.is_sra_present
> + : amd_sfh_op_idx_enabled(emp2);
> + if (!present)
> + return -ENODEV;
> +
> + mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
[Severity: High]
Will this read corrupted data on v1.0 hardware?
On v1.0 hardware, amd_start_sensor() uses a 64-bit writeq() to AMD_C2P_MSG2
to program the sensor's DMA address:
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:amd_start_sensor() {
...
writeq(info.dma_address, privdata->mmio + AMD_C2P_MSG2);
...
}
This 64-bit write spans both AMD_C2P_MSG2 and AMD_C2P_MSG3. Since
amd_get_c2p_val(emp2, 3) accesses AMD_C2P_MSG3, will it return the upper
32 bits of a DMA address instead of the actual operating mode?
> + dev_dbg(&emp2->pdev->dev,
> + "op-mode: raw=0x%08x mode=%u lid=%u angle=%u ontable=%u devmode=%u\n",
> + mode.val, mode.op_mode.mode, mode.op_mode.lidstatus,
> + mode.op_mode.angle, mode.op_mode.ontablestate,
> + mode.op_mode.devicemode);
> + *op_mode = mode.op_mode.mode;
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174422.3109166-1-Basavaraj.Natikar@amd.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] HID: amd_sfh: Register tablet-mode auxiliary device
2026-07-21 17:44 [PATCH 0/3] Add SW_TABLET_MODE support for AMD SFH convertibles Basavaraj Natikar
2026-07-21 17:44 ` [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor Basavaraj Natikar
@ 2026-07-21 17:44 ` Basavaraj Natikar
2026-07-21 17:44 ` [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver Basavaraj Natikar
2 siblings, 0 replies; 6+ messages in thread
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov; +Cc: linux-input, Basavaraj Natikar
Register an auxiliary device when the operating-mode sensor is present,
so a dedicated input driver can bind to it and report the device
posture. This keeps the input handling out of the sensor transport
driver.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/Kconfig | 1 +
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 2 ++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 30 ++++++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/Kconfig b/drivers/hid/amd-sfh-hid/Kconfig
index 3291786a5ee6..d86d83ffebd7 100644
--- a/drivers/hid/amd-sfh-hid/Kconfig
+++ b/drivers/hid/amd-sfh-hid/Kconfig
@@ -6,6 +6,7 @@ menu "AMD SFH HID Support"
config AMD_SFH_HID
tristate "AMD Sensor Fusion Hub"
depends on X86
+ select AUXILIARY_BUS
help
If you say yes to this option, support will be included for the
AMD Sensor Fusion Hub.
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index cad7d5f8fd4d..34270ad95cf0 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -10,6 +10,7 @@
#ifndef AMD_SFH_COMMON_H
#define AMD_SFH_COMMON_H
+#include <linux/auxiliary_bus.h>
#include <linux/mutex.h>
#include <linux/pci.h>
#include "amd_sfh_hid.h"
@@ -64,6 +65,7 @@ struct amd_mp2_dev {
struct mutex lock;
u8 init_done;
u8 rver;
+ struct auxiliary_device *tm_auxdev;
};
struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index a3672a9302c5..826fd762514f 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -8,6 +8,7 @@
* Basavaraj Natikar <Basavaraj.Natikar@amd.com>
*/
+#include <linux/auxiliary_bus.h>
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/devm-helpers.h>
@@ -391,6 +392,32 @@ static const struct attribute_group *amd_sfh_groups[] = {
NULL,
};
+static void amd_sfh_maybe_register_tm(struct amd_mp2_dev *mp2)
+{
+ bool present;
+
+ if (mp2->tm_auxdev)
+ return;
+
+ present = mp2->sfh1_1_ops ? mp2->dev_en.is_sra_present
+ : amd_sfh_op_idx_enabled(mp2);
+ if (!present)
+ return;
+
+ mp2->tm_auxdev = auxiliary_device_create(&mp2->pdev->dev, KBUILD_MODNAME,
+ "tabletmode", NULL, 0);
+ if (!mp2->tm_auxdev)
+ dev_warn(&mp2->pdev->dev, "tabletmode auxdev create failed\n");
+}
+
+static void amd_sfh_unregister_tm(struct amd_mp2_dev *mp2)
+{
+ if (mp2->tm_auxdev) {
+ auxiliary_device_destroy(mp2->tm_auxdev);
+ mp2->tm_auxdev = NULL;
+ }
+}
+
static void sfh1_1_init_work(struct work_struct *work)
{
struct amd_mp2_dev *mp2 = container_of(work, struct amd_mp2_dev, work);
@@ -407,6 +434,7 @@ static void sfh1_1_init_work(struct work_struct *work)
if (rc)
dev_warn(&mp2->pdev->dev, "failed to update sysfs group\n");
+ amd_sfh_maybe_register_tm(mp2);
}
static void sfh_init_work(struct work_struct *work)
@@ -425,6 +453,7 @@ static void sfh_init_work(struct work_struct *work)
amd_sfh_clear_intr(mp2);
mp2->init_done = 1;
+ amd_sfh_maybe_register_tm(mp2);
}
static void amd_sfh_remove(struct pci_dev *pdev)
@@ -432,6 +461,7 @@ static void amd_sfh_remove(struct pci_dev *pdev)
struct amd_mp2_dev *mp2 = pci_get_drvdata(pdev);
flush_work(&mp2->work);
+ amd_sfh_unregister_tm(mp2);
if (mp2->init_done)
mp2->mp2_ops->remove(mp2);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver
2026-07-21 17:44 [PATCH 0/3] Add SW_TABLET_MODE support for AMD SFH convertibles Basavaraj Natikar
2026-07-21 17:44 ` [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor Basavaraj Natikar
2026-07-21 17:44 ` [PATCH 2/3] HID: amd_sfh: Register tablet-mode auxiliary device Basavaraj Natikar
@ 2026-07-21 17:44 ` Basavaraj Natikar
2026-07-21 17:54 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov
Cc: linux-input, Basavaraj Natikar, Helge Bahmann
Report whether an AMD convertible is in laptop or tablet mode using the
operating-mode sensor provided by the Sensor Fusion Hub, and expose it
to userspace as SW_TABLET_MODE, so userspace can react to the device
being folded into tablet posture.
Cc: Helge Bahmann <hcb@chaoticmind.net>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
MAINTAINERS | 1 +
drivers/input/misc/Kconfig | 15 +++++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/amd_sfh_tabletmode.c | 81 +++++++++++++++++++++++++
4 files changed, 98 insertions(+)
create mode 100644 drivers/input/misc/amd_sfh_tabletmode.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1ab8736850ea..30cd879e4a3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1301,6 +1301,7 @@ L: linux-input@vger.kernel.org
S: Maintained
F: Documentation/hid/amd-sfh*
F: drivers/hid/amd-sfh-hid/
+F: drivers/input/misc/amd_sfh_tabletmode.c
AMD SPI DRIVER
M: Raju Rangoju <Raju.Rangoju@amd.com>
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 1f6c57dba030..5b76be31f687 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -899,6 +899,21 @@ config INPUT_SOC_BUTTON_ARRAY
To compile this driver as a module, choose M here: the
module will be called soc_button_array.
+config INPUT_AMD_SFH_TABLETMODE
+ tristate "AMD SFH tablet-mode switch"
+ depends on AMD_SFH_HID
+ select AUXILIARY_BUS
+ help
+ Expose SW_TABLET_MODE for AMD convertible laptops whose
+ operation-mode sensor is provided by the AMD Sensor Fusion Hub.
+
+ Userspace components such as systemd-logind and modern desktop
+ environments react to SW_TABLET_MODE when the device is folded
+ into tablet posture.
+
+ To compile this driver as a module, choose M here: the module
+ will be called amd_sfh_tabletmode.
+
config INPUT_DRV260X_HAPTICS
tristate "TI DRV260X haptics support"
depends on INPUT && I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 2281d6803fce..72aa0658ebb3 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_INPUT_RK805_PWRKEY) += rk805-pwrkey.o
obj-$(CONFIG_INPUT_SC27XX_VIBRA) += sc27xx-vibra.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
obj-$(CONFIG_INPUT_SOC_BUTTON_ARRAY) += soc_button_array.o
+obj-$(CONFIG_INPUT_AMD_SFH_TABLETMODE) += amd_sfh_tabletmode.o
obj-$(CONFIG_INPUT_SPARCSPKR) += sparcspkr.o
obj-$(CONFIG_INPUT_STPMIC1_ONKEY) += stpmic1_onkey.o
obj-$(CONFIG_INPUT_TPS65218_PWRBUTTON) += tps65218-pwrbutton.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..f88edebbf35d
--- /dev/null
+++ b/drivers/input/misc/amd_sfh_tabletmode.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD SFH tablet-mode switch driver
+ *
+ * Copyright (c) 2026, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
+ */
+
+#include <linux/amd-pmf-io.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/pci_ids.h>
+
+#define POLL_INTERVAL_MS 200
+
+static void sfh_tm_poll(struct input_dev *input)
+{
+ struct amd_sfh_info info = {};
+
+ if (amd_get_sfh_info(&info, MT_OP_MODE))
+ return;
+
+ input_report_switch(input, SW_TABLET_MODE,
+ info.op_mode == SFH_MODE_TABLET);
+ input_sync(input);
+}
+
+static int sfh_tm_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct amd_sfh_info info = {};
+ struct device *dev = &auxdev->dev;
+ struct input_dev *input;
+ int rc;
+
+ rc = amd_get_sfh_info(&info, MT_OP_MODE);
+ if (rc)
+ return rc == -EINVAL ? -ENODEV : rc;
+
+ input = devm_input_allocate_device(dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "AMD SFH tablet mode switch";
+ input->phys = "amd-sfh/tabletmode";
+ input->id.bustype = BUS_HOST;
+ input->id.vendor = PCI_VENDOR_ID_AMD;
+ input_set_capability(input, EV_SW, SW_TABLET_MODE);
+
+ rc = input_setup_polling(input, sfh_tm_poll);
+ if (rc)
+ return rc;
+ input_set_poll_interval(input, POLL_INTERVAL_MS);
+
+ rc = input_register_device(input);
+ if (rc)
+ return rc;
+
+ sfh_tm_poll(input);
+
+ return 0;
+}
+
+static const struct auxiliary_device_id sfh_tm_id_table[] = {
+ { .name = "amd_sfh.tabletmode" },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, sfh_tm_id_table);
+
+static struct auxiliary_driver sfh_tm_driver = {
+ .name = "tabletmode",
+ .id_table = sfh_tm_id_table,
+ .probe = sfh_tm_probe,
+};
+module_auxiliary_driver(sfh_tm_driver);
+
+MODULE_DESCRIPTION("AMD SFH tablet mode switch");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver
2026-07-21 17:44 ` [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver Basavaraj Natikar
@ 2026-07-21 17:54 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-21 17:54 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The initial hardware state of the tablet mode switch is redundantly queried and synchronized after `input_register_device()` exposes the device to userspace, creating a race condition.
- [Low] Violation of input subsystem naming convention for error variables.
--
--- Patch [3]: [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver ---
commit e88c97b2482f6ed434b346336b9c6f9164d73473
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Input: misc: Add AMD SFH tablet-mode switch driver
This commit introduces a new driver for AMD convertible laptops. It exposes
the SW_TABLET_MODE switch by polling the operating-mode sensor provided by
the AMD Sensor Fusion Hub.
> diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
> new file mode 100644
> index 0000000000000..f88edebbf35d5
> --- /dev/null
> +++ b/drivers/input/misc/amd_sfh_tabletmode.c
[ ... ]
> +static int sfh_tm_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *id)
> +{
> + struct amd_sfh_info info = {};
> + struct device *dev = &auxdev->dev;
> + struct input_dev *input;
> + int rc;
[Severity: Low]
This isn't a bug, but does using 'rc' instead of 'error' or 'err' violate
the input subsystem maintainer preference?
The input subsystem guidelines suggest using 'error' or 'err' for variables
that only hold negative error codes and 0 for success.
[ ... ]
> + rc = input_setup_polling(input, sfh_tm_poll);
> + if (rc)
> + return rc;
> + input_set_poll_interval(input, POLL_INTERVAL_MS);
> +
> + rc = input_register_device(input);
> + if (rc)
> + return rc;
> +
> + sfh_tm_poll(input);
[Severity: Medium]
Should sfh_tm_poll() be called before input_register_device()?
Calling it after registration exposes a race condition where userspace
components reacting to the device creation uevent might read the
SW_TABLET_MODE state before sfh_tm_poll() executes.
They will read the default uninitialized state (0), which may incorrectly
leave the keyboard and touchpad active if the device is actually folded into
tablet mode.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174422.3109166-1-Basavaraj.Natikar@amd.com?part=3
^ permalink raw reply [flat|nested] 6+ messages in thread