[U-Boot] [PATCH] ppc4xx: Remove duplicated code for Sequoia NAND booting version
Signed-off-by: Stefan Roese sr@denx.de --- board/amcc/sequoia/sdram.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/board/amcc/sequoia/sdram.c b/board/amcc/sequoia/sdram.c index c26e6ee..6df4c6d 100644 --- a/board/amcc/sequoia/sdram.c +++ b/board/amcc/sequoia/sdram.c @@ -44,7 +44,7 @@ extern void denali_core_search_data_eye(void); * for the 4k NAND boot image so define bus_frequency to 133MHz here * which is save for the refresh counter setup. */ -#define get_bus_freq(val) 133000000 +#define get_bus_freq(val) 133333333 #endif
/************************************************************************* @@ -55,11 +55,7 @@ extern void denali_core_search_data_eye(void); phys_size_t initdram (int board_type) { #if !defined(CONFIG_NAND_U_BOOT) || defined(CONFIG_NAND_SPL) -#if !defined(CONFIG_NAND_SPL) ulong speed = get_bus_freq(0); -#else - ulong speed = 133333333; /* 133MHz is on the safe side */ -#endif
mtsdram(DDR0_02, 0x00000000);
Dear Stefan Roese,
In message 1239788018-27468-1-git-send-email-sr@denx.de you wrote:
Signed-off-by: Stefan Roese sr@denx.de
board/amcc/sequoia/sdram.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/board/amcc/sequoia/sdram.c b/board/amcc/sequoia/sdram.c index c26e6ee..6df4c6d 100644 --- a/board/amcc/sequoia/sdram.c +++ b/board/amcc/sequoia/sdram.c @@ -44,7 +44,7 @@ extern void denali_core_search_data_eye(void);
- for the 4k NAND boot image so define bus_frequency to 133MHz here
- which is save for the refresh counter setup.
*/ -#define get_bus_freq(val) 133000000 +#define get_bus_freq(val) 133333333 #endif
To me that does not look exactly like duplicated code removal...
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Wednesday 15 April 2009, Wolfgang Denk wrote:
+++ b/board/amcc/sequoia/sdram.c @@ -44,7 +44,7 @@ extern void denali_core_search_data_eye(void);
- for the 4k NAND boot image so define bus_frequency to 133MHz here
- which is save for the refresh counter setup.
*/ -#define get_bus_freq(val) 133000000 +#define get_bus_freq(val) 133333333 #endif
To me that does not look exactly like duplicated code removal...
Here a closer look at the patch:
-#define get_bus_freq(val) 133000000 +#define get_bus_freq(val) 133333333 #endif
/************************************************************************* @@ -55,11 +55,7 @@ extern void denali_core_search_data_eye(void); phys_size_t initdram (int board_type) { #if !defined(CONFIG_NAND_U_BOOT) || defined(CONFIG_NAND_SPL) -#if !defined(CONFIG_NAND_SPL) ulong speed = get_bus_freq(0); -#else - ulong speed = 133333333; /* 133MHz is on the safe side */ -#endif
As you can see, the top patch part changes the define of get_bus_frequency() to 133333333. This define was only enabled (via ifdef) for the CONFIG_NAND_SPL part:
#if defined(CONFIG_NAND_SPL) /* Using cpu/ppc4xx/speed.c to calculate the bus frequency is too big * for the 4k NAND boot image so define bus_frequency to 133MHz here * which is save for the refresh counter setup. */ #define get_bus_freq(val) 133333333 #endif
And now the 2nd patch part makes sure that this define is really used.
So what the patch does is to remove an unused code part. Is this what you are complaining about? If really needed I could resend this patch with a modified patch subject.
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@denx.de =====================================================================
Dear Stefan Roese,
In message 200904160651.28962.sr@denx.de you wrote:
-#define get_bus_freq(val) 133000000 +#define get_bus_freq(val) 133333333 #endif
To me that does not look exactly like duplicated code removal...
...
So what the patch does is to remove an unused code part. Is this what you are complaining about? If really needed I could resend this patch with a modified patch subject.
No, I complain that you also change a vital costant without even mentioning.
Normally, this should be split into two separate patches., but at least it should be stated in the commit message.
Best regards,
Wolfgang Denk
On Thursday 16 April 2009, Wolfgang Denk wrote:
So what the patch does is to remove an unused code part. Is this what you are complaining about? If really needed I could resend this patch with a modified patch subject.
No, I complain that you also change a vital costant without even mentioning.
The constant was not used at all (as I mentioned in my last mail). So changing it to the really used value seems logical to me.
Normally, this should be split into two separate patches., but at least it should be stated in the commit message.
I'll resend the patch with a changed commit text later.
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@denx.de =====================================================================
participants (2)
-
Stefan Roese -
Wolfgang Denk