Received-SPF: pass (google.com: domain of linux-kernel+bounces-7349-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <b472d62b-47f9-428b-b5a9-9b3c526f487c@web.de>
Date: Wed, 20 Dec 2023 18:28:07 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Yazen Ghannam <yazen.ghannam@amd.com>, Borislav Petkov <bp@alien8.de>,
 linux-edac@vger.kernel.org, kernel-janitors@vger.kernel.org
Cc: LKML <linux-kernel@vger.kernel.org>, Avadhut Naik <avadhut.naik@amd.com>,
 Christophe Jaillet <christophe.jaillet@wanadoo.fr>,
 John Allen <john.allen@amd.com>, Muralidhara M K <muralidhara.mk@amd.com>,
 Tony Luck <tony.luck@intel.com>, William Roche <william.roche@oracle.com>
References: <20231218190406.27479-2-yazen.ghannam@amd.com>
Subject: Re: [PATCH v4 1/3] RAS: Introduce AMD Address Translation Library
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20231218190406.27479-2-yazen.ghannam@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:BZhrZ0n9XNQ=;orYPngzetXKwht6bs8pgPrNY/DX
 Y8iC3CaY4LFN12PIPDORRoVi12WvG6+MJj0+PJ1meq4yw0Q7FVhJV9TLF5Xwgxsln82R9mUCC
 qdEqXger5KJyNIudna1WPlBVMOeNdptednZOcT01J8wE1uuud7Jp9bC5q5T8d6E6SkCpCdVGx
 5ztLaX71l0vkgICJTsBdXB8cjFpMijpHUSq/Gqb+SyTN4ZmsNzKnU9I+hh6C0ZtROoOhWoEXD
 yZYAXrI6V4ioHKm7j8beZ/HFs+c4mnvt6lmpIm3yOiiozN6HDmS6jdQxQ7sZrXu+UluQ6D3CK
 affCPmCd3B9kPXm0eiyCnYNe8uqC5KOlS/Fz6j8PUfjVV7F7NYKY1TYhxfcko4U747PoUAJ5R
 1jRlWXFNZO8jLUlLik2fbx6uHaNaYGuUfXU+/xLsUburG6ZeY1V0gguoNgKh7vHjLhMc9Tq+d
 nYW9v1Gm2/BFOsEy+Eq6PuyYKiCJpdrpPUM4fwYQASUAESTjSTnA/8Hlm3pyluHAjbn8ruK4t
 0DR4rZxGZc+IxrjG4mVmGNtukNa65cIgjkHgFhzNrWLkQNOMmlLqLCHzCDw7/njIQRH8Si8hM
 Iu+Cm5LBVN7JcVB0MqnhP9daW15uUsEK6TYaU1WgmItx1VSkvuLsudPiOl1uRgg+6NbCIgT81
 gJAVE9CrOvnKzJNO0q0obdVXlqMNZ/wgMuduM/Dl0Y+JWUuRnGGu/yyZCXvKNDv+QmRMuhr7N
 qhfVdfk7sC1ib42IqZHx2SZRYRe0fsF5+cyFOLyhAHCg8CCPpN4ZUpc53dCNeJa3oC/gVhXqG
 ko1+1HkNq7NIOX2DIT5NYBlFQgKLXf1qCrjylPxkOkKOSSUYZBawwk/QVNw4nXXlMJSCFygfq
 5o0WWUAPf6Jzq3OYlEeMVUfoVpj1v6KwVcCfvT2PZF25RNMbrA4sg7L1i6dQzBw+kNgEJR92J
 e566SLzWy+ZQ1DJfm8W4w76Ri/o=

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

Is the following statement variant a bit nicer?

	ctx->coh_st_fabric_id =3D 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 >=3D DF4)
> +		return FIELD_GET(DF_ADDR_RANGE_VAL, ctx->map.ctl);
> +	else
> +		return FIELD_GET(DF_ADDR_RANGE_VAL, ctx->map.base);
> +}
=E2=80=A6

Can the following statement variant be integrated instead?

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


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

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

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

	if (ret)
		return ret;

	ret =3D 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/Do=
cumentation/process/coding-style.rst?h=3Dv6.7-rc6#n532

Regards,
Markus