* [PATCH 1/4] am33xx_generic: convert from switch to if/else
2013-10-02 19:30 BeagleBone fixes & cleanup Jan Luebbe
@ 2013-10-02 19:30 ` Jan Luebbe
2013-10-02 19:45 ` Lucas Stach
2013-10-02 19:30 ` [PATCH 2/4] arm: mmu: be more verbose if ttb setup fails Jan Luebbe
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Jan Luebbe @ 2013-10-02 19:30 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else
2013-10-02 19:30 ` [PATCH 1/4] am33xx_generic: convert from switch to if/else Jan Luebbe
@ 2013-10-02 19:45 ` Lucas Stach
2013-10-02 20:04 ` Jan Lübbe
0 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2013-10-02 19:45 UTC (permalink / raw)
To: Jan Luebbe; +Cc: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else
2013-10-02 19:45 ` Lucas Stach
@ 2013-10-02 20:04 ` Jan Lübbe
2013-10-02 20:16 ` Lucas Stach
0 siblings, 1 reply; 9+ messages in thread
From: Jan Lübbe @ 2013-10-02 20:04 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] am33xx_generic: convert from switch to if/else
2013-10-02 20:04 ` Jan Lübbe
@ 2013-10-02 20:16 ` Lucas Stach
0 siblings, 0 replies; 9+ messages in thread
From: Lucas Stach @ 2013-10-02 20:16 UTC (permalink / raw)
To: Jan Lübbe; +Cc: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] arm: mmu: be more verbose if ttb setup fails
2013-10-02 19:30 BeagleBone fixes & cleanup Jan Luebbe
2013-10-02 19:30 ` [PATCH 1/4] am33xx_generic: convert from switch to if/else Jan Luebbe
@ 2013-10-02 19:30 ` Jan Luebbe
2013-10-02 19:30 ` [PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2 Jan Luebbe
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Jan Luebbe @ 2013-10-02 19:30 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2
2013-10-02 19:30 BeagleBone fixes & cleanup Jan Luebbe
2013-10-02 19:30 ` [PATCH 1/4] am33xx_generic: convert from switch to if/else Jan Luebbe
2013-10-02 19:30 ` [PATCH 2/4] arm: mmu: be more verbose if ttb setup fails Jan Luebbe
@ 2013-10-02 19:30 ` Jan Luebbe
2013-10-02 19:30 ` [PATCH 4/4] beaglebone: use most recent timings for white variant Jan Luebbe
2013-10-06 11:15 ` BeagleBone fixes & cleanup Sascha Hauer
4 siblings, 0 replies; 9+ messages in thread
From: Jan Luebbe @ 2013-10-02 19:30 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] beaglebone: use most recent timings for white variant
2013-10-02 19:30 BeagleBone fixes & cleanup Jan Luebbe
` (2 preceding siblings ...)
2013-10-02 19:30 ` [PATCH 3/4] arm: omap: am33xx_generic: fix DDR setup for DDR2 Jan Luebbe
@ 2013-10-02 19:30 ` Jan Luebbe
2013-10-06 11:15 ` BeagleBone fixes & cleanup Sascha Hauer
4 siblings, 0 replies; 9+ messages in thread
From: Jan Luebbe @ 2013-10-02 19:30 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: BeagleBone fixes & cleanup
2013-10-02 19:30 BeagleBone fixes & cleanup Jan Luebbe
` (3 preceding siblings ...)
2013-10-02 19:30 ` [PATCH 4/4] beaglebone: use most recent timings for white variant Jan Luebbe
@ 2013-10-06 11:15 ` Sascha Hauer
4 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2013-10-06 11:15 UTC (permalink / raw)
To: Jan Luebbe; +Cc: barebox
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
^ permalink raw reply [flat|nested] 9+ messages in thread