| Message ID | 1313782128-7252-1-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Frysinger |
| Headers | show |
On Friday, August 19, 2011 15:28:48 Simon Glass wrote: > This adds a new SPI flash command which only rewrites blocks if the > contents need to change. This can speed up SPI flash programming when much > of the data is unchanged from what is already there. looks good to me. i'll give it a spin on a board of mine and then push into my sf branch. i really should write a spi flash simulation so i could just test this from gdb ... -mike
On Fri, Aug 19, 2011 at 3:15 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Friday, August 19, 2011 15:28:48 Simon Glass wrote: >> This adds a new SPI flash command which only rewrites blocks if the >> contents need to change. This can speed up SPI flash programming when much >> of the data is unchanged from what is already there. > > looks good to me. i'll give it a spin on a board of mine and then push into > my sf branch. i really should write a spi flash simulation so i could just > test this from gdb ... Hi Mike, Thanks. Funny you should say that. I rather badly need a way of testing the higher level U-Boot code (from the commands down to where it calls architecture/driver code). I am drafting up an email to send to the list with some thoughts on the matter. Regards, Simon > -mike >
On Friday, August 19, 2011 17:25:10 Simon Glass wrote: > On Fri, Aug 19, 2011 at 3:15 PM, Mike Frysinger wrote: > > On Friday, August 19, 2011 15:28:48 Simon Glass wrote: > >> This adds a new SPI flash command which only rewrites blocks if the > >> contents need to change. This can speed up SPI flash programming when > >> much of the data is unchanged from what is already there. > > > > looks good to me. i'll give it a spin on a board of mine and then push > > into my sf branch. i really should write a spi flash simulation so i > > could just test this from gdb ... > > Funny you should say that. I rather badly need a way of testing the > higher level U-Boot code (from the commands down to where it calls > architecture/driver code). I am drafting up an email to send to the > list with some thoughts on the matter. when i wrote the blackfin system level port of the gnu sim, it was so i could do this (and i thought it'd be bad-ass). i often use the gnu sim to do initial testing (sometimes down to the driver level) before i get around to loading up on actual hardware. $ bfin-elf-run --env operating --model bf537 ./u-boot U-Boot 2011.06-00375-g23ffb39-dirty (Aug 14 2011 - 16:54:03) CPU: ADSP bf537-0.2 (Detected Rev: 0.0) (bypass boot) Board: ADI BF537 stamp board Support: http://blackfin.uclinux.org/ Clock: VCO: 500 MHz, Core: 500 MHz, System: 125 MHz RAM: 64 MiB Flash: ## Unknown flash on Bank 1 - Size = 0x00000000 = 0 MB 0 Bytes MMC: *** Warning - bad CRC, using default environment In: serial Out: serial Err: serial KGDB: [on serial] ready Warning: Generating 'random' MAC address Net: bfin_mac Hit any key to stop autoboot: 5 0 bfin> help sf sf - SPI flash sub-system Usage: sf probe [bus:]cs [hz] [mode] - init flash device on given SPI bus and chip select sf read addr offset len - read `len' bytes starting at `offset' to memory at `addr' sf write addr offset len - write `len' bytes from memory at `addr' to flash at `offset' sf erase offset [+]len - erase `len' bytes from `offset' `+len' round up `len' to block size bfin> -mike
Hi Mike, On Fri, Aug 19, 2011 at 3:28 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Friday, August 19, 2011 17:25:10 Simon Glass wrote: >> On Fri, Aug 19, 2011 at 3:15 PM, Mike Frysinger wrote: >> > On Friday, August 19, 2011 15:28:48 Simon Glass wrote: >> >> This adds a new SPI flash command which only rewrites blocks if the >> >> contents need to change. This can speed up SPI flash programming when >> >> much of the data is unchanged from what is already there. >> > >> > looks good to me. i'll give it a spin on a board of mine and then push >> > into my sf branch. i really should write a spi flash simulation so i >> > could just test this from gdb ... >> >> Funny you should say that. I rather badly need a way of testing the >> higher level U-Boot code (from the commands down to where it calls >> architecture/driver code). I am drafting up an email to send to the >> list with some thoughts on the matter. > > when i wrote the blackfin system level port of the gnu sim, it was so i could > do this (and i thought it'd be bad-ass). i often use the gnu sim to do > initial testing (sometimes down to the driver level) before i get around to > loading up on actual hardware. > > $ bfin-elf-run --env operating --model bf537 ./u-boot > > > U-Boot 2011.06-00375-g23ffb39-dirty (Aug 14 2011 - 16:54:03) > > CPU: ADSP bf537-0.2 (Detected Rev: 0.0) (bypass boot) > Board: ADI BF537 stamp board > Support: http://blackfin.uclinux.org/ > Clock: VCO: 500 MHz, Core: 500 MHz, System: 125 MHz > RAM: 64 MiB > Flash: ## Unknown flash on Bank 1 - Size = 0x00000000 = 0 MB > 0 Bytes > MMC: > *** Warning - bad CRC, using default environment > > In: serial > Out: serial > Err: serial > KGDB: [on serial] ready > Warning: Generating 'random' MAC address > Net: bfin_mac > Hit any key to stop autoboot: 5 0 > bfin> help sf > sf - SPI flash sub-system > > Usage: > sf probe [bus:]cs [hz] [mode] - init flash device on given SPI bus > and chip select > sf read addr offset len - read `len' bytes starting at > `offset' to memory at `addr' > sf write addr offset len - write `len' bytes from memory > at `addr' to flash at `offset' > sf erase offset [+]len - erase `len' bytes from `offset' > `+len' round up `len' to block size > bfin> > -mike > That's a great trick. How much of the drivers did you implement in the simulator? How about this, running native under Linux: $ ./u-boot U-Boot 2011.03-00793-g0463aab-dirty (Aug 23 2011 - 14:56:06) DRAM: 128 MiB Using default environment In: serial Out: lcd Err: lcd =>sf probe 0 sf probe 0 SF: Detected test with page size 256, total 4 MiB 4096 KiB test at 0:0 is now current device =>sf read 0 0 0x2000 sf read 0 0 0x2000 =>md 0 md 0 00000000: 282c36b2 17938d43 d7dcbdfd 362e4362 .6,(C.......bC.6 00000010: 00000000 00000000 00000000 00000000 ................ 00000020: 00020001 0000000f 0000000b 00400000 ..............@. 00000030: 00000004 00000003 00000003 00000003 ................ 00000040: 00000003 00000000 00000116 00000000 ................ 00000050: 00000000 00000000 00000116 00000000 ................ 00000060: 00000000 00000000 00000116 00000000 ................ 00000070: 00000000 00000000 00000116 00000000 ................ 00000080: 00000000 00000002 00000003 00000008 ................ 00000090: 00000000 0000001a 000002f8 00000000 ................ 000000a0: 0000012c 00000002 00000000 e0a61818 ,............... 000000b0: 00000000 00000000 0000000b 00000026 ............&... 000000c0: 00000008 00000003 00000004 00000004 ................ 000000d0: 00000002 0000000b 00000003 00000003 ................ 000000e0: 00000002 00000001 00000003 00000004 ................ 000000f0: 00000003 00000009 0000000c 0000059f ................ =>^C $ Regards, Simon
On Tuesday, August 23, 2011 18:01:34 Simon Glass wrote: > That's a great trick. How much of the drivers did you implement in the > simulator? probably more than i'd like to admit, but not as many as i'd like ;) http://docs.blackfin.uclinux.org/doku.php?id=toolchain:sim#peripherals > How about this, running native under Linux: also pretty cool. i think both are worth while efforts. -mike
Hi Simon, I have a dumb question: How did you make u-boot run native under Linux? Did you mock out all platform functions? Or did you bundle u-boot with a emulator? Regards, Che-Liang On Wed, Aug 24, 2011 at 6:16 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tuesday, August 23, 2011 18:01:34 Simon Glass wrote: >> That's a great trick. How much of the drivers did you implement in the >> simulator? > > probably more than i'd like to admit, but not as many as i'd like ;) > http://docs.blackfin.uclinux.org/doku.php?id=toolchain:sim#peripherals > >> How about this, running native under Linux: > > also pretty cool. i think both are worth while efforts. > -mike > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > >
Hi Che-Liang. On Tue, Aug 23, 2011 at 8:41 PM, Che-liang Chiou <clchiou@chromium.org> wrote: > Hi Simon, > > I have a dumb question: How did you make u-boot run native under > Linux? Did you mock out all platform functions? Or did you bundle > u-boot with a emulator? Basically created a new architecture (like arm/x86) called 'native' which runs under Linux. Here is the first review, some of the rest follow, but I am still working on it: http://gerrit.chromium.org/gerrit/#change,6334 Regards, Simon > > Regards, > Che-Liang > > On Wed, Aug 24, 2011 at 6:16 AM, Mike Frysinger <vapier@gentoo.org> wrote: >> On Tuesday, August 23, 2011 18:01:34 Simon Glass wrote: >>> That's a great trick. How much of the drivers did you implement in the >>> simulator? >> >> probably more than i'd like to admit, but not as many as i'd like ;) >> http://docs.blackfin.uclinux.org/doku.php?id=toolchain:sim#peripherals >> >>> How about this, running native under Linux: >> >> also pretty cool. i think both are worth while efforts. >> -mike >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> >> >
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 11a491d..69f04e5 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -6,6 +6,7 @@ */ #include <common.h> +#include <malloc.h> #include <spi_flash.h> #include <asm/io.h> @@ -109,6 +110,78 @@ static int do_spi_flash_probe(int argc, char * const argv[]) return 0; } +/** + * Write a block of data to SPI flash, first checking if it is different from + * what is already there. + * + * If the data being written is the same, then *skipped is incremented by len. + * + * @param flash flash context pointer + * @param offset flash offset to write + * @param len number of bytes to write + * @param buf buffer to write from + * @param cmp_buf read buffer to use to compare data + * @param skipped Count of skipped data (incremented by this function) + * @return NULL if OK, else a string containing the stage which failed + */ +static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset, + size_t len, const char *buf, char *cmp_buf, size_t *skipped) +{ + debug("offset=%#x, sector_size=%#x, len=%#x\n", + offset, flash->sector_size, len); + if (spi_flash_read(flash, offset, len, cmp_buf)) + return "read"; + if (memcmp(cmp_buf, buf, len) == 0) { + debug("Skip region %x size %x: no change\n", + offset, len); + *skipped += len; + return NULL; + } + if (spi_flash_erase(flash, offset, len)) + return "erase"; + if (spi_flash_write(flash, offset, len, buf)) + return "write"; + return NULL; +} + +/** + * Update an area of SPI flash by erasing and writing any blocks which need + * to change. Existing blocks with the correct data are left unchanged. + * + * @param flash flash context pointer + * @param offset flash offset to write + * @param len number of bytes to write + * @param buf buffer to write from + * @return 0 if ok, 1 on error + */ +static int spi_flash_update(struct spi_flash *flash, u32 offset, + size_t len, const char *buf) +{ + const char *err_oper = NULL; + char *cmp_buf; + const char *end = buf + len; + size_t todo; /* number of bytes to do in this pass */ + size_t skipped; /* statistics */ + + cmp_buf = malloc(flash->sector_size); + if (cmp_buf) { + for (skipped = 0; buf < end; buf += todo, offset += todo) { + todo = min(end - buf, flash->sector_size); + err_oper = spi_flash_update_block(flash, offset, todo, + buf, cmp_buf, &skipped); + } + } else { + err_oper = "malloc"; + } + free(cmp_buf); + if (err_oper) { + printf("SPI flash failed in %s step\n", err_oper); + return 1; + } + printf("%d bytes written, %d bytes skipped\n", len - skipped, skipped); + return 0; +} + static int do_spi_flash_read_write(int argc, char * const argv[]) { unsigned long addr; @@ -137,7 +210,9 @@ static int do_spi_flash_read_write(int argc, char * const argv[]) return 1; } - if (strcmp(argv[0], "read") == 0) + if (strcmp(argv[0], "update") == 0) + ret = spi_flash_update(flash, offset, len, buf); + else if (strcmp(argv[0], "read") == 0) ret = spi_flash_read(flash, offset, len, buf); else ret = spi_flash_write(flash, offset, len, buf); @@ -203,7 +278,8 @@ static int do_spi_flash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ return 1; } - if (strcmp(cmd, "read") == 0 || strcmp(cmd, "write") == 0) + if (strcmp(cmd, "read") == 0 || strcmp(cmd, "write") == 0 || + strcmp(cmd, "update") == 0) ret = do_spi_flash_read_write(argc, argv); else if (strcmp(cmd, "erase") == 0) ret = do_spi_flash_erase(argc, argv); @@ -229,4 +305,6 @@ U_BOOT_CMD( " at `addr' to flash at `offset'\n" "sf erase offset [+]len - erase `len' bytes from `offset'\n" " `+len' round up `len' to block size" + "sf update addr offset len - erase and write `len' bytes from memory\n" + " at `addr' to flash at `offset'\n" );
This adds a new SPI flash command which only rewrites blocks if the contents need to change. This can speed up SPI flash programming when much of the data is unchanged from what is already there. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: -Moved loop into a function -Moved malloc outside loop -Other minor changes from review comments Changes in v3: -Change error message -Style and function name change common/cmd_sf.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 80 insertions(+), 2 deletions(-)