
On Thu, 2013-01-31 at 15:10 +0100, Wolfgang Denk wrote:
Dear Lubomir Rintel,
In message 1359630144.16475.6.camel@hobbes you wrote:
Mine is a Kobo Mini e-book reader, which is basically Freescale MX50 with the only storage there being an MMC/SD card (removable from a slot if disassembled), which contains uboot and its environment as well as partition table, root and storage volume.
OK - but you do not comment on my remarks about the actual code?
On Thu, 2013-01-31 at 07:48 +0100, Wolfgang Denk wrote:
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 37b60b8..0d8052d 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -838,7 +838,7 @@ static int flash_write_buf (int dev, int fd, void *buf, size_t count, ioctl (fd, MEMUNLOCK, &erase);
/* Dataflash does not need an explicit erase cycle */
if (mtd_type != MTD_DATAFLASH)
if (mtd_type && mtd_type != MTD_DATAFLASH)
This change appears to be redundant. If mtd_type is null, then this is already caught iun te test mtd_type != MTD_DATAFLASH, isn't it?
No. We don't want the erase ioctl to be called for non-MTD devices and files (where mtd_type is null).
- rc = ioctl (fd, MEMGETINFO, &mtdinfo);
- rc = fstat (fd, &st); if (rc < 0) {
perror ("Cannot get MTD information");
perror ("Cannot access MTD device");
I don't understand this. You talk about a MTD device here, but expect that MEMGETINFO will not work on it? Please explain in which exact circumstances such a situation wouldhappen.
The error message (mention of MTD at that point) is incorrect. fstat() is called to determine whether the device is a character device (such as MTD devices -- MEMGETINFO call will follow) or not (it might be a blockdevice or flat file).
if (mtdinfo.type != MTD_NORFLASH && mtdinfo.type != MTD_NANDFLASH &&
mtdinfo.type != MTD_DATAFLASH) {
mtdinfo.type != MTD_DATAFLASH &&
mtdinfo.type) {
Again, this last line appears to be redundant.
The same response again -- if the type is nul, then the device is not a flash device at all.
}
DEVTYPE(dev_current) = mtdinfo.type;
- rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE,
Please don't make such unrelated white space changes in this commit.
That was a mistake.
Have a nice day! -- Lubo