public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/3] Fixes for edb9315
@ 2010-02-06 19:53 Alessandro Rubini
  2010-02-06 19:53 ` [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition Alessandro Rubini
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alessandro Rubini @ 2010-02-06 19:53 UTC (permalink / raw)
  To: u-boot

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(-)

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

* [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition
  2010-02-06 19:53 [U-Boot] [PATCH 0/3] Fixes for edb9315 Alessandro Rubini
@ 2010-02-06 19:53 ` Alessandro Rubini
  2010-02-06 22:17   ` Matthias Kaehlcke
  2010-02-06 19:53 ` [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h Alessandro Rubini
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Alessandro Rubini @ 2010-02-06 19:53 UTC (permalink / raw)
  To: u-boot

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
-- 
1.6.0.2

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

* [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h
  2010-02-06 19:53 [U-Boot] [PATCH 0/3] Fixes for edb9315 Alessandro Rubini
  2010-02-06 19:53 ` [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition Alessandro Rubini
@ 2010-02-06 19:53 ` Alessandro Rubini
  2010-02-06 22:17   ` Matthias Kaehlcke
  2010-02-06 19:54 ` [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register Alessandro Rubini
  2010-02-06 22:15 ` [U-Boot] [PATCH 0/3] Fixes for edb9315 Matthias Kaehlcke
  3 siblings, 1 reply; 9+ messages in thread
From: Alessandro Rubini @ 2010-02-06 19:53 UTC (permalink / raw)
  To: u-boot

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"
-- 
1.6.0.2

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

* [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register
  2010-02-06 19:53 [U-Boot] [PATCH 0/3] Fixes for edb9315 Alessandro Rubini
  2010-02-06 19:53 ` [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition Alessandro Rubini
  2010-02-06 19:53 ` [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h Alessandro Rubini
@ 2010-02-06 19:54 ` Alessandro Rubini
  2010-02-06 22:24   ` Matthias Kaehlcke
  2010-02-06 22:15 ` [U-Boot] [PATCH 0/3] Fixes for edb9315 Matthias Kaehlcke
  3 siblings, 1 reply; 9+ messages in thread
From: Alessandro Rubini @ 2010-02-06 19:54 UTC (permalink / raw)
  To: u-boot

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;
 
-- 
1.6.0.2

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

* [U-Boot] [PATCH 0/3] Fixes for edb9315
  2010-02-06 19:53 [U-Boot] [PATCH 0/3] Fixes for edb9315 Alessandro Rubini
                   ` (2 preceding siblings ...)
  2010-02-06 19:54 ` [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register Alessandro Rubini
@ 2010-02-06 22:15 ` Matthias Kaehlcke
  3 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2010-02-06 22:15 UTC (permalink / raw)
  To: u-boot

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!

-- 
Matthias Kaehlcke
Embedded Linux Developer
Barcelona

      The assumption that what currently exists must necessarily
        exist is the acid that corrodes all visionary thinking
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

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

* [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition
  2010-02-06 19:53 ` [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition Alessandro Rubini
@ 2010-02-06 22:17   ` Matthias Kaehlcke
  0 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2010-02-06 22:17 UTC (permalink / raw)
  To: u-boot

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>

-- 
Matthias Kaehlcke
Embedded Linux Developer
Barcelona

             If liberty means anything at all, it means the
           right to tell people what they do not want to hear
                            (George Orwell)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

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

* [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h
  2010-02-06 19:53 ` [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h Alessandro Rubini
@ 2010-02-06 22:17   ` Matthias Kaehlcke
  0 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2010-02-06 22:17 UTC (permalink / raw)
  To: u-boot

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>

-- 
Matthias Kaehlcke
Embedded Linux Developer
Barcelona

              We build too many walls and not enough bridges
                             (Isaac Newton)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

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

* [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register
  2010-02-06 19:54 ` [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register Alessandro Rubini
@ 2010-02-06 22:24   ` Matthias Kaehlcke
  2010-02-07 23:08     ` Tom
  0 siblings, 1 reply; 9+ messages in thread
From: Matthias Kaehlcke @ 2010-02-06 22:24 UTC (permalink / raw)
  To: u-boot

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@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
Embedded Linux Developer
Barcelona

             If liberty means anything at all, it means the
           right to tell people what they do not want to hear
                            (George Orwell)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

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

* [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register
  2010-02-06 22:24   ` Matthias Kaehlcke
@ 2010-02-07 23:08     ` Tom
  0 siblings, 0 replies; 9+ messages in thread
From: Tom @ 2010-02-07 23:08 UTC (permalink / raw)
  To: u-boot

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

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

end of thread, other threads:[~2010-02-07 23:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-06 19:53 [U-Boot] [PATCH 0/3] Fixes for edb9315 Alessandro Rubini
2010-02-06 19:53 ` [U-Boot] [PATCH 1/3] EP93xx: fix syscon_regs definition Alessandro Rubini
2010-02-06 22:17   ` Matthias Kaehlcke
2010-02-06 19:53 ` [U-Boot] [PATCH 2/3] edb93xx: change calculation un early_udelay.h Alessandro Rubini
2010-02-06 22:17   ` Matthias Kaehlcke
2010-02-06 19:54 ` [U-Boot] [PATCH 3/3] edb93xx: enable the uart in devicecfg register Alessandro Rubini
2010-02-06 22:24   ` Matthias Kaehlcke
2010-02-07 23:08     ` Tom
2010-02-06 22:15 ` [U-Boot] [PATCH 0/3] Fixes for edb9315 Matthias Kaehlcke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox