Re: [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


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Richard Retanubun]

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 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.

If block_size is considered a kludgy name, please propose another.

Most of the driver (excluding atmel) will now only calculate this
unit once during probe and will be able to use it directly.

For atmel, I added code to populate the block_size structure member
but felt too ignorant to actually modify the erase unit check.

Code compiles cleanly. fo all drivers, but I can only test
stmicro (M25P40 and M25P16).
---
 common/cmd_sf.c|   63 +--
 drivers/mtd/spi/atmel.c|1 +
 drivers/mtd/spi/macronix.c |4 +-
 drivers/mtd/spi/spansion.c |3 +-
 drivers/mtd/spi/sst.c  |3 +-
 drivers/mtd/spi/stmicro.c  |3 +-
 drivers/mtd/spi/winbond.c  |6 ++--
 include/spi_flash.h|3 +-
 8 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 6e7be81..b1fed6e 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -19,6 +19,59 @@
 
 static struct spi_flash *flash;
 
+
+/*
+ * This function computes the length argument for the erase command.
+ * The length on which the command is to operate can be given in two forms:
+ * 1. cmd offset len  - operate on 'offset',  'len')
+ * 2. cmd offset +len - operate on 'offset',  'round_up(len)')
+ * If the second form is used and the length doesn't fall on the
+ * sector boundary, than it will be adjusted to the next sector boundary.
+ * If it isn't in the flash, the function will fail (return -1).
+ * Input:
+ *arg: length specification (i.e. both command arguments)
+ * Output:
+ *len: computed length for operation
+ * Return:
+ *1: success
+ *   -1: failure (bad format, bad address).
+*/
+static int sf_parse_len_arg(char *arg, ulong *len)
+{
+   char *ep;
+   char round_up_len; /* indicates if the +length form used */
+   ulong sector_size;
+   ulong len_arg;
+
+
+   round_up_len = 0;
+   if (arg  *arg == '+'){
+   round_up_len = 1;
+   ++arg;
+   }
+
+   len_arg = simple_strtoul(arg, ep, 16);
+   if (ep == arg || *ep != '\0')
+   return -1;
+
+   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);
+   if (*len  flash-size) {
+   return -1;
+   }
+   } else {
+   return -1;
+   }
+   } else {
+   *len = len_arg;
+   }
+
+   return 1;
+}
+
 static int do_spi_flash_probe(int argc, char * const argv[])
 {
unsigned int bus = 0;
@@ -135,9 +188,11 @@ static int do_spi_flash_erase(int argc, char * const 
argv[])
offset = simple_strtoul(argv[1], endp, 16);
if (*argv[1] == 0 || *endp != 0)
goto usage;
-   len = simple_strtoul(argv[2], endp, 16);
-   if (*argv[2] == 0 || *endp != 0)
+
+   ret = sf_parse_len_arg(argv[2], len);
+   if (ret != 1) {
goto usage;
+   }
 
ret = spi_flash_erase(flash, offset, len);
if (ret) {
@@ -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;
 }
 
@@ -189,5 +245,6 @@ U_BOOT_CMD(
 `offset' to memory at `addr'\n
sf write addr offset len   - write `len' bytes from memory\n
 at `addr' to flash at `offset'\n
-   sf erase offset len- erase `len' bytes from `offset'
+   sf erase offset [+]len - erase `len' bytes from `offset'\n
+  - `+len' round up `len' to block size
 );
diff --git a/drivers/mtd/spi/atmel.c b/drivers/mtd/spi/atmel.c
index 8d02169..ec1a7b7 100644
--- a/drivers/mtd/spi/atmel.c
+++ b/drivers/mtd/spi/atmel.c
@@ -539,6 +539,7 @@ struct spi_flash *spi_flash_probe_atmel(struct spi_slave 
*spi, u8 *idcode)
asf-flash.size = page_size * params-pages_per_block
* params-blocks_per_sector
* params-nr_sectors;
+   asf-flash.block_size = page_size;
 
printf(SF: Detected %s with page size %u, total ,
   params-name, page_size);
diff --git a/drivers/mtd/spi/macronix.c b/drivers/mtd/spi/macronix.c
index 76d5284..98e553b 100644
--- a/drivers/mtd/spi/macronix.c
+++ b/drivers/mtd/spi/macronix.c
@@ -247,8 +247,7 @@ int