[U-Boot] [PATCH 2/3 v3] sandbox: new SPI flash driver

[Mike Frysinger]

On Thursday 15 March 2012 00:09:59 Simon Glass wrote:
> On Mon, Mar 12, 2012 at 8:22 AM, Mike Frysinger wrote:
> > +/* Used to quickly bulk erase backing store */
> > +static u8 sb_sf_0xff[0x10000];
> 
> Ick, Does it really need to be so large?

in order to do a single write() for a single sector, yes.  it also simplifies 
the code as i don't have to loop over some smaller size and keep track of how 
many bytes i've written until a sector is cleared.

this is in the bss and it's 64KiB.  i'm really not worried about your 
development system running Chrome getting low on RAM :).

> > +       sbsf->fd = os_open(file, 02);
> > +       if (sbsf->fd == -1) {
> > +               free(sbsf);
> > +               goto error;
> 
> Prints incorrect error if the file couldn't be found/open. I wonder if
> we should have os_perror() ?

probably would be useful.  we prob have to document that while os_perror() 
would work, strerror(errno) would not.  the os_* funcs update the errno that 
the C library knows about (or *__errno_location() in glibc), but u-boot 
declares a local "errno" variable.  so attempting to reference "errno" in u-
boot code will always resolve to the local definition rather than the glibc 
one.  os_perror() would work because it calls __errno_location() internally 
just like the other os_* funcs.

> > +                       pos += os_read(sbsf->fd, tx + pos, cnt);
> 
> This can fail (return -1)

the ways in which read/write could fail aren't really possible for us, but i 
guess it won't hurt to assert() the values.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120315/b8c7fb73/attachment.pgp>