diff mbox

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

Message ID 1297286172-6343-1-git-send-email-RichardRetanubun@RuggedCom.com
State Superseded
Headers show

Commit Message

Richard Retanubun Feb. 9, 2011, 9:16 p.m. UTC 
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(-)

Comments

Mike Frysinger Feb. 15, 2011, 9:34 a.m. UTC | #1 
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
Richard Retanubun Feb. 16, 2011, 8:27 p.m. UTC | #2 
Hi Mike,

Thanks for the feedback, and for the cool unification.
Here is the updated patch that implements your comments and is
based on the head of the blackfin.git sf branch.
Mike Frysinger Feb. 17, 2011, 5:46 a.m. UTC | #3 
On Wednesday, February 16, 2011 15:27:52 Richard Retanubun wrote:
> Thanks for the feedback, and for the cool unification.
> Here is the updated patch that implements your comments and is
> based on the head of the blackfin.git sf branch.

thanks !  i'll try to get to it this weekend, but no promises as i currently 
have non-u-boot stuff pressing for my time.
-mike
diff mbox

Patch

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 macronix_erase(struct spi_flash *flash, u32 offset, size_t len)
 	 * when possible.
 	 */
 
-	sector_size = mcx->params->page_size * mcx->params->pages_per_sector
-			* mcx->params->sectors_per_block;
+	sector_size = mcx->flash.block_size;
 
 	if (offset % sector_size || len % sector_size) {
 		debug("SF: Erase offset/length not multiple of sector size\n");
@@ -329,6 +328,7 @@  struct spi_flash *spi_flash_probe_macronix(struct spi_slave *spi, u8 *idcode)
 	mcx->flash.read = macronix_read_fast;
 	mcx->flash.size = params->page_size * params->pages_per_sector
 	    * params->sectors_per_block * params->nr_blocks;
+	mcx->flash.block_size = mcx->flash.size/params->nr_blocks;
 
 	printf("SF: Detected %s with page size %u, total ",
 	       params->name, params->page_size);
diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c
index d6c1a5f..e5ba4f4 100644
--- a/drivers/mtd/spi/spansion.c
+++ b/drivers/mtd/spi/spansion.c
@@ -255,7 +255,7 @@  int spansion_erase(struct spi_flash *flash, u32 offset, size_t len)
 	 * when possible.
 	 */
 
-	sector_size = spsn->params->page_size * spsn->params->pages_per_sector;
+	sector_size = spsn->flash.block_size;
 
 	if (offset % sector_size || len % sector_size) {
 		debug("SF: Erase offset/length not multiple of sector size\n");
@@ -342,6 +342,7 @@  struct spi_flash *spi_flash_probe_spansion(struct spi_slave *spi, u8 *idcode)
 	spsn->flash.read = spansion_read_fast;
 	spsn->flash.size = params->page_size * params->pages_per_sector
 	    * params->nr_sectors;
+	spsn->flash.block_size = spsn->flash.size/params->nr_sectors;
 
 	printf("SF: Detected %s with page size %u, total ",
 	       params->name, params->page_size);
diff --git a/drivers/mtd/spi/sst.c b/drivers/mtd/spi/sst.c
index 2557891..4ac6783 100644
--- a/drivers/mtd/spi/sst.c
+++ b/drivers/mtd/spi/sst.c
@@ -261,7 +261,7 @@  sst_erase(struct spi_flash *flash, u32 offset, size_t len)
 	 * when possible.
 	 */
 
-	sector_size = SST_SECTOR_SIZE;
+	sector_size = flash->block_size;
 
 	if (offset % sector_size) {
 		debug("SF: Erase offset not multiple of sector size\n");
@@ -363,6 +363,7 @@  spi_flash_probe_sst(struct spi_slave *spi, u8 *idcode)
 	stm->flash.erase = sst_erase;
 	stm->flash.read = sst_read_fast;
 	stm->flash.size = SST_SECTOR_SIZE * params->nr_sectors;
+	stm->flash.block_size = SST_SECTOR_SIZE;
 
 	printf("SF: Detected %s with page size %u, total ",
 	       params->name, SST_SECTOR_SIZE);
diff --git a/drivers/mtd/spi/stmicro.c b/drivers/mtd/spi/stmicro.c
index 3134027..8b44fb9 100644
--- a/drivers/mtd/spi/stmicro.c
+++ b/drivers/mtd/spi/stmicro.c
@@ -269,7 +269,7 @@  int stmicro_erase(struct spi_flash *flash, u32 offset, size_t len)
 	 * when possible.
 	 */
 
-	sector_size = stm->params->page_size * stm->params->pages_per_sector;
+	sector_size = stm->flash.block_size;
 
 	if (offset % sector_size || len % sector_size) {
 		debug("SF: Erase offset/length not multiple of sector size\n");
@@ -364,6 +364,7 @@  struct spi_flash *spi_flash_probe_stmicro(struct spi_slave *spi, u8 * idcode)
 	stm->flash.read = stmicro_read_fast;
 	stm->flash.size = params->page_size * params->pages_per_sector
 	    * params->nr_sectors;
+	stm->flash.block_size = stm->flash.size/params->nr_sectors;
 
 	printf("SF: Detected %s with page size %u, total ",
 	       params->name, params->page_size);
diff --git a/drivers/mtd/spi/winbond.c b/drivers/mtd/spi/winbond.c
index ff1df25..f760bc2 100644
--- a/drivers/mtd/spi/winbond.c
+++ b/drivers/mtd/spi/winbond.c
@@ -225,7 +225,6 @@  int winbond_erase(struct spi_flash *flash, u32 offset, size_t len)
 {
 	struct winbond_spi_flash *stm = to_winbond_spi_flash(flash);
 	unsigned long sector_size;
-	unsigned int page_shift;
 	size_t actual;
 	int ret;
 	u8 cmd[4];
@@ -236,8 +235,7 @@  int winbond_erase(struct spi_flash *flash, u32 offset, size_t len)
 	 * when possible.
 	 */
 
-	page_shift = stm->params->l2_page_size;
-	sector_size = (1 << page_shift) * stm->params->pages_per_sector;
+	sector_size = stm->flash.block_size;
 
 	if (offset % sector_size || len % sector_size) {
 		debug("SF: Erase offset/length not multiple of sector size\n");
@@ -324,6 +322,8 @@  struct spi_flash *spi_flash_probe_winbond(struct spi_slave *spi, u8 *idcode)
 	stm->flash.size = page_size * params->pages_per_sector
 				* params->sectors_per_block
 				* params->nr_blocks;
+	stm->flash.block_size = (1 << stm->params->l2_page_size) *
+		stm->params->pages_per_sector;
 
 	printf("SF: Detected %s with page size %u, total ",
 	       params->name, page_size);
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 1f8ba29..462bef6 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -38,6 +38,8 @@  struct spi_flash {
 
 	u32		size;
 
+	u32		block_size;
+
 	int		(*read)(struct spi_flash *flash, u32 offset,
 				size_t len, void *buf);
 	int		(*write)(struct spi_flash *flash, u32 offset,
@@ -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_ */