diff mbox series

[v2,2/2] HID: amd_sfh: Cleanup all sensors when init fails

Message ID 20250403150026.1657538-3-superm1@kernel.org (mailing list archive)
State Superseded
Delegated to: Jiri Kosina
Headers show
Series Fixes for SRA sensor handling | expand

Commit Message

Mario Limonciello April 3, 2025, 2:59 p.m. UTC 
From: Mario Limonciello <mario.limonciello@amd.com>

If any sensors have been started when a failure occurs during client
init they are left enabled by the firmware.

As part of the cleanup routine check all sensors and if they're enabled
stop them.

Suggested-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * New patch
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Basavaraj Natikar April 7, 2025, 9:22 a.m. UTC | #1 
On 4/3/2025 8:29 PM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> If any sensors have been started when a failure occurs during client
> init they are left enabled by the firmware.
>
> As part of the cleanup routine check all sensors and if they're enabled
> stop them.
>
> Suggested-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2:
>   * New patch
> ---
>   drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index c1bdf1e0d44af..c61cc42fa7a33 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> @@ -235,12 +235,20 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
>   	return 0;
>   
>   cleanup:
> -	amd_sfh_hid_client_deinit(privdata);
>   	for (i = 0; i < cl_data->num_hid_devices; i++) {
> +		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> +			mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
> +			status = amd_sfh_wait_for_response
> +					(privdata, cl_data->sensor_idx[i], DISABLE_SENSOR);
> +			cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED;
> +		}

can we only add below condition as above case is already taken care in
amd_sfh_hid_client_deinit

Thanks,
--
Basavaraj

> +		if (cl_data->sensor_idx[i] == SRA_IDX)
> +			continue;
>   		devm_kfree(dev, cl_data->feature_report[i]);
>   		devm_kfree(dev, in_data->input_report[i]);
>   		devm_kfree(dev, cl_data->report_descr[i]);
>   	}
> +	amd_sfh_hid_client_deinit(privdata);
>   	return rc;
>   }
>
diff mbox series

Patch

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index c1bdf1e0d44af..c61cc42fa7a33 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -235,12 +235,20 @@  static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
 	return 0;
 
 cleanup:
-	amd_sfh_hid_client_deinit(privdata);
 	for (i = 0; i < cl_data->num_hid_devices; i++) {
+		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
+			mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
+			status = amd_sfh_wait_for_response
+					(privdata, cl_data->sensor_idx[i], DISABLE_SENSOR);
+			cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED;
+		}
+		if (cl_data->sensor_idx[i] == SRA_IDX)
+			continue;
 		devm_kfree(dev, cl_data->feature_report[i]);
 		devm_kfree(dev, in_data->input_report[i]);
 		devm_kfree(dev, cl_data->report_descr[i]);
 	}
+	amd_sfh_hid_client_deinit(privdata);
 	return rc;
 }