[U-Boot] [PATCH] Add bootstrap command
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
Signed-off-by: Dirk Eibach eibach@gdsys.de --- board/amcc/canyonlands/Makefile | 5 +- board/amcc/canyonlands/bootstrap.c | 177 ++++++------------------------------ common/Makefile | 1 + common/cmd_bootstrap.c | 75 +++++++++++++++ include/bootstrap.h | 36 +++++++ include/configs/canyonlands.h | 6 + 6 files changed, 148 insertions(+), 152 deletions(-) create mode 100644 common/cmd_bootstrap.c create mode 100644 include/bootstrap.h
diff --git a/board/amcc/canyonlands/Makefile b/board/amcc/canyonlands/Makefile index 2aeead6..be51cb9 100644 --- a/board/amcc/canyonlands/Makefile +++ b/board/amcc/canyonlands/Makefile @@ -25,10 +25,11 @@ include $(TOPDIR)/config.mk
LIB = $(obj)lib$(BOARD).a
-COBJS := $(BOARD).o -COBJS += bootstrap.o +COBJS-y := $(BOARD).o +COBJS-$(CONFIG_CMD_BOOTSTRAP) += bootstrap.o SOBJS := init.o
+COBJS := $(COBJS-y) SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS)) SOBJS := $(addprefix $(obj),$(SOBJS)) diff --git a/board/amcc/canyonlands/bootstrap.c b/board/amcc/canyonlands/bootstrap.c index 6dc2cca..8731b4e 100644 --- a/board/amcc/canyonlands/bootstrap.c +++ b/board/amcc/canyonlands/bootstrap.c @@ -22,174 +22,51 @@ * */
-#include <common.h> -#include <command.h> -#include <i2c.h> -#include <asm/io.h> - -/* - * NOR and NAND boot options change bytes 5, 6, 8, 9, 11. The - * values are independent of the rest of the clock settings. - */ - -#define NAND_COMPATIBLE 0x01 -#define NOR_COMPATIBLE 0x02 - -#define I2C_EEPROM_ADDR 0x52 - -static char *config_labels[] = { - "CPU: 600 PLB: 200 OPB: 100 EBC: 100", - "CPU: 800 PLB: 200 OPB: 100 EBC: 100", - "CPU:1000 PLB: 200 OPB: 100 EBC: 100", - "CPU:1066 PLB: 266 OPB: 88 EBC: 88", - NULL +#include <bootstrap.h> + +char *bootstrap_config_labels[] = { + "NOR CPU: 600 PLB: 200 OPB: 100 EBC: 100", + "NAND CPU: 600 PLB: 200 OPB: 100 EBC: 100", + "NOR CPU: 800 PLB: 200 OPB: 100 EBC: 100", + "NAND CPU: 800 PLB: 200 OPB: 100 EBC: 100", + "NOR CPU:1000 PLB: 200 OPB: 100 EBC: 100", + "NAND CPU:1000 PLB: 200 OPB: 100 EBC: 100", + "NOR CPU:1066 PLB: 266 OPB: 88 EBC: 88", + "NAND CPU:1066 PLB: 266 OPB: 88 EBC: 88", + "" };
-static u8 boot_configs[][17] = { +u8 bootstrap_configs[][CONFIG_BOOTSTRAP_BLOCKSIZE] = { { - (NAND_COMPATIBLE | NOR_COMPATIBLE), 0x86, 0x80, 0xce, 0x1f, 0x79, 0x80, 0x00, 0xa0, 0x40, 0x08, 0x23, 0x50, 0x0d, 0x05, 0x00, 0x00 }, { - (NAND_COMPATIBLE | NOR_COMPATIBLE), + 0x86, 0x80, 0xce, 0x1f, 0x79, 0x90, 0x01, 0xa0, 0xa0, 0xe8, + 0x23, 0x58, 0x0d, 0x05, 0x00, 0x00 + }, + { 0x86, 0x80, 0xba, 0x14, 0x99, 0x80, 0x00, 0xa0, 0x40, 0x08, 0x23, 0x50, 0x0d, 0x05, 0x00, 0x00 }, { - (NAND_COMPATIBLE | NOR_COMPATIBLE), + 0x86, 0x80, 0xba, 0x14, 0x99, 0x90, 0x01, 0xa0, 0xa0, 0xe8, + 0x23, 0x58, 0x0d, 0x05, 0x00, 0x00 + }, + { 0x86, 0x82, 0x96, 0x19, 0xb9, 0x80, 0x00, 0xa0, 0x40, 0x08, 0x23, 0x50, 0x0d, 0x05, 0x00, 0x00 }, { - (NAND_COMPATIBLE | NOR_COMPATIBLE), + 0x86, 0x82, 0x96, 0x19, 0xb9, 0x90, 0x01, 0xa0, 0xa0, 0xe8, + 0x23, 0x58, 0x0d, 0x05, 0x00, 0x00 + }, + { 0x86, 0x80, 0xb3, 0x01, 0x9d, 0x80, 0x00, 0xa0, 0x40, 0x08, 0x23, 0x50, 0x0d, 0x05, 0x00, 0x00 }, { - 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + 0x86, 0x80, 0xb3, 0x01, 0x9d, 0x90, 0x01, 0xa0, 0xa0, 0xe8, + 0x23, 0x58, 0x0d, 0x05, 0x00, 0x00 } }; - -/* - * Bytes 5,6,8,9,11 change for NAND boot - */ -#if 0 -/* - * Values for 512 page size NAND chips, not used anymore, just - * keep them here for reference - */ -static u8 nand_boot[] = { - 0x90, 0x01, 0xa0, 0x68, 0x58 -}; -#else -/* - * Values for 2k page size NAND chips - */ -static u8 nand_boot[] = { - 0x90, 0x01, 0xa0, 0xe8, 0x58 -}; -#endif - -static int do_bootstrap(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) -{ - u8 *buf, b_nand; - int x, y, nbytes, selcfg; - extern char console_buffer[]; - - if (argc < 2) { - cmd_usage(cmdtp); - return 1; - } - - if ((strcmp(argv[1], "nor") != 0) && - (strcmp(argv[1], "nand") != 0)) { - printf("Unsupported boot-device - only nor|nand support\n"); - return 1; - } - - /* set the nand flag based on provided input */ - if ((strcmp(argv[1], "nand") == 0)) - b_nand = 1; - else - b_nand = 0; - - printf("Available configurations: \n\n"); - - if (b_nand) { - for(x = 0, y = 0; boot_configs[x][0] != 0; x++) { - /* filter on nand compatible */ - if (boot_configs[x][0] & NAND_COMPATIBLE) { - printf(" %d - %s\n", (y+1), config_labels[x]); - y++; - } - } - } else { - for(x = 0, y = 0; boot_configs[x][0] != 0; x++) { - /* filter on nor compatible */ - if (boot_configs[x][0] & NOR_COMPATIBLE) { - printf(" %d - %s\n", (y+1), config_labels[x]); - y++; - } - } - } - - do { - nbytes = readline(" Selection [1-x / quit]: "); - - if (nbytes) { - if (strcmp(console_buffer, "quit") == 0) - return 0; - selcfg = simple_strtol(console_buffer, NULL, 10); - if ((selcfg < 1) || (selcfg > y)) - nbytes = 0; - } - } while (nbytes == 0); - - - y = (selcfg - 1); - - for (x = 0; boot_configs[x][0] != 0; x++) { - if (b_nand) { - if (boot_configs[x][0] & NAND_COMPATIBLE) { - if (y > 0) - y--; - else if (y < 1) - break; - } - } else { - if (boot_configs[x][0] & NOR_COMPATIBLE) { - if (y > 0) - y--; - else if (y < 1) - break; - } - } - } - - buf = &boot_configs[x][1]; - - if (b_nand) { - buf[5] = nand_boot[0]; - buf[6] = nand_boot[1]; - buf[8] = nand_boot[2]; - buf[9] = nand_boot[3]; - buf[11] = nand_boot[4]; - } - - if (i2c_write(I2C_EEPROM_ADDR, 0, 1, buf, 16) != 0) - printf("Error writing to EEPROM at address 0x%x\n", I2C_EEPROM_ADDR); - udelay(CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS * 1000); - - printf("Done\n"); - printf("Please power-cycle the board for the changes to take effect\n"); - - return 0; -} - -U_BOOT_CMD( - bootstrap, 2, 0, do_bootstrap, - "program the I2C bootstrap EEPROM", - "<nand|nor> - strap to boot from NAND or NOR flash" -); diff --git a/common/Makefile b/common/Makefile index c8e5d26..0a9a17b 100644 --- a/common/Makefile +++ b/common/Makefile @@ -71,6 +71,7 @@ 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 +COBJS-$(CONFIG_CMD_BOOTSTRAP) += cmd_bootstrap.o COBJS-$(CONFIG_CMD_CACHE) += cmd_cache.o COBJS-$(CONFIG_CMD_CONSOLE) += cmd_console.o COBJS-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o diff --git a/common/cmd_bootstrap.c b/common/cmd_bootstrap.c new file mode 100644 index 0000000..6fceb23 --- /dev/null +++ b/common/cmd_bootstrap.c @@ -0,0 +1,75 @@ +/* + * (C) Copyright 2009 + * Dirk Eibach, Guntermann & Drunck GmbH, eibach@gdsys.de + * + * based on code (C) Copyright 2008 + * Stefan Roese, DENX Software Engineering, sr@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 <bootstrap.h> +#include <common.h> +#include <command.h> +#include <i2c.h> +#include <asm/io.h> + +static int do_bootstrap(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + int x, nbytes, selcfg; + extern char console_buffer[]; + + printf("Available configurations: \n\n"); + + for (x = 0; bootstrap_config_labels[x][0] != 0; x++) + printf(" %d - %s\n", (x+1), bootstrap_config_labels[x]); + + do { + nbytes = readline(" Selection [1-x / quit]: "); + + if (nbytes) { + if (strcmp(console_buffer, "quit") == 0) + return 0; + selcfg = simple_strtol(console_buffer, NULL, 10); + if ((selcfg < 1) || (selcfg > x)) + nbytes = 0; + } + } while (nbytes == 0); + + + x = (selcfg - 1); + + if (i2c_write(CONFIG_BOOTSTRAP_I2C_EEPROM_ADDR, 0, 1, + bootstrap_configs[x], CONFIG_BOOTSTRAP_BLOCKSIZE) != 0) + printf("Error writing to EEPROM at address 0x%x\n", + CONFIG_BOOTSTRAP_I2C_EEPROM_ADDR); + udelay(CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS * 1000); + + printf("Done\n"); + printf("Please power-cycle the board for the changes to take effect\n"); + + return 0; +} + +U_BOOT_CMD( + bootstrap, 1, 0, do_bootstrap, + "program the I2C bootstrap EEPROM", + "" +); diff --git a/include/bootstrap.h b/include/bootstrap.h new file mode 100644 index 0000000..61919dc --- /dev/null +++ b/include/bootstrap.h @@ -0,0 +1,36 @@ +/* + * (C) Copyright 2009 + * Dirk Eibach, Guntermann & Drunck GmbH, eibach@gdsys.de + * + * based on code (C) Copyright 2008 + * Stefan Roese, DENX Software Engineering, sr@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 + * + */ + +#ifndef __BOOTSTRAP_H +#define __BOOTSTRAP_H + +#include <common.h> + +extern char *bootstrap_config_labels[]; +extern u8 bootstrap_configs[][CONFIG_BOOTSTRAP_BLOCKSIZE]; + +#endif /* __BOOTSTRAP_H */ diff --git a/include/configs/canyonlands.h b/include/configs/canyonlands.h index 48c5198..ab0bd59 100644 --- a/include/configs/canyonlands.h +++ b/include/configs/canyonlands.h @@ -330,6 +330,9 @@ #define CONFIG_SYS_EEPROM_PAGE_WRITE_BITS 3 #define CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS 10
+#define CONFIG_BOOTSTRAP_I2C_EEPROM_ADDR 0x54 +#define CONFIG_BOOTSTRAP_BLOCKSIZE 16 + /* I2C SYSMON (LM75, AD7414 is almost compatible) */ #define CONFIG_DTT_LM75 1 /* ON Semi's LM75 */ #define CONFIG_DTT_AD7414 1 /* use AD7414 */ @@ -443,10 +446,12 @@ * Commands additional to the ones defined in amcc-common.h */ #if defined(CONFIG_ARCHES) +#define CONFIG_CMD_BOOTSTRAP #define CONFIG_CMD_DTT #define CONFIG_CMD_PCI #define CONFIG_CMD_SDRAM #elif defined(CONFIG_CANYONLANDS) +#define CONFIG_CMD_BOOTSTRAP #define CONFIG_CMD_DATE #define CONFIG_CMD_DTT #define CONFIG_CMD_EXT2 @@ -457,6 +462,7 @@ #define CONFIG_CMD_SNTP #define CONFIG_CMD_USB #elif defined(CONFIG_GLACIER) +#define CONFIG_CMD_BOOTSTRAP #define CONFIG_CMD_DATE #define CONFIG_CMD_DTT #define CONFIG_CMD_NAND
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ? -mike
Hi Dirk,
On Wednesday 15 July 2009 16:46:12 Mike Frysinger wrote:
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"? Any other good ideas about this naming welcome. :)
Other than this, patch looks good. Thanks.
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 =====================================================================
On Thursday 16 July 2009 15:21, Stefan Roese wrote:
Hi Dirk,
On Wednesday 15 July 2009 16:46:12 Mike Frysinger wrote:
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'. But it works a little mit different that the bootstrap command.
Other than this, patch looks good. Thanks.
Matthias
Best regards, Stefan
On Thursday 16 July 2009 15:47:17 Matthias Fuchs wrote:
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'.
If you already forgot what it's supposed to mean, then it definitely is not a good name. Better a bit longer and more descriptive.
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one. Does you command support more features? What's the main difference?
Thanks.
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 =====================================================================
Hi,
Stefan Roese wrote:
On Thursday 16 July 2009 15:47:17 Matthias Fuchs wrote:
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'.
If you already forgot what it's supposed to mean, then it definitely is not a good name. Better a bit longer and more descriptive.
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one. Does you command support more features? What's the main difference?
BTW, there's also pllalter command for kilauea, that provides similar functionality.
Felix.
Thanks.
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 ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Thursday 16 July 2009 16:16:36 Felix Radensky wrote:
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one. Does you command support more features? What's the main difference?
BTW, there's also pllalter command for kilauea, that provides similar functionality.
Correct. Thanks for reminding me. I'll do some Kilauea work in the next days and so I'll probably change this command to the new 4xx-common bootstrap command (whatever this will be).
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 =====================================================================
On Thursday 16 July 2009 16:06, Stefan Roese wrote:
On Thursday 16 July 2009 15:47:17 Matthias Fuchs wrote:
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'.
If you already forgot what it's supposed to mean, then it definitely is not a good name. Better a bit longer and more descriptive.
I like 'sbe' - it short and I even gave it a meaning :-)
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one.
I was a little inspired by the sequoia code that also comes with a bootstrap command. So please don't ask me to merge it into common code. But your finger on your own nose.
Does you command support more features? What's the main difference?
Much simpler. Just call sbe with a descriptive argument like a CPU frequency or something like '667-66' on a 440EPx target with 66Mhz PCI clock or 'sr-test-only' for something you will remove later :-). This has two advantages over just using numbers: You can remove configurations without making the following configs in the table moving to the front and its a little more secure meaning you have to type a couple of valid character to reconfigure the clocking. Just using "bootstrap 5" is error-prone.
Well, I like my syntax and behavior, but I do not want to totally dismiss Dirk's idea as long as I can keep my sbe command :-)
Thanks.
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 ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Thursday 16 July 2009 17:00:00 Matthias Fuchs wrote:
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'.
If you already forgot what it's supposed to mean, then it definitely is not a good name. Better a bit longer and more descriptive.
I like 'sbe' - it short and I even gave it a meaning :-)
But unfortunately not specific enough for a "common" command.
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one.
I was a little inspired by the sequoia code that also comes with a bootstrap command. So please don't ask me to merge it into common code. But your finger on your own nose.
OK. But if your "sbe" command is "better" than the current bootstrap one, then let's see if it makes sense to use your command as the common one.
Does you command support more features? What's the main difference?
Much simpler. Just call sbe with a descriptive argument like a CPU frequency or something like '667-66' on a 440EPx target with 66Mhz PCI clock or 'sr-test-only' for something you will remove later :-). This has two advantages over just using numbers: You can remove configurations without making the following configs in the table moving to the front and its a little more secure meaning you have to type a couple of valid character to reconfigure the clocking. Just using "bootstrap 5" is error-prone.
Ack.
Well, I like my syntax and behavior, but I do not want to totally dismiss Dirk's idea as long as I can keep my sbe command :-)
Seems that "your" command is not so bad. ;) I'll take a look at it tomorrow. Perhaps we can use some of your ideas in such a new common (PPC4xx) implementation. :)
Thanks.
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 =====================================================================
On Thursday 16 July 2009 17:00:00 Matthias Fuchs wrote:
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'.
If you already forgot what it's supposed to mean, then it definitely is not a good name. Better a bit longer and more descriptive.
I like 'sbe' - it short and I even gave it a meaning :-)
But unfortunately not specific enough for a "common" command.
I think we are talking about a generic 4xx command to write data into the EEPROM that is read by the bootstrap controller. But I do not care about the name ;-)
But it works a little mit different that the bootstrap command.
It would be great if you could merge/consolidate such 4xx custom commands into this common one.
I was a little inspired by the sequoia code that also comes with a bootstrap command. So please don't ask me to merge it into common code. But your finger on your own nose.
OK. But if your "sbe" command is "better" than the current bootstrap one, then let's see if it makes sense to use your command as the common one.
Dirk's approach is very generic. Putting nothing but the EEPROM data and a descriptive table into the board file is what we need. I only suggest to add an additional label to each entry that is used instead of using numbers and I'd to pass this label as argument instead of the a number. One could use the numbers as labels also :-)
When you do no pass the argument we could either print the descriptive texts as menu (as DIrk did so far) and wait for input or just print the texts as kind of help and the must supply an argument if he want to change something (I prefer the latter).
Does you command support more features? What's the main difference?
Much simpler. Just call sbe with a descriptive argument like a CPU frequency or something like '667-66' on a 440EPx target with 66Mhz PCI clock or 'sr-test-only' for something you will remove later :-). This has two advantages over just using numbers: You can remove configurations without making the following configs in the table moving to the front and its a little more secure meaning you have to type a couple of valid character to reconfigure the clocking. Just using "bootstrap 5" is error-prone.
Ack.
Well, I like my syntax and behavior, but I do not want to totally dismiss Dirk's idea as long as I can keep my sbe command :-)
Seems that "your" command is not so bad. ;) I'll take a look at it tomorrow. Perhaps we can use some of your ideas in such a new common (PPC4xx) implementation. :)
My implementation is nothing but ar if-!strcmp-else-if-!strcmp implementation. But putting things together is a good idea.
Matthias
Thanks.
Best regards, Stefan
Dear Matthias Fuchs,
In message 200907162208.28398.matthias.fuchs@esd-electronics.com you wrote:
But unfortunately not specific enough for a "common" command.
I think we are talking about a generic 4xx command to write data into the EEPROM that is read by the bootstrap controller. But I do not care about the name ;-)
We're changing some board configuration data. Maybe the command name should start with "conf[ig]" then.
Best regards,
Wolfgang Denk
On Thursday 16 July 2009 22:08:27 Matthias Fuchs wrote:
OK. But if your "sbe" command is "better" than the current bootstrap one, then let's see if it makes sense to use your command as the common one.
Dirk's approach is very generic. Putting nothing but the EEPROM data and a descriptive table into the board file is what we need.
I'm aware of this, since I suggested to Dirk to implement it this way. ;)
I only suggest to add an additional label to each entry that is used instead of using numbers and I'd to pass this label as argument instead of the a number. One could use the numbers as labels also :-)
Ack.
When you do no pass the argument we could either print the descriptive texts as menu (as DIrk did so far) and wait for input or just print the texts as kind of help and the must supply an argument if he want to change something (I prefer the latter).
Yes, just printing all available config options when called without parameter is what I prefer as well. Something like this:
=> 4xx_config (or 4xx_bootstrap) Available configurations: Name CPU PLB OPB EBC Boot-Location 1 333 133 66 66 NOR 2 333 133 66 66 NAND 3 333 133 66 66 PCI 4 400 133 66 66 NOR 5 400 133 66 66 NAND 6 400 160 80 53 NOR ...
The board maintainer could of course use real "names" instead of the numbers here.
Does you command support more features? What's the main difference?
Much simpler. Just call sbe with a descriptive argument like a CPU frequency or something like '667-66' on a 440EPx target with 66Mhz PCI clock or 'sr-test-only' for something you will remove later :-). This has two advantages over just using numbers: You can remove configurations without making the following configs in the table moving to the front and its a little more secure meaning you have to type a couple of valid character to reconfigure the clocking. Just using "bootstrap 5" is error-prone.
Ack.
Well, I like my syntax and behavior, but I do not want to totally dismiss Dirk's idea as long as I can keep my sbe command :-)
Seems that "your" command is not so bad. ;) I'll take a look at it tomorrow. Perhaps we can use some of your ideas in such a new common (PPC4xx) implementation. :)
My implementation is nothing but ar if-!strcmp-else-if-!strcmp implementation. But putting things together is a good idea.
Yes, the implementation itself is non-optimal. I'll send an updated version of Dirk's patch with the "new" user interface and a new command-naming (hopefully) today.
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 Matthias Fuchs,
In message 200907161547.17433.matthias.fuchs@esd.eu you wrote:
Hi Dirk,
On Wednesday 15 July 2009 16:46:12 Mike Frysinger wrote:
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'. But it works
Indeed. "sbe" is not an acceptable name as nobody will know what it's suppost to mean.
a little mit different that the bootstrap command.
Actually the name "bootstrap" command itself is something that I really dislike.
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
Best regards,
Wolfgang Denk
On 7/16/2009 2:36 PM, Wolfgang Denk wrote:
Dear Matthias Fuchs,
In message200907161547.17433.matthias.fuchs@esd.eu you wrote:
Hi Dirk,
On Wednesday 15 July 2009 16:46:12 Mike Frysinger wrote:
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
a little mit different that the bootstrap command.
Actually the name "bootstrap" command itself is something that I really dislike.
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
Keep in mind the processor UM documentation section that covers these boot methods is labeled "Bootstrap Operations". It refers the registers that the EEPROM values populate as "bootstrap registers", etc.
There is some logic in tying the section of the processor manual to the command. Whatever the command ends up being labeled, it would be nice if its purpose was clear to those who actually work in 4xx space.
Not to mention there is documentation referencing the current commands used by the evaluation platforms.
Regards, -DM
Dear Dave Mitchell,
In message 4A5F8A1D.4030701@gmail.com you wrote:
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
Keep in mind the processor UM documentation section that covers these boot methods is labeled "Bootstrap Operations". It refers the registers that the EEPROM values populate as "bootstrap registers", etc.
Yes, I know. But then, bootstrapping [1] is really a name with a _very_ fuzzy meaning. It can be anything. The fact thay it was chosen in the UM documentation does't make it better - in U-Boot context I think about other activities, and in Linux context yet about other things.
There is some logic in tying the section of the processor manual to the command. Whatever the command ends up being labeled, it would be nice if its purpose was clear to those who actually work in 4xx space.
We are changing the CPU configuration, right? It has nothing to do with actually booting or bootstrapping? We are setting parameters that will be used by the CPU when it comes up out of reset, that's all.
So maybe we call this config_cpu ?
Not to mention there is documentation referencing the current commands used by the evaluation platforms.
Documentation should be updated when new software versions become available - that's a matter of fact.
[1] http://en.wikipedia.org/wiki/Bootstrap
Best regards,
Wolfgang Denk
On 7/16/2009 5:11 PM, Wolfgang Denk wrote:
Dear Dave Mitchell,
In message4A5F8A1D.4030701@gmail.com you wrote:
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
Keep in mind the processor UM documentation section that covers these boot methods is labeled "Bootstrap Operations". It refers the registers that the EEPROM values populate as "bootstrap registers", etc.
Yes, I know. But then, bootstrapping [1] is really a name with a _very_ fuzzy meaning. It can be anything. The fact thay it was chosen in the UM documentation does't make it better - in U-Boot context I think about other activities, and in Linux context yet about other things.
In general, the term does have a "fuzzy" meaning. But within the AMCC 4xx processors, it does not. And we are talking about a command that would only be available to AMCC 4xx processors.
A command that's been in use on several U-Boot 4xx ports for some time.
There is some logic in tying the section of the processor manual to the command. Whatever the command ends up being labeled, it would be nice if its purpose was clear to those who actually work in 4xx space.
We are changing the CPU configuration, right? It has nothing to do with actually booting or bootstrapping? We are setting parameters that will be used by the CPU when it comes up out of reset, that's all.
It does actually allow you to set the boot ROM location, just FYI.
So maybe we call this config_cpu ?
Not to mention there is documentation referencing the current commands used by the evaluation platforms.
Documentation should be updated when new software versions become available - that's a matter of fact.
Of course. And I suppose I'm wasting energy typing since you have already made up your mind on the subject but no harm in trying.
Best regards, -DM
On Friday 17 July 2009 02:12:45 Dave Mitchell wrote:
Keep in mind the processor UM documentation section that covers these boot methods is labeled "Bootstrap Operations". It refers the registers that the EEPROM values populate as "bootstrap registers", etc.
Yes, I know. But then, bootstrapping [1] is really a name with a _very_ fuzzy meaning. It can be anything. The fact thay it was chosen in the UM documentation does't make it better - in U-Boot context I think about other activities, and in Linux context yet about other things.
In general, the term does have a "fuzzy" meaning. But within the AMCC 4xx processors, it does not. And we are talking about a command that would only be available to AMCC 4xx processors.
A command that's been in use on several U-Boot 4xx ports for some time.
I'm aware of this fact. And changing this name is of course non-optimal. But from my point of view, the advantages of enhancing this command and making it easier usable/portable for all 4xx boards overweight this disadvantage.
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 Dave Mitchell,
In message 4A5FC1FD.1090201@gmail.com you wrote:
Yes, I know. But then, bootstrapping [1] is really a name with a _very_ fuzzy meaning. It can be anything. The fact thay it was chosen in the UM documentation does't make it better - in U-Boot context I think about other activities, and in Linux context yet about other things.
In general, the term does have a "fuzzy" meaning. But within the AMCC 4xx processors, it does not. And we are talking about a command that would only be available to AMCC 4xx processors.
A command that's been in use on several U-Boot 4xx ports for some time.
Right. and now we are generalizing it to add a common command for all these boards - and actually for other architectures as well. We will have to suffer from a changed command name, but we win by better functionality, and less code duplication.
Of course. And I suppose I'm wasting energy typing since you have already made up your mind on the subject but no harm in trying.
I appreciate your comments, and you know that I rate your opinion highly; but in this case my more global perspective sets other priorities than your more 4xx-focussed POV. Sorry.
Best regards,
Wolfgang Denk
Dear Matthias Fuchs,
In message 200907161547.17433.matthias.fuchs@esd.eu you wrote:
Hi Dirk,
On Wednesday 15 July 2009 16:46:12 Mike Frysinger wrote:
On Wednesday 15 July 2009 09:48:00 Dirk Eibach wrote:
This adds a generic command for programming I2C bootstrap eeproms. Implementation for Canyonlands board is included.
"bootstrap" is pretty generic. can you pick something a little more descriptive/specific ?
I agree with Mike. Please change the command name and the files to something more specific. How about "4xxbootstrap"? Or "4xxstrap"?
To long for my taste.
Any other good ideas about this naming welcome. :)
We called this command "sbe" on our PMC440 (440EPx) and upcoming PMC405DE (405EP) board. I must admit that I forget its meaning. Probably something like 'setup bootstrap eeprom'. But it works
Indeed. "sbe" is not an acceptable name as nobody will know what it's suppost to mean.
a little mit different that the bootstrap command.
Actually the name "bootstrap" command itself is something that I really dislike.
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
ccc - config chip|cpu configuration scc - setup chip|cpu configuration wcc - write chip|cpu configuration wcce - write chip|cpu configuration eeprom
I am not a friend of long names :-)
Matthias
Dear Matthias Fuchs,
In message 200907162224.23377.matthias.fuchs@esd-electronics.com you wrote:
Please find a better description (avoiding "bootstrap"), and then chose a descriptive name.
ccc - config chip|cpu configuration scc - setup chip|cpu configuration wcc - write chip|cpu configuration wcce - write chip|cpu configuration eeprom
Which of these will you still remember after the third glass of beer? ;-)
I am not a friend of long names :-)
Neiter am I. But there are some basic rules for user interface design. Short commands should only be used for "harmless" operations. Long names are OK for critical actions where you better should have your brain online and not only your fingers continue to type something after the brain detached. And short commands should bne used for frequently used commands, while long, self-explaining commands are OK for more exotic operations.
This _is_ a thing that requires attention, and you do not need it frequently - I guess if you run statistics about command usage it will be in the bottom 5% - agreed?
So a long name is perfect.
Something like "config_cpu" or "set_cpu_config" etc. (and the underscore is intentional to make you think twice what you are doing here).
Best regards,
Wolfgang Denk
Please see my comments below.
On Wednesday 15 July 2009 15:48, Dirk Eibach wrote:
...
diff --git a/common/cmd_bootstrap.c b/common/cmd_bootstrap.c new file mode 100644 index 0000000..6fceb23 --- /dev/null +++ b/common/cmd_bootstrap.c @@ -0,0 +1,75 @@ +/*
- (C) Copyright 2009
- Dirk Eibach, Guntermann & Drunck GmbH, eibach@gdsys.de
- based on code (C) Copyright 2008
- Stefan Roese, DENX Software Engineering, sr@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 <bootstrap.h> +#include <common.h> +#include <command.h> +#include <i2c.h> +#include <asm/io.h>
+static int do_bootstrap(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- int x, nbytes, selcfg;
- extern char console_buffer[];
- printf("Available configurations: \n\n");
- for (x = 0; bootstrap_config_labels[x][0] != 0; x++)
printf(" %d - %s\n", (x+1), bootstrap_config_labels[x]);- do {
nbytes = readline(" Selection [1-x / quit]: ");if (nbytes) {if (strcmp(console_buffer, "quit") == 0)return 0;selcfg = simple_strtol(console_buffer, NULL, 10);if ((selcfg < 1) || (selcfg > x))nbytes = 0;}- } while (nbytes == 0);
Do we really need to have this interactive? At least it would be fine to use "bootstrap 4" to get the fourth configuration. So when an additional parameter ist passed to bootstrap you could bypass alle the printf stuff and directly update the EEPROM.
- x = (selcfg - 1);
- if (i2c_write(CONFIG_BOOTSTRAP_I2C_EEPROM_ADDR, 0, 1,
bootstrap_configs[x], CONFIG_BOOTSTRAP_BLOCKSIZE) != 0)printf("Error writing to EEPROM at address 0x%x\n",CONFIG_BOOTSTRAP_I2C_EEPROM_ADDR);- udelay(CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS * 1000);
- printf("Done\n");
Can't we life without this 'Done' message?
- printf("Please power-cycle the board for the changes to take effect\n");
At least for 440EPx and 405EP calling 'reset' is enough and much easier for scripting :-)
Matthias
participants (8)
-
Dave Mitchell -
Dirk Eibach -
Felix Radensky -
Matthias Fuchs -
Matthias Fuchs -
Mike Frysinger -
Stefan Roese -
Wolfgang Denk