* [PATCH] HID: amd_sfh: Add sync across amd sfh work functions
@ 2025-09-18 12:32 Basavaraj Natikar
2025-09-18 19:27 ` Mario Limonciello (AMD) (kernel.org)
2025-09-19 15:30 ` Jiri Kosina
0 siblings, 2 replies; 3+ messages in thread
From: Basavaraj Natikar @ 2025-09-18 12:32 UTC (permalink / raw)
To: jikos, bentiss, linux-input
Cc: Basavaraj Natikar, Matthew Schwartz, Prakruthi SP,
Akshata MukundShetty
The process of the report is delegated across different work functions.
Hence, add a sync mechanism to protect SFH work data across functions.
Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH)")
Reported-by: Matthew Schwartz <matthew.schwartz@linux.dev>
Closes: https://lore.kernel.org/all/a21abca5-4268-449d-95f1-bdd7a25894a5@linux.dev/
Tested-by: Prakruthi SP <Prakruthi.SP@amd.com>
Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 12 ++++++++++--
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 3 +++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 0f2cbae39b2b..7017bfa59093 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -39,8 +39,12 @@ int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type)
struct amdtp_hid_data *hid_data = hid->driver_data;
struct amdtp_cl_data *cli_data = hid_data->cli_data;
struct request_list *req_list = &cli_data->req_list;
+ struct amd_input_data *in_data = cli_data->in_data;
+ struct amd_mp2_dev *mp2;
int i;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
for (i = 0; i < cli_data->num_hid_devices; i++) {
if (cli_data->hid_sensor_hubs[i] == hid) {
struct request_list *new = kzalloc(sizeof(*new), GFP_KERNEL);
@@ -75,6 +79,8 @@ void amd_sfh_work(struct work_struct *work)
u8 report_id, node_type;
u8 report_size = 0;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
req_node = list_last_entry(&req_list->list, struct request_list, list);
list_del(&req_node->list);
current_index = req_node->current_index;
@@ -83,7 +89,6 @@ void amd_sfh_work(struct work_struct *work)
node_type = req_node->report_type;
kfree(req_node);
- mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
mp2_ops = mp2->mp2_ops;
if (node_type == HID_FEATURE_REPORT) {
report_size = mp2_ops->get_feat_rep(sensor_index, report_id,
@@ -107,6 +112,8 @@ void amd_sfh_work(struct work_struct *work)
cli_data->cur_hid_dev = current_index;
cli_data->sensor_requested_cnt[current_index] = 0;
amdtp_hid_wakeup(cli_data->hid_sensor_hubs[current_index]);
+ if (!list_empty(&req_list->list))
+ schedule_delayed_work(&cli_data->work, 0);
}
void amd_sfh_work_buffer(struct work_struct *work)
@@ -117,9 +124,10 @@ void amd_sfh_work_buffer(struct work_struct *work)
u8 report_size;
int i;
+ mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
+ guard(mutex)(&mp2->lock);
for (i = 0; i < cli_data->num_hid_devices; i++) {
if (cli_data->sensor_sts[i] == SENSOR_ENABLED) {
- mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
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,
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index f44a3bb2fbd4..78f830c133e5 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/mutex.h>
#include <linux/pci.h>
#include "amd_sfh_hid.h"
@@ -59,6 +60,8 @@ struct amd_mp2_dev {
u32 mp2_acs;
struct sfh_dev_status dev_en;
struct work_struct work;
+ /* mp2 to protect data */
+ struct mutex lock;
u8 init_done;
u8 rver;
};
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 2983af969579..1d9f955573aa 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -466,6 +466,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (!privdata->cl_data)
return -ENOMEM;
+ rc = devm_mutex_init(&pdev->dev, &privdata->lock);
+ 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)
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] HID: amd_sfh: Add sync across amd sfh work functions
2025-09-18 12:32 [PATCH] HID: amd_sfh: Add sync across amd sfh work functions Basavaraj Natikar
@ 2025-09-18 19:27 ` Mario Limonciello (AMD) (kernel.org)
2025-09-19 15:30 ` Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-09-18 19:27 UTC (permalink / raw)
To: Basavaraj Natikar, jikos, bentiss, linux-input
Cc: Matthew Schwartz, Prakruthi SP, Akshata MukundShetty
On 9/18/2025 7:32 AM, Basavaraj Natikar wrote:
> The process of the report is delegated across different work functions.
> Hence, add a sync mechanism to protect SFH work data across functions.
>
> Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH)")
> Reported-by: Matthew Schwartz <matthew.schwartz@linux.dev>
> Closes: https://lore.kernel.org/all/a21abca5-4268-449d-95f1-bdd7a25894a5@linux.dev/
> Tested-by: Prakruthi SP <Prakruthi.SP@amd.com>
> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/hid/amd-sfh-hid/amd_sfh_client.c | 12 ++++++++++--
> drivers/hid/amd-sfh-hid/amd_sfh_common.h | 3 +++
> drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 0f2cbae39b2b..7017bfa59093 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> @@ -39,8 +39,12 @@ int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type)
> struct amdtp_hid_data *hid_data = hid->driver_data;
> struct amdtp_cl_data *cli_data = hid_data->cli_data;
> struct request_list *req_list = &cli_data->req_list;
> + struct amd_input_data *in_data = cli_data->in_data;
> + struct amd_mp2_dev *mp2;
> int i;
>
> + mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
> + guard(mutex)(&mp2->lock);
> for (i = 0; i < cli_data->num_hid_devices; i++) {
> if (cli_data->hid_sensor_hubs[i] == hid) {
> struct request_list *new = kzalloc(sizeof(*new), GFP_KERNEL);
> @@ -75,6 +79,8 @@ void amd_sfh_work(struct work_struct *work)
> u8 report_id, node_type;
> u8 report_size = 0;
>
> + mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
> + guard(mutex)(&mp2->lock);
> req_node = list_last_entry(&req_list->list, struct request_list, list);
> list_del(&req_node->list);
> current_index = req_node->current_index;
> @@ -83,7 +89,6 @@ void amd_sfh_work(struct work_struct *work)
> node_type = req_node->report_type;
> kfree(req_node);
>
> - mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
> mp2_ops = mp2->mp2_ops;
> if (node_type == HID_FEATURE_REPORT) {
> report_size = mp2_ops->get_feat_rep(sensor_index, report_id,
> @@ -107,6 +112,8 @@ void amd_sfh_work(struct work_struct *work)
> cli_data->cur_hid_dev = current_index;
> cli_data->sensor_requested_cnt[current_index] = 0;
> amdtp_hid_wakeup(cli_data->hid_sensor_hubs[current_index]);
> + if (!list_empty(&req_list->list))
> + schedule_delayed_work(&cli_data->work, 0);
> }
>
> void amd_sfh_work_buffer(struct work_struct *work)
> @@ -117,9 +124,10 @@ void amd_sfh_work_buffer(struct work_struct *work)
> u8 report_size;
> int i;
>
> + mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
> + guard(mutex)(&mp2->lock);
> for (i = 0; i < cli_data->num_hid_devices; i++) {
> if (cli_data->sensor_sts[i] == SENSOR_ENABLED) {
> - mp2 = container_of(in_data, struct amd_mp2_dev, in_data);
> 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,
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index f44a3bb2fbd4..78f830c133e5 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/mutex.h>
> #include <linux/pci.h>
> #include "amd_sfh_hid.h"
>
> @@ -59,6 +60,8 @@ struct amd_mp2_dev {
> u32 mp2_acs;
> struct sfh_dev_status dev_en;
> struct work_struct work;
> + /* mp2 to protect data */
> + struct mutex lock;
> u8 init_done;
> u8 rver;
> };
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 2983af969579..1d9f955573aa 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -466,6 +466,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
> if (!privdata->cl_data)
> return -ENOMEM;
>
> + rc = devm_mutex_init(&pdev->dev, &privdata->lock);
> + 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)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] HID: amd_sfh: Add sync across amd sfh work functions
2025-09-18 12:32 [PATCH] HID: amd_sfh: Add sync across amd sfh work functions Basavaraj Natikar
2025-09-18 19:27 ` Mario Limonciello (AMD) (kernel.org)
@ 2025-09-19 15:30 ` Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2025-09-19 15:30 UTC (permalink / raw)
To: Basavaraj Natikar
Cc: bentiss, linux-input, Matthew Schwartz, Prakruthi SP,
Akshata MukundShetty
On Thu, 18 Sep 2025, Basavaraj Natikar wrote:
> The process of the report is delegated across different work functions.
> Hence, add a sync mechanism to protect SFH work data across functions.
>
> Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH)")
> Reported-by: Matthew Schwartz <matthew.schwartz@linux.dev>
> Closes: https://lore.kernel.org/all/a21abca5-4268-449d-95f1-bdd7a25894a5@linux.dev/
> Tested-by: Prakruthi SP <Prakruthi.SP@amd.com>
> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-19 15:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 12:32 [PATCH] HID: amd_sfh: Add sync across amd sfh work functions Basavaraj Natikar
2025-09-18 19:27 ` Mario Limonciello (AMD) (kernel.org)
2025-09-19 15:30 ` Jiri Kosina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox