linux-kernel - Re: [PATCH v4 3/6] ACPI: CPPC: Add macros to generally implement registers getting and setting functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10a624c3-66c1-420c-860d-2ef9104b59d8@huawei.com>
Date: Wed, 15 Jan 2025 16:58:55 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: <lenb@...nel.org>, <robert.moore@...el.com>, <viresh.kumar@...aro.org>,
	<mario.limonciello@....com>, <gautham.shenoy@....com>, <ray.huang@....com>,
	<pierre.gondois@....com>, <acpica-devel@...ts.linux.dev>,
	<linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-pm@...r.kernel.org>, <linuxarm@...wei.com>,
	<jonathan.cameron@...wei.com>, <zhanjie9@...ilicon.com>,
	<lihuisong@...wei.com>, <hepeng68@...wei.com>, <fanghao11@...wei.com>
Subject: Re: [PATCH v4 3/6] ACPI: CPPC: Add macros to generally implement
 registers getting and setting functions

On 2025/1/15 1:58, Rafael J. Wysocki wrote:

> On Mon, Jan 13, 2025 at 1:21 PM Lifeng Zheng <zhenglifeng1@...wei.com> 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@...wei.com>
> 
> 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.