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

[Thierry Reding]

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.

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().

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

Thierry

Attachment: signature.asc
Description: PGP signature