[U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards

[mjmarx]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Wednesday, February 18, 2009 10:00 AM
> To: mjmarx
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] ADS5121 Add mem config for current rev4
> boards
> 
> Dear Martha,
> 
> in message <22077983.post at talk.nabble.com> you wrote:
> >
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_PCHG_ALL;
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_RFSH;
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_RFSH;
> > > > -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> > > > +		im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> > > > +
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_PCHG_ALL;
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
> > > > +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> > >
> > > While performing such a global rename, please let's get rid of the
> > > CONFIG_SYS_ names. These are NOT options that can be changed by the
> > > user in the board config file, so they should not look like such.
> >
> > CONFIG_SYS_ was in there already for all the memory #defs
> > (as of the global rename in October)
> 
> Yes, you are right. But as I mentioned - while performing such a
> global rename, we should use the opportunity and do it right this
> time.
> 
As far as my name change -- I DID NOT DO A WHOLESALE RENAME OF THE MEMORY
#defs

The init settings that Elpida and Micron share went from _MICRON_ to _DDR_
The setting they don't share stayed as _MICRON_ and now has a counterpart
_ELPIDA_  This name change should not be combined with any other name change
and belongs in the memory change patch.
In fact it will be especially required if I keep one binary.

Code section with elipses

+#if defined(CONFIG_ADS5121_REV2) ||  \
+    defined(CONFIG_ADS5121_REV3) || \
+    defined(CONFIG_ADS5121_REV4M)
+	/* Micron init sequence */
 	im->mddrc.ddr_command = CONFIG_SYS_MICRON_INIT_DEV_OP;
...
+	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
+	im->mddrc.ddr_command = CONFIG_SYS_DDR_EM2;
+	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
...
+#else
+	/* Elpida init sequence */
+	im->mddrc.ddr_command = CONFIG_SYS_DDR_EM2;
...
+	im->mddrc.ddr_command = CONFIG_SYS_ELPIDA_INIT_DEV_OP;
+	udelay(200);
+#endif

> > IF you look -- the only rename I did was to change _MICRON_ to _DDR_
> > and it is very appropriate for it to be in this patch.
> 
> But if we change the names anyway, we could correct the other mistake
> as well, right?
> 
> > > > +#elif defined(CONFIG_ADS5121_REV3) || \
> > > > +      defined(CONFIG_ADS5121_REV4M)
> > > >  #define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA804A00
> > > >  #define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA804A00
> > > >  #define CONFIG_SYS_MDDRC_TIME_CFG1	 0x68EC1168
> > > >  #define CONFIG_SYS_MDDRC_TIME_CFG2	 0x34310864
> > > > +#else
> > > > +#define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA802b40
> > > > +#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA802b40
> > > > +#define CONFIG_SYS_MDDRC_TIME_CFG1	 0x690e1189
> > > > +#define CONFIG_SYS_MDDRC_TIME_CFG2	 0x35410864
> > > >  #endif
> > >
> > > If I understand correctly, these are the only  real  changes  to  the
> > > memory  init sequence  right? This is virtually invisible in the huge
> > > number of variable renames. You should split this into  two  separate
> > > patches,  one  doing  the  renames  only, and one adding the real new
> > > configuration.
> >
> > See comment above.
> 
> In any case: please split this into two patches: one doing the rename
> only, and another one adding the new functionality.
> 
> > The memory change was made due to a supply problem .. all new
> > boards will have the new memory but unfortunately there is no
> > new rev (or CPLD reg setting) for these boards  (it is not an
> > unwillingness on my part)
> 
> I understand this. On the other hand, there might  be  a  new  supply
> problem in N days/weeks/months from now, and we definitely don't want
> to  have  an  ever  growing number of incompatible configurations and
> binary images if it can be avoided (and I'm sure it can be avoided).
> 
> > I will attempt to create one binary using Micron settings for rev 4.0
> > and below, Elpida settings for rev 4.1 and beyond and for the small
> > number of 4.1 Micron boards out there I will attempt to have them run
> > with Elpida settings (the memory will run slightly more slowly)  I would
> > like to keep an #ifdef in there to force Micron settings, however.
> 
> Let's try and find a solution that works with only one binary image,
> without additional target names and #ifdef's.

I will very much try my darn'dest!!!!!

-Martha
> 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> "Engineering without management is art."               - Jeff Johnson

-- 
View this message in context: http://www.nabble.com/-U-Boot---PATCH--ADS5121-Add-mem-config-for-current-rev4-boards-tp22067475p22082366.html
Sent from the Uboot - Users mailing list archive at Nabble.com.