+ Andrzej
On 30/07/2018, 18:45, "will sanfilippo" <wi...@runtime.io> wrote:
I guess you could also use some error correction/detection coding scheme as
well if you wanted to try and actually recover data that had errors in it (but
that seems a bit much). Another thing I have seen done is that just the header
has FEC on it. This way, you can almost always get the proper length. Again,
not sure if any of this is warranted.
Will
> On Jul 30, 2018, at 6:44 AM, marko kiiskila <ma...@runtime.io> wrote:
>
> Hi,
>
>> On Jul 30, 2018, at 1:47 PM, Laczen JMS <laczen...@gmail.com> wrote:
>>
>> Hi Marko and Will,
>>
>> I have been studying fcb and I think you can leave it at 8 bit as it
>> was. The crc can only be interpreted as a check that
>> the closing was done properly. As soon as the crc check fails this
>> only means that the closing failed. It could just as well
>> be fixed to zero instead of a crc.
>
> That is a valid point. If you can’t trust your data path to flash, what
can you trust?
>
>>
>> When writing errors (bit errors) would occur fcb would need a lot more
>> protection, the length which is written first could
>> no longer be trusted and it would be impossible to find the crc. The
>> writing of the length can also be a more problematic
>> case to solve, what happens when the write of the length fails and the
>> length that is read is pointing outside the sector ?
>
> On the other hand, there are flashes where multiple write cycles to
> same location are allowed; you can keep turning more bits to zero.
> There you can corrupt the data after writing. And on some the default,
> unwritten state is 0 (this we need to address for FCB, image management).
>
> You’re right; adding mechanisms to detect corruption of length field, for
> example, starts to get complicated and costly. Recovery is easier to do,
> however, if we use a stronger version of CRC. I.e. if CRC does not match,
> then just go forward a byte at a time until it does.
>
>
>>
>> Kind regards,
>>
>> Jehudi
>>
>>
>> Op ma 30 jul. 2018 om 11:10 schreef marko kiiskila <ma...@runtime.io>:
>>>
>>> Thanks for the response, Will.
>>>
>>> I made it one-byte because the scenario where the CRC check strength
comes into play is somewhat rare;
>>> it is to detect partial writes. I.e. when fcb_append_finish() fails to
get called. This, presumably, would
>>> onlly happen on crash/power outage within a specific time window. This
is not used as an error detection
>>> mechanism on a channel where we expect bit errors.
>>>
>>> The way I did it was I added 2 syscfg knobs to control which CRC is
included in the build. In case you get
>>> really tight on code space. Of course, newtmgr uses CRC16, so if you
have that enabled,
>>> there is no gain.
>>> There’s 3 different options when starting FCB: inherit from flash,
force 16 bit, or force 8 bits. If the flash region
>>> has not been initialized with anything, then the ‘inherit’ option will
prefer 16 bits over 8 bits.
>>> So if you need to worry about backwards compatibility, set ‘inherit’.
If you want to use 16 bit, and are flashing
>>> a new device, use ‘inherit’ or set ‘force 16bits’. If you need to keep
FCB region backwards compatible, use
>>> option ‘force 8 bits’.
>>>
>>>
>>>> On Jul 27, 2018, at 11:05 PM, will sanfilippo <wi...@runtime.io> wrote:
>>>>
>>>> I realize I am a bit late with these comments but better late than
never I guess. Well, maybe some or all of these comments should never be made
:-) This could be a horrible idea but why not just make it backward
incompatible? (for #2). Maybe have a syscfg for folks that want to use the old
format in the code? I would even consider making all entries have a two-byte
CRC (regardless of length of data written). I presume the resume that it was
one-byte was to save flash space? Heck, maybe even allow the syscfg to specify
a 32-bit CRC (for those with lots of flash and/or really do not want a false
positive to occur).
>>>>
>>>> Will
>>>>> On Jul 26, 2018, at 5:10 AM, marko kiiskila <ma...@runtime.io> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> there’s few issues with FCB which I need to fix.
>>>>>
>>>>> 1) Compressing flash sector in the middle of write.
>>>>> Currently the cycle of operation is expected to be the following:
>>>>> fcb_append() -> get offset to write data to
>>>>> flash_area_write() -> write data
>>>>> fcb_append_finish() -> writes CRC etc
>>>>>
>>>>> This is not too great if the location returned by fcb_append() gets
rotated to be
>>>>> scratch area before fcb_append_finish() is called. Which is quite
possible
>>>>> if you’re using FCB to store logs, and data comes from different
tasks.
>>>>>
>>>>> 2) CRC is 1 byte only. It’s ok for short config entries, but for
longer things
>>>>> (log entries) is not good enough.
>>>>>
>>>>> To fix 1 I was going to add linkage to fcb_entry such that FCB can
track
>>>>> of incomplete writes, and add a write routine which should be used
when
>>>>> doing the write.
>>>>>
>>>>> Issue 2 is a little bit more cumbersome. I want to be able to at least
>>>>> allow backwards compatibility, where code figures out from data in
>>>>> the flash which CRC format is there. But then be able to switch to
>>>>> 16 bit CRC format as new flash areas are written. Or keep using
>>>>> the old format, because you want to do downgrade to old SW version
>>>>> if necessary.
>>>>>
>>>>> So that’s what I’m currently thinking of trying to achieve. Let me
know
>>>>> if you guys have other things to take into consideration here.
>>>>
>>>
>
