[PATCH] cmd: sf: Support unaligned flash updates with 'sf update'

[Michael Walle]

Am 2021-09-30 19:17, schrieb Frieder Schrempf:
> On 30.09.21 18:35, Michael Walle wrote:
>> Am 2021-09-30 18:19, schrieb Frieder Schrempf:
>>> In order to support unaligned updates, we simply read the first full
>>> block and check only the requested part of the block for changes. If
>>> necessary, the block is erased, the first (unchanged) part of the 
>>> block
>>> is written back together with the second part of the block (updated
>>> data).
>> 
>> I'm not sure what I should think of this. You might loose that 
>> unchanged
>> part if there is an power loss in the middle, even worse, you likely 
>> don't
>> have this part anymore because it isn't part of the data you want to 
>> write
>> to the flash.
>> 
>> Maybe add an parameter for allow (unsafe) unaligned updates?
>> 
>> Now you might argue, that with "sf erase, sf write" you can do the 
>> same
>> harm, to which i reply: but then it is intentional ;) Because "sf 
>> erase"
>> just works on sector boundaries (which isn't really checked in the 
>> command,
>> i just realized, but at least my driver returns EINVAL) and then the
>> user has to include the "unchanged part" for the arguments on the
>> commandline.
> 
> True, but "sf update" already is "unsafe" even without supporting
> unaligned start offsets. The code already handles partial sector writes
> for the last sector in the same way (read, erase, write), which is also
> prone to data loss in case of power loss.

Ah, I missed that. Yes, in this case, we don't loose anything. Agreed.

> So this patch in fact just adds support for partial sector updates for
> the first sector. It is slightly more probable to loose data, but it
> doesn't introduce new "unsafe" behavior.
> 
> Maybe we could cover this by adding a warning to the documentation, or
> even printing a warning?

Heh, although I was using "sf update" all the time, I wasn't aware of
the read - "partly modify" - write cycle. Maybe it's just me being
over cautious.

Printing a warning might scare users, though. I'd prefer to fix the
online help, where only "erase and write" is mentioned.

-michael