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

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

[Sumit Gupta]

Add support for representing each memory-controller instance (one
per NUMA node / socket) as its own interconnect (ICC) provider,
with its own MC client nodes, to match the hardware topology on
multi-socket Tegra SoCs.

Use the NUMA node ID to make client IDs globally unique across
per-socket providers, since the ICC framework allocates node IDs
from a single global IDR. Per-socket MC and EMC node names are
also derived from dev_name() so they match the corresponding
debugfs subdirectory. On single-socket platforms (NUMA_NO_NODE)
the existing client IDs and node-name strings are preserved.

Each socket's MC and EMC therefore get their own debugfs
subdirectory under /sys/kernel/debug/{mc,emc}/. Bandwidth requests
from MC clients in a socket are routed to that socket's local
BPMP.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/memory/tegra/mc.c           | 34 ++++++++++++++++++++------
 drivers/memory/tegra/mc.h           | 31 +++++++++++++++++++++++
 drivers/memory/tegra/tegra186-emc.c | 38 ++++++++++++++++++++++-------
 3 files changed, 86 insertions(+), 17 deletions(-)

diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index ec80ea9cc173..7bef758d0049 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -22,6 +22,8 @@
 
 #include "mc.h"
 
+static struct dentry *mc_debugfs_root;
+
 static const struct of_device_id tegra_mc_of_match[] = {
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 	{ .compatible = "nvidia,tegra20-mc-gart", .data = &tegra20_mc_soc },
@@ -778,7 +780,7 @@ struct icc_node *tegra_mc_icc_xlate(const struct of_phandle_args *spec, void *da
 	struct icc_node *node;
 
 	list_for_each_entry(node, &mc->provider.nodes, node_list) {
-		if (node->id == spec->args[0])
+		if (tegra_mc_client_id_from_node(node) == spec->args[0])
 			return node;
 	}
 
@@ -834,6 +836,7 @@ const struct tegra_mc_icc_ops tegra_mc_icc_ops = {
  */
 static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
 {
+	int node_id = dev_to_node(mc->dev);
 	struct icc_node *node;
 	unsigned int i;
 	int err;
@@ -854,31 +857,40 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
 	icc_provider_init(&mc->provider);
 
 	/* create Memory Controller node */
-	node = icc_node_create(TEGRA_ICC_MC);
+	node = tegra_mc_icc_node_create(node_id, TEGRA_ICC_MC);
 	if (IS_ERR(node))
 		return PTR_ERR(node);
 
-	node->name = "Memory Controller";
+	if (node_id == NUMA_NO_NODE)
+		node->name = "Memory Controller";
+	else
+		node->name = dev_name(mc->dev);
+
 	icc_node_add(node, &mc->provider);
 
 	/* link Memory Controller to External Memory Controller */
-	err = icc_link_create(node, TEGRA_ICC_EMC);
+	err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_EMC);
 	if (err)
 		goto remove_nodes;
 
 	for (i = 0; i < mc->soc->num_clients; i++) {
 		/* create MC client node */
-		node = icc_node_create(mc->soc->clients[i].id);
+		node = tegra_mc_icc_node_create(node_id, mc->soc->clients[i].id);
 		if (IS_ERR(node)) {
 			err = PTR_ERR(node);
 			goto remove_nodes;
 		}
 
-		node->name = mc->soc->clients[i].name;
+		if (node_id == NUMA_NO_NODE)
+			node->name = mc->soc->clients[i].name;
+		else
+			node->name = devm_kasprintf(mc->dev, GFP_KERNEL, "%d-%s",
+						    node_id, mc->soc->clients[i].name);
+
 		icc_node_add(node, &mc->provider);
 
 		/* link Memory Client to Memory Controller */
-		err = icc_link_create(node, TEGRA_ICC_MC);
+		err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_MC);
 		if (err)
 			goto remove_nodes;
 
@@ -957,7 +969,13 @@ static int tegra_mc_probe(struct platform_device *pdev)
 	if (IS_ERR(mc->regs))
 		return PTR_ERR(mc->regs);
 
-	mc->debugfs.root = debugfs_create_dir("mc", NULL);
+	if (!mc_debugfs_root)
+		mc_debugfs_root = debugfs_create_dir("mc", NULL);
+
+	if (dev_to_node(mc->dev) == NUMA_NO_NODE)
+		mc->debugfs.root = mc_debugfs_root;
+	else
+		mc->debugfs.root = debugfs_create_dir(dev_name(mc->dev), mc_debugfs_root);
 
 	if (mc->soc->ops && mc->soc->ops->probe) {
 		err = mc->soc->ops->probe(mc);
diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
index e94d265d7b67..be6ec0f63f59 100644
--- a/drivers/memory/tegra/mc.h
+++ b/drivers/memory/tegra/mc.h
@@ -8,6 +8,7 @@
 
 #include <linux/bits.h>
 #include <linux/io.h>
+#include <linux/numa.h>
 #include <linux/types.h>
 
 #include <soc/tegra/mc.h>
@@ -167,6 +168,36 @@ icc_provider_to_tegra_mc(struct icc_provider *provider)
 	return container_of(provider, struct tegra_mc, provider);
 }
 
+/*
+ * Compose a globally-unique ICC node ID. On single-socket
+ * systems (NUMA_NO_NODE), the SoC client ID is returned unchanged.
+ * On multi-socket systems, the NUMA node ID is encoded in the
+ * upper bits of the returned ID.
+ */
+static inline u32 tegra_mc_get_client_id(int node_id, int id)
+{
+	if (node_id == NUMA_NO_NODE)
+		return id;
+
+	return ((node_id + 1) << 16) | id;
+}
+
+static inline struct icc_node *tegra_mc_icc_node_create(int node_id, int id)
+{
+	return icc_node_create(tegra_mc_get_client_id(node_id, id));
+}
+
+static inline int tegra_mc_icc_link_create(struct icc_node *node, int node_id, int id)
+{
+	return icc_link_create(node, tegra_mc_get_client_id(node_id, id));
+}
+
+/* Return the SoC client ID encoded in an ICC node ID. */
+static inline u32 tegra_mc_client_id_from_node(const struct icc_node *node)
+{
+	return node->id & GENMASK(15, 0);
+}
+
 static inline u32 mc_ch_readl(const struct tegra_mc *mc, int ch,
 			      unsigned long offset)
 {
diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
index 03ebab6fbe68..e02f4d0229b9 100644
--- a/drivers/memory/tegra/tegra186-emc.c
+++ b/drivers/memory/tegra/tegra186-emc.c
@@ -13,6 +13,8 @@
 #include <soc/tegra/bpmp.h>
 #include "mc.h"
 
+static struct dentry *emc_debugfs_root;
+
 struct tegra186_emc_dvfs {
 	unsigned long latency;
 	unsigned long rate;
@@ -207,7 +209,14 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
 		return err;
 	}
 
-	emc->debugfs.root = debugfs_create_dir("emc", NULL);
+	if (!emc_debugfs_root)
+		emc_debugfs_root = debugfs_create_dir("emc", NULL);
+
+	if (dev_to_node(emc->dev) == NUMA_NO_NODE)
+		emc->debugfs.root = emc_debugfs_root;
+	else
+		emc->debugfs.root = debugfs_create_dir(dev_name(emc->dev), emc_debugfs_root);
+
 	debugfs_create_file("available_rates", 0444, emc->debugfs.root, emc,
 			    &tegra186_emc_debug_available_rates_fops);
 	debugfs_create_file("min_rate", 0644, emc->debugfs.root, emc,
@@ -239,7 +248,7 @@ tegra186_emc_of_icc_xlate(const struct of_phandle_args *spec, void *data)
 
 	/* External Memory is the only possible ICC route */
 	list_for_each_entry(node, &provider->nodes, node_list) {
-		if (node->id != TEGRA_ICC_EMEM)
+		if (tegra_mc_client_id_from_node(node) != TEGRA_ICC_EMEM)
 			continue;
 
 		return node;
@@ -260,6 +269,7 @@ static int tegra186_emc_interconnect_init(struct tegra186_emc *emc)
 {
 	struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
 	const struct tegra_mc_soc *soc = mc->soc;
+	int node_id = dev_to_node(mc->dev);
 	struct icc_node *node;
 	int err;
 
@@ -273,26 +283,36 @@ static int tegra186_emc_interconnect_init(struct tegra186_emc *emc)
 	icc_provider_init(&emc->provider);
 
 	/* create External Memory Controller node */
-	node = icc_node_create(TEGRA_ICC_EMC);
-	if (IS_ERR(node))
-		return PTR_ERR(node);
+	node = tegra_mc_icc_node_create(node_id, TEGRA_ICC_EMC);
+	if (IS_ERR(node)) {
+		err = PTR_ERR(node);
+		goto remove_nodes;
+	}
+
+	if (node_id == NUMA_NO_NODE)
+		node->name = "External Memory Controller";
+	else
+		node->name = dev_name(emc->dev);
 
-	node->name = "External Memory Controller";
 	icc_node_add(node, &emc->provider);
 
 	/* link External Memory Controller to External Memory (DRAM) */
-	err = icc_link_create(node, TEGRA_ICC_EMEM);
+	err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_EMEM);
 	if (err)
 		goto remove_nodes;
 
 	/* create External Memory node */
-	node = icc_node_create(TEGRA_ICC_EMEM);
+	node = tegra_mc_icc_node_create(node_id, TEGRA_ICC_EMEM);
 	if (IS_ERR(node)) {
 		err = PTR_ERR(node);
 		goto remove_nodes;
 	}
 
-	node->name = "External Memory (DRAM)";
+	if (node_id == NUMA_NO_NODE)
+		node->name = "External Memory (DRAM)";
+	else
+		node->name = devm_kasprintf(emc->dev, GFP_KERNEL, "%d-dram", node_id);
+
 	icc_node_add(node, &emc->provider);
 
 	err = icc_provider_register(&emc->provider);
-- 
2.34.1

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

[Krzysztof Kozlowski]

On 21/05/2026 16:05, Sumit Gupta wrote:
> -		err = icc_link_create(node, TEGRA_ICC_MC);
> +		err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_MC);
>  		if (err)
>  			goto remove_nodes;
>  
> @@ -957,7 +969,13 @@ static int tegra_mc_probe(struct platform_device *pdev)
>  	if (IS_ERR(mc->regs))
>  		return PTR_ERR(mc->regs);
>  
> -	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.

> +		mc_debugfs_root = debugfs_create_dir("mc", NULL);
> +
> +	if (dev_to_node(mc->dev) == NUMA_NO_NODE)
> +		mc->debugfs.root = mc_debugfs_root;
> +	else
> +		mc->debugfs.root = debugfs_create_dir(dev_name(mc->dev), mc_debugfs_root);
>  


Best regards,
Krzysztof

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

[Sumit Gupta]

On 27/05/26 18:25, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 21/05/2026 16:05, Sumit Gupta wrote:
>> -             err = icc_link_create(node, TEGRA_ICC_MC);
>> +             err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_MC);
>>                if (err)
>>                        goto remove_nodes;
>>
>> @@ -957,7 +969,13 @@ static int tegra_mc_probe(struct platform_device *pdev)
>>        if (IS_ERR(mc->regs))
>>                return PTR_ERR(mc->regs);
>>
>> -     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);

Thank you,
Sumit Gupta


>
>> +             mc_debugfs_root = debugfs_create_dir("mc", NULL);
>> +
>> +     if (dev_to_node(mc->dev) == NUMA_NO_NODE)
>> +             mc->debugfs.root = mc_debugfs_root;
>> +     else
>> +             mc->debugfs.root = debugfs_create_dir(dev_name(mc->dev), mc_debugfs_root);
>>
>
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 27/05/2026 16:21, Sumit Gupta wrote:
> 
> On 27/05/26 18:25, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 21/05/2026 16:05, Sumit Gupta wrote:
>>> -             err = icc_link_create(node, TEGRA_ICC_MC);
>>> +             err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_MC);
>>>                if (err)
>>>                        goto remove_nodes;
>>>
>>> @@ -957,7 +969,13 @@ static int tegra_mc_probe(struct platform_device *pdev)
>>>        if (IS_ERR(mc->regs))
>>>                return PTR_ERR(mc->regs);
>>>
>>> -     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.



Best regards,
Krzysztof

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

On Wed, May 27, 2026 at 04:46:38PM +0200, Krzysztof Kozlowski wrote:
> On 27/05/2026 16:21, Sumit Gupta wrote:
> > 
> > On 27/05/26 18:25, Krzysztof Kozlowski wrote:
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 21/05/2026 16:05, Sumit Gupta wrote:
> >>> -             err = icc_link_create(node, TEGRA_ICC_MC);
> >>> +             err = tegra_mc_icc_link_create(node, node_id, TEGRA_ICC_MC);
> >>>                if (err)
> >>>                        goto remove_nodes;
> >>>
> >>> @@ -957,7 +969,13 @@ static int tegra_mc_probe(struct platform_device *pdev)
> >>>        if (IS_ERR(mc->regs))
> >>>                return PTR_ERR(mc->regs);
> >>>
> >>> -     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. Granted, that means we
can't use any of those helper macros, but we don't use that for the MC
driver anyway.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Krzysztof Kozlowski]

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.

> Granted, that means we
> can't use any of those helper macros, but we don't use that for the MC
> driver anyway.
> 
> Thierry


Best regards,
Krzysztof

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

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

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4192 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Sumit Gupta]

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