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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Newly added BPMP-managed memory clients are missing their `.bpmp_id` and `.type` initializations.
- [Medium] The `tegra264_mc_clients` array is not strictly sorted by client ID, contradicting the commit message.
--

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

memory: tegra264: Add full set of MC clients

This commit extends the Tegra264 MC dt-bindings header and
tegra264_mc_clients table to cover the full set of memory clients exposed by
the SoC.

> Extend the Tegra264 MC dt-bindings header and tegra264_mc_clients
> table to cover the full set of memory clients exposed by the SoC.
> Client name is used for MC fault reporting. Clients managed by the
> bandwidth manager in BPMP additionally carry their bpmp_id and type.

The commit message states that clients managed by the bandwidth manager in
BPMP carry their bpmp_id and type. Should these fields be initialized for the
newly added clients like NVENC, NVJPG, ISP, PVA, XUSB, and TSEC?

> diff --git a/drivers/memory/tegra/tegra264.c b/drivers/memory/tegra/tegra264.c
> --- a/drivers/memory/tegra/tegra264.c
> +++ b/drivers/memory/tegra/tegra264.c
[ ... ]
> @@ -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",
>  	}, {
>  		.id = TEGRA264_MEMORY_CLIENT_PCIE0W,
>  		.name = "pcie0w",

If the bpmp_id and type fields are omitted here, they default to 0. Would
this cause drivers for these components to inadvertently pass 0 as the client
ID to the BPMP firmware when attempting to configure interconnect bandwidth?

> Entries in tegra264_mc_clients[] are sorted in increasing order of
> their client IDs, which matches the order of the override and
> security register offsets used in previous SoCs.

This isn't a functional bug, but are these entries strictly sorted by their
client IDs as the commit message indicates?

[ ... ]
> @@ -185,6 +289,402 @@ static const struct tegra_mc_client tegra264_mc_clients[] = {
>  		.name = "nvdecswr2mc",
>  		.bpmp_id = TEGRA264_BWMGR_NVDEC,
>  		.type = TEGRA_ICC_NISO,
> +	}, {
> +		.id = TEGRA264_MEMORY_CLIENT_MIU5R,
> +		.name = "miu5r",
> +	}, {

Looking at the tegra264_mc_clients array, GPUR02MC (0xb6), GPUW02MC (0xb7),
NVDECSRD2MC (0x78), and NVDECSWR2MC (0x79) appear immediately after
PCIE5W (0xe3), and the newly added MIU5R (0xfc) is appended after them.

Could this mismatch between the array's actual order and the commit
message's claim cause confusion during future maintenance?

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