On 2025/1/15 1:58, Rafael J. Wysocki wrote:
> On Mon, Jan 13, 2025 at 1:21 PM Lifeng Zheng <zhenglifeng1@xxxxxxxxxx> wrote:
>>
>> Add CPPC_REG_VAL_READ() to implement registers getting functions.
>>
>> Add CPPC_REG_VAL_WRITE() to implement registers setting functions.
>>
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
>
> I don't particularly like these macros as they will generally make it
> harder to follow the code.
>
>> ---
>> drivers/acpi/cppc_acpi.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 571f94855dce..6326a1536cda 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1279,6 +1279,20 @@ static int cppc_set_reg_val(int cpu, enum cppc_regs reg_idx, u64 val)
>> return cpc_write(cpu, reg, val);
>> }
>>
>> +#define CPPC_REG_VAL_READ(reg_name, reg_idx) \
>> +int cppc_get_##reg_name(int cpu, u64 *val) \
>> +{ \
>> + return cppc_get_reg_val(cpu, reg_idx, val); \
>> +} \
>> +EXPORT_SYMBOL_GPL(cppc_get_##reg_name)
>
> What about if defining something like
>
> #define CPPC_READ_REG_VAL(cpu, reg_name, val)
> cppc_get_reg_val((cpu), CPPC_REG_IDX(reg_name), (val))
>
> (and analogously for the WRITE_ part), where CPPC_REG_IDX(reg_name) is
>
> #define CPPC_REG_IDX(reg_name) CPPC_REG_##reg_name_IDX
>
> and there are CPPC_REG_##reg_name_IDX macros defined for all register
> names in use?
>
> For example
>
> #define CPPC_REG_desired_perf_IDX DESIRED_PERF
What about keeping these two macros but replace reg_idx with
CPPC_REG_IDX(reg_name)? With this, the only needed parameter for these two
macros is reg_name.
