Re: [PATCH v4 1/3] RAS: Introduce AMD Address Translation Library

[Markus Elfring <Markus.Elfring@xxxxxx>]

…
> +++ b/drivers/ras/amd/atl/map.c
> @@ -0,0 +1,667 @@
…
> +static int get_coh_st_fabric_id(struct addr_ctx *ctx)
> +{
…
> +	if (df_cfg.rev < DF4p5)
> +		ctx->coh_st_fabric_id = FIELD_GET(DF2_COH_ST_FABRIC_ID, reg);
> +	else
> +		ctx->coh_st_fabric_id = FIELD_GET(DF4p5_COH_ST_FABRIC_ID, reg);
…
> +}
…

Is the following statement variant a bit nicer?

	ctx->coh_st_fabric_id = FIELD_GET((df_cfg.rev < DF4p5)
					  ? DF2_COH_ST_FABRIC_ID
					  : DF4p5_COH_ST_FABRIC_ID,
					  reg);



> +static bool valid_map(struct addr_ctx *ctx)
> +{
> +	if (df_cfg.rev >= DF4)
> +		return FIELD_GET(DF_ADDR_RANGE_VAL, ctx->map.ctl);
> +	else
> +		return FIELD_GET(DF_ADDR_RANGE_VAL, ctx->map.base);
> +}
…

Can the following statement variant be integrated instead?

	return FIELD_GET(DF_ADDR_RANGE_VAL, (df_cfg.rev >= DF4) ? ctx->map.ctl : ctx->map.base);



> +int get_address_map(struct addr_ctx *ctx)
> +{
> +	int ret = 0;
> +
> +	ret = get_address_map_common(ctx);
> +	if (ret)
> +		goto out;
> +
> +	ret = get_global_map_data(ctx);
> +	if (ret)
> +		goto out;
> +
> +	dump_address_map(&ctx->map);
> +
> +out:
> +	return ret;
> +}
…

Would you like to use a function implementation like the following instead?

int get_address_map(struct addr_ctx *ctx)
{
	int ret = get_address_map_common(ctx);

	if (ret)
		return ret;

	ret = get_global_map_data(ctx);
	if (ret)
		return ret;

	dump_address_map(&ctx->map);
	return 0;
}


See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.7-rc6#n532

Regards,
Markus