* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
@ 2009-02-17 22:04 John Rigby
2009-02-17 22:57 ` Wolfgang Denk
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: John Rigby @ 2009-02-17 22:04 UTC (permalink / raw)
To: u-boot
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
ads5121_rev3_config
rev3 boards which also have Micron memory but have the
older rev silicon
Signed-off-by: Martha Marx <mmarx@silicontkx.com>
---
Makefile | 8 ++++++
board/ads5121/ads5121.c | 61 ++++++++++++++++++++++++++++++--------------
include/configs/ads5121.h | 28 ++++++++++++++------
3 files changed, 68 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index ee82c5d..b77532c 100644
--- a/Makefile
+++ b/Makefile
@@ -792,11 +792,19 @@ v38b_config: unconfig
ads5121_config \
ads5121_rev2_config \
+ads5121_rev3_config \
+ads5121_rev4m_config \
: unconfig
@mkdir -p $(obj)include
@if [ "$(findstring rev2,$@)" ] ; then \
echo "#define CONFIG_ADS5121_REV2 1" > $(obj)include/config.h; \
fi
+ @if [ "$(findstring rev3,$@)" ] ; then \
+ echo "#define CONFIG_ADS5121_REV3 1" > $(obj)include/config.h; \
+ fi
+ @if [ "$(findstring rev4m,$@)" ] ; then \
+ echo "#define CONFIG_ADS5121_REV4M 1" > $(obj)include/config.h; \
+ fi
@$(MKCONFIG) -a ads5121 ppc mpc512x ads5121
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;
+#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_MICRON_NOP;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_EM2;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_PCHG_ALL;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_EM2;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_EM3;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_EN_DLL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EM2;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_PCHG_ALL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EM2;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EM3;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EN_DLL;
im->mddrc.ddr_command = CONFIG_SYS_MICRON_INIT_DEV_OP;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_PCHG_ALL;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_PCHG_ALL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
im->mddrc.ddr_command = CONFIG_SYS_MICRON_INIT_DEV_OP;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_OCD_DEFAULT;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_PCHG_ALL;
- im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
+#else
+ /* Elpida init sequence */
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EM2;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EM3;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_EN_DLL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RES_DLL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_PCHG_ALL;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
+ im->mddrc.ddr_command = CONFIG_SYS_ELPIDA_INIT_DEV_OP;
+ udelay(200);
+#endif
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_OCD_DEFAULT;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_OCD_EXIT;
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
+ for (i = 0; i < 10; i++)
+ im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
/* Start MDDRC */
im->mddrc.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0_RUN;
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
@@ -135,24 +135,34 @@
#define CONFIG_SYS_MDDRC_SYS_CFG_RUN 0xE8604A00
#define CONFIG_SYS_MDDRC_TIME_CFG1 0x54EC1168
#define CONFIG_SYS_MDDRC_TIME_CFG2 0x35210864
-#else
+#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
#define CONFIG_SYS_MDDRC_SYS_CFG_EN 0xF0000000
+#define CONFIG_SYS_MDDRC_SYS_CFG_CLK_BIT (1 << 29)
+#define CONFIG_SYS_MDDRC_SYS_CFG_CKE_BIT (1 << 30)
#define CONFIG_SYS_MDDRC_TIME_CFG0 0x00003D2E
#define CONFIG_SYS_MDDRC_TIME_CFG0_RUN 0x06183D2E
-
-#define CONFIG_SYS_MICRON_NOP 0x01380000
-#define CONFIG_SYS_MICRON_PCHG_ALL 0x01100400
-#define CONFIG_SYS_MICRON_EM2 0x01020000
-#define CONFIG_SYS_MICRON_EM3 0x01030000
-#define CONFIG_SYS_MICRON_EN_DLL 0x01010000
-#define CONFIG_SYS_MICRON_RFSH 0x01080000
#define CONFIG_SYS_MICRON_INIT_DEV_OP 0x01000432
-#define CONFIG_SYS_MICRON_OCD_DEFAULT 0x01010780
+#define CONFIG_SYS_ELPIDA_INIT_DEV_OP 0x01000842
+#define CONFIG_SYS_DDR_NOP 0x01380000
+#define CONFIG_SYS_DDR_PCHG_ALL 0x01100400
+#define CONFIG_SYS_DDR_EM2 0x01020000
+#define CONFIG_SYS_DDR_EM3 0x01030000
+#define CONFIG_SYS_DDR_EN_DLL 0x01010000
+#define CONFIG_SYS_DDR_RES_DLL 0x01000932
+#define CONFIG_SYS_DDR_RFSH 0x01080000
+#define CONFIG_SYS_DDR_OCD_DEFAULT 0x01010780
+#define CONFIG_SYS_DDR_OCD_EXIT 0x01010400
/* DDR Priority Manager Configuration */
#define CONFIG_SYS_MDDRCGRP_PM_CFG1 0x00077777
--
1.5.6.2.255.gbed62
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
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
2009-02-18 12:53 ` mjmarx
2009-02-18 16:23 ` mjmarx
2 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2009-02-17 22:57 UTC (permalink / raw)
To: u-boot
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.
> 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
--
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
A committee is a group that keeps the minutes and loses hours.
-- Milton Berle
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
2009-02-17 22:57 ` Wolfgang Denk
@ 2009-02-17 23:29 ` John Rigby
0 siblings, 0 replies; 6+ messages in thread
From: John Rigby @ 2009-02-17 23:29 UTC (permalink / raw)
To: u-boot
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
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-18 12:53 ` mjmarx
2009-02-18 14:59 ` Wolfgang Denk
2009-02-18 16:23 ` mjmarx
2 siblings, 1 reply; 6+ messages in thread
From: mjmarx @ 2009-02-18 12:53 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Tuesday, February 17, 2009 5:58 PM
> To: John Rigby
> Cc: u-boot at lists.denx.de; Martha Marx
> Subject: Re: [U-Boot] [PATCH] ADS5121 Add mem config for current rev4
> boards
>
> 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.
>
> > 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.
>
CONFIG_SYS_ was in there already for all the memory #defs
(as of the global rename in October)
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.
> > 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.
>
See comment above.
> 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.
>
>
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 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.
> 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
>
> --
> 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
> A committee is a group that keeps the minutes and loses hours.
>
--
View this message in context: http://www.nabble.com/-U-Boot---PATCH--ADS5121-Add-mem-config-for-current-rev4-boards-tp22067475p22077983.html
Sent from the Uboot - Users mailing list archive at Nabble.com.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
2009-02-18 12:53 ` mjmarx
@ 2009-02-18 14:59 ` Wolfgang Denk
0 siblings, 0 replies; 6+ messages in thread
From: Wolfgang Denk @ 2009-02-18 14:59 UTC (permalink / raw)
To: u-boot
Dear Martha,
in message <22077983.post@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.
> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards
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-18 12:53 ` mjmarx
@ 2009-02-18 16:23 ` mjmarx
2 siblings, 0 replies; 6+ messages in thread
From: mjmarx @ 2009-02-18 16:23 UTC (permalink / raw)
To: u-boot
> -----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@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-02-18 16:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2009-02-18 12:53 ` mjmarx
2009-02-18 14:59 ` Wolfgang Denk
2009-02-18 16:23 ` mjmarx
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox