[U-Boot] [PATCH] ppc4xx: Fix initialization of SDRAM_CODT register in ppc_4xx_ddr2.c

[U-Boot] [PATCH] ppc4xx: Fix initialization of SDRAM_CODT register in ppc_4xx_ddr2.c

[carolyn.j.smith at tektronix.com]

This fixes the initialization of the SDRAM_CODT register in the ppc4xx DDR2 initialization code. The changes were discussed with Stefan Roese and Adam Graham and tested by Adam.

Signed-off-by: Carolyn Smith <carolyn.smith@tektronix.com>
---
diff -w -uprN old/cpu/ppc4xx/44x_spd_ddr2.c new/cpu/ppc4xx/44x_spd_ddr2.c
--- old/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-20 13:00:33.000000000 -0800
+++ new/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-23 16:00:49.000000000 -0800
@@ -1104,11 +1104,14 @@ static void program_codt(unsigned long *
         * Set the SDRAM Controller On Die Termination Register
         *-----------------------------------------------------------------*/
        mfsdram(SDRAM_CODT, codt);
-       codt |= (SDRAM_CODT_IO_NMODE
-                & (~SDRAM_CODT_DQS_SINGLE_END
-                   & ~SDRAM_CODT_CKSE_SINGLE_END
-                   & ~SDRAM_CODT_FEEBBACK_RCV_SINGLE_END
-                   & ~SDRAM_CODT_FEEBBACK_DRV_SINGLE_END));
+
+       codt |= SDRAM_CODT_IO_NMODE;
+
+       codt &= ~(SDRAM_CODT_DQS_SINGLE_END |
+                 SDRAM_CODT_CKSE_SINGLE_END |
+                 SDRAM_CODT_FEEBBACK_RCV_SINGLE_END |
+                 SDRAM_CODT_FEEBBACK_DRV_SINGLE_END);
+

        for (dimm_num = 0; dimm_num < num_dimm_banks; dimm_num++) {
                if (dimm_populated[dimm_num] != SDRAM_NONE) {

[U-Boot] [PATCH] ppc4xx: Fix initialization of SDRAM_CODT register in ppc_4xx_ddr2.c

[Stefan Roese]

On Monday 26 January 2009, carolyn.j.smith at tektronix.com wrote:
> This fixes the initialization of the SDRAM_CODT register in the ppc4xx DDR2
> initialization code. The changes were discussed with Stefan Roese and Adam
> Graham and tested by Adam.

General comment: The line "The changes were discussed..." should be placed 
below the "---" since it doesn't really belong to the patch description.

> Signed-off-by: Carolyn Smith <carolyn.smith@tektronix.com>
> ---
> diff -w -uprN old/cpu/ppc4xx/44x_spd_ddr2.c new/cpu/ppc4xx/44x_spd_ddr2.c
> --- old/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-20 13:00:33.000000000 -0800
> +++ new/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-23 16:00:49.000000000 -0800
> @@ -1104,11 +1104,14 @@ static void program_codt(unsigned long *
>          * Set the SDRAM Controller On Die Termination Register
>         
> *-----------------------------------------------------------------*/
> mfsdram(SDRAM_CODT, codt);
> -       codt |= (SDRAM_CODT_IO_NMODE
> -                & (~SDRAM_CODT_DQS_SINGLE_END
> -                   & ~SDRAM_CODT_CKSE_SINGLE_END
> -                   & ~SDRAM_CODT_FEEBBACK_RCV_SINGLE_END
> -                   & ~SDRAM_CODT_FEEBBACK_DRV_SINGLE_END));
> +
> +       codt |= SDRAM_CODT_IO_NMODE;
> +
> +       codt &= ~(SDRAM_CODT_DQS_SINGLE_END |
> +                 SDRAM_CODT_CKSE_SINGLE_END |
> +                 SDRAM_CODT_FEEBBACK_RCV_SINGLE_END |
> +                 SDRAM_CODT_FEEBBACK_DRV_SINGLE_END);
> +

I don't think that this is correct. As I understood from Adam, those bits it 
questions are undefined/reserevd on all 4xx PPC's. So we should better remove 
the code referencing those values. Here is what Adam wrote:

> It is in fact the case for all the AMCC 4xx SoC chips that have the IBM
> SDRAM memory controller core IP, the SDRAM_CODT register, bits 29 and 30
> are reserved bits.  Thanks Carolyn for finding this issue.  We will
> update the 44x_spd_ddr2.c file and send out a patch shortly for this
> SDRAM_CODT register bits 29-30 issue.

Adam, do you still plan to send a patch for this? Or did I misunderstand you 
and the patch above is ok?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] ppc4xx: Fix initialization of SDRAM_CODT register in ppc_4xx_ddr2.c

[Adam Graham]

Stefan, Carolyn,

Yes, I do intend on sending out an AMCC patch to remove the definition
for SDRAM_CODT_FEEBBACK_RCV_SINGLE_END and
SDRAM_CODT_FEEBBACK_DRV_SINGLE_END.

My apologies Carolyn as I should have notified you of this intent.

My AMCC patch to remove these SDRAM_CODT bit definitions may come after
your patch Carolyn, so in your patch do not include the usage of these
(SDRAM_CODT_FEEBBACK_RCV_SINGLE_END and
SDRAM_CODT_FEEBBACK_DRV_SINGLE_END) bits.

According to our AMCC PPC4xx chip hardware designers, the SDRAM_CODT IP
feedback signal is single ended (i.e. SDRAM_CODT bits 29 and 30).

The PPC4xx SDRAM_CODT bits 29 and 30 are reserved non-changeable bits as
they are set in the IP to single-ended mode:

SDRAM_CODT bit 29  0 Sets feedback receiver to differential mode
                   1 Sets feedback receiver to single ended mode (mode
set for the AMCC 4xx SoC chips)
           bit 30  0 Sets feedback driver to differential mode
                   1 Sets feedback driver to single ended mode   (mode
set for the AMCC 4xx SoC chips)


Best,
Adam

> -----Original Message-----
> From: Stefan Roese [mailto:sr at denx.de] 
> Sent: Monday, January 26, 2009 9:47 PM
> To: u-boot at lists.denx.de
> Cc: carolyn.j.smith at tektronix.com; Adam Graham
> Subject: Re: [U-Boot] [PATCH] ppc4xx: Fix initialization of 
> SDRAM_CODT register in ppc_4xx_ddr2.c
> 
> On Monday 26 January 2009, carolyn.j.smith at tektronix.com wrote:
> > This fixes the initialization of the SDRAM_CODT register in 
> the ppc4xx 
> > DDR2 initialization code. The changes were discussed with 
> Stefan Roese 
> > and Adam Graham and tested by Adam.
> 
> General comment: The line "The changes were discussed..." 
> should be placed below the "---" since it doesn't really 
> belong to the patch description.
> 
> > Signed-off-by: Carolyn Smith <carolyn.smith@tektronix.com>
> > ---
> > diff -w -uprN old/cpu/ppc4xx/44x_spd_ddr2.c 
> new/cpu/ppc4xx/44x_spd_ddr2.c
> > --- old/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-20 
> 13:00:33.000000000 -0800
> > +++ new/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-23 
> 16:00:49.000000000 -0800
> > @@ -1104,11 +1104,14 @@ static void program_codt(unsigned long *
> >          * Set the SDRAM Controller On Die Termination Register
> >         
> > *-----------------------------------------------------------------*/
> > mfsdram(SDRAM_CODT, codt);
> > -       codt |= (SDRAM_CODT_IO_NMODE
> > -                & (~SDRAM_CODT_DQS_SINGLE_END
> > -                   & ~SDRAM_CODT_CKSE_SINGLE_END
> > -                   & ~SDRAM_CODT_FEEBBACK_RCV_SINGLE_END
> > -                   & ~SDRAM_CODT_FEEBBACK_DRV_SINGLE_END));
> > +
> > +       codt |= SDRAM_CODT_IO_NMODE;
> > +
> > +       codt &= ~(SDRAM_CODT_DQS_SINGLE_END |
> > +                 SDRAM_CODT_CKSE_SINGLE_END |
> > +                 SDRAM_CODT_FEEBBACK_RCV_SINGLE_END |
> > +                 SDRAM_CODT_FEEBBACK_DRV_SINGLE_END);
> > +
> 
> I don't think that this is correct. As I understood from 
> Adam, those bits it questions are undefined/reserevd on all 
> 4xx PPC's. So we should better remove the code referencing 
> those values. Here is what Adam wrote:
> 
> > It is in fact the case for all the AMCC 4xx SoC chips that have the 
> > IBM SDRAM memory controller core IP, the SDRAM_CODT 
> register, bits 29 
> > and 30 are reserved bits.  Thanks Carolyn for finding this 
> issue.  We 
> > will update the 44x_spd_ddr2.c file and send out a patch 
> shortly for 
> > this SDRAM_CODT register bits 29-30 issue.
> 
> Adam, do you still plan to send a patch for this? Or did I 
> misunderstand you and the patch above is ok?
> 
> Thanks.
> 
> Best regards,
> Stefan
> 
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: 
> office at denx.de 
> =====================================================================
> 

[U-Boot] [PATCH] ppc4xx: Fix initialization of SDRAM_CODT register in ppc_4xx_ddr2.c

[carolyn.j.smith at tektronix.com]

Okay, I will resubmit my patch.

Carolyn

>-----Original Message-----
>From: Adam Graham [mailto:agraham at amcc.com]
>Sent: Tuesday, January 27, 2009 11:22 PM
>To: Stefan Roese; u-boot at lists.denx.de
>Cc: Smith, Carolyn J; Adam Graham; Victor Gallardo
>Subject: RE: [U-Boot] [PATCH] ppc4xx: Fix initialization of
>SDRAM_CODT register in ppc_4xx_ddr2.c
>
>Stefan, Carolyn,
>
>Yes, I do intend on sending out an AMCC patch to remove the definition
>for SDRAM_CODT_FEEBBACK_RCV_SINGLE_END and
>SDRAM_CODT_FEEBBACK_DRV_SINGLE_END.
>
>My apologies Carolyn as I should have notified you of this intent.
>
>My AMCC patch to remove these SDRAM_CODT bit definitions may come after
>your patch Carolyn, so in your patch do not include the usage of these
>(SDRAM_CODT_FEEBBACK_RCV_SINGLE_END and
>SDRAM_CODT_FEEBBACK_DRV_SINGLE_END) bits.
>
>According to our AMCC PPC4xx chip hardware designers, the SDRAM_CODT IP
>feedback signal is single ended (i.e. SDRAM_CODT bits 29 and 30).
>
>The PPC4xx SDRAM_CODT bits 29 and 30 are reserved
>non-changeable bits as
>they are set in the IP to single-ended mode:
>
>SDRAM_CODT bit 29  0 Sets feedback receiver to differential mode
>                   1 Sets feedback receiver to single ended mode (mode
>set for the AMCC 4xx SoC chips)
>           bit 30  0 Sets feedback driver to differential mode
>                   1 Sets feedback driver to single ended mode   (mode
>set for the AMCC 4xx SoC chips)
>
>
>Best,
>Adam
>
>> -----Original Message-----
>> From: Stefan Roese [mailto:sr at denx.de]
>> Sent: Monday, January 26, 2009 9:47 PM
>> To: u-boot at lists.denx.de
>> Cc: carolyn.j.smith at tektronix.com; Adam Graham
>> Subject: Re: [U-Boot] [PATCH] ppc4xx: Fix initialization of
>> SDRAM_CODT register in ppc_4xx_ddr2.c
>>
>> On Monday 26 January 2009, carolyn.j.smith at tektronix.com wrote:
>> > This fixes the initialization of the SDRAM_CODT register in
>> the ppc4xx
>> > DDR2 initialization code. The changes were discussed with
>> Stefan Roese
>> > and Adam Graham and tested by Adam.
>>
>> General comment: The line "The changes were discussed..."
>> should be placed below the "---" since it doesn't really
>> belong to the patch description.
>>
>> > Signed-off-by: Carolyn Smith <carolyn.smith@tektronix.com>
>> > ---
>> > diff -w -uprN old/cpu/ppc4xx/44x_spd_ddr2.c
>> new/cpu/ppc4xx/44x_spd_ddr2.c
>> > --- old/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-20
>> 13:00:33.000000000 -0800
>> > +++ new/cpu/ppc4xx/44x_spd_ddr2.c       2009-01-23
>> 16:00:49.000000000 -0800
>> > @@ -1104,11 +1104,14 @@ static void program_codt(unsigned long *
>> >          * Set the SDRAM Controller On Die Termination Register
>> >
>> >
>*-----------------------------------------------------------------*/
>> > mfsdram(SDRAM_CODT, codt);
>> > -       codt |= (SDRAM_CODT_IO_NMODE
>> > -                & (~SDRAM_CODT_DQS_SINGLE_END
>> > -                   & ~SDRAM_CODT_CKSE_SINGLE_END
>> > -                   & ~SDRAM_CODT_FEEBBACK_RCV_SINGLE_END
>> > -                   & ~SDRAM_CODT_FEEBBACK_DRV_SINGLE_END));
>> > +
>> > +       codt |= SDRAM_CODT_IO_NMODE;
>> > +
>> > +       codt &= ~(SDRAM_CODT_DQS_SINGLE_END |
>> > +                 SDRAM_CODT_CKSE_SINGLE_END |
>> > +                 SDRAM_CODT_FEEBBACK_RCV_SINGLE_END |
>> > +                 SDRAM_CODT_FEEBBACK_DRV_SINGLE_END);
>> > +
>>
>> I don't think that this is correct. As I understood from
>> Adam, those bits it questions are undefined/reserevd on all
>> 4xx PPC's. So we should better remove the code referencing
>> those values. Here is what Adam wrote:
>>
>> > It is in fact the case for all the AMCC 4xx SoC chips that have the
>> > IBM SDRAM memory controller core IP, the SDRAM_CODT
>> register, bits 29
>> > and 30 are reserved bits.  Thanks Carolyn for finding this
>> issue.  We
>> > will update the 44x_spd_ddr2.c file and send out a patch
>> shortly for
>> > this SDRAM_CODT register bits 29-30 issue.
>>
>> Adam, do you still plan to send a patch for this? Or did I
>> misunderstand you and the patch above is ok?
>>
>> Thanks.
>>
>> Best regards,
>> Stefan
>>
>> =====================================================================
>> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email:
>> office at denx.de
>> =====================================================================
>>
>
>