[U-Boot] [PATCH] fw_env: Make env flash lock optional

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <dirk.eibach@gdsys.cc>

Since locking flash is horribly broken on some platforms,
offer an option to build fw_env tools without locking.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>

---

 tools/env/fw_env.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 06cf63d..e47232a 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -908,8 +908,10 @@ static int flash_write_buf (int dev, int fd, void *buf, size_t count,
 			return -1;
 		}
 
+#ifndef NO_MEMLOCK
 		if (mtd_type != MTD_ABSENT)
 			ioctl(fd, MEMLOCK, &erase);
+#endif
 
 		processed  += erasesize;
 		block_seek = 0;
@@ -941,7 +943,9 @@ static int flash_flag_obsolete (int dev, int fd, off_t offset)
 	}
 	ioctl (fd, MEMUNLOCK, &erase);
 	rc = write (fd, &obsolete_flag, sizeof (obsolete_flag));
+#ifndef NO_MEMLOCK
 	ioctl (fd, MEMLOCK, &erase);
+#endif
 	if (rc < 0)
 		perror ("Could not set obsolete flag");
 
-- 
2.1.3

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Anatolij Gustschin]

On Thu,  2 Jun 2016 09:09:14 +0200
dirk.eibach at gdsys.cc dirk.eibach at gdsys.cc wrote:
...
> @@ -941,7 +943,9 @@ static int flash_flag_obsolete (int dev, int fd, off_t offset)
>  	}
>  	ioctl (fd, MEMUNLOCK, &erase);

when MEMLOCK ist broken on a platform and disabled by your patch,
shouldn't MEMUNLOCK be isolated as well?

--
Anatolij

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Dirk Eibach]

Hi Anatolij,

2016-06-02 10:14 GMT+02:00 Anatolij Gustschin <agust@denx.de>:
...
> when MEMLOCK ist broken on a platform and disabled by your patch,
> shouldn't MEMUNLOCK be isolated as well?

There are  cornercases where lock/unlock works properly in the kernel
but not in u-boot. So we might have an environment that was locked by
u-boot and are able to unlock it with fw_env.

Cheers
Dirk

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Andreas Fenkart]

Hi,

I faced the same problem a while back, thanks for taking a look at this.
Does it have to be compiled in? Why not make it a command line option,
evtl. making the default a compile time option.

/Andi

2016-06-02 10:27 GMT+02:00 Dirk Eibach <dirk.eibach@gdsys.cc>:
> Hi Anatolij,
>
> 2016-06-02 10:14 GMT+02:00 Anatolij Gustschin <agust@denx.de>:
> ...
>> when MEMLOCK ist broken on a platform and disabled by your patch,
>> shouldn't MEMUNLOCK be isolated as well?
>
> There are  cornercases where lock/unlock works properly in the kernel
> but not in u-boot. So we might have an environment that was locked by
> u-boot and are able to unlock it with fw_env.
>
> Cheers
> Dirk

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Wolfgang Denk]

Dear Dirk,

In message <CANVMifJG+MdmMSjBpciuk0xcfdhMDywDkg9WZ4Skrez0ccFwYw@mail.gmail.com> you wrote:
> 
> There are  cornercases where lock/unlock works properly in the kernel
> but not in u-boot. So we might have an environment that was locked by
> u-boot and are able to unlock it with fw_env.

But then such a "fix" looks wrong to me.  If it works in Linux, why
not fix the code so it works in U-Boot, too?  I mean, if you have a
working example, that should be possible, or not?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
That said, there may be good reasons for what you did beyond obsequi-
ous sycophantic parody. Perhaps you might be so kind as to elucidate.
         -- Tom Christiansen in <5ldjbm$jtk$1@csnews.cs.colorado.edu>

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Dirk Eibach]

Hi Wolfgang,

2016-06-02 15:36 GMT+02:00 Wolfgang Denk <wd@denx.de>:
> ...
>> There are  cornercases where lock/unlock works properly in the kernel
>> but not in u-boot. So we might have an environment that was locked by
>> u-boot and are able to unlock it with fw_env.
>
> But then such a "fix" looks wrong to me.  If it works in Linux, why
> not fix the code so it works in U-Boot, too?  I mean, if you have a
> working example, that should be possible, or not?
> ...

Sometimes I simply need a tool to repair a device in the field where
updating u-boot or kernel is not an option.
But uploading modified fw_env tools is.

Cheers
Dirk

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Tom Rini]

On Thu, Jun 02, 2016 at 08:57:05PM +0200, Dirk Eibach wrote:
> Hi Wolfgang,
> 
> 2016-06-02 15:36 GMT+02:00 Wolfgang Denk <wd@denx.de>:
> > ...
> >> There are  cornercases where lock/unlock works properly in the kernel
> >> but not in u-boot. So we might have an environment that was locked by
> >> u-boot and are able to unlock it with fw_env.
> >
> > But then such a "fix" looks wrong to me.  If it works in Linux, why
> > not fix the code so it works in U-Boot, too?  I mean, if you have a
> > working example, that should be possible, or not?
> 
> Sometimes I simply need a tool to repair a device in the field where
> updating u-boot or kernel is not an option.
> But uploading modified fw_env tools is.

So in sum, things are already in a bad state and this is the only way to
correct things?  Can you please make this a cli option instead with
appropriately scary help/man text?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160602/e7e0e07f/attachment.sig>

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Wolfgang Denk]

Dear Dirk,

In message <CANVMifL_VKFX-G9Xsm7yJ-Bu6YrUPOqFbpbSus+f3OL184A0-w@mail.gmail.com> you wrote:
> 
> > But then such a "fix" looks wrong to me.  If it works in Linux, why
> > not fix the code so it works in U-Boot, too?  I mean, if you have a
> > working example, that should be possible, or not?
> 
> Sometimes I simply need a tool to repair a device in the field where
> updating u-boot or kernel is not an option.
> But uploading modified fw_env tools is.

Understood.

Am I interpreting this correctly, that the root cause is fixed in
recent versions of U-Boot, and you need it only to fix old, broken
versions?

Or is the problem still present in current mainline, too?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the realm of scientific observation, luck is granted only to those
who are prepared.                                     - Louis Pasteur

[U-Boot] [PATCH] fw_env: Make env flash lock optional

[Fabio Estevam]

Hi Dirk,

On Thu, Jun 2, 2016 at 4:09 AM,  <dirk.eibach@gdsys.cc> wrote:
> From: Dirk Eibach <dirk.eibach@gdsys.cc>
>
> Since locking flash is horribly broken on some platforms,
> offer an option to build fw_env tools without locking.

What type of flash are you referring to: NAND, SPI NOR, parallel NOR?