Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> wrote: > >This patch trades off the removal of most of the #ifdef ugly for > > a lot of duplication. Which is the lesser of two evils? > Only 4 archs share actually the same code avr32, i386, mips and sh > which actually I've plan to modify for sh soon And the avr32 code is mostly wrong. So it looks like the current situation is #ifdef mess _and_ duplication. This patch definitely improves things. The reason why the avr32 part is wrong is that I simply didn't notice that I had to do anything in there until quite recently. So IMO making this thing arch-specific will make it easier to get it right for new architectures (since you'll get a nice and friendly link error reminding you that you missed it.) Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
On Wed, Nov 12, 2008 at 1:08 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 12:55 Wed 12 Nov , Mike Frysinger wrote: >> On Wed, Nov 12, 2008 at 11:31 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> >> > Is this a good idea? It takes one centralized mess (that is deprecated, >> >> > but we don't have a good track record of death after deprecation) and >> >> > spreads it out over a bunch of files. Reminds me of cancer. :-( >> >> > >> >> > The centralized mess had no duplication of code, but a lot of #ifdef >> >> > ugly. This patch trades off the removal of most of the #ifdef ugly for >> >> > a lot of duplication. Which is the lesser of two evils? >> >> > >> >> > If you continue down the fragmentation path, would it work to keep the >> >> > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to >> >> > it that processor families and boards can hook to add in their extra >> >> > processor- and board-specific stuff? This may result in some >> >> > rearrangement of the print output (which I don't view as a problem, but >> >> > manual writers might not like it). It also results in some additional >> >> > obscurity since a processor/board porter needs to understand that there >> >> > is an additional hook to grab for customization. >> >> >> >> i think the split version proposed is a lot nicer than the current >> >> one, but going the route of having an arch hook would be best. i dont >> >> think we even need a weak function ... force every arch to implement >> >> *something*. >> > >> > It's the case >> > The idea is to allow soc and board to allow them to print more info >> >> so you have one hard arch hook and one weak board hook. every > > two weak hook there's no point in making the arch one weak if every arch implements it. you simply add useless overhead. > to allow board AND soc to print more info which is exactly what i said -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
On 12:55 Wed 12 Nov , Mike Frysinger wrote: > On Wed, Nov 12, 2008 at 11:31 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >> > Is this a good idea? It takes one centralized mess (that is deprecated, > >> > but we don't have a good track record of death after deprecation) and > >> > spreads it out over a bunch of files. Reminds me of cancer. :-( > >> > > >> > The centralized mess had no duplication of code, but a lot of #ifdef > >> > ugly. This patch trades off the removal of most of the #ifdef ugly for > >> > a lot of duplication. Which is the lesser of two evils? > >> > > >> > If you continue down the fragmentation path, would it work to keep the > >> > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to > >> > it that processor families and boards can hook to add in their extra > >> > processor- and board-specific stuff? This may result in some > >> > rearrangement of the print output (which I don't view as a problem, but > >> > manual writers might not like it). It also results in some additional > >> > obscurity since a processor/board porter needs to understand that there > >> > is an additional hook to grab for customization. > >> > >> i think the split version proposed is a lot nicer than the current > >> one, but going the route of having an arch hook would be best. i dont > >> think we even need a weak function ... force every arch to implement > >> *something*. > > > > It's the case > > The idea is to allow soc and board to allow them to print more info > > so you have one hard arch hook and one weak board hook. every two weak hook to allow board AND soc to print more info Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
On Wed, Nov 12, 2008 at 11:31 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> > Is this a good idea? It takes one centralized mess (that is deprecated, >> > but we don't have a good track record of death after deprecation) and >> > spreads it out over a bunch of files. Reminds me of cancer. :-( >> > >> > The centralized mess had no duplication of code, but a lot of #ifdef >> > ugly. This patch trades off the removal of most of the #ifdef ugly for >> > a lot of duplication. Which is the lesser of two evils? >> > >> > If you continue down the fragmentation path, would it work to keep the >> > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to >> > it that processor families and boards can hook to add in their extra >> > processor- and board-specific stuff? This may result in some >> > rearrangement of the print output (which I don't view as a problem, but >> > manual writers might not like it). It also results in some additional >> > obscurity since a processor/board porter needs to understand that there >> > is an additional hook to grab for customization. >> >> i think the split version proposed is a lot nicer than the current >> one, but going the route of having an arch hook would be best. i dont >> think we even need a weak function ... force every arch to implement >> *something*. > > It's the case > The idea is to allow soc and board to allow them to print more info so you have one hard arch hook and one weak board hook. every lib_/ needs to implement a bdinfo hook. -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
> > > Hi Jean-Christophe, > > > > Is this a good idea? It takes one centralized mess (that is deprecated, > > but we don't have a good track record of death after deprecation) and > > spreads it out over a bunch of files. Reminds me of cancer. :-( > > > > The centralized mess had no duplication of code, but a lot of #ifdef > > ugly. This patch trades off the removal of most of the #ifdef ugly for > > a lot of duplication. Which is the lesser of two evils? > > > > If you continue down the fragmentation path, would it work to keep the > > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to > > it that processor families and boards can hook to add in their extra > > processor- and board-specific stuff? This may result in some > > rearrangement of the print output (which I don't view as a problem, but > > manual writers might not like it). It also results in some additional > > obscurity since a processor/board porter needs to understand that there > > is an additional hook to grab for customization. > > i think the split version proposed is a lot nicer than the current > one, but going the route of having an arch hook would be best. i dont > think we even need a weak function ... force every arch to implement > *something*. It's the case The idea is to allow soc and board to allow them to print more info Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
On Wed, Nov 12, 2008 at 9:02 AM, Jerry Van Baren wrote: > Jean-Christophe PLAGNIOL-VILLARD wrote: >> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> >> --- >> apply after my precedent fix for cmd_bdinfo >> >> Best Regards, >> J. >> common/Makefile |1 - >> common/cmd_bdinfo.c | 447 >> --- >> include/common.h| 15 ++ >> lib_arm/Makefile|1 + >> lib_arm/bdinfo.c| 69 >> lib_avr32/Makefile |1 + >> lib_avr32/bdinfo.c | 62 +++ >> lib_blackfin/Makefile |1 + >> lib_blackfin/bdinfo.c | 68 +++ >> lib_i386/Makefile |1 + >> lib_i386/bdinfo.c | 62 +++ >> lib_m68k/Makefile |1 + >> lib_m68k/bdinfo.c | 101 +++ >> lib_microblaze/Makefile |1 + >> lib_microblaze/bdinfo.c | 65 +++ >> lib_mips/Makefile |1 + >> lib_mips/bdinfo.c | 62 +++ >> lib_nios/Makefile |1 + >> lib_nios/bdinfo.c | 61 +++ >> lib_nios2/Makefile |1 + >> lib_nios2/bdinfo.c | 71 >> lib_ppc/Makefile|1 + >> lib_ppc/bdinfo.c| 141 +++ >> lib_sh/Makefile |1 + >> lib_sh/bdinfo.c | 62 +++ >> lib_sparc/Makefile | 13 +- >> lib_sparc/bdinfo.c | 78 > > Hi Jean-Christophe, > > Is this a good idea? It takes one centralized mess (that is deprecated, > but we don't have a good track record of death after deprecation) and > spreads it out over a bunch of files. Reminds me of cancer. :-( > > The centralized mess had no duplication of code, but a lot of #ifdef > ugly. This patch trades off the removal of most of the #ifdef ugly for > a lot of duplication. Which is the lesser of two evils? > > If you continue down the fragmentation path, would it work to keep the > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to > it that processor families and boards can hook to add in their extra > processor- and board-specific stuff? This may result in some > rearrangement of the print output (which I don't view as a problem, but > manual writers might not like it). It also results in some additional > obscurity since a processor/board porter needs to understand that there > is an additional hook to grab for customization. i think the split version proposed is a lot nicer than the current one, but going the route of having an arch hook would be best. i dont think we even need a weak function ... force every arch to implement *something*. -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
Hi Jerry, On 09:02 Wed 12 Nov , Jerry Van Baren wrote: > Jean-Christophe PLAGNIOL-VILLARD wrote: >> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> >> --- >> apply after my precedent fix for cmd_bdinfo >> >> Best Regards, >> J. >> common/Makefile |1 - >> common/cmd_bdinfo.c | 447 >> --- >> include/common.h| 15 ++ >> lib_arm/Makefile|1 + >> lib_arm/bdinfo.c| 69 >> lib_avr32/Makefile |1 + >> lib_avr32/bdinfo.c | 62 +++ >> lib_blackfin/Makefile |1 + >> lib_blackfin/bdinfo.c | 68 +++ >> lib_i386/Makefile |1 + >> lib_i386/bdinfo.c | 62 +++ >> lib_m68k/Makefile |1 + >> lib_m68k/bdinfo.c | 101 +++ >> lib_microblaze/Makefile |1 + >> lib_microblaze/bdinfo.c | 65 +++ >> lib_mips/Makefile |1 + >> lib_mips/bdinfo.c | 62 +++ >> lib_nios/Makefile |1 + >> lib_nios/bdinfo.c | 61 +++ >> lib_nios2/Makefile |1 + >> lib_nios2/bdinfo.c | 71 >> lib_ppc/Makefile|1 + >> lib_ppc/bdinfo.c| 141 +++ >> lib_sh/Makefile |1 + >> lib_sh/bdinfo.c | 62 +++ >> lib_sparc/Makefile | 13 +- >> lib_sparc/bdinfo.c | 78 > > Hi Jean-Christophe, > > Is this a good idea? It takes one centralized mess (that is deprecated, > but we don't have a good track record of death after deprecation) and > spreads it out over a bunch of files. Reminds me of cancer. :-( As you said it's a mess of ifdef this files and it's hard to understand and to be sure which part we modify. By splitting it, it will simplify arch implementation and maintaining. BTW the header are already split for each in 'include/asm-arch/global_data.h' > > The centralized mess had no duplication of code, but a lot of #ifdef > ugly. in the c implmentation maybe but we already have in the header >This patch trades off the removal of most of the #ifdef ugly for > a lot of duplication. Which is the lesser of two evils? Only 4 archs share actually the same code avr32, i386, mips and sh which actually I've plan to modify for sh soon > > If you continue down the fragmentation path, would it work to keep the > primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to > it that processor families and boards can hook to add in their extra > processor- and board-specific stuff? why not in this case we will avoid a lots of ifdef > This may result in some > rearrangement of the print output (which I don't view as a problem, but > manual writers might not like it). I prefer to have customizable code and update a manual than have hard maintaining code. > It also results in some additional > obscurity since a processor/board porter needs to understand that there > is an additional hook to grab for customization. We may need to write a guideline Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
Jean-Christophe PLAGNIOL-VILLARD wrote: > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> > --- > apply after my precedent fix for cmd_bdinfo > > Best Regards, > J. > common/Makefile |1 - > common/cmd_bdinfo.c | 447 > --- > include/common.h| 15 ++ > lib_arm/Makefile|1 + > lib_arm/bdinfo.c| 69 > lib_avr32/Makefile |1 + > lib_avr32/bdinfo.c | 62 +++ > lib_blackfin/Makefile |1 + > lib_blackfin/bdinfo.c | 68 +++ > lib_i386/Makefile |1 + > lib_i386/bdinfo.c | 62 +++ > lib_m68k/Makefile |1 + > lib_m68k/bdinfo.c | 101 +++ > lib_microblaze/Makefile |1 + > lib_microblaze/bdinfo.c | 65 +++ > lib_mips/Makefile |1 + > lib_mips/bdinfo.c | 62 +++ > lib_nios/Makefile |1 + > lib_nios/bdinfo.c | 61 +++ > lib_nios2/Makefile |1 + > lib_nios2/bdinfo.c | 71 > lib_ppc/Makefile|1 + > lib_ppc/bdinfo.c| 141 +++ > lib_sh/Makefile |1 + > lib_sh/bdinfo.c | 62 +++ > lib_sparc/Makefile | 13 +- > lib_sparc/bdinfo.c | 78 Hi Jean-Christophe, Is this a good idea? It takes one centralized mess (that is deprecated, but we don't have a good track record of death after deprecation) and spreads it out over a bunch of files. Reminds me of cancer. :-( The centralized mess had no duplication of code, but a lot of #ifdef ugly. This patch trades off the removal of most of the #ifdef ugly for a lot of duplication. Which is the lesser of two evils? If you continue down the fragmentation path, would it work to keep the primary bdinfo command (cmd_bdinfo.c) and add two weak function calls to it that processor families and boards can hook to add in their extra processor- and board-specific stuff? This may result in some rearrangement of the print output (which I don't view as a problem, but manual writers might not like it). It also results in some additional obscurity since a processor/board porter needs to understand that there is an additional hook to grab for customization. Thanks, gvb ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]>
---
apply after my precedent fix for cmd_bdinfo
Best Regards,
J.
common/Makefile |1 -
common/cmd_bdinfo.c | 447 ---
include/common.h| 15 ++
lib_arm/Makefile|1 +
lib_arm/bdinfo.c| 69
lib_avr32/Makefile |1 +
lib_avr32/bdinfo.c | 62 +++
lib_blackfin/Makefile |1 +
lib_blackfin/bdinfo.c | 68 +++
lib_i386/Makefile |1 +
lib_i386/bdinfo.c | 62 +++
lib_m68k/Makefile |1 +
lib_m68k/bdinfo.c | 101 +++
lib_microblaze/Makefile |1 +
lib_microblaze/bdinfo.c | 65 +++
lib_mips/Makefile |1 +
lib_mips/bdinfo.c | 62 +++
lib_nios/Makefile |1 +
lib_nios/bdinfo.c | 61 +++
lib_nios2/Makefile |1 +
lib_nios2/bdinfo.c | 71
lib_ppc/Makefile|1 +
lib_ppc/bdinfo.c| 141 +++
lib_sh/Makefile |1 +
lib_sh/bdinfo.c | 62 +++
lib_sparc/Makefile | 13 +-
lib_sparc/bdinfo.c | 78
27 files changed, 937 insertions(+), 452 deletions(-)
delete mode 100644 common/cmd_bdinfo.c
create mode 100644 lib_arm/bdinfo.c
create mode 100644 lib_avr32/bdinfo.c
create mode 100644 lib_blackfin/bdinfo.c
create mode 100644 lib_i386/bdinfo.c
create mode 100644 lib_m68k/bdinfo.c
create mode 100644 lib_microblaze/bdinfo.c
create mode 100644 lib_mips/bdinfo.c
create mode 100644 lib_nios/bdinfo.c
create mode 100644 lib_nios2/bdinfo.c
create mode 100644 lib_ppc/bdinfo.c
create mode 100644 lib_sh/bdinfo.c
create mode 100644 lib_sparc/bdinfo.c
diff --git a/common/Makefile b/common/Makefile
index 6484b23..7412abb 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -63,7 +63,6 @@ COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
COBJS-$(CONFIG_CMD_AMBAPP) += cmd_ambapp.o
COBJS-$(CONFIG_AUTOSCRIPT) += cmd_autoscript.o
COBJS-$(CONFIG_CMD_AUTOSCRIPT) += cmd_autoscript.o
-COBJS-$(CONFIG_CMD_BDI) += cmd_bdinfo.o
COBJS-$(CONFIG_CMD_BEDBUG) += bedbug.o cmd_bedbug.o
COBJS-$(CONFIG_CMD_BMP) += cmd_bmp.o
COBJS-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
deleted file mode 100644
index 087eda7..000
--- a/common/cmd_bdinfo.c
+++ /dev/null
@@ -1,447 +0,0 @@
-/*
- * (C) Copyright 2003
- * Wolfgang Denk, DENX Software Engineering, [EMAIL PROTECTED]
- *
- * 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
- */
-
-/*
- * Boot support
- */
-#include
-#include
-#include/* for print_IPaddr */
-
-DECLARE_GLOBAL_DATA_PTR;
-
-static void print_num(const char *, ulong);
-
-#ifndef CONFIG_ARM /* PowerPC and other */
-static void print_lnum(const char *, u64);
-
-#ifdef CONFIG_PPC
-static void print_str(const char *, const char *);
-
-int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
-{
- int i;
- bd_t *bd = gd->bd;
- char buf[32];
-
-#ifdef DEBUG
- print_num ("bd address",(ulong)bd );
-#endif
- print_num ("memstart", bd->bi_memstart );
- print_lnum ("memsize", bd->bi_memsize );
- print_num ("flashstart",bd->bi_flashstart );
- print_num ("flashsize", bd->bi_flashsize);
- print_num ("flashoffset", bd->bi_flashoffset );
- print_num ("sramstart", bd->bi_sramstart);
- print_num ("sramsize", bd->bi_sramsize );
-#if defined(CONFIG_5xx) || defined(CONFIG_8xx) || \
-defined(CONFIG_8260) || defined(CONFIG_E500)
- print_num ("immr_base", bd->bi_immr_base);
-#endif
- print_num ("bootflags", bd->bi_bootflags);
-#if defined(CONFIG_405GP) || defined(CONFIG_405CR) || \
-defined(CONFIG_405EP) || defined(CONFIG_XILINX_405) || \
-defined(CONFIG_440EP) || defined(CONFIG_440GR) || \
-defined(CONFIG_440EPX) || defined(CONFIG_440GRX) ||\
-defined(CONFIG_440SP) || defined(CONFIG_440SPE)
- print_str ("procfreq", strmhz(buf, bd->bi_procfreq));
- print_str ("plb_busfreq", strmhz(buf, bd->bi_plb_busfreq));
-#if defined(CONFIG_405GP) || define
