Hi Mike, On Fri, Aug 19, 2011 at 1:03 PM, Mike Frysinger <vap...@gentoo.org> wrote: > On Friday, August 19, 2011 14:19:42 Simon Glass wrote: >> +static const char *write_block(struct spi_flash *flash, u32 offset, >> + size_t len, const char *buf, char *cmp_buf, size_t *skipped) > > "spi_flash_update_block" is probably a better name. after all, you're doing > more (and sometimes less) than "writing a block". > >> + return"read"; > > need a space in the middle there
Done > >> + printf("SPI flash %s failed\n", err_oper); > > how about: > SPI flash update failed in %s step > running 'sf update' on the command line would be obvious where the problem is > coming from, but if you have a bunch of sf commands in a script and only see > one error, it's probably confusing to quickly pick out where the error > occurred. Done > >> - 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); >> ... >> - 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); > > these duplicate strcmp's make me wonder if a follow up patch should turn these > into a tokenize step first and then later code works off that Yes I agree. Still the sf code is quite a bit cleaner than March. > > rest looks fine > -mike > Thanks, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot