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

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

Hi Hans,

On 11/26/2025 15:06, Hans de Goede wrote:
> 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.
> 

Thank you for the quick feedback. I sent out a v2 now.

Thanks,
Shyam