[U-Boot] [PATCH v4] cmd_sf: add "update" subcommand to do smart SPI flash update

[Simon Glass]

On Sun, Aug 21, 2011 at 9:35 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Sunday, August 21, 2011 06:37:30 Simon Glass wrote:
>> On Sat, Aug 20, 2011 at 5:04 PM, Marek Vasut wrote:
>> > On Sunday, August 21, 2011 12:35:51 AM Mike Frysinger wrote:
>> >> +{
>> >> +     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";
>> >
>> > Numeric value won't be ok ? You can have these in the calling function
>> > instead of returning a char *.
>>
>> Yes it's a bit odd, but the alternative is quite a bit more verbose:
>>
>> enum {
>>    OPER_MALLOC,
>>    OPER_READ,
>>    OPER_ERASE,
>>    ...
>> };
>>
>> static const char *names[OPER...] = {
>
> static const char * const names[] = {
>
>>    "malloc",
>>    "read",
>>    "erase"
>> ...
>> };
>>
>> Is that better?
>
> only if the code size is smaller ;)

Hi Mike,

Well it is 4 bytes larger, or 8 if I avoid char * for the array and
fix the string length. So I will leave that as is for now, and send a
new v5 patch.

Regards,
Simon

> -mike
>