Patchwork PPC440EPx SDRAM width

login
register
mail settings
Submitter Steven A. Falco
Date April 23, 2009, 1:36 p.m.
Message ID <49F06ECC.7030904@harris.com>
Download mbox | patch
Permalink /patch/26367/
State Superseded
Headers show

Comments

Steven A. Falco - April 23, 2009, 1:36 p.m.
There is an error in the way ibm4xx_denali_fixup_memsize calculates
memory size.  When testing the DDR_REDUC bit, the polarity is
backwards.  A "1" implies 32-bit wide memory while a "0" implies
64-bit wide memory.

For a 32-bit wide system, this bug causes twice the memory to be
reported, leading to boot failure.

Signed-off-by: Steven A. Falco <sfalco@harris.com>
---

Here is a partial log showing what happens in the
ibm4xx_denali_fixup_memsize routine.  This board has
128 Mbytes of RAM in a 32-bit wide configuration, but
the REDUC bug causes the path width to report as "8",
and the memory size to be doubled to 255 MB.  Once I
applied my patch, the memory correctly reported as
127 MB.

max_cs=00000002 max_col=0000000c max_row=0000000e
cs 00000001
path width 00000008
row=0000000d col=0000000a bank=00000004
ibm4xx_denali_fixup_memsize 10000000
Memory <- <0x0 0x0 0xffff000> (255MB)
Josh Boyer - April 23, 2009, 1:45 p.m.
On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
>There is an error in the way ibm4xx_denali_fixup_memsize calculates
>memory size.  When testing the DDR_REDUC bit, the polarity is
>backwards.  A "1" implies 32-bit wide memory while a "0" implies
>64-bit wide memory.
>
>For a 32-bit wide system, this bug causes twice the memory to be
>reported, leading to boot failure.
>
>Signed-off-by: Steven A. Falco <sfalco@harris.com>

So we had a previous patch for this, and a very long discussion on what the
right solution was.  Either we never came to a resolution, or I have just
forgotten what it was.

Stefan, Valentine, do either of you remember?  IIRC, it wasn't something that
effected Sequoia or Rainier, but it could (and obviously does) effect custom
boards.  I don't remember what we agreed on for the proper fix.

josh
Stefan Roese - April 23, 2009, 2:05 p.m.
On Thursday 23 April 2009, Josh Boyer wrote:
> On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
> >There is an error in the way ibm4xx_denali_fixup_memsize calculates
> >memory size.  When testing the DDR_REDUC bit, the polarity is
> >backwards.  A "1" implies 32-bit wide memory while a "0" implies
> >64-bit wide memory.
> >
> >For a 32-bit wide system, this bug causes twice the memory to be
> >reported, leading to boot failure.
> >
> >Signed-off-by: Steven A. Falco <sfalco@harris.com>
>
> So we had a previous patch for this, and a very long discussion on what the
> right solution was.  Either we never came to a resolution, or I have just
> forgotten what it was.
>
> Stefan, Valentine, do either of you remember?

Not really, sorry. Must be longer than 2 weeks ago, so it's already flushed 
from my cache. :)

> IIRC, it wasn't something 
> that effected Sequoia or Rainier, but it could (and obviously does) effect
> custom boards.  I don't remember what we agreed on for the proper fix.

It would effect all 32-bit wide 440EPx/GRx boards using the boot wrapper. I 
never used the wrapper on those platforms though. Sorry, I don't remember the 
outcome of the discussion either.

Thanks,
Stefan
Valentine Barshak - April 23, 2009, 2:40 p.m.
Stefan Roese wrote:
> On Thursday 23 April 2009, Josh Boyer wrote:
>> On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
>>> There is an error in the way ibm4xx_denali_fixup_memsize calculates
>>> memory size.  When testing the DDR_REDUC bit, the polarity is
>>> backwards.  A "1" implies 32-bit wide memory while a "0" implies
>>> 64-bit wide memory.
>>>
>>> For a 32-bit wide system, this bug causes twice the memory to be
>>> reported, leading to boot failure.
>>>
>>> Signed-off-by: Steven A. Falco <sfalco@harris.com>
>> So we had a previous patch for this, and a very long discussion on what the
>> right solution was.  Either we never came to a resolution, or I have just
>> forgotten what it was.
>>
>> Stefan, Valentine, do either of you remember?
> 

The patch will break sequia/rainier since u-boot doesn't set the number 
of chipselects correctly for them. IIRC, the last conversation didn't 
come to any conclusion. We sort of wanted to fix that regardless of 
whether we had corrected u-boot or not.

Could we use a "model" property to distinguish between the "real" 
sequoia/rainier and other custom boards?
If yes, we could add a workaround the ibm4xx_denali_fixup_memsize to 
hardcode the chipselect number to 1 for sequoia/rainier.

Thanks,
Val.

> Not really, sorry. Must be longer than 2 weeks ago, so it's already flushed 
> from my cache. :)
> 
>> IIRC, it wasn't something 
>> that effected Sequoia or Rainier, but it could (and obviously does) effect
>> custom boards.  I don't remember what we agreed on for the proper fix.
> 
> It would effect all 32-bit wide 440EPx/GRx boards using the boot wrapper. I 
> never used the wrapper on those platforms though. Sorry, I don't remember the 
> outcome of the discussion either.
> 
> Thanks,
> Stefan
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
Josh Boyer - April 23, 2009, 2:59 p.m.
On Thu, Apr 23, 2009 at 06:40:48PM +0400, Valentine Barshak wrote:
> Stefan Roese wrote:
>> On Thursday 23 April 2009, Josh Boyer wrote:
>>> On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
>>>> There is an error in the way ibm4xx_denali_fixup_memsize calculates
>>>> memory size.  When testing the DDR_REDUC bit, the polarity is
>>>> backwards.  A "1" implies 32-bit wide memory while a "0" implies
>>>> 64-bit wide memory.
>>>>
>>>> For a 32-bit wide system, this bug causes twice the memory to be
>>>> reported, leading to boot failure.
>>>>
>>>> Signed-off-by: Steven A. Falco <sfalco@harris.com>
>>> So we had a previous patch for this, and a very long discussion on what the
>>> right solution was.  Either we never came to a resolution, or I have just
>>> forgotten what it was.
>>>
>>> Stefan, Valentine, do either of you remember?
>>
>
> The patch will break sequia/rainier since u-boot doesn't set the number  
> of chipselects correctly for them. IIRC, the last conversation didn't  
> come to any conclusion. We sort of wanted to fix that regardless of  
> whether we had corrected u-boot or not.
>
> Could we use a "model" property to distinguish between the "real"  
> sequoia/rainier and other custom boards?
> If yes, we could add a workaround the ibm4xx_denali_fixup_memsize to  
> hardcode the chipselect number to 1 for sequoia/rainier.

We could do that perhaps, yes.  In cases where the board has a newer U-Boot
with the fix already, it shouldn't really cause any harm, correct?

josh
Valentine Barshak - April 23, 2009, 3:16 p.m.
Josh Boyer wrote:
> On Thu, Apr 23, 2009 at 06:40:48PM +0400, Valentine Barshak wrote:
>> Stefan Roese wrote:
>>> On Thursday 23 April 2009, Josh Boyer wrote:
>>>> On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
>>>>> There is an error in the way ibm4xx_denali_fixup_memsize calculates
>>>>> memory size.  When testing the DDR_REDUC bit, the polarity is
>>>>> backwards.  A "1" implies 32-bit wide memory while a "0" implies
>>>>> 64-bit wide memory.
>>>>>
>>>>> For a 32-bit wide system, this bug causes twice the memory to be
>>>>> reported, leading to boot failure.
>>>>>
>>>>> Signed-off-by: Steven A. Falco <sfalco@harris.com>
>>>> So we had a previous patch for this, and a very long discussion on what the
>>>> right solution was.  Either we never came to a resolution, or I have just
>>>> forgotten what it was.
>>>>
>>>> Stefan, Valentine, do either of you remember?
>> The patch will break sequia/rainier since u-boot doesn't set the number  
>> of chipselects correctly for them. IIRC, the last conversation didn't  
>> come to any conclusion. We sort of wanted to fix that regardless of  
>> whether we had corrected u-boot or not.
>>
>> Could we use a "model" property to distinguish between the "real"  
>> sequoia/rainier and other custom boards?
>> If yes, we could add a workaround the ibm4xx_denali_fixup_memsize to  
>> hardcode the chipselect number to 1 for sequoia/rainier.
> 
> We could do that perhaps, yes.  In cases where the board has a newer U-Boot
> with the fix already, it shouldn't really cause any harm, correct?

Yes, that's correct.

Thanks,
Val

> 
> josh
Steven A. Falco - April 23, 2009, 4 p.m.
Valentine Barshak wrote:
> 
> Josh Boyer wrote:
>> On Thu, Apr 23, 2009 at 06:40:48PM +0400, Valentine Barshak wrote:
>>> Stefan Roese wrote:
>>>> On Thursday 23 April 2009, Josh Boyer wrote:
>>>>> On Thu, Apr 23, 2009 at 09:36:12AM -0400, Steven A. Falco wrote:
>>>>>> There is an error in the way ibm4xx_denali_fixup_memsize calculates
>>>>>> memory size.  When testing the DDR_REDUC bit, the polarity is
>>>>>> backwards.  A "1" implies 32-bit wide memory while a "0" implies
>>>>>> 64-bit wide memory.
>>>>>>
>>>>>> For a 32-bit wide system, this bug causes twice the memory to be
>>>>>> reported, leading to boot failure.
>>>>>>
>>>>>> Signed-off-by: Steven A. Falco <sfalco@harris.com>
>>>>> So we had a previous patch for this, and a very long discussion on
>>>>> what the
>>>>> right solution was.  Either we never came to a resolution, or I
>>>>> have just
>>>>> forgotten what it was.
>>>>>
>>>>> Stefan, Valentine, do either of you remember?
>>> The patch will break sequia/rainier since u-boot doesn't set the
>>> number  of chipselects correctly for them. 

I had wondered about that.  I was surprised that sequoia uboot
enabled both cs bits - now I know why it worked.  Two wrongs
sometimes do make a right. :-)

>>> IIRC, the last
>>> conversation didn't  come to any conclusion. We sort of wanted to fix
>>> that regardless of  whether we had corrected u-boot or not.

That would have saved me a bit of grief.  It will probably
help someone else someday too.

>>> Could we use a "model" property to distinguish between the "real" 
>>> sequoia/rainier and other custom boards?
>>> If yes, we could add a workaround the ibm4xx_denali_fixup_memsize to 
>>> hardcode the chipselect number to 1 for sequoia/rainier.
>>
>> We could do that perhaps, yes.  In cases where the board has a newer
>> U-Boot
>> with the fix already, it shouldn't really cause any harm, correct?
> 
> Yes, that's correct.

Not sure if you want me to do something further in my patch.
Are you suggesting testing for model = "amcc,sequoia" and
forcing cs to 1, or is there more to it?

	Steve

> 
> Thanks,
> Val
> 
>>
>> josh
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

Patch

diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 5c87843..3595c0a 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -193,9 +193,9 @@  void ibm4xx_denali_fixup_memsize(void)
        val = SDRAM0_READ(DDR0_14);

        if (DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT))
-               dpath = 8; /* 64 bits */
-       else
                dpath = 4; /* 32 bits */
+       else
+               dpath = 8; /* 64 bits */

        /* get address pins (rows) */
        val = SDRAM0_READ(DDR0_42);