Re: [PATCH v2 3/3] memory: tegra264: Add full set of MC clients

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Newly added BPMP-managed memory clients are missing their `.bpmp_id` and `.type` interconnect properties.
--

commit 1230942ec14ede13ea86a9f95a6ff94371903b6d
Author: Sumit Gupta <sumitg@nvidia.com>

memory: tegra264: Add full set of MC clients

This commit extends the tegra264_mc_clients table to include the full set of
memory clients exposed by the SoC. The commit message notes that clients
managed by the BPMP bandwidth manager additionally carry their bpmp_id and
type.

> diff --git a/drivers/memory/tegra/tegra264.c b/drivers/memory/tegra/tegra264.c
> index e628908417255..b8d6c4ce48369 100644
> --- a/drivers/memory/tegra/tegra264.c
> +++ b/drivers/memory/tegra/tegra264.c
[ ... ]
> @@ -21,45 +21,62 @@
>   */
>  static const struct tegra_mc_client tegra264_mc_clients[] = {
[ ... ]
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_MGBE2R,
> +		.name = "mgbe2r",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_OFAR2MC,
> +		.name = "ofar2mc",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_OFAW2MC,
> +		.name = "ofaw2mc",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_MGBE2W,
> +		.name = "mgbe2w",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_MGBE3R,
> +		.name = "mgbe3r",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_MGBE3W,
> +		.name = "mgbe3w",

Are the new MGBE2 and MGBE3 clients missing their interconnect properties?

Looking at the other EQOS clients like MGBE0R and MGBE1R, they correctly
initialize .bpmp_id = TEGRA264_BWMGR_EQOS and .type = TEGRA_ICC_NISO. If
these fields are omitted for MGBE2 and MGBE3, won't interconnect bandwidth
scaling requests for these blocks be silently ignored, potentially causing
hardware bandwidth starvation?

[ ... ]
> @@ -70,6 +87,15 @@ static const struct tegra_mc_client tegra264_mc_clients[] = {
>  		.name = "vicw",
>  		.bpmp_id = TEGRA264_BWMGR_VIC,
>  		.type = TEGRA_ICC_NISO,
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_VIW,
> +		.name = "viw",

Does this missing bpmp_id issue also apply to other BPMP-managed
subsystems added in this patch, such as VIW?

[ ... ]
> @@ -110,6 +208,12 @@ static const struct tegra_mc_client tegra264_mc_clients[] = {
>  		.name = "rcew",
>  		.bpmp_id = TEGRA264_BWMGR_RCE,
>  		.type = TEGRA_ICC_NISO,
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_NVENC1SRD2MC,
> +		.name = "nvenc1srd2mc",
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_NVENC1SWR2MC,
> +		.name = "nvenc1swr2mc",

Are these newly added NVENC clients also missing their TEGRA264_BWMGR_NVENC
manager definitions?

Since the memory controller interconnect implementation skips bandwidth
scaling requests for clients without a bpmp_id, should these clients also
include their respective bpmp_id and type to ensure bandwidth requests
are processed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518124306.2071481-1-sumitg@nvidia.com?part=3