ARM: exynos: clear L310_AUX_CTRL_FULL_LINE_ZERO in default l2c_aux_val
diff mbox series

Message ID c6d979c0050502cd1bca8619cb5ff95db239db12.1591969825.git.guillaume.tucker@collabora.com
State Accepted
Commit c49f06c376f15d82b1db52dca94512d95d64c0cd
Headers show
Series
  • ARM: exynos: clear L310_AUX_CTRL_FULL_LINE_ZERO in default l2c_aux_val
Related show

Commit Message

Guillaume Tucker June 12, 2020, 1:58 p.m. UTC
This "alert" error message can be seen on exynos4412-odroidx2:

    L2C: platform modifies aux control register: 0x02070000 -> 0x3e470001
    L2C: platform provided aux values permit register corruption.

Followed by this plain error message:

    L2C-310: enabling full line of zeros but not enabled in Cortex-A9

To fix it, don't set the L310_AUX_CTRL_FULL_LINE_ZERO flag (bit 0) in
the default value of l2c_aux_val.  It may instead be enabled when
applicable by the logic in l2c310_enable() if the attribute
"arm,full-line-zero-disable" was set in the device tree.

The initial commit that introduced this default value was in v2.6.38:

  1cf0eb799759 "ARM: S5PV310: Add L2 cache init function in cpu.c"

However, the code to set the L310_AUX_CTRL_FULL_LINE_ZERO flag and
manage that feature was added much later and the default value was not
updated then.  So this seems to have been a subtle oversight
especially since enabling it only in the cache and not in the A9 core
doesn't actually prevent the platform from running.  According to the
TRM, the opposite would be a real issue, if the feature was enabled in
the A9 core but not in the cache controller.

Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 arch/arm/mach-exynos/exynos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Kozlowski June 22, 2020, 5:50 p.m. UTC | #1
On Fri, Jun 12, 2020 at 02:58:37PM +0100, Guillaume Tucker wrote:
> This "alert" error message can be seen on exynos4412-odroidx2:
> 
>     L2C: platform modifies aux control register: 0x02070000 -> 0x3e470001
>     L2C: platform provided aux values permit register corruption.
> 
> Followed by this plain error message:
> 
>     L2C-310: enabling full line of zeros but not enabled in Cortex-A9
> 
> To fix it, don't set the L310_AUX_CTRL_FULL_LINE_ZERO flag (bit 0) in
> the default value of l2c_aux_val.  It may instead be enabled when
> applicable by the logic in l2c310_enable() if the attribute
> "arm,full-line-zero-disable" was set in the device tree.
> 
> The initial commit that introduced this default value was in v2.6.38:
> 
>   1cf0eb799759 "ARM: S5PV310: Add L2 cache init function in cpu.c"
> 
> However, the code to set the L310_AUX_CTRL_FULL_LINE_ZERO flag and
> manage that feature was added much later and the default value was not
> updated then.  So this seems to have been a subtle oversight
> especially since enabling it only in the cache and not in the A9 core
> doesn't actually prevent the platform from running.  According to the
> TRM, the opposite would be a real issue, if the feature was enabled in
> the A9 core but not in the cache controller.
> 
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  arch/arm/mach-exynos/exynos.c | 2 +-

Thanks, applied.

Best regards,
Krzysztof
Krzysztof Kozlowski June 23, 2020, 6:12 a.m. UTC | #2
On Fri, Jun 12, 2020 at 02:58:37PM +0100, Guillaume Tucker wrote:
> This "alert" error message can be seen on exynos4412-odroidx2:
> 
>     L2C: platform modifies aux control register: 0x02070000 -> 0x3e470001
>     L2C: platform provided aux values permit register corruption.
> 
> Followed by this plain error message:
> 
>     L2C-310: enabling full line of zeros but not enabled in Cortex-A9
> 
> To fix it, don't set the L310_AUX_CTRL_FULL_LINE_ZERO flag (bit 0) in
> the default value of l2c_aux_val.  It may instead be enabled when
> applicable by the logic in l2c310_enable() if the attribute
> "arm,full-line-zero-disable" was set in the device tree.
> 
> The initial commit that introduced this default value was in v2.6.38:
> 
>   1cf0eb799759 "ARM: S5PV310: Add L2 cache init function in cpu.c"

Run checkpatch in the future on your patches.

I fixed it up.

Best regards,
Krzysztof

> 
> However, the code to set the L310_AUX_CTRL_FULL_LINE_ZERO flag and
> manage that feature was added much later and the default value was not
> updated then.  So this seems to have been a subtle oversight
> especially since enabling it only in the cache and not in the A9 core
> doesn't actually prevent the platform from running.  According to the
> TRM, the opposite would be a real issue, if the feature was enabled in
> the A9 core but not in the cache controller.
> 
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  arch/arm/mach-exynos/exynos.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 7a8d1555db40..36c37444485a 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -193,7 +193,7 @@ static void __init exynos_dt_fixup(void)
>  }
>  
>  DT_MACHINE_START(EXYNOS_DT, "Samsung Exynos (Flattened Device Tree)")
> -	.l2c_aux_val	= 0x3c400001,
> +	.l2c_aux_val	= 0x3c400000,
>  	.l2c_aux_mask	= 0xc20fffff,
>  	.smp		= smp_ops(exynos_smp_ops),
>  	.map_io		= exynos_init_io,
> -- 
> 2.20.1
>

Patch
diff mbox series

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 7a8d1555db40..36c37444485a 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -193,7 +193,7 @@  static void __init exynos_dt_fixup(void)
 }
 
 DT_MACHINE_START(EXYNOS_DT, "Samsung Exynos (Flattened Device Tree)")
-	.l2c_aux_val	= 0x3c400001,
+	.l2c_aux_val	= 0x3c400000,
 	.l2c_aux_mask	= 0xc20fffff,
 	.smp		= smp_ops(exynos_smp_ops),
 	.map_io		= exynos_init_io,