Fcb erase value support #28839
Fcb erase value support #28839
Conversation
de-nordic
requested review from
nashif,
nvlsianpu and
wentongwu
as code owners
github-actions
bot
added
area: API
|
@nvlsianpu please review |
|
sure |
|
@erwango This might be used now on some STM32L1 SoCs which has 0x00 as flash erased state. |
I was gonna try it a few weeks ago but seems like the stm32 flash driver does not support L1 as of yet. |
Thanks @utzig ! |
|
Thanks for pinging me. Though, I don't have ways to test this right now and I'm not sure my review has much added value. |
|
@erwango ^^ PR is no related to where erase-value property is defined. |
subsys/fs/fcb/fcb.c
Outdated
| #include <drivers/flash.h> | ||
|
|
||
| #define MK32(val) ((((uint32_t)val) << 24) | ((val) << 16) | \ | ||
| ((val) << 8) | (val)) |
There was a problem hiding this comment.
This would not work correctly if a uint8_t is passed in because you need to also cast << 16 and << 8. Also it would be better to cast the the MSB with (((uint32_t)(val)) << 24) (one extra set of parenthesis around val. But in the end this would be better being just an inline function so no casts are required.
subsys/fs/fcb/fcb_priv.h
Outdated
| { | ||
| const uint8_t ev = fcb->f_erase_value; | ||
|
|
||
| return (fcb->f_magic ^ ~((ev << 24) | (ev << 16) | (ev << 8) | ev)); |
There was a problem hiding this comment.
If MK32 was declared in this file you could use it with ev here.
subsys/fs/fcb/fcb.c
Outdated
| return 1; | ||
| } else if (len < FCB_MAX_LEN) { | ||
| buf[0] = (len & 0x7f) | 0x80; | ||
| buf[1] = len >> 7; | ||
| buf[0] = ((len & 0x7f) | 0x80) ^ ~fcb->f_erase_value; |
There was a problem hiding this comment.
This is a nitpick, but can ((len & 0x7f) | 0x80) ever result in something that is not equal to just (len | 0x80) when you're storing it in a uint8_t?
de-nordic
force-pushed
the
fcb-erase-value-support
branch
from
d882fb3
to
52e9594
Compare
|
cc @carlescufi This is ready. |
|
@de-nordic can you rebase this? seems like the doc build error was solved in master |
de-nordic
force-pushed
the
fcb-erase-value-support
branch
from
7e77c90
to
976b232
Compare
Overlays that enable FCB tests on qemu_x86 plaftorm have been added. Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
The FCB has been strongly tied to 0xff erased flash devices and this commit adds support for other erase values. Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
Fcb tests have been modified to be able to work on non-0xff erase value flash devices. Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
de-nordic
force-pushed
the
fcb-erase-value-support
branch
from
976b232
to
1b5cfc0
Compare
Adds support to non-0xff erasable flash devices to FCB.
Resolves #28693