Re: [PATCH v2 1/3] ACPI: ACPI 6.5: RAS2: Shorten RAS2 table structure and variable names

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Wed, 5 Mar 2025 18:02:22 +0000
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Shorten RAS2 table structure and variable names.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Hi Shiju,

Generally looks reasonable to me, but I'm not sure what your
decision process was for which to shorten and which to leave alone.
Perhaps it is worth mentioning that in the patch description?

> ---
>  include/acpi/actbl2.h | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 2e917a8f8bca..5cfc65ba6e9e 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -2802,20 +2802,20 @@ struct acpi_ras2_pcc_desc {
>  
>  /* RAS2 Platform Communication Channel Shared Memory Region */
>  
> -struct acpi_ras2_shared_memory {
> +struct acpi_ras2_shmem {
>  	u32 signature;
> -	u16 command;
> +	u16 cmd;
>  	u16 status;
>  	u16 version;
>  	u8 features[16];
> -	u8 set_capabilities[16];
> -	u16 num_parameter_blocks;
> -	u32 set_capabilities_status;
> +	u8 set_caps[16];
> +	u16 num_param_blks;
> +	u32 set_caps_status;

I assume focus was on fields that were leading to long line lengths? 
If it was just generally shortening things to common form
sig, sts, ver, feats etc would also seem reasonable to me
(all subject to Tony's question on whether we can touch this at all.)

>  };
>  
>  /* RAS2 Parameter Block Structure for PATROL_SCRUB */
>  
> -struct acpi_ras2_parameter_block {
> +struct acpi_ras2_param_blk {
>  	u16 type;
>  	u16 version;
>  	u16 length;
> @@ -2823,11 +2823,11 @@ struct acpi_ras2_parameter_block {
>  
>  /* RAS2 Parameter Block Structure for PATROL_SCRUB */
>  
> -struct acpi_ras2_patrol_scrub_parameter {
> -	struct acpi_ras2_parameter_block header;
> -	u16 patrol_scrub_command;
> -	u64 requested_address_range[2];
> -	u64 actual_address_range[2];
> +struct acpi_ras2_patrol_scrub_param {
> +	struct acpi_ras2_param_blk header;
> +	u16 cmd;
> +	u64 req_addr_range[2];
> +	u64 actl_addr_range[2];
>  	u32 flags;
>  	u32 scrub_params_out;
>  	u32 scrub_params_in;
> @@ -2839,12 +2839,12 @@ struct acpi_ras2_patrol_scrub_parameter {
>  
>  /* RAS2 Parameter Block Structure for LA2PA_TRANSLATION */
>  
> -struct acpi_ras2_la2pa_translation_parameter {
> -	struct acpi_ras2_parameter_block header;
> -	u16 addr_translation_command;
> +struct acpi_ras2_la2pa_transln_param {
> +	struct acpi_ras2_param_blk header;
> +	u16 cmd;
>  	u64 sub_inst_id;
> -	u64 logical_address;
> -	u64 physical_address;
> +	u64 logical_addr;
> +	u64 phy_addr;
>  	u32 status;
>  };
>  
> @@ -2863,7 +2863,7 @@ enum acpi_ras2_features {
>  
>  /* RAS2 Patrol Scrub Commands */
>  
> -enum acpi_ras2_patrol_scrub_commands {
> +enum acpi_ras2_patrol_scrub_cmds {
>  	ACPI_RAS2_GET_PATROL_PARAMETERS = 1,
>  	ACPI_RAS2_START_PATROL_SCRUBBER = 2,
>  	ACPI_RAS2_STOP_PATROL_SCRUBBER = 3
> @@ -2871,13 +2871,13 @@ enum acpi_ras2_patrol_scrub_commands {
>  
>  /* RAS2 LA2PA Translation Commands */
>  
> -enum acpi_ras2_la2_pa_translation_commands {
> +enum acpi_ras2_la2_pa_transln_cmds {
>  	ACPI_RAS2_GET_LA2PA_TRANSLATION = 1,
>  };
>  
>  /* RAS2 LA2PA Translation Status values */
>  
> -enum acpi_ras2_la2_pa_translation_status {
> +enum acpi_ras2_la2_pa_transln_status {

Do we touch this in the main code?  If not I'd be tempted
to leave decision to shorten this or not to whowever
writes code that uses it.

>  	ACPI_RAS2_LA2PA_TRANSLATION_SUCCESS = 0,
>  	ACPI_RAS2_LA2PA_TRANSLATION_FAIL = 1,
>  };