[U-Boot] [PATCH] [RFC] SF: Add "sf erase offset +len" command handler.

[Mike Frysinger]

On Wednesday, February 09, 2011 16:16:12 Richard Retanubun wrote:
> From hints by Wolfgang, this patch adds the ability to handle +len
> argument for spi flash erase, which will round up the length to the
> nearest [sector|page|block]_size.

this should be split up into two patches.  one that unifies the erase sizes 
and one that modifies cmd_sf.c to use the new field.

ive already mostly unified the erase functions here:
	git://git.denx.de/u-boot-blackfin.git sf

but the one piece missing is what you're proposing.  so i'll want to merge the 
unification part you have here into that patch.  if you could test out that sf 
branch now to see if it works for you, that'd be nice ;).

> This is done by adding a new member to
> struct spi_flash::u32 block_size
> 
> The name 'block_size' is chosen to mean:
> "the smallest unit erase commands will check against".

let's use "sector_size" as this is what Linux already uses

> +static int sf_parse_len_arg(char *arg, ulong *len)

constify arg

> +
> +

one new line only

> +	if (arg && *arg == '+'){

NULL check is useless as the caller already took care of it

> +	if (round_up_len) {
> +		/* Get sector size from flash and round up */
> +		sector_size = 	flash->block_size;
> +		if (sector_size > 0) {
> +			*len = ((((len_arg -1)/sector_size) + 1) * sector_size);

we have a DIV_ROUND_UP macro already

> +			if (*len > flash->size) {
> +				return -1;
> +			}
> +		} else {
> +			return -1;
> +		}
> +	} else {
> +		*len = len_arg;
> +	}

pretty much all these braces can be punted

> @@ -149,6 +204,7 @@ static int do_spi_flash_erase(int argc, char * const
> argv[])
> 
>  usage:
>  	puts("Usage: sf erase offset len\n");
> +	puts("       sf erase offset +len\n");
>  	return 1;
>  }

hrm, a sep patch should be written and sent out before yours that drops this 
"usage" label and converts all "goto usage" to "return cmd_usage(cmdtp)".

> --- a/drivers/mtd/spi/macronix.c
> +++ b/drivers/mtd/spi/macronix.c

i'll ignore all the spi flash changes per my earlier highlight of rewrites 
pending in this area.

> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -38,6 +38,8 @@ struct spi_flash {
> 
>  	u32		size;
> 
> +	u32		block_size;

not sure what it'll do to code size, but a u16 should be large enough to hold 
the base erase size.

> @@ -67,5 +69,4 @@ static inline int spi_flash_erase(struct spi_flash
> *flash, u32 offset, {
>  	return flash->erase(flash, offset, len);
>  }
> -
>  #endif /* _SPI_FLASH_H_ */

please avoid useless whitespace changes
-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/20110215/a24def07/attachment.pgp