[U-Boot] [PATCH] Consolidate strmhz() implementation
ARM, i386, m68k and ppc all have identical implementations of strmhz(). Other architectures don't provide this function at all.
This patch moves strmhz() into lib_generic, reducing code duplication and providing a more unified API across architectures.
Signed-off-by: Haavard Skinnemoen haavard.skinnemoen@atmel.com --- include/common.h | 4 +++- lib_arm/board.c | 13 ------------- lib_generic/Makefile | 1 + lib_generic/strmhz.c | 36 ++++++++++++++++++++++++++++++++++++ lib_i386/board.c | 13 ------------- lib_m68k/board.c | 17 ----------------- lib_ppc/board.c | 13 ------------- 7 files changed, 40 insertions(+), 57 deletions(-) create mode 100644 lib_generic/strmhz.c
diff --git a/include/common.h b/include/common.h index 2fcb1fd..7c80fb9 100644 --- a/include/common.h +++ b/include/common.h @@ -222,7 +222,6 @@ void board_init_r (gd_t *, ulong) __attribute__ ((noreturn)); int checkboard (void); int checkflash (void); int checkdram (void); -char * strmhz(char *buf, long hz); int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void); @@ -613,6 +612,9 @@ int sprintf(char * buf, const char *fmt, ...) __attribute__ ((format (__printf__, 2, 3))); int vsprintf(char *buf, const char *fmt, va_list args);
+/* lib_generic/strmhz.c */ +char * strmhz(char *buf, long hz); + /* lib_generic/crc32.c */ uint32_t crc32 (uint32_t, const unsigned char *, uint); uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint); diff --git a/lib_arm/board.c b/lib_arm/board.c index a093860..6e3ef08 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -115,19 +115,6 @@ void *sbrk (ptrdiff_t increment) return ((void *) old); }
-char *strmhz(char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} -
/************************************************************************ * Coloured LED functionality diff --git a/lib_generic/Makefile b/lib_generic/Makefile index 4f6ce73..bf0e31d 100644 --- a/lib_generic/Makefile +++ b/lib_generic/Makefile @@ -40,6 +40,7 @@ COBJS-$(CONFIG_MD5) += md5.o COBJS-y += sha1.o COBJS-$(CONFIG_SHA256) += sha256.o COBJS-y += string.o +COBJS-y += strmhz.o COBJS-y += vsprintf.o COBJS-y += zlib.o
diff --git a/lib_generic/strmhz.c b/lib_generic/strmhz.c new file mode 100644 index 0000000..d0b6bc6 --- /dev/null +++ b/lib_generic/strmhz.c @@ -0,0 +1,36 @@ +/* + * (C) Copyright 2002-2006 + * Wolfgang Denk, DENX Software Engineering, wd@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ +#include <common.h> + +char *strmhz (char *buf, long hz) +{ + long l, n; + long m; + + n = hz / 1000000L; + l = sprintf (buf, "%ld", n); + m = (hz % 1000000L) / 1000L; + if (m != 0) + sprintf (buf + l, ".%03ld", m); + return (buf); +} diff --git a/lib_i386/board.c b/lib_i386/board.c index 22191e6..55fa42a 100644 --- a/lib_i386/board.c +++ b/lib_i386/board.c @@ -108,19 +108,6 @@ void *sbrk (ptrdiff_t increment) return ((void *) old); }
-char *strmhz (char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} - /************************************************************************ * Init Utilities * ************************************************************************ diff --git a/lib_m68k/board.c b/lib_m68k/board.c index a13ea26..39e8f23 100644 --- a/lib_m68k/board.c +++ b/lib_m68k/board.c @@ -136,23 +136,6 @@ void *sbrk (ptrdiff_t increment) return ((void *)old); }
-char *strmhz(char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - - l = sprintf (buf, "%ld", n); - - m = (hz % 1000000L) / 1000L; - - if (m != 0) - sprintf (buf+l, ".%03ld", m); - - return (buf); -} - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 71a70db..c8f075f 100644 --- a/lib_ppc/board.c +++ b/lib_ppc/board.c @@ -168,19 +168,6 @@ void *sbrk (ptrdiff_t increment) return ((void *) old); }
-char *strmhz (char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the
On 13:41 Mon 18 Aug , Haavard Skinnemoen wrote:
ARM, i386, m68k and ppc all have identical implementations of strmhz(). Other architectures don't provide this function at all.
This patch moves strmhz() into lib_generic, reducing code duplication and providing a more unified API across architectures.
Signed-off-by: Haavard Skinnemoen haavard.skinnemoen@atmel.com
include/common.h | 4 +++- lib_arm/board.c | 13 ------------- lib_generic/Makefile | 1 + lib_generic/strmhz.c | 36 ++++++++++++++++++++++++++++++++++++ lib_i386/board.c | 13 ------------- lib_m68k/board.c | 17 ----------------- lib_ppc/board.c | 13 ------------- 7 files changed, 40 insertions(+), 57 deletions(-) create mode 100644 lib_generic/strmhz.c
When I read the patch, I really like it. But when I take a look on all board.c
I've seen 2 others functions which are commom.
Could we use the patch that reply to this message instead?
Best Regards, J.
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20080819210013.GE9094@game.jcrosoft.org you wrote:
When I read the patch, I really like it. But when I take a look on all board.c
I've seen 2 others functions which are commom.
Could we use the patch that reply to this message instead?
Sorry, which specific patch are you talking about? Do you have a message ID or URL or somthing we can look up?
Best regards,
Wolfgang Denk
From: Haavard Skinnemoen haavard.skinnemoen@atmel.com
ARM, i386, m68k and ppc all have identical implementations of strmhz(). Other architectures don't provide this function at all.
ARM, avr32, blackfin, i386, m68k, mips and sparc all have identical implementations of init_baudrate(). Other architectures don't provide this function at all.
This patch moves strmhz() and init_baudrate() into lib_generic, reducing code duplication and providing a more unified API across architectures.
Signed-off-by: Haavard Skinnemoen haavard.skinnemoen@atmel.com Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com --- include/common.h | 5 +++- lib_arm/board.c | 25 ----------------------- lib_avr32/board.c | 15 ------------- lib_blackfin/board.c | 10 --------- lib_generic/Makefile | 1 + lib_generic/board.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib_i386/board.c | 25 ----------------------- lib_m68k/board.c | 30 --------------------------- lib_mips/board.c | 13 ------------ lib_ppc/board.c | 14 ------------- lib_sparc/board.c | 13 ------------ 11 files changed, 59 insertions(+), 146 deletions(-) create mode 100644 lib_generic/board.c
diff --git a/include/common.h b/include/common.h index 06ed278..3698788 100644 --- a/include/common.h +++ b/include/common.h @@ -224,7 +224,6 @@ void board_init_r (gd_t *, ulong) __attribute__ ((noreturn)); int checkboard (void); int checkflash (void); int checkdram (void); -char * strmhz(char *buf, long hz); int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void); @@ -615,6 +614,10 @@ int sprintf(char * buf, const char *fmt, ...) __attribute__ ((format (__printf__, 2, 3))); int vsprintf(char *buf, const char *fmt, va_list args);
+/* lib_generic/strmhz.c */ +char * strmhz(char *buf, long hz); +int init_baudrate(void); + /* lib_generic/crc32.c */ uint32_t crc32 (uint32_t, const unsigned char *, uint); uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint); diff --git a/lib_arm/board.c b/lib_arm/board.c index a093860..4b48736 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -115,20 +115,6 @@ void *sbrk (ptrdiff_t increment) return ((void *) old); }
-char *strmhz(char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} - - /************************************************************************ * Coloured LED functionality ************************************************************************ @@ -157,17 +143,6 @@ void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off"))) * but let's get it working (again) first... */
-static int init_baudrate (void) -{ - char tmp[64]; /* long enough for environment variables */ - int i = getenv_r ("baudrate", tmp, sizeof (tmp)); - gd->bd->bi_baudrate = gd->baudrate = (i > 0) - ? (int) simple_strtoul (tmp, NULL, 10) - : CONFIG_BAUDRATE; - - return (0); -} - static int display_banner (void) { printf ("\n\n%s\n\n", version_string); diff --git a/lib_avr32/board.c b/lib_avr32/board.c index f3d0c52..709b9d9 100644 --- a/lib_avr32/board.c +++ b/lib_avr32/board.c @@ -120,21 +120,6 @@ static inline void dma_alloc_init(void) } #endif
-static int init_baudrate(void) -{ - char tmp[64]; - int i; - - i = getenv_r("baudrate", tmp, sizeof(tmp)); - if (i > 0) { - gd->baudrate = simple_strtoul(tmp, NULL, 10); - } else { - gd->baudrate = CONFIG_BAUDRATE; - } - return 0; -} - - static int display_banner (void) { printf ("\n\n%s\n\n", version_string); diff --git a/lib_blackfin/board.c b/lib_blackfin/board.c index 0b8e664..c51d746 100644 --- a/lib_blackfin/board.c +++ b/lib_blackfin/board.c @@ -117,16 +117,6 @@ static int display_banner(void) return 0; }
-static int init_baudrate(void) -{ - char baudrate[15]; - int i = getenv_r("baudrate", baudrate, sizeof(baudrate)); - gd->bd->bi_baudrate = gd->baudrate = (i > 0) - ? simple_strtoul(baudrate, NULL, 10) - : CONFIG_BAUDRATE; - return 0; -} - static void display_global_data(void) { #ifdef CONFIG_DEBUG_EARLY_SERIAL diff --git a/lib_generic/Makefile b/lib_generic/Makefile index 4f6ce73..a38550b 100644 --- a/lib_generic/Makefile +++ b/lib_generic/Makefile @@ -25,6 +25,7 @@ include $(TOPDIR)/config.mk
LIB = $(obj)libgeneric.a
+COBJS-y += board.o COBJS-y += bzlib.o COBJS-y += bzlib_crctable.o COBJS-y += bzlib_decompress.o diff --git a/lib_generic/board.c b/lib_generic/board.c new file mode 100644 index 0000000..1970bdb --- /dev/null +++ b/lib_generic/board.c @@ -0,0 +1,54 @@ +/* + * (C) Copyright 2002-2006 + * Wolfgang Denk, DENX Software Engineering, wd@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ +#include <common.h> + +DECLARE_GLOBAL_DATA_PTR; + +char *strmhz (char *buf, long hz) +{ + long l, n; + long m; + + n = hz / 1000000L; + l = sprintf (buf, "%ld", n); + m = (hz % 1000000L) / 1000L; + if (m != 0) + sprintf (buf + l, ".%03ld", m); + return (buf); +} + +int init_baudrate(void) +{ + char tmp[64]; + int i; + + i = getenv_r("baudrate", tmp, sizeof(tmp)); + + if (i > 0) + gd->baudrate = simple_strtoul(tmp, NULL, 10); + else + gd->baudrate = CONFIG_BAUDRATE; + + return 0; +} + diff --git a/lib_i386/board.c b/lib_i386/board.c index 22191e6..b145521 100644 --- a/lib_i386/board.c +++ b/lib_i386/board.c @@ -108,19 +108,6 @@ void *sbrk (ptrdiff_t increment) return ((void *) old); }
-char *strmhz (char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} - /************************************************************************ * Init Utilities * ************************************************************************ @@ -128,18 +115,6 @@ char *strmhz (char *buf, long hz) * or dropped completely, * but let's get it working (again) first... */ -static int init_baudrate (void) -{ - char tmp[64]; /* long enough for environment variables */ - int i = getenv_r("baudrate", tmp, 64); - - gd->baudrate = (i != 0) - ? (int) simple_strtoul (tmp, NULL, 10) - : CONFIG_BAUDRATE; - - return (0); -} - static int display_banner (void) {
diff --git a/lib_m68k/board.c b/lib_m68k/board.c index fe3180e..f4fb4d3 100644 --- a/lib_m68k/board.c +++ b/lib_m68k/board.c @@ -140,23 +140,6 @@ void *sbrk (ptrdiff_t increment) return ((void *)old); }
-char *strmhz(char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - - l = sprintf (buf, "%ld", n); - - m = (hz % 1000000L) / 1000L; - - if (m != 0) - sprintf (buf+l, ".%03ld", m); - - return (buf); -} - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the @@ -178,19 +161,6 @@ typedef int (init_fnc_t) (void); * but let's get it working (again) first... */
-static int init_baudrate (void) -{ - char tmp[64]; /* long enough for environment variables */ - int i = getenv_r ("baudrate", tmp, sizeof (tmp)); - - gd->baudrate = (i > 0) - ? (int) simple_strtoul (tmp, NULL, 10) - : CONFIG_BAUDRATE; - return (0); -} - -/***********************************************************************/ - static int init_func_ram (void) { int board_type = 0; /* use dummy arg */ diff --git a/lib_mips/board.c b/lib_mips/board.c index fb31e33..a8b21e8 100644 --- a/lib_mips/board.c +++ b/lib_mips/board.c @@ -131,19 +131,6 @@ static void display_flash_config(ulong size) } #endif
-static int init_baudrate (void) -{ - char tmp[64]; /* long enough for environment variables */ - int i = getenv_r ("baudrate", tmp, sizeof (tmp)); - - gd->baudrate = (i > 0) - ? (int) simple_strtoul (tmp, NULL, 10) - : CONFIG_BAUDRATE; - - return (0); -} - - /* * Breath some life into the board... * diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 5816ebd..2aad921 100644 --- a/lib_ppc/board.c +++ b/lib_ppc/board.c @@ -167,20 +167,6 @@ void *sbrk (ptrdiff_t increment) mem_malloc_brk = new; return ((void *) old); } - -char *strmhz (char *buf, long hz) -{ - long l, n; - long m; - - n = hz / 1000000L; - l = sprintf (buf, "%ld", n); - m = (hz % 1000000L) / 1000L; - if (m != 0) - sprintf (buf + l, ".%03ld", m); - return (buf); -} - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the diff --git a/lib_sparc/board.c b/lib_sparc/board.c index 85e3f1f..10ad7bf 100644 --- a/lib_sparc/board.c +++ b/lib_sparc/board.c @@ -118,19 +118,6 @@ void *sbrk(ptrdiff_t increment) * but let's get it working (again) first... */
-static int init_baudrate(void) -{ - char tmp[64]; /* long enough for environment variables */ - int i = getenv_r("baudrate", tmp, sizeof(tmp)); - - gd->baudrate = (i > 0) - ? (int)simple_strtoul(tmp, NULL, 10) - : CONFIG_BAUDRATE; - return (0); -} - -/***********************************************************************/ - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the
Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
I've seen 2 others functions which are commom.
Could we use the patch that reply to this message instead?
The only downside that I can think of is that configuration that truly don't require strmhz() and don't use --gc-sections will end up slightly larger.
All the configurations I care about use --gc-sections, so it's fine with me.
Haavard
Dear Jean-Christophe,
In message 20080820140702.59a7ef68@hskinnemo-gx745.norway.atmel.com Haavard Skinnemoen wrote:
I've seen 2 others functions which are commom.
Could we use the patch that reply to this message instead?
The only downside that I can think of is that configuration that truly don't require strmhz() and don't use --gc-sections will end up slightly larger.
I agree with Haavard's comment - the other functions should be added as separate source files, too.
BTW: you mentioned "two other functions", but your patch handles only one other function - init_baudrate() ?
Best regards,
Wolfgang Denk
participants (3)
-
Haavard Skinnemoen -
Jean-Christophe PLAGNIOL-VILLARD -
Wolfgang Denk