Re: [Patch v4 03/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 28/03/23 16:35, Thierry Reding wrote:
On Tue, Mar 28, 2023 at 09:31:58AM +0200, Krzysztof Kozlowski wrote:
On 27/03/2023 18:14, Sumit Gupta wrote:
[...]
diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
[...]
@@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
  	if (!emc)
  		return -ENOMEM;
+ platform_set_drvdata(pdev, emc);
+	emc->dev = &pdev->dev;

This patch looks like stiched from two or more patches... emc->dev does
not look like new member of emc, thus why do you set in exisitng
function in this patch? Why it wasn't needed before?

This looks like it may be leftover from some development. These two
lines exist in this driver a few lines further down. Either one pair
should be removed. I don't see why this would need to be moved, so
probably the above additions can just be dropped.

Thierry

Yes, sorry i was left over. Will remove this.
Thank you for catching.

  	emc->bpmp = tegra_bpmp_get(&pdev->dev);
  	if (IS_ERR(emc->bpmp))
  		return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
@@ -236,6 +341,25 @@ 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);
+ mc = dev_get_drvdata(emc->dev->parent);
+	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;
+		else
+

Drop blank line.

Ok.

+			dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");

And what user is supposed to do with this? Either make it descriptive or
drop.

Agreed. I think we can just drop this. If the intention was to provide a
quick way for people to detect whether BWMGR is available or not, using
something from sysfs/debugfs would be preferable.

Thierry

Sure, will drop this.

Thank you,
Sumit Gupta



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux