From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steve Sakoman <steve@sakoman.com>
Date: Sat, 04 Dec 2010 06:48:46 -0800
Subject: [U-Boot] [RFC 2/3] tools/env: Support writing to files
In-Reply-To: <4CFA24A8.7030709@denx.de>
References: <1291436933-26861-1-git-send-email-steve@sakoman.com>
	<1291436933-26861-3-git-send-email-steve@sakoman.com>
	<4CFA24A8.7030709@denx.de>
Message-ID: <1291474126.23352.31.camel@quadra>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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