Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0808271748520.6718@axis700.grange you wrote:
--- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -44,6 +44,12 @@ #define CMD_GETENV "fw_printenv" #define CMD_SETENV "fw_setenv"
+#define min(x, y) ({ \
- typeof(x) _min1 = (x); \
- typeof(y) _min2 = (y); \
- (void) (&_min1 == &_min2); \
What does this do?
- _min1 < _min2 ? _min1 : _min2; })
typedef struct envdev_s { char devname[16]; /* Device name */ ulong devoff; /* Device offset */ @@ -413,179 +419,290 @@ int fw_setenv (int argc, char *argv[]) return 0; }
+static int flash_bad_block (int dev, int fd, struct mtd_info_user *mtdinfo,
loff_t *blockstart, size_t blocklen)+{
- if (mtdinfo->type == MTD_NANDFLASH) {
int badblock = ioctl (fd, MEMGETBADBLOCK, blockstart);if (badblock < 0) {perror ("Cannot read bad block mark");
It would be probably helpful to print the block address.
return badblock;}if (badblock) {fprintf (stderr, "Bad block at 0x%llx, ""skipping\n", *blockstart);*blockstart += blocklen;return badblock;}- }
- return 0;
+}
+/*
- We are called with count == 0 for backing up as much data from the
- range as possible
- */
Backing up?
static int flash_read_buf (int dev, int fd, void *buf, size_t count,
off_t offset)
off_t offset, size_t range){
- struct mtd_info_user mtdinfo;
- size_t blocklen, processed = 0;
- size_t readlen = count ? : range;
- off_t erase_offset, block_seek;
- loff_t blockstart; int rc;
- int backup_mode = !count;
backup_mode ?
I think there should be an explanation what exactly you are trying to do.
- rc = lseek (fd, offset, SEEK_SET);
- if (rc == -1) {
fprintf (stderr,"seek error on %s: %s\n",DEVNAME (dev), strerror (errno));
- if (!count)
count = range;- rc = ioctl (fd, MEMGETINFO, &mtdinfo);
- if (rc < 0) {
perror ("Cannot get MTD information");
Did you verify that the code still builds when MTD_OLD is set?
- rc = read (fd, buf, count);
- if (rc != count) {
fprintf (stderr,"Read error on %s: %s\n",DEVNAME (dev), strerror (errno));return -1;
- /* Erase sector size is always a power of 2 */
- erase_offset = offset & ~(mtdinfo.erasesize - 1);
Please explain this logic.
- blockstart = erase_offset;
- /* Offset inside a block */
- block_seek = offset - erase_offset;
- if (mtdinfo.type == MTD_NANDFLASH) {
/** NAND: calculate which blocks we are reading. We have* to read one block at a time to skip bad blocks.*/blocklen = mtdinfo.erasesize;/* Limit to one block for the first read */if (readlen > blocklen - block_seek)readlen = blocklen - block_seek;- } else {
blocklen = 0;
- return rc;
- /* This only runs once for NOR flash */
- while (processed < count) {
rc = flash_bad_block (dev, fd, &mtdinfo, &blockstart, blocklen);
But - NOR flash does not have bad block, so all of this is not needed at all?
if (rc < 0)return -1;else if (blockstart + block_seek + readlen > offset + range) {
I do not understand what you are doing here. Comment?
/* End of range is reached */if (backup_mode) {return processed;} else {fprintf (stderr,"Too few good blocks within range\n");return -1;}} else if (rc)continue;/** If a block is bad, we retry in the next block* at the same offset - see common/env_nand.c::* writeenv()*/lseek (fd, blockstart + block_seek, SEEK_SET);
I don't see that you remember which blocks were bad. Does that mean that you will attemopt to write the environment to known bad blocks? Sonds not like a good idea to me.
rc = read (fd, buf + processed, readlen);if (rc != readlen) {fprintf (stderr,"Read error on %s: %s\n",DEVNAME (dev), strerror (errno));return -1;}processed += readlen;readlen = min(blocklen, count - processed);block_seek = 0;blockstart += blocklen;- }
- return processed;
}
-static int flash_write (void) +static int flash_write_buf (int dev, int fd, void *buf, size_t count,
off_t offset){
- int fd_current, fd_target, rc, dev_target;
- erase_info_t erase_current = {}, erase_target; char *data = NULL;
- off_t erase_offset;
- struct mtd_info_user mtdinfo_target;
- erase_info_t erase;
- struct mtd_info_user mtdinfo;
- size_t blocklen, erase_len, processed = 0;
- size_t writelen, write_total = DEVESIZE (dev);
- off_t erase_offset, block_seek;
- loff_t blockstart;
- int rc;
- /* dev_current: fd_current, erase_current */
- if ((fd_current = open (DEVNAME (dev_current), O_RDWR)) < 0) {
fprintf (stderr,"Can't open %s: %s\n",DEVNAME (dev_current), strerror (errno));
- rc = ioctl (fd, MEMGETINFO, &mtdinfo);
- if (rc < 0) {
perror ("Cannot get MTD information");
- if (HaveRedundEnv) {
/* switch to next partition for writing */dev_target = !dev_current;/* dev_target: fd_target, erase_target */if ((fd_target = open (DEVNAME (dev_target), O_RDWR)) < 0) {fprintf (stderr,"Can't open %s: %s\n",DEVNAME (dev_target),strerror (errno));return -1;}- } else {
dev_target = dev_current;fd_target = fd_current;- }
/* Erase sector size is always a power of 2 */
erase_offset = offset & ~(mtdinfo.erasesize - 1);
/* Maximum area we may use */
erase_len = (offset - erase_offset + DEVESIZE (dev) +
mtdinfo.erasesize - 1) & ~(mtdinfo.erasesize - 1);blockstart = erase_offset;
/* Offset inside a block */
block_seek = offset - erase_offset;
/*
- Support environment anywhere within erase sectors: read out the
- complete area to be erased, replace the environment image, write
- the whole block back again.
*/
- if (DEVESIZE (dev_target) > CFG_ENV_SIZE) {
data = malloc (DEVESIZE (dev_target));
- if (erase_len > DEVESIZE (dev)) {
data = malloc (erase_len);
"Cannot malloc %lu bytes: %s\n",DEVESIZE (dev_target),strerror (errno));
"Cannot malloc %u bytes: %s\n",erase_len, strerror (errno)); return -1;
rc = ioctl (fd_target, MEMGETINFO, &mtdinfo_target);if (rc < 0) {perror ("Cannot get MTD information");
/** This is different from a normal read. We have to read as much* as we can from a certain area, and it should be at least X* bytes, instead of having to read a fixed number of bytes as* usual. This also tells us how much data "fits" in the good* blocks in the area.*/write_total = flash_read_buf (dev, fd, data, 0,erase_offset, erase_len);if (write_total < block_seek + CFG_ENV_SIZE)
Ummm...this is flash_write_buf(), and we start reading data?
Please explain your code.
Best regards,
Wolfgang Denk