Skip to contentSkip to conversation

ARM: imx: don't abort MMDC probe if power saving status doesn't match

PATCH New Lucas Stach 2 participants 4 messages Nov 27, 2017 – Nov 29, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: imx: don't abort MMDC probe if power saving status doesn't match

The power saving status bit will not signal if the MMDC is under load, which is likely during kernel boot. There is no point in checking this bit and aborting the probe, as there is nothing depending on power saving being enabled, so we can trust the memory controller to enable power saving when we allow it.

---
 arch/arm/mach-imx/mmdc.c | 11 -----------
 1 file changed, 11 deletions(-)
arch/arm/mach-imx/mmdc.c +0 -11
index 78262899a590..5fb1d2254b5e 100644
@@ -547,7 +547,6 @@ static int imx_mmdc_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	void __iomem *mmdc_base, *reg;
 	u32 val;
-	int timeout = 0x400;
 
 	mmdc_base = of_iomap(np, 0);
 	WARN_ON(!mmdc_base);
@@ -565,16 +564,6 @@ static int imx_mmdc_probe(struct platform_device *pdev)
 	val &= ~(1 << BP_MMDC_MAPSR_PSD);
 	writel_relaxed(val, reg);
 
-	/* Ensure it's successfully enabled */
-	while (!(readl_relaxed(reg) & 1 << BP_MMDC_MAPSR_PSS) && --timeout)
-		cpu_relax();
-
-	if (unlikely(!timeout)) {
-		pr_warn("%s: failed to enable automatic power saving\n",
-			__func__);
-		return -EBUSY;
-	}
-
 	return imx_mmdc_perf_init(pdev, mmdc_base);
 }
 
Signature · 1 line
2.11.0
#1 lore raw

PATCH 1/1 ARM: imx: don't abort MMDC probe if power saving status doesn't match

Quoted 2 lines · Lucas Stach

The power saving status bit will not signal if the MMDC is under load, which is likely during kernel boot.

What is exactly the condition of MMDC is under load? If it's likely hit during kernel boot, why have we not heard any report regarding to it before?

Shawn

Quoted 42 lines

There is no point in checking this
bit and aborting the probe, as there is nothing depending on power saving being enabled, so we can trust the memory controller to enable power saving when we allow it.

---
 arch/arm/mach-imx/mmdc.c | 11 -----------
 1 file changed, 11 deletions(-)
arch/arm/mach-imx/mmdc.c +0 -11
index 78262899a590..5fb1d2254b5e 100644
@@ -547,7 +547,6 @@ static int imx_mmdc_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	void __iomem *mmdc_base, *reg;
 	u32 val;
-	int timeout = 0x400;
 
 	mmdc_base = of_iomap(np, 0);
 	WARN_ON(!mmdc_base);
@@ -565,16 +564,6 @@ static int imx_mmdc_probe(struct platform_device *pdev)
 	val &= ~(1 << BP_MMDC_MAPSR_PSD);
 	writel_relaxed(val, reg);
 
-	/* Ensure it's successfully enabled */
-	while (!(readl_relaxed(reg) & 1 << BP_MMDC_MAPSR_PSS) && --timeout)
-		cpu_relax();
-
-	if (unlikely(!timeout)) {
-		pr_warn("%s: failed to enable automatic power saving\n",
-			__func__);
-		return -EBUSY;
-	}
-
 	return imx_mmdc_perf_init(pdev, mmdc_base);
 }
 
Signature · 1 line
2.11.0
#2 lore raw

PATCH 1/1 ARM: imx: don't abort MMDC probe if power saving status doesn't match

Am Mittwoch, den 29.11.2017, 22:06 +0800 schrieb Shawn Guo:

Quoted 8 lines · Lucas Stach
Quoted 3 lines

The power saving status bit will not signal if the MMDC is under load,
which is likely during kernel boot.

What is exactly the condition of MMDC is under load???If it's likely hit during kernel boot, why have we not heard any report regarding to it before?

There were reports of this before:
https://patchwork.kernel.org/patch/9289077/

Maybe nobody really cared about the issue before, as nothing bad happened on this check failing. But with the MMDC perf support getting skipped now, this is rather unfortunate.

I do hit this condition pretty reliably with a recent kernel. Also I would say it's expected that the MMDC is loaded with memory transactions during system boot.

Regards,
Lucas

Applied
#3 lore raw

PATCH 1/1 ARM: imx: don't abort MMDC probe if power saving status doesn't match

In reply to Lucas Stach
Quoted 7 lines · Lucas Stach

The power saving status bit will not signal if the MMDC is under load, which is likely during kernel boot. There is no point in checking this bit and aborting the probe, as there is nothing depending on power saving being enabled, so we can trust the memory controller to enable power saving when we allow it.

Applied, thanks.