[U-Boot] [RFC 2/3] tools/env: Support writing to files

[Stefano Babic]

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: office at denx.de
=====================================================================