On Tue, 23 Jul 2024, Shyam Sundar S K wrote:
> If the Ambient Light Sensor (ALS) is disabled, the current code in the PMF
> driver does not query for Human Presence Detection (HPD) data in
> amd_pmf_get_sensor_info(). As a result, stale HPD data is used by PMF-TA
> to evaluate policy conditions, leading to unexpected behavior in the policy
> output actions.
>
> To resolve this issue, modify the PMF driver to query HPD data
> independently of ALS.
>
> With this change, amd_pmf_get_sensor_info() now returns void instead of
> int.
>
> Fixes: cedecdba60f4 ("platform/x86/amd/pmf: Get ambient light information from AMD SFH driver")
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> ---
> drivers/platform/x86/amd/pmf/spc.c | 33 +++++++++++++++---------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index a3dec14c3004..d9e60d63553c 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,7 +150,7 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> return 0;
> }
>
> -static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> {
> struct amd_sfh_info sfh_info;
> int ret;
> @@ -160,26 +160,25 @@ static int amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> if (!ret)
> in->ev_info.ambient_light = sfh_info.ambient_light;
> else
> - return ret;
> + dev_dbg(dev->dev, "ALS is not enabled\n");
>
> /* get HPD data */
> ret = amd_get_sfh_info(&sfh_info, MT_HPD);
> - if (ret)
> - return ret;
> -
> - switch (sfh_info.user_present) {
> - case SFH_NOT_DETECTED:
> - in->ev_info.user_present = 0xff; /* assume no sensors connected */
> - break;
> - case SFH_USER_PRESENT:
> - in->ev_info.user_present = 1;
> - break;
> - case SFH_USER_AWAY:
> - in->ev_info.user_present = 0;
> - break;
> + if (!ret) {
> + switch (sfh_info.user_present) {
> + case SFH_NOT_DETECTED:
> + in->ev_info.user_present = 0xff; /* assume no sensors connected */
> + break;
> + case SFH_USER_PRESENT:
> + in->ev_info.user_present = 1;
> + break;
> + case SFH_USER_AWAY:
> + in->ev_info.user_present = 0;
> + break;
> + }
> + } else {
> + dev_dbg(dev->dev, "HPD is not enabled\n");
Is it okay to leave in->ev_info.user_present as 0 this case? Should it be
set to same as with SFH_NOT_DETECTED?
...I now realize your print out doesn't cover that case at all and
user_present is not used for anything else than debug printing.
--
i.
