All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Rigby <jrigby@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
Date: Tue, 17 Feb 2009 16:29:15 -0700	[thread overview]
Message-ID: <499B484B.7090605@freescale.com> (raw)
In-Reply-To: <20090217225734.DCA9A832E893@gemini.denx.de>

Wolfgang Denk wrote:
> Dear John Rigby,
>
> In message <1234908276-26381-1-git-send-email-jrigby@freescale.com> you wrote:
>   
>> From: Martha Marx <mmarx@silicontkx.com>
>>
>> The following configurations are now supported:
>>
>> ads5121_config
>> 	rev 4.1 boards with Elpida memory
>>
>> ads5121_rev4m_config
>> 	rev 4 boards with Micron memory
>>     
>
> This shall not be done. All possible memory configurations shall be
> handled by a single binary image of U-Boot.
>   
That would be wonderful but Martha has no idea how to do this.  Last I 
heard STX was unwilling to put a board rev in the cpld.

Martha, I'll leave this to you to work out with Wolfgang.
>   
>> ads5121_rev3_config
>> 	rev3 boards which also have Micron memory but have the
>> 	older rev silicon
>>     
>
> It makes little sense to me to add new config options for old boards
> that are EOL and not distributed any more. 
>
>   
>> diff --git a/board/ads5121/ads5121.c b/board/ads5121/ads5121.c
>> index 6c40e94..cb607b4 100644
>> --- a/board/ads5121/ads5121.c
>> +++ b/board/ads5121/ads5121.c
>> @@ -182,29 +182,50 @@ long int fixed_sdram (void)
>>  
>>  	/* Initialize DDR */
>>  	for (i = 0; i < 10; i++)
>> -		im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
>> -
>> -	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.
>
>   
>> diff --git a/include/configs/ads5121.h b/include/configs/ads5121.h
>> index 8fda3f2..2e6ceef 100644
>> --- a/include/configs/ads5121.h
>> +++ b/include/configs/ads5121.h
>>     
> ...
>   
>> +#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.
>
> But as mentioned above - the key point is that we  need  to  come  up
> with a solution to support all used memory types with a single binary
> image.
>
>
> I think such a patch cannot go into  mainline  (even  if  the  formal
> issues  mentioned  above  were  fixed). Can we please have a solution
> that avoilds an explosin of differnt, incompatible binary images  and
> config  options  just  for  minor  board modifications like different
> memory types?
>
> Thanks.
>
> Wolfgang Denk
>
>   

  reply	other threads:[~2009-02-17 23:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-17 22:04 [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards John Rigby
2009-02-17 22:57 ` Wolfgang Denk
2009-02-17 23:29   ` John Rigby [this message]
2009-02-18 12:53 ` mjmarx
2009-02-18 14:59   ` Wolfgang Denk
2009-02-18 16:23 ` mjmarx

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=499B484B.7090605@freescale.com \
    --to=jrigby@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.