| Message ID | 20250403031242.1267561-1-superm1@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: amd_sfh: Fix SRA sensor when it's the only sensor | expand |
Internal Use - Confidential > -----Original Message----- > From: Mario Limonciello <superm1@kernel.org> > Sent: Thursday, April 3, 2025 11:12 AM > To: mario.limonciello@amd.com; basavaraj.natikar@amd.com; > jikos@kernel.org; bentiss@kernel.org; ilpo.jarvinen@linux.intel.com; Shyam- > sundar.S-k@amd.com; akshata.mukundshetty@amd.com > Cc: Shen, Yijun <Yijun_Shen@Dell.com>; stable@vger.kernel.org; linux- > input@vger.kernel.org > Subject: [PATCH] HID: amd_sfh: Fix SRA sensor when it's the only sensor > > > [EXTERNAL EMAIL] > > From: Mario Limonciello <mario.limonciello@amd.com> > > On systems that only have an SRA sensor connected to SFH the sensor > doesn't get enabled due to a bad optimization condition of breaking the > sensor walk loop. > > This optimization is unnecessary in the first place because if there is only one > device then the loop only runs once. Drop the condition and explicitly mark > sensor as enabled. > > Reported-by: Yijun Shen <Yijun.Shen@dell.com> > Fixes: d1c444b47100d ("HID: amd_sfh: Add support to export device > operating states") > Cc: stable@vger.kernel.org > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Verified the patch on the issued system, the issue is gone. Tested-By: Yijun Shen <Yijun_Shen@Dell.com> > --- > drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > 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 25f0ebfcbd5f5..c1bdf1e0d44af 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 > @@ -134,9 +134,6 @@ static int amd_sfh1_1_hid_client_init(struct > amd_mp2_dev *privdata) > for (i = 0; i < cl_data->num_hid_devices; i++) { > cl_data->sensor_sts[i] = SENSOR_DISABLED; > > - if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0] > == SRA_IDX) > - break; > - > if (cl_data->sensor_idx[i] == SRA_IDX) { > info.sensor_idx = cl_data->sensor_idx[i]; > writel(0, privdata->mmio + > amd_get_p2c_val(privdata, 0)); @@ -145,8 +142,10 @@ static int > amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata) > (privdata, cl_data->sensor_idx[i], > ENABLE_SENSOR); > > cl_data->sensor_sts[i] = (status == 0) ? > SENSOR_ENABLED : SENSOR_DISABLED; > - if (cl_data->sensor_sts[i] == SENSOR_ENABLED) > + if (cl_data->sensor_sts[i] == SENSOR_ENABLED) { > + cl_data->is_any_sensor_enabled = true; > privdata->dev_en.is_sra_present = true; > + } > continue; > } > > -- > 2.43.0
On 4/3/2025 8:42 AM, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > On systems that only have an SRA sensor connected to SFH the sensor > doesn't get enabled due to a bad optimization condition of breaking > the sensor walk loop. > > This optimization is unnecessary in the first place because if there > is only one device then the loop only runs once. Drop the condition > and explicitly mark sensor as enabled. > > Reported-by: Yijun Shen <Yijun.Shen@dell.com> > Fixes: d1c444b47100d ("HID: amd_sfh: Add support to export device operating states") > Cc: stable@vger.kernel.org > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > 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 25f0ebfcbd5f5..c1bdf1e0d44af 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 > @@ -134,9 +134,6 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata) > for (i = 0; i < cl_data->num_hid_devices; i++) { > cl_data->sensor_sts[i] = SENSOR_DISABLED; > > - if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0] == SRA_IDX) > - break; > - > if (cl_data->sensor_idx[i] == SRA_IDX) { > info.sensor_idx = cl_data->sensor_idx[i]; > writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0)); > @@ -145,8 +142,10 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata) > (privdata, cl_data->sensor_idx[i], ENABLE_SENSOR); > > cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED; > - if (cl_data->sensor_sts[i] == SENSOR_ENABLED) > + if (cl_data->sensor_sts[i] == SENSOR_ENABLED) { > + cl_data->is_any_sensor_enabled = true; > privdata->dev_en.is_sra_present = true; > + } > continue; > } > Looks like once SRA is enabled then SRA is not handled properly in failure cases. Thanks, -- Basavaraj
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 25f0ebfcbd5f5..c1bdf1e0d44af 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 @@ -134,9 +134,6 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata) for (i = 0; i < cl_data->num_hid_devices; i++) { cl_data->sensor_sts[i] = SENSOR_DISABLED; - if (cl_data->num_hid_devices == 1 && cl_data->sensor_idx[0] == SRA_IDX) - break; - if (cl_data->sensor_idx[i] == SRA_IDX) { info.sensor_idx = cl_data->sensor_idx[i]; writel(0, privdata->mmio + amd_get_p2c_val(privdata, 0)); @@ -145,8 +142,10 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata) (privdata, cl_data->sensor_idx[i], ENABLE_SENSOR); cl_data->sensor_sts[i] = (status == 0) ? SENSOR_ENABLED : SENSOR_DISABLED; - if (cl_data->sensor_sts[i] == SENSOR_ENABLED) + if (cl_data->sensor_sts[i] == SENSOR_ENABLED) { + cl_data->is_any_sensor_enabled = true; privdata->dev_en.is_sra_present = true; + } continue; }