Tegra234 Memory interconnect support

Add Interconnect framework support to dynamically set the DRAM bandwidth from different clients. Both the MC and EMC drivers are added as ICC providers. The path for any request is:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM

drivers/memory/tegra/tegra186-emc.c +66 -0

+static int tegra_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;
+	struct icc_node *node;
+	int err;
+
+	emc->provider.dev = emc->dev;
+	emc->provider.set = tegra_emc_icc_set_bw;
+	emc->provider.data = &emc->provider;
+	emc->provider.aggregate = soc->icc_ops->aggregate;
+	emc->provider.xlate = tegra_emc_of_icc_xlate;
+	emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+	icc_provider_init(&emc->provider);
+
+	/* create External Memory Controller node */
+	node = icc_node_create(TEGRA_ICC_EMC);
+	if (IS_ERR(node)) {
+		err = PTR_ERR(node);
+		goto err_msg;
+	}
+
+	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);
+	if (err)
+		goto remove_nodes;
+
+	/* create External Memory node */
+	node = icc_node_create(TEGRA_ICC_EMEM);
+	if (IS_ERR(node)) {
+		err = PTR_ERR(node);
+		goto remove_nodes;
+	}
+
+	node->name = "External Memory (DRAM)";
+	icc_node_add(node, &emc->provider);
+
+	err = icc_provider_register(&emc->provider);
+	if (err)
+		goto remove_nodes;
+
+	return 0;
+
+remove_nodes:
+	icc_nodes_remove(&emc->provider);
+err_msg:
+	dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+	return err;
+}
+
 static int tegra186_emc_probe(struct platform_device *pdev)
 {
+	struct tegra_mc *mc = dev_get_drvdata(pdev->dev.parent);
 	struct mrq_emc_dvfs_latency_response response;
 	struct tegra_bpmp_message msg;
 	struct tegra186_emc *emc;

@@ -236,6 +339,29 @@ static int tegra186_emc_probe(struct platform_device *pdev)
 	debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
 			    emc, &tegra186_emc_debug_max_rate_fops);
 
+	if (mc && mc->soc->icc_ops) {
+		/*
+		 * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
+		 * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
+		 * EINVAL instead of passing the request to BPMP-FW later when the BW
+		 * request is made by client with 'icc_set_bw()' call.
+		 */
+		err = tegra_emc_interconnect_init(emc);
+		if (err)
+			goto put_bpmp;

External email: Use caution opening links or attachments

Quoted 5 lines · Sumit Gupta

Add Interconnect framework support to dynamically set the DRAM bandwidth from different clients. Both the MC and EMC drivers are added as ICC providers. The path for any request is:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM

Thank you for your patch. There is something to discuss/improve.

Quoted 75 lines

drivers/memory/tegra/tegra186-emc.c +66 -0

+static int tegra_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;
+     struct icc_node *node;
+     int err;
+
+     emc->provider.dev = emc->dev;
+     emc->provider.set = tegra_emc_icc_set_bw;
+     emc->provider.data = &emc->provider;
+     emc->provider.aggregate = soc->icc_ops->aggregate;
+     emc->provider.xlate = tegra_emc_of_icc_xlate;
+     emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+     icc_provider_init(&emc->provider);
+
+     /* create External Memory Controller node */
+     node = icc_node_create(TEGRA_ICC_EMC);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto err_msg;
+     }
+
+     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);
+     if (err)
+             goto remove_nodes;
+
+     /* create External Memory node */
+     node = icc_node_create(TEGRA_ICC_EMEM);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto remove_nodes;
+     }
+
+     node->name = "External Memory (DRAM)";
+     icc_node_add(node, &emc->provider);
+
+     err = icc_provider_register(&emc->provider);
+     if (err)
+             goto remove_nodes;
+
+     return 0;
+
+remove_nodes:
+     icc_nodes_remove(&emc->provider);
+err_msg:
+     dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+     return err;
+}
+
  static int tegra186_emc_probe(struct platform_device *pdev)
  {
+     struct tegra_mc *mc = dev_get_drvdata(pdev->dev.parent);
       struct mrq_emc_dvfs_latency_response response;
       struct tegra_bpmp_message msg;
       struct tegra186_emc *emc;

@@ -236,6 +339,29 @@ static int tegra186_emc_probe(struct platform_device *pdev)
       debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
                           emc, &tegra186_emc_debug_max_rate_fops);
 
+     if (mc && mc->soc->icc_ops) {
+             /*
+              * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
+              * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
+              * EINVAL instead of passing the request to BPMP-FW later when the BW
+              * request is made by client with 'icc_set_bw()' call.
+              */
+             err = tegra_emc_interconnect_init(emc);
+             if (err)
+                     goto put_bpmp;

drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 361

Quoted 10 lines

drivers/memory/tegra/tegra186-emc.c +10 -0

+
+             if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
+                     mc->bwmgr_mrq_supported = true;
+
+                     /*
+                      * MC driver probe can't get BPMP reference as it gets probed
+                      * earlier than BPMP. So, save the BPMP ref got from the EMC
+                      * DT node in the mc->bpmp and use it in MC's icc_set hook.
+                      */
+                     mc->bpmp = emc->bpmp;

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Best regards,
Krzysztof

Quoted 111 lines · Krzysztof Kozlowski

External email: Use caution opening links or attachments

Quoted 5 lines · Sumit Gupta

Add Interconnect framework support to dynamically set the DRAM bandwidth from different clients. Both the MC and EMC drivers are added as ICC providers. The path for any request is:
  MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM

Thank you for your patch. There is something to discuss/improve.

Quoted 85 lines

drivers/memory/tegra/tegra186-emc.c +55 -0

+static int tegra_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;
+     struct icc_node *node;
+     int err;
+
+     emc->provider.dev = emc->dev;
+     emc->provider.set = tegra_emc_icc_set_bw;
+     emc->provider.data = &emc->provider;
+     emc->provider.aggregate = soc->icc_ops->aggregate;
+     emc->provider.xlate = tegra_emc_of_icc_xlate;
+     emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+     icc_provider_init(&emc->provider);
+
+     /* create External Memory Controller node */
+     node = icc_node_create(TEGRA_ICC_EMC);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto err_msg;
+     }
+
+     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);
+     if (err)
+             goto remove_nodes;
+
+     /* create External Memory node */
+     node = icc_node_create(TEGRA_ICC_EMEM);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto remove_nodes;
+     }
+
+     node->name = "External Memory (DRAM)";
+     icc_node_add(node, &emc->provider);
+
+     err = icc_provider_register(&emc->provider);
+     if (err)
+             goto remove_nodes;
+
+     return 0;
+
+remove_nodes:
+     icc_nodes_remove(&emc->provider);
+err_msg:
+     dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+     return err;
+}
+

  static int tegra186_emc_probe(struct platform_device *pdev) {
+     struct tegra_mc *mc = dev_get_drvdata(pdev->dev.parent); struct mrq_emc_dvfs_latency_response response; struct tegra_bpmp_message msg;
       struct tegra186_emc *emc;

drivers/memory/tegra/tegra186-emc.c +3 -0

@@ -236,6 +339,29 @@ static int tegra186_emc_probe(struct 
 platform_device *pdev)
        debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
                            emc, &tegra186_emc_debug_max_rate_fops);
 
+     if (mc && mc->soc->icc_ops) {
+             /*
+              * Initialize the ICC even if BPMP-FW doesn't support 

'MRQ_BWMGR_INT'.
+              * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
+              * EINVAL instead of passing the request to BPMP-FW later when the BW
+              * request is made by client with 'icc_set_bw()' call. +              */
+             err = tegra_emc_interconnect_init(emc);
+             if (err)
+                     goto put_bpmp;
+
+             if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
+                     mc->bwmgr_mrq_supported = true;
+
+                     /*

Quoted 8 lines

+                      * MC driver probe can't get BPMP reference as it gets probed
+                      * earlier than BPMP. So, save the BPMP ref got from the EMC
+                      * DT node in the mc->bpmp and use it in MC's icc_set hook.
+                      */
+                     mc->bpmp = emc->bpmp;

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Quoted 3 lines

Best regards,
Krzysztof

Currently, the tegra234_mc_icc_set() has NULL check. So, it will give this error.
 if (!mc->bpmp) {
         dev_err(mc->dev, "BPMP reference NULL\n");
         return -ENOENT;
 }

To fix, we can do the below change and swap the order. Please let me know if this sounds fine ?
 if (mc && mc->soc->icc_ops) {
         if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) { mc->bwmgr_mrq_supported = true;
                 mc->bpmp = emc->bpmp;
         }

         err = tegra_emc_interconnect_init(emc);
         if (err)
                 goto put_bpmp;

 }

Thank you,
Sumit Gupta

Quoted 115 lines · Sumit Gupta

Quoted 112 lines · Krzysztof Kozlowski

External email: Use caution opening links or attachments

Quoted 5 lines · Sumit Gupta

Add Interconnect framework support to dynamically set the DRAM bandwidth from different clients. Both the MC and EMC drivers are added as ICC providers. The path for any request is:
  MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM

Thank you for your patch. There is something to discuss/improve.

Quoted 86 lines

drivers/memory/tegra/tegra186-emc.c +55 -0

+static int tegra_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;
+     struct icc_node *node;
+     int err;
+
+     emc->provider.dev = emc->dev;
+     emc->provider.set = tegra_emc_icc_set_bw;
+     emc->provider.data = &emc->provider;
+     emc->provider.aggregate = soc->icc_ops->aggregate;
+     emc->provider.xlate = tegra_emc_of_icc_xlate;
+     emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+     icc_provider_init(&emc->provider);
+
+     /* create External Memory Controller node */
+     node = icc_node_create(TEGRA_ICC_EMC);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto err_msg;
+     }
+
+     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);
+     if (err)
+             goto remove_nodes;
+
+     /* create External Memory node */
+     node = icc_node_create(TEGRA_ICC_EMEM);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto remove_nodes;
+     }
+
+     node->name = "External Memory (DRAM)";
+     icc_node_add(node, &emc->provider);
+
+     err = icc_provider_register(&emc->provider);
+     if (err)
+             goto remove_nodes;
+
+     return 0;
+
+remove_nodes:
+     icc_nodes_remove(&emc->provider);
+err_msg:
+     dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+     return err;
+}
+

  static int tegra186_emc_probe(struct platform_device *pdev) {
+     struct tegra_mc *mc = dev_get_drvdata(pdev->dev.parent); struct mrq_emc_dvfs_latency_response response; struct tegra_bpmp_message msg;
       struct tegra186_emc *emc;

drivers/memory/tegra/tegra186-emc.c +3 -0

@@ -236,6 +339,29 @@ static int tegra186_emc_probe(struct 
 platform_device *pdev)
        debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, 
 emc->debugfs.root,
                            emc, &tegra186_emc_debug_max_rate_fops);
 
+     if (mc && mc->soc->icc_ops) {
+             /*
+              * Initialize the ICC even if BPMP-FW doesn't support 

'MRQ_BWMGR_INT'.
+              * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
+              * EINVAL instead of passing the request to BPMP-FW later when the BW
+              * request is made by client with 'icc_set_bw()' call. +              */
+             err = tegra_emc_interconnect_init(emc);
+             if (err)
+                     goto put_bpmp;
+
+             if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
+                     mc->bwmgr_mrq_supported = true;
+
+                     /*

Quoted 8 lines

+                      * MC driver probe can't get BPMP reference as it gets probed
+                      * earlier than BPMP. So, save the BPMP ref got from the EMC
+                      * DT node in the mc->bpmp and use it in MC's icc_set hook.
+                      */
+                     mc->bpmp = emc->bpmp;

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Quoted 7 lines

Quoted 3 lines

Best regards,
Krzysztof

Currently, the tegra234_mc_icc_set() has NULL check. So, it will give this error.
 if (!mc->bpmp) {

Quoted 12 lines

         dev_err(mc->dev, "BPMP reference NULL\n"); return -ENOENT;
 }

To fix, we can do the below change and swap the order. Please let me know if this sounds fine ?
 if (mc && mc->soc->icc_ops) {
         if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) { mc->bwmgr_mrq_supported = true;
                 mc->bpmp = emc->bpmp;
         }

Quoted 5 lines

         err = tegra_emc_interconnect_init(emc);
         if (err)
                 goto put_bpmp;

 }

Sorry, replied too soon. Missed setting "mc->bpmp" to NULL. Sharing new proposed change. Please let me know if this looks OK.

  if (mc && mc->soc->icc_ops) {
     if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
             mc->bwmgr_mrq_supported = true;
             mc->bpmp = emc->bpmp;
     }

     err = tegra_emc_interconnect_init(emc);
     if (err) {
             mc->bpmp = NULL;
             goto put_bpmp;

External email: Use caution opening links or attachments

Quoted 122 lines · Sumit Gupta

Quoted 119 lines · Sumit Gupta

Quoted 115 lines · Krzysztof Kozlowski

External email: Use caution opening links or attachments

Quoted 5 lines · Sumit Gupta

Add Interconnect framework support to dynamically set the DRAM bandwidth from different clients. Both the MC and EMC drivers are added as ICC providers. The path for any request is:

   MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM

Thank you for your patch. There is something to discuss/improve.

Quoted 92 lines

drivers/memory/tegra/tegra186-emc.c +59 -0

+static int tegra_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;
+     struct icc_node *node;
+     int err;
+
+     emc->provider.dev = emc->dev;
+     emc->provider.set = tegra_emc_icc_set_bw;
+     emc->provider.data = &emc->provider;
+     emc->provider.aggregate = soc->icc_ops->aggregate;
+     emc->provider.xlate = tegra_emc_of_icc_xlate;
+     emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+     icc_provider_init(&emc->provider);
+
+     /* create External Memory Controller node */
+     node = icc_node_create(TEGRA_ICC_EMC);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto err_msg;
+     }
+
+     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);
+     if (err)
+             goto remove_nodes;
+
+     /* create External Memory node */
+     node = icc_node_create(TEGRA_ICC_EMEM);
+     if (IS_ERR(node)) {
+             err = PTR_ERR(node);
+             goto remove_nodes;
+     }
+
+     node->name = "External Memory (DRAM)";
+     icc_node_add(node, &emc->provider);
+
+     err = icc_provider_register(&emc->provider);
+     if (err)
+             goto remove_nodes;
+
+     return 0;
+
+remove_nodes:
+     icc_nodes_remove(&emc->provider);
+err_msg:
+     dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+     return err;
+}
+
   static int tegra186_emc_probe(struct platform_device *pdev)
   {
+     struct tegra_mc *mc = dev_get_drvdata(pdev->dev.parent);
        struct mrq_emc_dvfs_latency_response response;
        struct tegra_bpmp_message msg;
        struct tegra186_emc *emc;

@@ -236,6 +339,29 @@ static int tegra186_emc_probe(struct
 platform_device *pdev)
        debugfs_create_file("max_rate", S_IRUGO | S_IWUSR,
 emc->debugfs.root,
                            emc, &tegra186_emc_debug_max_rate_fops);
 
+     if (mc && mc->soc->icc_ops) {
+             /*
+              * Initialize the ICC even if BPMP-FW doesn't support

'MRQ_BWMGR_INT'.
+ * Use the flag 'mc->bwmgr_mrq_supported' within MC
driver and return
+ * EINVAL instead of passing the request to BPMP-FW
later when the BW

drivers/memory/tegra/tegra186-emc.c +7 -0

+              * request is made by client with 'icc_set_bw()' call.
+              */
+             err = tegra_emc_interconnect_init(emc);
+             if (err)
+                     goto put_bpmp;
+
+             if (tegra_bpmp_mrq_is_supported(emc->bpmp,

MRQ_BWMGR_INT)) {

drivers/memory/tegra/tegra186-emc.c +4 -0

+                     mc->bwmgr_mrq_supported = true;
+
+                     /*
+                      * MC driver probe can't get BPMP reference as

it gets probed
+ * earlier than BPMP. So, save the BPMP ref got
from the EMC
+ * DT node in the mc->bpmp and use it in MC's
icc_set hook.

Review

Quoted 2 lines

(quoted diff) +2 -0

+                      */
+                     mc->bpmp = emc->bpmp;

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Best regards,
Krzysztof

Quoted 3 lines

Quoted 3 lines

Currently, the tegra234_mc_icc_set() has NULL check. So, it will give this error.
if (!mc->bpmp) {

How does it solve concurrent accesses and re-ordering of instructions by compiler or CPU?

Quoted 26 lines

Quoted 16 lines

           dev_err(mc->dev, "BPMP reference NULL\n");
           return -ENOENT;

}

To fix, we can do the below change and swap the order. Please let me know if this sounds fine ?

   if (mc && mc->soc->icc_ops) {
           if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
                   mc->bwmgr_mrq_supported = true;
                   mc->bpmp = emc->bpmp;
           }

           err = tegra_emc_interconnect_init(emc);
           if (err)
                   goto put_bpmp;

Quoted 1 line

   }

Sorry, replied too soon. Missed setting "mc->bpmp" to NULL. Sharing new proposed change. Please let me know if this looks OK.

   if (mc && mc->soc->icc_ops) {
      if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT)) {
              mc->bwmgr_mrq_supported = true;
              mc->bpmp = emc->bpmp;
      }

Quoted 4 lines

      err = tegra_emc_interconnect_init(emc);
      if (err) {
              mc->bpmp = NULL;
              goto put_bpmp;

Sorry, I didn't check the code. I assume you should do it...

Best regards,
Krzysztof

Quoted 23 lines

Quoted 22 lines

Quoted 22 lines

Quoted 10 lines

Review

Quoted 9 lines

(quoted diff) +2 -0

+                     /*
+                      * MC driver probe can't get BPMP reference as

it gets probed
+ * earlier than BPMP. So, save the BPMP ref got
from the EMC
+ * DT node in the mc->bpmp and use it in MC's
icc_set hook.

(quoted diff) +2 -0

+                      */
+                     mc->bpmp = emc->bpmp;

Quoted 8 lines

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Best regards,
Krzysztof

Currently, the tegra234_mc_icc_set() has NULL check. So, it will give this error.
if (!mc->bpmp) {

Quoted 3 lines

How does it solve concurrent accesses and re-ordering of instructions by compiler or CPU?

Now, the "mc->bpmp" is set before tegra_emc_interconnect_init(). So, until the EMC interconnect initializes, set() won't be called as the devm_of_icc_get() call will fail.

External email: Use caution opening links or attachments

Quoted 23 lines · Sumit Gupta

Quoted 23 lines

Quoted 22 lines

Quoted 22 lines

Quoted 10 lines

Review

Quoted 9 lines

(quoted diff) +2 -0

+                     /*
+                      * MC driver probe can't get BPMP reference as

it gets probed
+ * earlier than BPMP. So, save the BPMP ref got
from the EMC
+ * DT node in the mc->bpmp and use it in MC's
icc_set hook.

(quoted diff) +2 -0

+                      */
+                     mc->bpmp = emc->bpmp;

Quoted 8 lines

This (and ()) are called without any locking. You register first the interconnect, so set() callback can be used, right? Then set() could be called anytime between tegra_emc_interconnect_init() and assignment above. How do you synchronize these?

Best regards,
Krzysztof

Currently, the tegra234_mc_icc_set() has NULL check. So, it will give this error.

    if (!mc->bpmp) {

Quoted 7 lines

Quoted 3 lines

How does it solve concurrent accesses and re-ordering of instructions by compiler or CPU?

Now, the "mc->bpmp" is set before tegra_emc_interconnect_init(). So, until the EMC interconnect initializes, set() won't be called as the devm_of_icc_get() call will fail.

What if compiler puts "mc->bpmp" assignment after
tegra_emc_interconnect_init()?

What if CPU executes above assignment also after
tegra_emc_interconnect_init()?

Considering amount of code inside tegra_emc_interconnect_init() second case is rather unlikely, but first possible, right?

Best regards,
Krzysztof

Hi All,

Thank you for the review.
Requesting for ACK on the remaining patches and please consider for merging.