[CCing the three reporters and the regressions list]
On 03.10.24 18:04, Basavaraj Natikar wrote:
> There is no need for additional cleanup, as all resources are managed.
> Additionally, if no sensor is found, there will be no initialization of
> HID devices. Therefore, return immediately if no sensor is detected.
I'm not a reviewer, so feel free to ignore the follow comment:
I think the patch description should mentioned that this bug caused
Memory Errors / Page Faults / btrfs going read-only / btrfs disk
corruption, as that is a crucial detail for later and downstreams that
need to consider when deciding about backporting.
> Fixes: 8031b001da70 ("HID: amd_sfh: Move sensor discovery before HID device initialization")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219331
Some reported-by tags IMHO would be appropriate to give credit; all
three reporters already agreed to use their email address in public.
There meanwhile is also one comment in the bugzilla ticket that could be
read as a tested-by tag.
Maybe a Link: to
https://lore.kernel.org/all/90f6ee64-df5e-43b2-ad04-fa3a35efc1d5@xxxxxxxxxxxxx/
might be appropriate as well.
Ohh, and participation in stable is optional, but given the severeness
on the problem: would you maybe be willing to add a stable tag to the
commit to ensure this is backported to affected stable series quickly?
Ciao, Thorsten
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
> ---
> drivers/hid/amd-sfh-hid/amd_sfh_client.c | 3 +--
> drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 +++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 4b59687ff5d8..3fcb971d5fda 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> @@ -297,8 +297,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
> (mp2_ops->discovery_status && mp2_ops->discovery_status(privdata) == 0)) {
> dev_warn(dev, "Failed to discover, sensors not enabled is %d\n",
> cl_data->is_any_sensor_enabled);
> - rc = -EOPNOTSUPP;
> - goto cleanup;
> + return -EOPNOTSUPP;
> }
>
> for (i = 0; i < cl_data->num_hid_devices; i++) {
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 0c28ca349bcd..1300f122b524 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -351,7 +351,9 @@ static void sfh_init_work(struct work_struct *work)
>
> rc = amd_sfh_hid_client_init(mp2);
> if (rc) {
> - amd_sfh_clear_intr(mp2);
> + if (rc != -EOPNOTSUPP)
> + amd_sfh_clear_intr(mp2);
> +
> dev_err(&pdev->dev, "amd_sfh_hid_client_init failed err %d\n", rc);
> return;
> }
