Re: [PATCH] memory: tegra: add multi-socket support to the memory interconnect

[Sumit Gupta <sumitg@nvidia.com>]

On 29/05/26 20:53, Thierry Reding wrote:
> On Fri, May 29, 2026 at 02:55:35PM +0530, Sumit Gupta wrote:
>> On 28/05/26 18:35, Thierry Reding wrote:
>>> On Thu, May 28, 2026 at 02:20:07PM +0200, Krzysztof Kozlowski wrote:
>>>> On 28/05/2026 13:56, Thierry Reding wrote:
>>>>>>>>> -     mc->debugfs.root = debugfs_create_dir("mc", NULL);
>>>>>>>>> +     if (!mc_debugfs_root)
>>>>>>>> That's a probe path and you created a singletone. Looks like preventing
>>>>>>>> async probing for no real reason.
>>>>>>>>
>>>>>>>> I am very against singletons and debugfs does not look like justified
>>>>>>>> exception.
>>>>>>> The singleton was added so multi-socket MC/EMC instances could
>>>>>>> share a "mc"/"emc" parent. I'll drop it in v2.
>>>>>>>
>>>>>>> On single-socket SoCs, the "mc"/"emc" names will be unchanged.
>>>>>>> On multi-socket SoCs, each instance will create a top-level debugfs
>>>>>>> dir named with dev_name(). Same pattern in tegra186-emc.c.
>>>>>>>
>>>>>>>      if (dev_to_node(mc->dev) == NUMA_NO_NODE)
>>>>>>>          mc->debugfs.root = debugfs_create_dir("mc", NULL);
>>>>>>>      else
>>>>>>>          mc->debugfs.root = debugfs_create_dir(dev_name(mc->dev), NULL);
>>>>>> You assume this is fully synced, so you as well could do a look up and
>>>>>> then use what you found or create new dir. If you think that is racy, so
>>>>>> is this approach... How are other drivers handling per-device debugfs
>>>>>> directories? Do they also create such in the top-level? I think no.
>>>>> I think we want a top-level directory for a bit more structure in
>>>>> debugfs. But I also think we want to create that top-level directory in
>>>>> the module's init function rather than _probe.
>>>> I was thinking about this as well but that would mean your driver will
>>>> create it on every multi-arch kernel.
>>>>
>>>> This should be then moved to some core bus (and there are examples of
>>>> that, e.g. USB), except there is no core-MC bus code to do that.
>>> We have a utility function (soc_is_tegra()) that we've used in similar
>>> situations in the past. We haven't used them in a little while, but it
>>> could be useful here. It's not for free, but should be fairly quick to
>>> error out early on multi-arch kernels.
>>>
>>> Thierry
>> soc_is_tegra()'s match table currently has entries up to Tegra210
>> (seems only used by a legacy 32-bit ARM path), so it would skip
>> the SoCs this patch targets (Tegra186+).
>> Could we follow tegra_init_soc() in fuse-tegra.c. Only create the
>> "mc"/"emc" parent at module init when a matching DT node is present:
>>
>>      static int __init tegra_mc_init(void)
>>      {
>>          struct device_node *np;
>>
>>          np = of_find_matching_node(NULL, tegra_mc_of_match);
>>          if (np) {
>>              tegra_mc_debugfs_root = debugfs_create_dir("mc", NULL);
>>              of_node_put(np);
>>          }
>>
>>          return platform_driver_register(&tegra_mc_driver);
>>      }
>>      arch_initcall(tegra_mc_init);
>>
>> Each probe just creates its per-device child under that parent
>> without touching any shared state. Same in tegra186-emc.c.
>> Or would you prefer a different approach, e.g. extending the
>> soc_is_tegra() match table for arm64 SoCs, before I respin?
> of_find_matching_node() has the big downside that it scans the entire
> device tree to find matching nodes, which can be quite expensive. The
> soc_is_tegra() helper uses of_machine_device_match() which only checks
> the compatible string on the root node and therefore is much faster.

Good point. Will skip that approach.

>
> Jon is working on a different solution for a similar case for BPMP, so
> maybe we can also look at doing something similar for MC. It's a bit
> involved because it adds an extra mutex to make sure we don't end up
> racing the creation of the parent directory, but it has the advantage
> that it's only ever triggered when really needed and doesn't need any
> "tricks" like soc_is_tegra().

Sure, will do a similar change as Jon's v3 [1] for debugfs.
[1] 
https://lore.kernel.org/linux-tegra/20260529173337.155722-2-jonathanh@nvidia.com/

>
> I wonder if maybe a helper could be extracted from that to make this a
> bit easier to replicate elsewhere.
>
> Thierry

Will do it as a follow up if we have more users.

Thank you,
Sumit Gupta