Re: [U-Boot] [RFC 2/3] tools/env: Support writing to files
On Sat, Dec 04, 2010, Steve Sakoman wrote: > I don't have a big endian target for testing, but perhaps Loïc has > access to one. Otherwise we'll come back to the list with a request for > testing help. (I don't have access to one either) -- Loïc Minier ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 2/3] tools/env: Support writing to files
On Sat, 2010-12-04 at 12:23 +0100, Stefano Babic wrote:
> On 12/04/2010 05:28 AM, Steve Sakoman wrote:
> > -static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart)
> > +static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart,
> > + int is_mtd)
>
> You add an additional parameter to the flash function to check if it is
> a real mtd, but we have already a structure to store which type of flash
> we have. We could use envdevices[].mtd_type to store that we want to
> write into a file and not into a mtd device. There is already a
> MTD_ABSENT constant that we could reuse.
>
> > {
> > + if (!is_mtd)
> > + return 0;
> > +
>
> Because this function does nothing with the parameter, it should be
> better to check its value in the caller without calling this function.
>
> > /* This only runs once on NOR flash */
> > while (processed < count) {
> > - rc = flash_bad_block (fd, mtd_type, &blockstart);
> > + rc = flash_bad_block (fd, mtd_type, &blockstart, is_mtd);
>
> Ditto.
>
> > rc = flash_read_buf (dev, fd, data, write_total, erase_offset,
> > -mtd_type);
> > +mtd_type, is_mtd);
>
> See my first comment. mtd_type can already tell us if we want to write
> into a file or into a real mtd device, without adding an additional
> parameter.
>
> I have an additional question: if we want to add support to prepare the
> environment on the host and to transfer later on the target, should we
> not to care about the endianess ? I think the crc is written without
> checking the endianess of the target. Does it work for powerpc (usually
> big endian) when we run on a host PC ?
Good points! I'll work with Loïc to revise the code and get a new
version submitted.
I don't have a big endian target for testing, but perhaps Loïc has
access to one. Otherwise we'll come back to the list with a request for
testing help.
Steve
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 2/3] tools/env: Support writing to files
On Sat, 2010-12-04 at 05:37 -0500, Mike Frysinger wrote:
> On Friday, December 03, 2010 23:28:52 Steve Sakoman wrote:
> > - ioctl (fd, MEMLOCK, &erase);
> > + if (is_mtd) {
> > + ioctl (fd, MEMLOCK, &erase);
> > + }
>
> useless braces
Good catch, I will correct in the next revision.
Steve
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 2/3] tools/env: Support writing to files
On 12/04/2010 05:28 AM, Steve Sakoman wrote:
> -static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart)
> +static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart,
> + int is_mtd)
You add an additional parameter to the flash function to check if it is
a real mtd, but we have already a structure to store which type of flash
we have. We could use envdevices[].mtd_type to store that we want to
write into a file and not into a mtd device. There is already a
MTD_ABSENT constant that we could reuse.
> {
> + if (!is_mtd)
> + return 0;
> +
Because this function does nothing with the parameter, it should be
better to check its value in the caller without calling this function.
> /* This only runs once on NOR flash */
> while (processed < count) {
> - rc = flash_bad_block (fd, mtd_type, &blockstart);
> + rc = flash_bad_block (fd, mtd_type, &blockstart, is_mtd);
Ditto.
> rc = flash_read_buf (dev, fd, data, write_total, erase_offset,
> - mtd_type);
> + mtd_type, is_mtd);
See my first comment. mtd_type can already tell us if we want to write
into a file or into a real mtd device, without adding an additional
parameter.
I have an additional question: if we want to add support to prepare the
environment on the host and to transfer later on the target, should we
not to care about the endianess ? I think the crc is written without
checking the endianess of the target. Does it work for powerpc (usually
big endian) when we run on a host PC ?
Best regards,
Stefano Babic
--
=
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 2/3] tools/env: Support writing to files
On Friday, December 03, 2010 23:28:52 Steve Sakoman wrote:
> - ioctl (fd, MEMLOCK, &erase);
> + if (is_mtd) {
> + ioctl (fd, MEMLOCK, &erase);
> + }
useless braces
-mike
signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC 2/3] tools/env: Support writing to files
From: Loïc Minier
Track st_mode and use it to skip ioctls on file-backed fds. This allows
writing to regular files transparently.
Signed-off-by: Loïc Minier
Tested-by: Steve Sakoman
---
tools/env/fw_env.c | 92 ++-
1 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 75f6a98..d2f9748 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -60,6 +60,7 @@ struct envdev_s {
ulong erase_size; /* device erase size */
ulong env_sectors; /* number of environment sectors */
uint8_t mtd_type; /* type of the MTD device */
+ mode_t st_mode; /* protection / file type */
};
static struct envdev_s envdevices[2] =
@@ -78,6 +79,7 @@ static int dev_current;
#define DEVESIZE(i) envdevices[(i)].erase_size
#define ENVSECTORS(i) envdevices[(i)].env_sectors
#define DEVTYPE(i)envdevices[(i)].mtd_type
+#define STMODE(i) envdevices[(i)].st_mode
#define CONFIG_ENV_SIZE ENVSIZE(dev_current)
@@ -633,8 +635,12 @@ int fw_parse_script(char *fname)
* > 0 - block is bad
* < 0 - failed to test
*/
-static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart)
+static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart,
+ int is_mtd)
{
+ if (!is_mtd)
+ return 0;
+
if (mtd_type == MTD_NANDFLASH) {
int badblock = ioctl (fd, MEMGETBADBLOCK, blockstart);
@@ -661,7 +667,7 @@ static int flash_bad_block (int fd, uint8_t mtd_type,
loff_t *blockstart)
* the DEVOFFSET (dev) block. On NOR the loop is only run once.
*/
static int flash_read_buf (int dev, int fd, void *buf, size_t count,
- off_t offset, uint8_t mtd_type)
+ off_t offset, uint8_t mtd_type, int is_mtd)
{
size_t blocklen;/* erase / write length - one block on NAND,
0 on NOR */
@@ -683,7 +689,7 @@ static int flash_read_buf (int dev, int fd, void *buf,
size_t count,
/* Offset inside a block */
block_seek = offset - blockstart;
- if (mtd_type == MTD_NANDFLASH) {
+ if (!is_mtd || mtd_type == MTD_NANDFLASH) {
/*
* NAND: calculate which blocks we are reading. We have
* to read one block at a time to skip bad blocks.
@@ -707,7 +713,7 @@ static int flash_read_buf (int dev, int fd, void *buf,
size_t count,
/* This only runs once on NOR flash */
while (processed < count) {
- rc = flash_bad_block (fd, mtd_type, &blockstart);
+ rc = flash_bad_block (fd, mtd_type, &blockstart, is_mtd);
if (rc < 0) /* block test failed */
return -1;
@@ -754,7 +760,7 @@ static int flash_read_buf (int dev, int fd, void *buf,
size_t count,
* the whole data at once.
*/
static int flash_write_buf (int dev, int fd, void *buf, size_t count,
- off_t offset, uint8_t mtd_type)
+ off_t offset, uint8_t mtd_type, int is_mtd)
{
void *data;
struct erase_info_user erase;
@@ -812,7 +818,7 @@ static int flash_write_buf (int dev, int fd, void *buf,
size_t count,
}
rc = flash_read_buf (dev, fd, data, write_total, erase_offset,
-mtd_type);
+mtd_type, is_mtd);
if (write_total != rc)
return -1;
@@ -826,7 +832,7 @@ static int flash_write_buf (int dev, int fd, void *buf,
size_t count,
data = buf;
}
- if (mtd_type == MTD_NANDFLASH) {
+ if (!is_mtd || mtd_type == MTD_NANDFLASH) {
/*
* NAND: calculate which blocks we are writing. We have
* to write one block at a time to skip bad blocks.
@@ -840,7 +846,7 @@ static int flash_write_buf (int dev, int fd, void *buf,
size_t count,
/* This only runs once on NOR flash */
while (processed < write_total) {
- rc = flash_bad_block (fd, mtd_type, &blockstart);
+ rc = flash_bad_block (fd, mtd_type, &blockstart, is_mtd);
if (rc < 0) /* block test failed */
return rc;
@@ -854,14 +860,16 @@ static int flash_write_buf (int dev, int fd, void *buf,
size_t count,
continue;
}
- erase.start = blockstart;
- ioctl (fd, MEMUNLOCK, &erase);
+ if (is_mtd) {
+ erase.start = blockstart;
+ ioctl (fd, MEMUNLOCK, &erase);
- if (ioctl (fd, MEMERASE, &erase) != 0) {
- fprintf (stderr, "MTD erase error on %s: %s\n",
-DEVN
