* [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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2020-07-31 21:40 UTC | newest]
Thread overview: 5+ 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
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.