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
