Re: [PATCH v2 3/4] firmware: tegra: bpmp: Add per-SoC EMC clock ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] firmware: tegra: bpmp: missing emc_clk_id for Tegra186 and Tegra194
--

commit cbaa54ccaf9df29aa946e309eb402aa8387ad5ef
Author: Johnny Liu <johnliu@nvidia.com>

firmware: tegra: bpmp: Add per-SoC EMC clock ID

This commit adds an emc_clk_id field to struct tegra_bpmp_soc and populates
it for Tegra234, Tegra238, and Tegra264 while leaving it unset for Tegra186
and Tegra194.

> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index beeb385870f59..06da888535dc0 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c

[ ... ]

> @@ -1158,14 +1162,17 @@ static const struct tegra_bpmp_soc tegra186_soc = {
>  
>  static const struct tegra_bpmp_soc tegra234_soc = {
>  	TEGRA186_BPMP_SOC_COMMON,
> +	.emc_clk_id = TEGRA234_CLK_EMC,
>  };

[Severity: Medium]
The commit message states that Tegra186 and Tegra194 have no EMC clock ID
and leaves emc_clk_id unset in tegra186_soc. Is that accurate for those two
SoCs?

Both SoCs define BPMP EMC clock IDs in their device tree binding headers:

include/dt-bindings/clock/tegra186-clock.h:
    #define TEGRA186_CLK_EMC 58

include/dt-bindings/clock/tegra194-clock.h:
    #define TEGRA194_CLK_EMC			31

Their external-memory-controller device tree nodes (handled by
drivers/memory/tegra/tegra186-emc.c) also consume those clocks from the BPMP
node:

arch/arm64/boot/dts/nvidia/tegra186.dtsi:
    emc: external-memory-controller@2c60000 {
        compatible = "nvidia,tegra186-emc";
        ...
        clocks = <&bpmp TEGRA186_CLK_EMC>;

arch/arm64/boot/dts/nvidia/tegra194.dtsi:
    emc: external-memory-controller@2c60000 {
        compatible = "nvidia,tegra194-emc";
        ...
        clocks = <&bpmp TEGRA194_CLK_EMC>;

In the follow-up commit ("clk: tegra: bpmp: Do not cache the EMC clock
rate"), tegra_bpmp_clk_register() in drivers/clk/tegra/clk-bpmp.c checks
if (bpmp->soc->emc_clk_id && info->id == bpmp->soc->emc_clk_id) before
setting CLK_GET_RATE_NOCACHE. With emc_clk_id left at 0 in tegra186_soc,
tegra_bpmp_clk_register() will skip setting CLK_GET_RATE_NOCACHE on both
Tegra186 and Tegra194.

In addition, because the BPMP node in arch/arm64/boot/dts/nvidia/tegra194.dtsi
uses compatible = "nvidia,tegra186-bpmp", Tegra186 and Tegra194 currently
share tegra186_soc even though their EMC clock IDs differ (58 vs 31). Should
Tegra186 and Tegra194 also have their EMC clock IDs configured?

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