BeagleBone fixes & cleanup

BeagleBone fixes & cleanup

[Jan Luebbe]

Hi,

I've tried the recent changes in next on both BeagleBones.

These are actual fixes:
[PATCH 1/4] am33xx_generic: convert from switch to if/else
[PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2

These are more 'nice to have':
[PATCH 2/4] arm: mmu: be more verbose if ttb setup fails
[PATCH 4/4] beaglebone: use most recent timings for white variant

Regards,
Jan

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/4] am33xx_generic: convert from switch to if/else

[Jan Luebbe]

The function am33xx_get_cpu_rev may be called before barebox_arm_entry(),
so we need to avoid switch statements. One example is the BeagleBone,
where we use this function to differenciate between the white and black
variants.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 arch/arm/mach-omap/am33xx_generic.c |   31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap/am33xx_generic.c b/arch/arm/mach-omap/am33xx_generic.c
index 3690ce1..251c8d4 100644
--- a/arch/arm/mach-omap/am33xx_generic.c
+++ b/arch/arm/mach-omap/am33xx_generic.c
@@ -43,30 +43,21 @@ void __noreturn reset_cpu(unsigned long addr)
  * The significance of the CPU revision depends upon the cpu type.
  * Latest known revision is considered default.
  *
+ * This function is called before barebox_arm_entry(), so avoid switch
+ * statements.
+ *
  * @return silicon version
  */
 u32 am33xx_get_cpu_rev(void)
 {
-	u32 version, retval;
-
-	version = (readl(AM33XX_IDCODE_REG) >> 28) & 0xF;
-
-	switch (version) {
-	case 0:
-		retval = AM335X_ES1_0;
-		break;
-	case 1:
-		retval = AM335X_ES2_0;
-		break;
-	case 2:
-		/*
-		 * Fall through the default case.
-		 */
-	default:
-		retval = AM335X_ES2_1;
-	}
-
-	return retval;
+	u32 version = (readl(AM33XX_IDCODE_REG) >> 28) & 0xF;
+
+	if (version == 0)
+		return AM335X_ES1_0;
+	else if (version == 1)
+		return AM335X_ES2_0;
+	else
+		return AM335X_ES2_1;
 }
 
 /**
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] arm: mmu: be more verbose if ttb setup fails

[Jan Luebbe]

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 arch/arm/cpu/mmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index e3ea3b6..fae83f3 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -285,7 +285,7 @@ static int mmu_init(void)
 		ttb = (unsigned long *)((unsigned long)ttb & ~0x3fff);
 
 		if (!request_sdram_region("ttb", (unsigned long)ttb, SZ_16K))
-			pr_err("Error: Can't request SDRAM region for ttb\n");
+			pr_err("Error: Can't request SDRAM region for ttb at %p\n", ttb);
 	} else {
 		ttb = memalign(0x10000, 0x4000);
 	}
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2

[Jan Luebbe]

For DDR2 RAMs, regs->zq_config is not used, which causes the
AM33XX_EMIF4_0_REG(SDRAM_CONFIG) register to be left unconfigured, resulting
in boot failure.

It seems that the DDR2 case was missed during the consolidation in commit
9f122f8bf023a12ad5f84b61d1d74d3ff06104dd. The actual call for the Bone was
removed in 88659d9c4a87a730f6efe4f38c011e8e0214a67b.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
I've tested this on White and Black BeagleBones. An additional test on the
PCM051 would be good.

 arch/arm/mach-omap/am33xx_generic.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap/am33xx_generic.c b/arch/arm/mach-omap/am33xx_generic.c
index 251c8d4..3e2b6c4 100644
--- a/arch/arm/mach-omap/am33xx_generic.c
+++ b/arch/arm/mach-omap/am33xx_generic.c
@@ -302,7 +302,7 @@ void am33xx_config_sdram(const struct am33xx_emif_regs *regs)
 
 	writel(regs->sdram_ref_ctrl, AM33XX_EMIF4_0_REG(SDRAM_REF_CTRL));
 	writel(regs->sdram_ref_ctrl, AM33XX_EMIF4_0_REG(SDRAM_REF_CTRL_SHADOW));
-	writel(regs->sdram_config, CM_EMIF_SDRAM_CONFIG);
+	writel(regs->sdram_config, AM33XX_EMIF4_0_REG(SDRAM_CONFIG));
 }
 
 void am33xx_config_io_ctrl(int ioctrl)
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] beaglebone: use most recent timings for white variant

[Jan Luebbe]

These values come from the U-Boot source code.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 arch/arm/boards/beaglebone/lowlevel.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
index 2ff7c71..e993c89 100644
--- a/arch/arm/boards/beaglebone/lowlevel.c
+++ b/arch/arm/boards/beaglebone/lowlevel.c
@@ -36,13 +36,13 @@ static const struct am33xx_cmd_control ddr2_cmd_ctrl = {
 };
 
 static const struct am33xx_emif_regs ddr2_regs = {
-	.emif_read_latency	= 0x5,
-	.emif_tim1		= 0x0666B3D6,
-	.emif_tim2		= 0x143731DA,
-	.emif_tim3		= 0x00000347,
-	.sdram_config		= 0x43805332,
-	.sdram_config2		= 0x43805332,
-	.sdram_ref_ctrl		= 0x0000081a,
+	.emif_read_latency	= 0x100005,
+	.emif_tim1		= 0x0666B3C9,
+	.emif_tim2		= 0x243631CA,
+	.emif_tim3		= 0x0000033F,
+	.sdram_config		= 0x41805332,
+	.sdram_config2		= 0x41805332,
+	.sdram_ref_ctrl		= 0x0000081A,
 };
 
 static const struct am33xx_ddr_data ddr2_data = {
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else

[Lucas Stach]

Am Mittwoch, den 02.10.2013, 21:30 +0200 schrieb Jan Luebbe:
> The function am33xx_get_cpu_rev may be called before barebox_arm_entry(),
> so we need to avoid switch statements. 

Uhm, could you please be more verbose on _why_ we need to avoid switch
statements at this stage? I might be running into the same failure with
Tegra at some point if there's a real issue.

Thanks,
Lucas

> One example is the BeagleBone,
> where we use this function to differenciate between the white and black
> variants.
> 
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>  arch/arm/mach-omap/am33xx_generic.c |   31 +++++++++++--------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm/mach-omap/am33xx_generic.c b/arch/arm/mach-omap/am33xx_generic.c
> index 3690ce1..251c8d4 100644
> --- a/arch/arm/mach-omap/am33xx_generic.c
> +++ b/arch/arm/mach-omap/am33xx_generic.c
> @@ -43,30 +43,21 @@ void __noreturn reset_cpu(unsigned long addr)
>   * The significance of the CPU revision depends upon the cpu type.
>   * Latest known revision is considered default.
>   *
> + * This function is called before barebox_arm_entry(), so avoid switch
> + * statements.
> + *
>   * @return silicon version
>   */
>  u32 am33xx_get_cpu_rev(void)
>  {
> -	u32 version, retval;
> -
> -	version = (readl(AM33XX_IDCODE_REG) >> 28) & 0xF;
> -
> -	switch (version) {
> -	case 0:
> -		retval = AM335X_ES1_0;
> -		break;
> -	case 1:
> -		retval = AM335X_ES2_0;
> -		break;
> -	case 2:
> -		/*
> -		 * Fall through the default case.
> -		 */
> -	default:
> -		retval = AM335X_ES2_1;
> -	}
> -
> -	return retval;
> +	u32 version = (readl(AM33XX_IDCODE_REG) >> 28) & 0xF;
> +
> +	if (version == 0)
> +		return AM335X_ES1_0;
> +	else if (version == 1)
> +		return AM335X_ES2_0;
> +	else
> +		return AM335X_ES2_1;
>  }
>  
>  /**



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else

[Jan Lübbe]

On Wed, 2013-10-02 at 21:45 +0200, Lucas Stach wrote: 
> Am Mittwoch, den 02.10.2013, 21:30 +0200 schrieb Jan Luebbe:
> > The function am33xx_get_cpu_rev may be called before barebox_arm_entry(),
> > so we need to avoid switch statements. 
> 
> Uhm, could you please be more verbose on _why_ we need to avoid switch
> statements at this stage? I might be running into the same failure with
> Tegra at some point if there's a real issue.

I believe the problem is that the table is accessed using absolute
addresses. Before the call to barebox_arm_entry() barebox may be running
at a different address.

Regards,
Jan


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else

[Lucas Stach]

Am Mittwoch, den 02.10.2013, 22:04 +0200 schrieb Jan Lübbe:
> On Wed, 2013-10-02 at 21:45 +0200, Lucas Stach wrote: 
> > Am Mittwoch, den 02.10.2013, 21:30 +0200 schrieb Jan Luebbe:
> > > The function am33xx_get_cpu_rev may be called before barebox_arm_entry(),
> > > so we need to avoid switch statements. 
> > 
> > Uhm, could you please be more verbose on _why_ we need to avoid switch
> > statements at this stage? I might be running into the same failure with
> > Tegra at some point if there's a real issue.
> 
> I believe the problem is that the table is accessed using absolute
> addresses. Before the call to barebox_arm_entry() barebox may be running
> at a different address.
> 
Ah, so it's producing a jump table with absolute offsets even for this
rather trivial switch statement? I can see how this would cause issues.
How about using "-fno-jump-tables" for the objects that are referenced
in the early startup code? This may allow you to drop this code
workaround.

Regards,
Lucas



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: BeagleBone fixes & cleanup

[Sascha Hauer]

On Wed, Oct 02, 2013 at 09:30:07PM +0200, Jan Luebbe wrote:
> Hi,
> 
> I've tried the recent changes in next on both BeagleBones.
> 
> These are actual fixes:
> [PATCH 1/4] am33xx_generic: convert from switch to if/else
> [PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2
> 
> These are more 'nice to have':
> [PATCH 2/4] arm: mmu: be more verbose if ttb setup fails
> [PATCH 4/4] beaglebone: use most recent timings for white variant

I applied all for master now. Anyway, Jan, could you give the
-fno-jump-labels thingy a test? Maybe we want to switch to this later.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox