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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 26 Nov 2025, 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>
> ---
> v2:
>  - Drop if() check for bios_idx
> 
> Note: This patch can be applied on ilpo-next
> 
>  drivers/platform/x86/amd/pmf/tee-if.c | 61 +++++++++++++++------------
>  1 file changed, 33 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..e4562caf98fe 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,22 @@ 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);

You probably realized yourself too this is a bit too long line. :-)

There are multiple things that could be done to alleviate it (not a list 
of requirements, just ideas where to pick from):

- Create a local var for out->actions_list[idx] in the loop (with a 
  relatively short name).

- Try to make the function's name slightly shorter though I'm not sure 
  it's easy beyond "index" -> "idx".

- Place .action_index to a local var before the call or before the 
  switch ().

- Call some function and put this code there.

I don't necessarily know which results in the best experience from code 
reading PoV but something needs to be done.

> +			amd_pmf_smartpc_apply_bios_output(dev, val, BIT(bios_idx), bios_idx);
>  			break;
>  		}
> +		}
>  	}
>  }
>  
> 

-- 
 i.