Hi Wolfgang,
On Wed, Jun 27, 2012 at 03:12:07, Wolfgang Denk wrote:
Dear Prabhakar Lad,
In message 1340708826-26707-1-git-send-email-prabhakar.lad@ti.com you wrote:
From: Alagu Sankar alagusankar@embwise.com
This is a Linux command line tool specific to TI's Davinci platforms, for flashing UBL (User Boot Loader), u-boot and u-boot Environment in the MMC/SD card. This MMC/SD card can be used for booting Davinci platforms that supports MMC/SD boot option.
Do we also build UBL as part of the U-Boot source tree?
No, we do not build UBL as part of U-Boot.
If not, then why is this tool supposed to be part of the U-Boot tree?
How does this work with a SPL?
This command has options to flash u-boot images to MMC/SD card. When SPL is supported, this command can be used to flash the single SPL image to MMC/SD card.
--- a/Makefile +++ b/Makefile @@ -726,7 +726,7 @@ clean: @rm -f $(obj)examples/api/demo{,.bin} @rm -f $(obj)tools/bmp_logo $(obj)tools/easylogo/easylogo \ $(obj)tools/env/{fw_printenv,fw_setenv} \
$(obj)tools/envcrc \
$(obj)tools/envcrc $(obj)tools/uflash/uflash \ $(obj)tools/gdb/{astest,gdbcont,gdbsend} \ $(obj)tools/gen_eth_addr $(obj)tools/img2srec \ $(obj)tools/mk{env,}image $(obj)tools/mpc86x_clk \Please keep list sorted.
Ok.
+e. Using the 'uflash' utility, place the UBL and u-uoot binaries on the MMC
- card. Copy the u-boot.bin to tools/uflash directory
Why is this copy operation needed?
This copy is not needed as long as the path to u-boot.bin is specified Correctly in command line.
And where is the UBL binary coming from?
UBL binary is optional. We can flash only u-boot.bin.
diff --git a/tools/uflash/config.txt b/tools/uflash/config.txt new file mode 100644 index 0000000..f6acb22 --- /dev/null +++ b/tools/uflash/config.txt @@ -0,0 +1,11 @@ +bootargs=console=ttyS0,115200n8 root=/dev/mmcblk0p1 rootwait rootfstype=ext3 rw +bootcmd=ext2load mmc 0 0x80700000 boot/uImage; bootm 0x80700000 +bootdelay=1 +baudrate=115200 +bootfile="uImage" +stdin=serial +stdout=serial +stderr=serial +ethact=dm9000 +videostd=ntsc
This looks like U-Boot environment settings? Why are these in the tools/uflash/ directory? I would expect these are board specific? For example, what in case a board uses a different baud rate?
Is this really supposed to be board independent? It doesn't look so...
I agree with this. Can you think of any other scenario?
And please, no trailing empty lines!
Ok.
...
- if (!strcmp(platform, "DM3XX")) {
if (!uboot_load_address)uboot_load_address = DM3XX_UBOOT_LOAD_ADDRESS;if (!uboot_entry_point)uboot_entry_point = DM3XX_UBOOT_LOAD_ADDRESS;- }
- if (!strcmp(platform, "OMAPL138")) {
if (!uboot_load_address)uboot_load_address = DA850_UBOOT_LOAD_ADDRESS;if (!uboot_entry_point)uboot_entry_point = DA850_UBOOT_LOAD_ADDRESS;- }
So this is actually all hardwired for a few very specific board configurations, right?
Yes.
.
+static int get_file_size(char *fname) +{
- FILE *fp;
- int size;
- fp = fopen(fname, "rb");
- if (fp == NULL) {
fprintf(stdout, "File %s Open Error : %s\n",fname, strerror(errno));return -1;- }
- fseek(fp, 0, SEEK_END);
- size = ftell(fp);
- fclose(fp);
Why not simply using stat() ?
Yes that makes sense.
+static int write_file(int devfd, char *fname) +{
- FILE *fp;
- int readlen, writelen;
- fp = fopen(fname, "rb");
- if (fp == NULL) {
fprintf(stderr, "File %s Open Error: %s",fname, strerror(errno));return -1;- }
- while ((readlen = fread(readbuf, 1, BLOCK_SIZE, fp)) > 0) {
if (readlen < BLOCK_SIZE)memset(&readbuf[readlen], 0, BLOCK_SIZE-readlen);writelen = write(devfd, readbuf, BLOCK_SIZE);if (writelen < BLOCK_SIZE) {close(devfd);return -1;}- }
- fclose(fp);
You don't even print a warning or error message in case of read errors? Ouch...
Ok , I'll fix it in V2 version.
Thx, --Prabhakar Lad
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@denx.de C makes it easy for you to shoot yourself in the foot. C++ makes that harder, but when you do, it blows away your whole leg. -- Bjarne Stroustrup