Re: [PATCH] platform/x86/amd/pmf: Refactor repetitive BIOS output handling

[Hans de Goede <hansg@kernel.org>]

Hi Shyam,

On 26-Nov-25 10:31 AM, Shyam Sundar S K wrote:
> Replace repetitive switch-case statements for PMF_POLICY_BIOS_OUTPUT_*
> with a helper function and consolidated case handling. This reduces code
> duplication and improves maintainability.
> 
> The 10 BIOS output policies (PMF_POLICY_BIOS_OUTPUT_1 through
> PMF_POLICY_BIOS_OUTPUT_10) previously each had individual case statements
> with identical logic. This patch introduces
> pmf_policy_to_bios_output_index() to map policy values to array indices,
> consolidating the handling into a single case block with fallthrough.
> 
> This approach handles non-sequential policy enum values gracefully and
> makes future additions easier to implement.
> 
> No functional changes.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> This can be applied on ilpo-next branch.
> 
>  drivers/platform/x86/amd/pmf/tee-if.c | 64 +++++++++++++++------------
>  1 file changed, 36 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 2c74ba2a0b51..a379556032fe 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -73,6 +73,34 @@ static void amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
>  	input_sync(dev->pmf_idev);
>  }
>  
> +static int amd_pmf_policy_to_bios_output_index(u32 action_idx)
> +{
> +	switch (action_idx) {
> +	case PMF_POLICY_BIOS_OUTPUT_1:
> +		return 0;
> +	case PMF_POLICY_BIOS_OUTPUT_2:
> +		return 1;
> +	case PMF_POLICY_BIOS_OUTPUT_3:
> +		return 2;
> +	case PMF_POLICY_BIOS_OUTPUT_4:
> +		return 3;
> +	case PMF_POLICY_BIOS_OUTPUT_5:
> +		return 4;
> +	case PMF_POLICY_BIOS_OUTPUT_6:
> +		return 5;
> +	case PMF_POLICY_BIOS_OUTPUT_7:
> +		return 6;
> +	case PMF_POLICY_BIOS_OUTPUT_8:
> +		return 7;
> +	case PMF_POLICY_BIOS_OUTPUT_9:
> +		return 8;
> +	case PMF_POLICY_BIOS_OUTPUT_10:
> +		return 9;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>  {
>  	u32 val;
> @@ -183,45 +211,25 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  			break;
>  
>  		case PMF_POLICY_BIOS_OUTPUT_1:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(0), 0);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_2:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(1), 1);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_3:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(2), 2);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_4:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(3), 3);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_5:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(4), 4);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_6:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(5), 5);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_7:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(6), 6);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_8:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(7), 7);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_9:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(8), 8);
> -			break;
> -
>  		case PMF_POLICY_BIOS_OUTPUT_10:
> -			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(9), 9);
> +		{
> +			u32 bios_idx = amd_pmf_policy_to_bios_output_index(out->actions_list[idx]
> +					.action_index);
> +			if (bios_idx >= 0) {
> +				amd_pmf_smartpc_apply_bios_output(dev, val,
> +								  BIT(bios_idx), bios_idx);
> +			}

Since you know you are in case PMF_POLICY_BIOS_OUTPUT_1 .. PMF_POLICY_BIOS_OUTPUT_10 here
amd_pmf_policy_to_bios_output_index() will always return a valid index, so IMHO you
can drop the "if (bios_idx >= 0)" check here.

Otherwise this is a nice cleanup, thank you for cleaning this up.

Regards,

Hans