Patchwork [U-Boot] v2011.12 and later do not boot on dreamplug.

login
register
mail settings
Submitter Ian Campbell
Date Jan. 8, 2012, 7:49 p.m.
Message ID <1326052145.29084.72.camel@dagon.hellion.org.uk>
Download mbox | patch
Permalink /patch/134938/
State Superseded
Headers show

Comments

Ian Campbell - Jan. 8, 2012, 7:49 p.m.
On Sun, 2012-01-08 at 15:30 +0000, Ian Campbell wrote:
> v2011.12 hangs on boot on my dreamplug with:
> 
>         U-Boot 2011.12-dirty (Jan 08 2012 - 15:16:44)
>         Marvell-DreamPlug
>         
>         SoC:   Kirkwood 88F6281_A1
>         DRAM:  512 MiB
>         WARNING: Caches not enabled
> 
> I bisected it down to:
>         commit ca4b55800ed74207c35271bf7335a092d4955416
>         Author: Heiko Schocher <hs@denx.de>
>         Date:   Wed Nov 9 20:06:23 2011 +0000
>         
>             arm, arm926ejs: always do cpu critical inits
>             
>             always do the cpu critical inits in cpu_init_crit,
>             and only jump to lowlevel_init, if CONFIG_SKIP_LOWLEVEL_INIT
>             is not defined.
>             
>             Signed-off-by: Heiko Schocher <hs@denx.de>
>             Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>             Cc: Wolfgang Denk <hs@denx.de>
>             Cc: Sandeep Paulraj <s-paulraj@ti.com>
>             Cc: Tom Rini <tom.rini@gmail.com>
>             Cc: Christian Riesch <christian.riesch@omicron.at>
> 
> and in particular the line
> 	orr	r0, r0, #0x00000002	/* set bit 2 (A) Align */
> 
> Commenting out just that one line causes things to work. Is the error
> setting this bit or is it that there is a bug somewhere else which
> prevents us setting this bit?

Seem that the latter is the case. The following fixes it for me.

(BTW, my mails to u-boot@ seem to be getting moderated, even though I am
subscribed, is that normal?)

8<-------------------------------------------------------------

From d6bc6e1a4f49314577e0be9af3cd020078593fdb Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Sun, 8 Jan 2012 19:44:38 +0000
Subject: [PATCH] kirkwood_spi: correct access to irq_mask register

Problem appears to have been present since day one but masked because alignment
aborts were not enabled. ca4b55800ed7 "arm, arm926ejs: always do cpu critical
inits" turned on alignment aborts and uncovered this latent problem.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 drivers/spi/kirkwood_spi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
u-boot@lakedaemon.net - Jan. 8, 2012, 10:54 p.m.
On Sun, Jan 08, 2012 at 07:49:05PM +0000, Ian Campbell wrote:
> On Sun, 2012-01-08 at 15:30 +0000, Ian Campbell wrote:
> > v2011.12 hangs on boot on my dreamplug with:
> > 
> >         U-Boot 2011.12-dirty (Jan 08 2012 - 15:16:44)
> >         Marvell-DreamPlug
> >         
> >         SoC:   Kirkwood 88F6281_A1
> >         DRAM:  512 MiB
> >         WARNING: Caches not enabled
> > 
> > I bisected it down to:
> >         commit ca4b55800ed74207c35271bf7335a092d4955416
> >         Author: Heiko Schocher <hs@denx.de>
> >         Date:   Wed Nov 9 20:06:23 2011 +0000
> >         
> >             arm, arm926ejs: always do cpu critical inits
> >             
> >             always do the cpu critical inits in cpu_init_crit,
> >             and only jump to lowlevel_init, if CONFIG_SKIP_LOWLEVEL_INIT
> >             is not defined.
> >             
> >             Signed-off-by: Heiko Schocher <hs@denx.de>
> >             Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >             Cc: Wolfgang Denk <hs@denx.de>
> >             Cc: Sandeep Paulraj <s-paulraj@ti.com>
> >             Cc: Tom Rini <tom.rini@gmail.com>
> >             Cc: Christian Riesch <christian.riesch@omicron.at>
> > 
> > and in particular the line
> > 	orr	r0, r0, #0x00000002	/* set bit 2 (A) Align */
> > 
> > Commenting out just that one line causes things to work. Is the error
> > setting this bit or is it that there is a bug somewhere else which
> > prevents us setting this bit?
> 
> Seem that the latter is the case. The following fixes it for me.
> 
> (BTW, my mails to u-boot@ seem to be getting moderated, even though I am
> subscribed, is that normal?)
> 
> 8<-------------------------------------------------------------
> 
> From d6bc6e1a4f49314577e0be9af3cd020078593fdb Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc@hellion.org.uk>
> Date: Sun, 8 Jan 2012 19:44:38 +0000
> Subject: [PATCH] kirkwood_spi: correct access to irq_mask register
> 
> Problem appears to have been present since day one but masked because alignment
> aborts were not enabled. ca4b55800ed7 "arm, arm926ejs: always do cpu critical
> inits" turned on alignment aborts and uncovered this latent problem.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> ---
>  drivers/spi/kirkwood_spi.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index dfe542d..db8ba8b 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -66,7 +66,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  	debug("data = 0x%08x \n", data);
>  
>  	writel(KWSPI_SMEMRDIRQ, &spireg->irq_cause);
> -	writel(KWSPI_IRQMASK, spireg->irq_mask);
> +	writel(KWSPI_IRQMASK, &spireg->irq_mask);
>  
>  	/* program mpp registers to select  SPI_CSn */
>  	if (cs) {
> -- 
> 1.7.5.4

Acked-By: Jason Cooper <u-boot@lakedaemon.net>

Dammit.  My apologies.  I should've caught that.  Thanks for the patch.

Jason.
Holger Brunck - Jan. 9, 2012, 10:59 a.m.
On 01/08/2012 09:00 PM, Ian Campbell wrote:
> On Sun, 2012-01-08 at 15:30 +0000, Ian Campbell wrote:
>> v2011.12 hangs on boot on my dreamplug with:
>>
>>         U-Boot 2011.12-dirty (Jan 08 2012 - 15:16:44)
>>         Marvell-DreamPlug
>>         
>>         SoC:   Kirkwood 88F6281_A1
>>         DRAM:  512 MiB
>>         WARNING: Caches not enabled
>>
>> I bisected it down to:
>>         commit ca4b55800ed74207c35271bf7335a092d4955416
>>         Author: Heiko Schocher <hs@denx.de>
>>         Date:   Wed Nov 9 20:06:23 2011 +0000
>>         
>>             arm, arm926ejs: always do cpu critical inits
>>             
>>             always do the cpu critical inits in cpu_init_crit,
>>             and only jump to lowlevel_init, if CONFIG_SKIP_LOWLEVEL_INIT
>>             is not defined.
>>             
>>             Signed-off-by: Heiko Schocher <hs@denx.de>
>>             Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>             Cc: Wolfgang Denk <hs@denx.de>
>>             Cc: Sandeep Paulraj <s-paulraj@ti.com>
>>             Cc: Tom Rini <tom.rini@gmail.com>
>>             Cc: Christian Riesch <christian.riesch@omicron.at>
>>
>> and in particular the line
>> 	orr	r0, r0, #0x00000002	/* set bit 2 (A) Align */
>>
>> Commenting out just that one line causes things to work. Is the error
>> setting this bit or is it that there is a bug somewhere else which
>> prevents us setting this bit?
> 
> Seem that the latter is the case. The following fixes it for me.
> 
> (BTW, my mails to u-boot@ seem to be getting moderated, even though I am
> subscribed, is that normal?)
> 
> 8<-------------------------------------------------------------
> 
> From d6bc6e1a4f49314577e0be9af3cd020078593fdb Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc@hellion.org.uk>
> Date: Sun, 8 Jan 2012 19:44:38 +0000
> Subject: [PATCH] kirkwood_spi: correct access to irq_mask register
> 
> Problem appears to have been present since day one but masked because alignment
> aborts were not enabled. ca4b55800ed7 "arm, arm926ejs: always do cpu critical
> inits" turned on alignment aborts and uncovered this latent problem.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> ---
>  drivers/spi/kirkwood_spi.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index dfe542d..db8ba8b 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -66,7 +66,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  	debug("data = 0x%08x \n", data);
>  
>  	writel(KWSPI_SMEMRDIRQ, &spireg->irq_cause);
> -	writel(KWSPI_IRQMASK, spireg->irq_mask);
> +	writel(KWSPI_IRQMASK, &spireg->irq_mask);
>  
>  	/* program mpp registers to select  SPI_CSn */
>  	if (cs) {

we had also SPI problems on our km_kirkwood boards with u-boot v2011.12. This
patch fixes it. Thanks! So:

Tested-By: Holger Brunck <holger.brunck@keymile.com>

Regards
Holger
Ian Campbell - Jan. 12, 2012, 8:01 a.m.
On Sun, 2012-01-08 at 17:54 -0500, Jason wrote:
> On Sun, Jan 08, 2012 at 07:49:05PM +0000, Ian Campbell wrote:
> > On Sun, 2012-01-08 at 15:30 +0000, Ian Campbell wrote:
> > diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> > index dfe542d..db8ba8b 100644
> > --- a/drivers/spi/kirkwood_spi.c
> > +++ b/drivers/spi/kirkwood_spi.c
> > @@ -66,7 +66,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> >  	debug("data = 0x%08x \n", data);
> >  
> >  	writel(KWSPI_SMEMRDIRQ, &spireg->irq_cause);
> > -	writel(KWSPI_IRQMASK, spireg->irq_mask);
> > +	writel(KWSPI_IRQMASK, &spireg->irq_mask);
> >  
> >  	/* program mpp registers to select  SPI_CSn */
> >  	if (cs) {
> > -- 
> > 1.7.5.4
> 
> Acked-By: Jason Cooper <u-boot@lakedaemon.net>
> 
> Dammit.  My apologies.  I should've caught that.  Thanks for the patch.

No problem, was interesting to dig into u-boot for the first time!

Did I get the CC list right to cover whoever will eventually apply this
patch?

You are also working on upstreaming dreamplug support into Linux, is
that right? What is the current state of the art? I'm currently using a
3.1 kernel + patches from
http://people.debian.org/~zumbi/dreamplug/patches/ . (BTW feel free to
reply off list or switch to e.g. linux-arm-kernel since I guess this is
strict speaking off-topic here).

Thanks,
Ian.
u-boot@lakedaemon.net - Jan. 12, 2012, 3:11 p.m.
Ian,

On Thu, Jan 12, 2012 at 08:01:45AM +0000, Ian Campbell wrote:
> On Sun, 2012-01-08 at 17:54 -0500, Jason wrote:
> > On Sun, Jan 08, 2012 at 07:49:05PM +0000, Ian Campbell wrote:
> > > On Sun, 2012-01-08 at 15:30 +0000, Ian Campbell wrote:
> > > diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> > > index dfe542d..db8ba8b 100644
> > > --- a/drivers/spi/kirkwood_spi.c
> > > +++ b/drivers/spi/kirkwood_spi.c
> > > @@ -66,7 +66,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> > >  	debug("data = 0x%08x \n", data);
> > >  
> > >  	writel(KWSPI_SMEMRDIRQ, &spireg->irq_cause);
> > > -	writel(KWSPI_IRQMASK, spireg->irq_mask);
> > > +	writel(KWSPI_IRQMASK, &spireg->irq_mask);
> > >  
> > >  	/* program mpp registers to select  SPI_CSn */
> > >  	if (cs) {
> > > -- 
> > > 1.7.5.4
> > 
> > Acked-By: Jason Cooper <u-boot@lakedaemon.net>
> > 
> > Dammit.  My apologies.  I should've caught that.  Thanks for the patch.
> 
> No problem, was interesting to dig into u-boot for the first time!

Well, at least my mistake created another u-boot hacker.  ;-)

> Did I get the CC list right to cover whoever will eventually apply this
> patch?

Meh, I've been pondering this.  The merge window is open right now, go
ahead and resubmit according to [1].  Make sure to put the Message-Id:
of the original patch email you sent as an In-Reply-To:, and mention in
the changelog that this 'v2' follows [1] for patch submissions.

Also, make sure Prafulla and Albert are in the CC if they aren't
already.

I think the original will get lost in the shuffle because it doesn't
have [PATCH 1/1] in the subject line.

> You are also working on upstreaming dreamplug support into Linux, is
> that right? What is the current state of the art? I'm currently using a
> 3.1 kernel + patches from
> http://people.debian.org/~zumbi/dreamplug/patches/ . (BTW feel free to
> reply off list or switch to e.g. linux-arm-kernel since I guess this is
> strict speaking off-topic here).

I use this [2] for 3.1, since it uses the same nor flash partitioning as
u-boot does.  This way, you could edit the u-boot config from userspace
or upgrade u-boot from userspace.

hth,

Jason.

[1] http://www.denx.de/wiki/U-Boot/Patches
[2] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/143116

Patch

diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index dfe542d..db8ba8b 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -66,7 +66,7 @@  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	debug("data = 0x%08x \n", data);
 
 	writel(KWSPI_SMEMRDIRQ, &spireg->irq_cause);
-	writel(KWSPI_IRQMASK, spireg->irq_mask);
+	writel(KWSPI_IRQMASK, &spireg->irq_mask);
 
 	/* program mpp registers to select  SPI_CSn */
 	if (cs) {