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

[Sumit Gupta]

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?

Thank you,
Sumit Gupta