[U-Boot] [PATCH 0/3] Fixes for edb9315

I'm porting current u-boot to a board similar to EP9315A, so I'm using arm/master as a basis, as it includes the patches by Matthias Kaehlcke. I'm currently running from RAM (SKIP_LOWLEVEL_INIT), after setting up sdram and pll elsewhere (older vendor u-boot code, still to be ported).
"fix syscon_regs definition" is needed to access any software-locked syscon register from C code (e.g., in reset_cpu() and devicecfg used by me in patch 3).
"change calculation un early_udelay.h" is needed at least for eldk-4.2 (gcc-4.2.2), as without this patch it will use the stack before setting SP. Actually, we could use the ether buffer as a stack, if needed, but it's not really needed here.
"enable the uart in devicecfg register" prevents u-boot from freezing at least with SKIP_LOWLEVEL_INIT set, but I'm pretty sure lowlevel_setup assembly code doesn't enable the uart, either.
Alessandro Rubini (3): EP93xx: fix syscon_regs definition edb93xx: change calculation un early_udelay.h edb93xx: enable the uart in devicecfg register
board/edb93xx/early_udelay.h | 2 +- board/edb93xx/edb93xx.c | 6 ++++++ include/asm-arm/arch-ep93xx/ep93xx.h | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-)

The structure was missing a reserved entry (not listed in the manual, actually), so the last registers had a wrong offset. This prevented all swlocked registers to be modified as swlock is last in the structure.
Signed-off-by: Alessandro Rubini rubini@gnudd.com --- include/asm-arm/arch-ep93xx/ep93xx.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/include/asm-arm/arch-ep93xx/ep93xx.h b/include/asm-arm/arch-ep93xx/ep93xx.h index 6cafe54..806557a 100644 --- a/include/asm-arm/arch-ep93xx/ep93xx.h +++ b/include/asm-arm/arch-ep93xx/ep93xx.h @@ -558,8 +558,9 @@ struct syscon_regs { uint32_t i2sclkdiv; uint32_t keytchclkdiv; uint32_t chipid; + uint32_t reserved4; uint32_t syscfg; - uint32_t reserved4[8]; + uint32_t reserved5[8]; uint32_t sysswlock; }; #else

El Sat, Feb 06, 2010 at 08:53:43PM +0100 Alessandro Rubini ha dit:
The structure was missing a reserved entry (not listed in the manual, actually), so the last registers had a wrong offset. This prevented all swlocked registers to be modified as swlock is last in the structure.
Signed-off-by: Alessandro Rubini rubini@gnudd.com
Acked-by: Matthias Kaehlcke matthias@kaehlcke.net

Previous code compiled with gcc-4.2.2 makes a call to __aeabi_uidiv to divide by 20. As a side effect it was not inline any more, and so sdram_cfg used the stack as well, but this is early code that has no stack yet. The patch explicitly removes the division, so no stack is used.
The calculation of the counter calls a division by 20
Signed-off-by: Alessandro Rubini rubini@gnudd.com --- board/edb93xx/early_udelay.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/board/edb93xx/early_udelay.h b/board/edb93xx/early_udelay.h index 3b26b3f..185283d 100644 --- a/board/edb93xx/early_udelay.h +++ b/board/edb93xx/early_udelay.h @@ -26,7 +26,7 @@ static inline void early_udelay(uint32_t usecs) { /* loop takes 4 cycles at 5.0ns (fastest case, running at 200MHz) */ - register uint32_t loops = (usecs * 1000) / 20; + register uint32_t loops = usecs * (1000 / 20);
__asm__ volatile ("1:\n" "subs %0, %1, #1\n"

El Sat, Feb 06, 2010 at 08:53:54PM +0100 Alessandro Rubini ha dit:
Previous code compiled with gcc-4.2.2 makes a call to __aeabi_uidiv to divide by 20. As a side effect it was not inline any more, and so sdram_cfg used the stack as well, but this is early code that has no stack yet. The patch explicitly removes the division, so no stack is used.
The calculation of the counter calls a division by 20
Signed-off-by: Alessandro Rubini rubini@gnudd.com
Acked-by: Matthias Kaehlcke matthias@kaehlcke.net

printf goes to uart1, but it will block forever waiting for busy to go off unless the uart is enabled first.
Signed-off-by: Alessandro Rubini rubini@gnudd.com --- board/edb93xx/edb93xx.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/board/edb93xx/edb93xx.c b/board/edb93xx/edb93xx.c index 4df2246..dde30ff 100644 --- a/board/edb93xx/edb93xx.c +++ b/board/edb93xx/edb93xx.c @@ -64,6 +64,12 @@ int board_init(void) value |= SYSCON_PWRCNT_UART_BAUD; writel(value, &syscon->pwrcnt);
+ /* Enable the uart in devicecfg */ + value = readl(&syscon->devicecfg); + value |= 1<<18 /* U1EN */; + writel(0xAA, &syscon->sysswlock); + writel(value, &syscon->devicecfg); + /* Machine number, as defined in linux/arch/arm/tools/mach-types */ gd->bd->bi_arch_number = CONFIG_MACH_TYPE;

Hi Alessandro,
El Sat, Feb 06, 2010 at 08:54:05PM +0100 Alessandro Rubini ha dit:
printf goes to uart1, but it will block forever waiting for busy to go off unless the uart is enabled first.
Signed-off-by: Alessandro Rubini rubini@gnudd.com
board/edb93xx/edb93xx.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/board/edb93xx/edb93xx.c b/board/edb93xx/edb93xx.c index 4df2246..dde30ff 100644 --- a/board/edb93xx/edb93xx.c +++ b/board/edb93xx/edb93xx.c @@ -64,6 +64,12 @@ int board_init(void) value |= SYSCON_PWRCNT_UART_BAUD; writel(value, &syscon->pwrcnt);
- /* Enable the uart in devicecfg */
- value = readl(&syscon->devicecfg);
- value |= 1<<18 /* U1EN */;
using a constant like DEVCFG_U1EN would be preferrable, as the patch is correct at the functional and coding style level i'll leave it to Tom to decide if we fix this now or later
- writel(0xAA, &syscon->sysswlock);
- writel(value, &syscon->devicecfg);
- /* Machine number, as defined in linux/arch/arm/tools/mach-types */ gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
Acked-by: Matthias Kaehlcke matthias@kaehlcke.net

Matthias Kaehlcke wrote:
Hi Alessandro,
El Sat, Feb 06, 2010 at 08:54:05PM +0100 Alessandro Rubini ha dit:
printf goes to uart1, but it will block forever waiting for busy to go off unless the uart is enabled first.
Signed-off-by: Alessandro Rubini rubini@gnudd.com
board/edb93xx/edb93xx.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/board/edb93xx/edb93xx.c b/board/edb93xx/edb93xx.c index 4df2246..dde30ff 100644 --- a/board/edb93xx/edb93xx.c +++ b/board/edb93xx/edb93xx.c @@ -64,6 +64,12 @@ int board_init(void) value |= SYSCON_PWRCNT_UART_BAUD; writel(value, &syscon->pwrcnt);
- /* Enable the uart in devicecfg */
- value = readl(&syscon->devicecfg);
- value |= 1<<18 /* U1EN */;
using a constant like DEVCFG_U1EN would be preferrable, as the patch is correct at the functional and coding style level i'll leave it to Tom to decide if we fix this now or later
It is ok to fix this later. The patch set have been applied. Thanks Tom

Hi Alessandro,
El Sat, Feb 06, 2010 at 08:53:30PM +0100 Alessandro Rubini ha dit:
I'm porting current u-boot to a board similar to EP9315A, so I'm using arm/master as a basis, as it includes the patches by Matthias Kaehlcke. I'm currently running from RAM (SKIP_LOWLEVEL_INIT), after setting up sdram and pll elsewhere (older vendor u-boot code, still to be ported).
"fix syscon_regs definition" is needed to access any software-locked syscon register from C code (e.g., in reset_cpu() and devicecfg used by me in patch 3).
you are right, i missed the gap between ChipID and SysCfg
"change calculation un early_udelay.h" is needed at least for eldk-4.2 (gcc-4.2.2), as without this patch it will use the stack before setting SP. Actually, we could use the ether buffer as a stack, if needed, but it's not really needed here.
i feared this could happen with different compiler versions, your fix certainly reduces the risk
"enable the uart in devicecfg register" prevents u-boot from freezing at least with SKIP_LOWLEVEL_INIT set, but I'm pretty sure lowlevel_setup assembly code doesn't enable the uart, either.
my ep9301 and ep9307 based boards for some reason boot with the uart enabled, though according to the datasheet it is disabled until enabled explicitly. therefore your fix is necessary for all cpus behaving as expected.
thanks a lot for your patches!
participants (3)
-
Alessandro Rubini
-
Matthias Kaehlcke
-
Tom