All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match
@ 2017-11-27 12:11 Lucas Stach
  2017-11-29 14:06 ` Shawn Guo
  2017-11-29 14:20 ` Shawn Guo
  0 siblings, 2 replies; 4+ messages in thread
From: Lucas Stach @ 2017-11-27 12:11 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/mach-imx/mmdc.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
index 78262899a590..5fb1d2254b5e 100644
--- a/arch/arm/mach-imx/mmdc.c
+++ b/arch/arm/mach-imx/mmdc.c
@@ -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);
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match
  2017-11-27 12:11 [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match Lucas Stach
@ 2017-11-29 14:06 ` Shawn Guo
  2017-11-29 14:12   ` Lucas Stach
  2017-11-29 14:20 ` Shawn Guo
  1 sibling, 1 reply; 4+ messages in thread
From: Shawn Guo @ 2017-11-29 14:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 27, 2017 at 01:11:39PM +0100, Lucas Stach wrote:
> 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

> 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.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  arch/arm/mach-imx/mmdc.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
> index 78262899a590..5fb1d2254b5e 100644
> --- a/arch/arm/mach-imx/mmdc.c
> +++ b/arch/arm/mach-imx/mmdc.c
> @@ -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);
>  }
>  
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match
  2017-11-29 14:06 ` Shawn Guo
@ 2017-11-29 14:12   ` Lucas Stach
  0 siblings, 0 replies; 4+ messages in thread
From: Lucas Stach @ 2017-11-29 14:12 UTC (permalink / raw)
  To: linux-arm-kernel

Am Mittwoch, den 29.11.2017, 22:06 +0800 schrieb Shawn Guo:
> On Mon, Nov 27, 2017 at 01:11:39PM +0100, Lucas Stach wrote:
> > 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match
  2017-11-27 12:11 [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match Lucas Stach
  2017-11-29 14:06 ` Shawn Guo
@ 2017-11-29 14:20 ` Shawn Guo
  1 sibling, 0 replies; 4+ messages in thread
From: Shawn Guo @ 2017-11-29 14:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 27, 2017 at 01:11:39PM +0100, Lucas Stach wrote:
> 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.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Applied, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-11-29 14:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 12:11 [PATCH] ARM: imx: don't abort MMDC probe if power saving status doesn't match Lucas Stach
2017-11-29 14:06 ` Shawn Guo
2017-11-29 14:12   ` Lucas Stach
2017-11-29 14:20 ` Shawn Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.