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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
