[U-Boot] question about U-Boot's cpu/ppc4xx/44x_spd_ddr2.c

[Adam Graham]

Thanks Stefan for handling this and thank you Carolyn for finding this
bug.
 

> -----Original Message-----
> From: Stefan Roese [mailto:sr at denx.de] 
> Sent: Monday, January 19, 2009 8:10 AM
> To: u-boot at lists.denx.de
> Cc: carolyn.j.smith at tektronix.com; Adam Graham
> Subject: Re: [U-Boot] question about U-Boot's 
> cpu/ppc4xx/44x_spd_ddr2.c
> 
> Hi Carolyn,
> 
> On Thursday 15 January 2009, carolyn.j.smith at tektronix.com wrote:
> > I have a question about some of the code in U-Boot's 
> > cpu/ppc4xx/44x_spd_ddr2.c, in particular the program_codt 
> function and 
> > these lines of code:
> >
> >         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));
> >
> > What is the intention of this code? As far as I can tell, all it is 
> > doing is or'ing SDRAM_CODT_IO_NMODE (= 0x00000001) into the codt 
> > variable and the
> >
> >                  & (~SDRAM_CODT_DQS_SINGLE_END
> >                     & ~SDRAM_CODT_CKSE_SINGLE_END
> >                     & ~SDRAM_CODT_FEEBBACK_RCV_SINGLE_END
> >                     & ~SDRAM_CODT_FEEBBACK_DRV_SINGLE_END)
> >
> > part of it has no real effect.
> 
> Good catch.

Yes, good catch.


> 
> > Was the intention to turn on the SDRAM_CODT_IO_NMODE bit 
> and turn off 
> > the SDRAM_CODT_DQS_SINGLE_END, SDRAM_CODT_CKSE_SINGLE_END, 
> > SDRAM_CODT_FEEBBACK_RCV_SINGLE_END and 
> > SDRAM_CODT_FEEBBACK_DRV_SINGLE_END
> > bits in which case the code should be something like this?
> >
> >         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);
> 
> Not being the original author of this code, I can only guess 
> that this is what this code *should* do. Please send a patch 
> to fix this.
> 

Yes, this is the intent of the code.  Basically the intent is as Carolyn
proposed which is to reset the bits for SDRAM_CODT_DQS_SINGLE_END,
SDRAM_CODT_CKSE_SINGLE_END, SDRAM_CODT_FEEBBACK_RCV_SINGLE_END, and
SDRAM_CODT_FEEBBACK_DRV_SINGLE_END, and then to set the bit for
SDRAM_CODT_IO_NMODE.  Putting this in 2 operations is the right thing to
do, it is easier to read the code, and it conveys the intention.

Your code above is the fix for a patch.


> > Also the 460EX manual I have (revision 1.16 - November 17, 
> 2008) shows 
> > bits
> > 29 and 30 of the CODT register as Reserved (in which case they 
> > shouldn't be
> > modified) while U-Boot has them as
> >
> > #define SDRAM_CODT_FEEBBACK_RCV_SINGLE_END      0x00000004
> > #define SDRAM_CODT_FEEBBACK_DRV_SINGLE_END      0x00000002
> >
> > Do I have an out-of-date manual or are they reserved only for the 
> > 460EX and not for other processors using this code?
> 
> No, you seem to be correct here too. I couldn't find those 
> defines in the 460EX users manual (same version) or in the 
> current 440SPe users manual.

It does appear that for the 460EX/GT SDRAM_CODT register bits 29 and 30
are documented as reserved.  These bits are defined in the PPC405EX
chip.  I'll ask the PPC460EX/GT chip designers about these bits 29 and
30 and why they are defined in the PPC405EX and not the PPC460EX/GT and
send a follow up email and we can then #ifdef the code appropriately.
The PPC405EX and the PPC460EX/GT share a common IBM SDRAM controller
core IP, and as such the code in the file 44x_spd_ddr2.c will apply to
both chips.

Best,
Adam
AMCC


> 
> Perhaps somebody from AMCC could jump in here. Adam? Any 
> ideas on this?
> 
> 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 
> =====================================================================
>