* [PATCH 0/1] fw_setenv always locks flash whenever it tries to write @ 2020-07-10 16:54 Ivan Mikhaylov 2020-07-10 16:54 ` [PATCH 1/1] fw_setenv: lock the flash only if it was locked before Ivan Mikhaylov 2020-07-23 22:20 ` [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov 0 siblings, 2 replies; 6+ messages in thread From: Ivan Mikhaylov @ 2020-07-10 16:54 UTC (permalink / raw) To: u-boot fw_setenv usage always locks u-boot-env mtd device without questions. Locking of flash can be disruptive and may affect not only u-boot-env region due to different problems with chips and lock callbacks on kernel side. I'm not sure if my fix is right, but also I thought about possible option in fw_env config about locking/unlocking or even option into fw_setenv/printenv. Any ideas? Ivan Mikhaylov (1): fw_setenv: lock the flash only if it was locked before tools/env/fw_env.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] fw_setenv: lock the flash only if it was locked before 2020-07-10 16:54 [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov @ 2020-07-10 16:54 ` Ivan Mikhaylov 2020-07-31 21:40 ` Tom Rini 2020-07-23 22:20 ` [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov 1 sibling, 1 reply; 6+ messages in thread From: Ivan Mikhaylov @ 2020-07-10 16:54 UTC (permalink / raw) To: u-boot u-boot-env flash region lock/unlock may affect other regions than u-boot-env, fw_setenv does this in a way 'Lock it in any cases'. Change this to 'lock it if it was locked before'. Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> --- tools/env/fw_env.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 381739d28d..3c01f52f3b 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -989,6 +989,7 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) of the data */ loff_t blockstart; /* running start of the current block - MEMGETBADBLOCK needs 64 bits */ + int was_locked; /* flash lock flag */ int rc; /* @@ -1074,6 +1075,12 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) } erase.length = erasesize; + if (DEVTYPE(dev) != MTD_ABSENT) { + was_locked = ioctl(fd, MEMISLOCKED, &erase); + /* treat any errors as unlocked flash */ + if (was_locked < 0) + was_locked = 0; + } /* This only runs once on NOR flash and SPI-dataflash */ while (processed < write_total) { @@ -1093,7 +1100,8 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) if (DEVTYPE(dev) != MTD_ABSENT) { erase.start = blockstart; - ioctl(fd, MEMUNLOCK, &erase); + if (was_locked) + ioctl(fd, MEMUNLOCK, &erase); /* These do not need an explicit erase cycle */ if (DEVTYPE(dev) != MTD_DATAFLASH) if (ioctl(fd, MEMERASE, &erase) != 0) { @@ -1121,8 +1129,10 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) return -1; } - if (DEVTYPE(dev) != MTD_ABSENT) - ioctl(fd, MEMLOCK, &erase); + if (DEVTYPE(dev) != MTD_ABSENT) { + if (was_locked) + ioctl(fd, MEMLOCK, &erase); + } processed += erasesize; block_seek = 0; @@ -1143,7 +1153,9 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset) int rc; struct erase_info_user erase; char tmp = ENV_REDUND_OBSOLETE; + int was_locked; /* flash lock flag */ + was_locked = ioctl(fd, MEMISLOCKED, &erase); erase.start = DEVOFFSET(dev); erase.length = DEVESIZE(dev); /* This relies on the fact, that ENV_REDUND_OBSOLETE == 0 */ @@ -1153,9 +1165,11 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset) DEVNAME(dev)); return rc; } - ioctl(fd, MEMUNLOCK, &erase); + if (was_locked) + ioctl(fd, MEMUNLOCK, &erase); rc = write(fd, &tmp, sizeof(tmp)); - ioctl(fd, MEMLOCK, &erase); + if (was_locked) + ioctl(fd, MEMLOCK, &erase); if (rc < 0) perror("Could not set obsolete flag"); -- 2.21.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] fw_setenv: lock the flash only if it was locked before 2020-07-10 16:54 ` [PATCH 1/1] fw_setenv: lock the flash only if it was locked before Ivan Mikhaylov @ 2020-07-31 21:40 ` Tom Rini 0 siblings, 0 replies; 6+ messages in thread From: Tom Rini @ 2020-07-31 21:40 UTC (permalink / raw) To: u-boot On Fri, Jul 10, 2020 at 07:54:18PM +0300, Ivan Mikhaylov wrote: > u-boot-env flash region lock/unlock may affect other regions than > u-boot-env, fw_setenv does this in a way 'Lock it in any cases'. > Change this to 'lock it if it was locked before'. > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> With a much reworded commit message, applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200731/320dda67/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/1] fw_setenv always locks flash whenever it tries to write 2020-07-10 16:54 [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov 2020-07-10 16:54 ` [PATCH 1/1] fw_setenv: lock the flash only if it was locked before Ivan Mikhaylov @ 2020-07-23 22:20 ` Ivan Mikhaylov 2020-07-30 14:24 ` Tom Rini 1 sibling, 1 reply; 6+ messages in thread From: Ivan Mikhaylov @ 2020-07-23 22:20 UTC (permalink / raw) To: u-boot On Fri, 2020-07-10 at 19:54 +0300, Ivan Mikhaylov wrote: > fw_setenv usage always locks u-boot-env mtd device without questions. > Locking of flash can be disruptive and may affect not only u-boot-env > region due to different problems with chips and lock callbacks on > kernel > side. I'm not sure if my fix is right, but also I thought about > possible option > in fw_env config about locking/unlocking or even option into > fw_setenv/printenv. Any ideas? > > Ivan Mikhaylov (1): > fw_setenv: lock the flash only if it was locked before > > tools/env/fw_env.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > A little bit more description: With current implementation of fw_setenv, it is always locks u-boot-env region if lock interface is implemented for such mtd device. You can not control lock of this region with fw_setenv, there is no option for it in config or in application itself. Because of this situation may happen problems like in this thread on xilinx forum: https://forums.xilinx.com/t5/Embedded-Linux/Flash-be-locked-after-use-fw-setenv-from-user-space/td-p/1027851 Short desc link: some person has issue with some spi chip which has lock interface but doesn't locks properly which leads to lock of whole flash memory on lock of u-boot-env region. As resulted solution hack was added into spi-nor.c driver for this chip with lock disablement. And for solving such issue usually: 1. do manual unlock after each fw_setenv usage. 2. do hacks inside driver representing your device, as example in spi- nor.c linux driver. And, yes, in this case it is problem of two sides: 1. driver lock implementation for some chips. 2. fw_setenv's way of work which is not trying be convenient in this case and is not providing any option for not doing locks. I want to implement some mechanism which will help user do not lock flash when it's not needed. So there is some points how it can be: 1. lock this region if it's already locked. 2. add option into fw_setenv application. 3. add some option about locks into fw_env.config. Patch what I've, just dirty implementation of first one. Any ideas or suggestions? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/1] fw_setenv always locks flash whenever it tries to write 2020-07-23 22:20 ` [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov @ 2020-07-30 14:24 ` Tom Rini 0 siblings, 0 replies; 6+ messages in thread From: Tom Rini @ 2020-07-30 14:24 UTC (permalink / raw) To: u-boot On Thu, Jul 23, 2020 at 10:20:55PM +0000, Ivan Mikhaylov wrote: > On Fri, 2020-07-10 at 19:54 +0300, Ivan Mikhaylov wrote: > > fw_setenv usage always locks u-boot-env mtd device without questions. > > Locking of flash can be disruptive and may affect not only u-boot-env > > region due to different problems with chips and lock callbacks on > > kernel > > side. I'm not sure if my fix is right, but also I thought about > > possible option > > in fw_env config about locking/unlocking or even option into > > fw_setenv/printenv. Any ideas? > > > > Ivan Mikhaylov (1): > > fw_setenv: lock the flash only if it was locked before > > > > tools/env/fw_env.c | 24 +++++++++++++++++++----- > > 1 file changed, 19 insertions(+), 5 deletions(-) > > > > A little bit more description: > > With current implementation of fw_setenv, it is always locks u-boot-env > region if lock interface is implemented for such mtd device. You can > not control lock of this region with fw_setenv, there is no option for > it in config or in application itself. Because of this situation may > happen problems like in this thread on xilinx forum: > https://forums.xilinx.com/t5/Embedded-Linux/Flash-be-locked-after-use-fw-setenv-from-user-space/td-p/1027851 > > Short desc link: some person has issue with some spi chip which has > lock interface but doesn't locks properly which leads to lock of whole > flash memory on lock of u-boot-env region. As resulted solution hack > was added into spi-nor.c driver for this chip with lock disablement. > > And for solving such issue usually: > 1. do manual unlock after each fw_setenv usage. > 2. do hacks inside driver representing your device, as example in spi- > nor.c linux driver. > > And, yes, in this case it is problem of two sides: > 1. driver lock implementation for some chips. > 2. fw_setenv's way of work which is not trying be convenient in this > case and is not providing any option for not doing locks. > > I want to implement some mechanism which will help user do not lock > flash when it's not needed. So there is some points how it can be: > 1. lock this region if it's already locked. > 2. add option into fw_setenv application. > 3. add some option about locks into fw_env.config. > > Patch what I've, just dirty implementation of first one. > Any ideas or suggestions? Thanks. I've taken the above and updated the commit message more. I'm currently reviewing the patch with other env changes now. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200730/a8d62d61/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/1] fw_setenv always locks flash whenever it tries to write @ 2020-05-06 1:24 Ivan Mikhaylov 0 siblings, 0 replies; 6+ messages in thread From: Ivan Mikhaylov @ 2020-05-06 1:24 UTC (permalink / raw) To: u-boot From: Ivan Mikhaylov <i.mikhaylov@yadro.com> fw_setenv usage always locks u-boot-env mtd device without questions. Locking of flash can be disruptive and may affect not only u-boot-env region due to different problems with chips and lock callbacks on kernel side. I'm not sure if my fix is right, but also I thought about possible option in fw_env config about locking/unlocking or even option into fw_setenv/printenv. Any ideas? Ivan Mikhaylov (1): fw_setenv: lock the flash only if it's already locked before tools/env/fw_env.c | 24 +++++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-31 21:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-10 16:54 [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov 2020-07-10 16:54 ` [PATCH 1/1] fw_setenv: lock the flash only if it was locked before Ivan Mikhaylov 2020-07-31 21:40 ` Tom Rini 2020-07-23 22:20 ` [PATCH 0/1] fw_setenv always locks flash whenever it tries to write Ivan Mikhaylov 2020-07-30 14:24 ` Tom Rini -- strict thread matches above, loose matches on Subject: below -- 2020-05-06 1:24 Ivan Mikhaylov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.