Re: [PATCH 1/2] memory: tegra: Add T238 MC support

Ashish Mhetre
2026-04-06T07:21:03Z

On 3/31/2026 5:08 PM, Jon Hunter wrote:
>
>
> On 31/03/2026 12:23, Ashish Mhetre wrote:
>> Add Memory Controller driver support for Tegra238 SOC, including:
>> - MC client definitions with Tegra238-specific stream IDs
>> - Reuse of Tegra234 ICC operations for bandwidth management via BPMP-FW
>> - Device tree compatible string "nvidia,tegra238-mc"
>>
>> Export tegra234_mc_icc_ops so it can be shared with the Tegra238 MC
>> driver, as both SoCs use the same ICC aggregation and bandwidth
>> management logic.
>>
>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
>> ---
>>   drivers/memory/tegra/Makefile   |   1 +
>>   drivers/memory/tegra/mc.c       |   3 +
>>   drivers/memory/tegra/mc.h       |   6 +
>>   drivers/memory/tegra/tegra234.c |   2 +-
>>   drivers/memory/tegra/tegra238.c | 395 ++++++++++++++++++++++++++++++++
>>   5 files changed, 406 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/memory/tegra/tegra238.c
>>
>> diff --git a/drivers/memory/tegra/Makefile
>> b/drivers/memory/tegra/Makefile
>> index 6334601e6120..0d50e37d43af 100644
>> --- a/drivers/memory/tegra/Makefile
>> +++ b/drivers/memory/tegra/Makefile
>> @@ -10,6 +10,7 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
>>   tegra-mc-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
>>   tegra-mc-$(CONFIG_ARCH_TEGRA_194_SOC) += tegra186.o tegra194.o
>>   tegra-mc-$(CONFIG_ARCH_TEGRA_234_SOC) += tegra186.o tegra234.o
>> +tegra-mc-$(CONFIG_ARCH_TEGRA_238_SOC) += tegra186.o tegra238.o
>>   tegra-mc-$(CONFIG_ARCH_TEGRA_264_SOC) += tegra186.o tegra264.o
>>     obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>> index d620660da331..10ef3c323e22 100644
>> --- a/drivers/memory/tegra/mc.c
>> +++ b/drivers/memory/tegra/mc.c
>> @@ -49,6 +49,9 @@ static const struct of_device_id
>> tegra_mc_of_match[] = {
>>   #ifdef CONFIG_ARCH_TEGRA_234_SOC
>>       { .compatible = "nvidia,tegra234-mc", .data = &tegra234_mc_soc },
>>   #endif
>> +#ifdef CONFIG_ARCH_TEGRA_238_SOC
>> +    { .compatible = "nvidia,tegra238-mc", .data = &tegra238_mc_soc },
>> +#endif
>
> It is always better/preferred for the dt-binding patch to be 1st in
> the series. The above does not exist until after patch 2 is applied.
>

Ack, I will resend with correct ordering.

>>   #ifdef CONFIG_ARCH_TEGRA_264_SOC
>>       { .compatible = "nvidia,tegra264-mc", .data = &tegra264_mc_soc },
>>   #endif
>> diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
>> index 649b54369263..d0da4a5f192d 100644
>> --- a/drivers/memory/tegra/mc.h
>> +++ b/drivers/memory/tegra/mc.h
>> @@ -238,6 +238,11 @@ extern const struct tegra_mc_soc tegra194_mc_soc;
>>     #ifdef CONFIG_ARCH_TEGRA_234_SOC
>>   extern const struct tegra_mc_soc tegra234_mc_soc;
>> +extern const struct tegra_mc_icc_ops tegra234_mc_icc_ops;
>> +#endif
>> +
>> +#ifdef CONFIG_ARCH_TEGRA_238_SOC
>> +extern const struct tegra_mc_soc tegra238_mc_soc;
>>   #endif
>
> Does this work? Tegra238 is dependent upon stuff in Tegra234, but
> there is no guarantee that both of these CONFIG options are always
> enabled?
>

Good point, thanks for catching this Jon. If CONFIG_ARCH_TEGRA_238_SOC
is enabled without CONFIG_ARCH_TEGRA_234_SOC, the build would fail
since tegra234_mc_icc_ops wouldn't be compiled or declared.
I'll fix this in V2 with combined Tegra234 and Tegra238 SOC guard for
struct.

>>     #ifdef CONFIG_ARCH_TEGRA_264_SOC
>> @@ -256,6 +261,7 @@ extern const struct tegra_mc_ops tegra30_mc_ops;
>>   #if defined(CONFIG_ARCH_TEGRA_186_SOC) || \
>>       defined(CONFIG_ARCH_TEGRA_194_SOC) || \
>>       defined(CONFIG_ARCH_TEGRA_234_SOC) || \
>> +    defined(CONFIG_ARCH_TEGRA_238_SOC) || \
>>       defined(CONFIG_ARCH_TEGRA_264_SOC)
>>   extern const struct tegra_mc_ops tegra186_mc_ops;
>>   #endif
>> diff --git a/drivers/memory/tegra/tegra234.c
>> b/drivers/memory/tegra/tegra234.c
>> index 87b22038a5fb..9fbd34d4abe0 100644
>> --- a/drivers/memory/tegra/tegra234.c
>> +++ b/drivers/memory/tegra/tegra234.c
>> @@ -1125,7 +1125,7 @@ static int tegra234_mc_icc_get_init_bw(struct
>> icc_node *node, u32 *avg, u32 *pea
>>       return 0;
>>   }
>>   -static const struct tegra_mc_icc_ops tegra234_mc_icc_ops = {
>> +const struct tegra_mc_icc_ops tegra234_mc_icc_ops = {
>>       .xlate = tegra_mc_icc_xlate,
>>       .aggregate = tegra234_mc_icc_aggregate,
>>       .get_bw = tegra234_mc_icc_get_init_bw,
>> diff --git a/drivers/memory/tegra/tegra238.c
>> b/drivers/memory/tegra/tegra238.c
>> new file mode 100644
>> index 000000000000..5abdca16a275
>> --- /dev/null
>> +++ b/drivers/memory/tegra/tegra238.c
>> @@ -0,0 +1,395 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2026, NVIDIA CORPORATION.  All rights reserved.
>> + */
>
> ...
>
>> +const struct tegra_mc_soc tegra238_mc_soc = {
>> +    .num_clients = ARRAY_SIZE(tegra238_mc_clients),
>> +    .clients = tegra238_mc_clients,
>> +    .num_address_bits = 40,
>> +    .num_channels = 8,
>> +    .client_id_mask = 0x1ff,
>> +    .intmasks = tegra238_mc_intmasks,
>> +    .num_intmasks = ARRAY_SIZE(tegra238_mc_intmasks),
>> +    .has_addr_hi_reg = true,
>> +    .ops = &tegra186_mc_ops,
>> +    .icc_ops = &tegra234_mc_icc_ops,
>> +    .ch_intmask = 0x0000ff00,
>> +    .global_intstatus_channel_shift = 8,
>> +    /*
>> +     * Additionally, there are lite carveouts but those are not
>> currently
>> +     * supported.
>> +     */
>
> I don't know what this means?
>

I have kept this comment similar to Tegra234. tegra_mc_get_carveout_info()
function uses num_carveouts variable as upper limit for supported carveouts.
On top of it, there are few lite carveouts which are supported by SOC
but are
not used by the driver.
It's redundant info IMO and can be removed from both Tegra234 and Tegra238.
Jon, Can you please share your thoughts on this?

Thanks,
Ashish Mhetre

>> +    .num_carveouts = 32,
>> +    .regs = &tegra20_mc_regs,
>> +    .handle_irq = tegra30_mc_irq_handlers,
>> +    .num_interrupts = ARRAY_SIZE(tegra30_mc_irq_handlers),
>> +    .mc_addr_hi_mask = 0x3,
>> +    .mc_err_status_type_mask = (0x7 << 28),
>> +};
>
> Jon
>