[U-Boot] [U-boot] sf: API for spi_flash_get_sector_size
Hello,
I am working on hooks to build into my board to allow for SPI flash (STMicro M25P40 and M25P16) reflash. When calling spi_flash_erase, which eventually calls stmicro_erase, one must do so while knowing what the sector_size is of the flash. Is there a recommened API to getting this from struct stmicro_spi_flash_params?
There are things like the "to_stmicro_spi_flash" #define in stmicro.c, but that is private.
Will adding this API be okay?
int spi_flash_get_sector_size(struct spi_flash *sf) that will simply return the sector_size number or an errno.
thanks for everyone's time
- Richard Retanubun
Dear Richard Retanubun,
In message 4D514F6E.6040101@RuggedCom.com you wrote:
When calling spi_flash_erase, which eventually calls stmicro_erase, one must do so while knowing what the sector_size is of the flash. Is there a recommened API to getting this from struct stmicro_spi_flash_params?
Why would you need to know the sector size?
To align your erase command accordingly? I think you are taking the wrong approach here. Look at how NOR flash is handled: we don't need to know the sector sizes there either. Instead we support the format "+N" (often used as "+${fileszize}") which will _internally_ round up N to match the next erase block boundary.
Do the same for SPI flash.
Best regards,
Wolfgang Denk
On 02/08/11 09:40, Wolfgang Denk wrote:
Thanks for the speedy response Wolfgang,
Dear Richard Retanubun,
In message4D514F6E.6040101@RuggedCom.com you wrote:
When calling spi_flash_erase, which eventually calls stmicro_erase, one must do so while knowing what the sector_size is of the flash. Is there a recommened API to getting this from struct stmicro_spi_flash_params?
Why would you need to know the sector size?
To align your erase command accordingly?
Yep.
I think you are taking the wrong approach here. Look at how NOR flash is handled: we don't need
to know the sector sizes there either. Instead we support the format "+N" (often used as "+${fileszize}") which will _internally_ round up N to match the next erase block boundary.
Do the same for SPI flash.
If by "how NOR flash in handled" you mean cmd_flash.c::flash_sect_roundb() then yes, I do want to do this but I need to know what number to round it to, no?
In the context of cmd_flash, it has access to flash_info_t which contains the sector_size information. Also cfi_flash.c also provides a function called flash_sector_size() that is called by other components that needs to know. If I have misunderstood what you mean, sorry, please clarify.
To have SPI flash do this, for example; stmicro.c::stmicro_erase() function will auto round up to the nearest sector_size. Which is indeed the simpler way. Right now, most drivers/mtd/spi/*.c just does this check:
<quoted_code> sector_size = stm->params->page_size * stm->params->pages_per_sector;
if (offset % sector_size || len % sector_size) { debug("SF: Erase offset/length not multiple of sector size\n"); return -1; } </quoted_code>
I am worried about unintentionally erasing more than what the caller requested. Would it be better to round up manually and then calls the erase?
Thanks for your time
- Richard
Dear Richard Retanubun,
In message 4D515D06.7020209@RuggedCom.com you wrote:
If by "how NOR flash in handled" you mean cmd_flash.c::flash_sect_roundb() then yes, I do want to do this but I need to know what number to round it to, no?
The code needs to know that, you don't ;-)
In the context of cmd_flash, it has access to flash_info_t which contains the sector_size information. Also cfi_flash.c also provides a function called flash_sector_size() that is called by other components that needs to know. If I have misunderstood what you mean, sorry, please clarify.
You are right.
To have SPI flash do this, for example; stmicro.c::stmicro_erase() function will auto round up to the nearest sector_size. Which is indeed the simpler way. Right now, most drivers/mtd/spi/*.c just does this check:
<quoted_code> sector_size = stm->params->page_size * stm->params->pages_per_sector;
if (offset % sector_size || len % sector_size) { debug("SF: Erase offset/length not multiple of sector size\n"); return -1; }
</quoted_code>
This is ok if an explicit size is given - you will se ethe same behaviour on NOR flash when trying something like
erase 40000000 40002345
I am worried about unintentionally erasing more than what the caller requested. Would it be better to round up manually and then calls the erase?
No. If the caller uses the "+<size>" notation he explictly requests to round up. He is supposed to know what he is doing.
Being able to use "+${filesize}" allows for many powerful solutions to standard tasks that without this would result in cumbersome manipulation of the size - which would then probably break if you issue a new revision of your hardware with different flash chips fit.
Best regards,
Wolfgang Denk
On 02/08/11 10:19, Wolfgang Denk wrote:
Dear Richard Retanubun,
In message4D515D06.7020209@RuggedCom.com you wrote:
If by "how NOR flash in handled" you mean cmd_flash.c::flash_sect_roundb() then yes, I do want to do this but I need to know what number to round it to, no?
The code needs to know that, you don't ;-)
In the context of cmd_flash, it has access to flash_info_t which contains the sector_size information. Also cfi_flash.c also provides a function called flash_sector_size() that is called by other components that needs to know. If I have misunderstood what you mean, sorry, please clarify.
You are right.
<sensing a wavering in your resolve> ;-)
Thus we come to my original question, in the same way that cfi_flash.c provides flash_sector_size();
I propose we add spi_flash.c::spi_flash_sector_size() which returns the same information so that other code can validate (or round up if requested) the length of the operation to the nearest sector size.
right now (unless I misunderstood), there is no easy way to get the spi flash sector_size, no?
would this be acceptable?
To have SPI flash do this, for example; stmicro.c::stmicro_erase() function will auto round up to the nearest sector_size. Which is indeed the simpler way. Right now, most drivers/mtd/spi/*.c just does this check:
<quoted_code> sector_size = stm->params->page_size * stm->params->pages_per_sector;
if (offset % sector_size || len % sector_size) { debug("SF: Erase offset/length not multiple of sector size\n"); return -1; }
</quoted_code>
This is ok if an explicit size is given - you will see the same behaviour on NOR flash when trying something like
erase 40000000 40002345
Understood. So the spi flash driver erase function itself should only validate the args and not auto round up. This means that the decision of [warn or auto-round-up] should be handled by other code that parses the user's commands that is operating on spi flash.
To do this, they need a way to ask the spi flash driver what the sector size to round up to is.
I am worried about unintentionally erasing more than what the caller requested. Would it be better to round up manually and then calls the erase?
No. If the caller uses the "+<size>" notation he explictly requests to round up. He is supposed to know what he is doing.
Being able to use "+${filesize}" allows for many powerful solutions to standard tasks that without this would result in cumbersome manipulation of the size - which would then probably break if you issue a new revision of your hardware with different flash chips fit.
This is a cool convention. Should we add this to "sf erase command?" if it is not there already. if so, what command (or better yet, git commit) shows how to add handling of "+N"
Thanks!
- Richard
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 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_ */
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
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.
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
This patch adds a new member to struct spi_flash (u16 sector_size) and updates the spi flash drivers to start populating it.
This parameter can be used by spi flash commands that need to round up units of operation to the flash's sector_size. --- drivers/mtd/spi/atmel.c | 1 + drivers/mtd/spi/macronix.c | 1 + drivers/mtd/spi/spansion.c | 4 ++-- drivers/mtd/spi/sst.c | 3 ++- drivers/mtd/spi/stmicro.c | 4 ++-- drivers/mtd/spi/winbond.c | 5 +++-- include/spi_flash.h | 2 ++ 7 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/spi/atmel.c b/drivers/mtd/spi/atmel.c index a9910b1..180a52b 100644 --- a/drivers/mtd/spi/atmel.c +++ b/drivers/mtd/spi/atmel.c @@ -498,6 +498,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.sector_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 4155d4d..4a8e17f 100644 --- a/drivers/mtd/spi/macronix.c +++ b/drivers/mtd/spi/macronix.c @@ -217,6 +217,7 @@ struct spi_flash *spi_flash_probe_macronix(struct spi_slave *spi, u8 *idcode) mcx->flash.read = spi_flash_cmd_read_fast; mcx->flash.size = params->page_size * params->pages_per_sector * params->sectors_per_block * params->nr_blocks; + mcx->flash.sector_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 d54a5fa..c88457b 100644 --- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -199,8 +199,7 @@ static int spansion_write(struct spi_flash *flash, int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) { struct spansion_spi_flash *spsn = to_spansion_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_S25FLXX_SE, - spsn->params->page_size * spsn->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_S25FLXX_SE, flash->sector_size, offset, len); }
@@ -242,6 +241,7 @@ struct spi_flash *spi_flash_probe_spansion(struct spi_slave *spi, u8 *idcode) spsn->flash.read = spi_flash_cmd_read_fast; spsn->flash.size = params->page_size * params->pages_per_sector * params->nr_sectors; + spsn->flash.sector_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 792d04d..15de12b 100644 --- a/drivers/mtd/spi/sst.c +++ b/drivers/mtd/spi/sst.c @@ -201,7 +201,7 @@ sst_write(struct spi_flash *flash, u32 offset, size_t len, const void *buf)
int sst_erase(struct spi_flash *flash, u32 offset, size_t len) { - return spi_flash_cmd_erase(flash, CMD_SST_SE, SST_SECTOR_SIZE, + return spi_flash_cmd_erase(flash, CMD_SST_SE, flash->sector_size, offset, len); }
@@ -257,6 +257,7 @@ spi_flash_probe_sst(struct spi_slave *spi, u8 *idcode) stm->flash.write = sst_write; stm->flash.erase = sst_erase; stm->flash.size = SST_SECTOR_SIZE * params->nr_sectors; + stm->flash.sector_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 7ef690d..80d838e 100644 --- a/drivers/mtd/spi/stmicro.c +++ b/drivers/mtd/spi/stmicro.c @@ -200,8 +200,7 @@ static int stmicro_write(struct spi_flash *flash, int stmicro_erase(struct spi_flash *flash, u32 offset, size_t len) { struct stmicro_spi_flash *stm = to_stmicro_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_M25PXX_SE, - stm->params->page_size * stm->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_M25PXX_SE, flash->sector_size, offset, len); }
@@ -251,6 +250,7 @@ struct spi_flash *spi_flash_probe_stmicro(struct spi_slave *spi, u8 * idcode) stm->flash.read = spi_flash_cmd_read_fast; stm->flash.size = params->page_size * params->pages_per_sector * params->nr_sectors; + stm->flash.sector_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 e88802f..ec6fb79 100644 --- a/drivers/mtd/spi/winbond.c +++ b/drivers/mtd/spi/winbond.c @@ -173,8 +173,7 @@ out: int winbond_erase(struct spi_flash *flash, u32 offset, size_t len) { struct winbond_spi_flash *stm = to_winbond_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_W25_SE, - (1 << stm->params->l2_page_size) * stm->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_W25_SE, flash->sector_size, offset, len); }
@@ -216,6 +215,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.sector_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..039b94e 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -38,6 +38,8 @@ struct spi_flash {
u32 size;
+ u16 sector_size; + int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf); int (*write)(struct spi_flash *flash, u32 offset,
Dear Richard Retanubun,
In message 1297888074-8344-2-git-send-email-RichardRetanubun@RuggedCom.com you wrote:
This patch adds a new member to struct spi_flash (u16 sector_size) and updates the spi flash drivers to start populating it.
This parameter can be used by spi flash commands that need to round up units of operation to the flash's sector_size.
drivers/mtd/spi/atmel.c | 1 + drivers/mtd/spi/macronix.c | 1 + drivers/mtd/spi/spansion.c | 4 ++-- drivers/mtd/spi/sst.c | 3 ++- drivers/mtd/spi/stmicro.c | 4 ++-- drivers/mtd/spi/winbond.c | 5 +++-- include/spi_flash.h | 2 ++ 7 files changed, 13 insertions(+), 7 deletions(-)
Rejected - Signed-off-by: line missing.
Best regards,
Wolfgang Denk
This patch adds [+]len handler for the erase command that will automatically round up the requested erase length to the flash's sector_size. --- common/cmd_sf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 6e7be81..bbd4842 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -19,6 +19,48 @@
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 len_arg; + + + round_up_len = 0; + if (*arg == '+'){ + round_up_len = 1; + ++arg; + } + + len_arg = simple_strtoul(arg, &ep, 16); + if (ep == arg || *ep != '\0') + return -1; + + if (round_up_len && flash->sector_size > 0) + *len = DIV_ROUND_UP(len_arg, flash->sector_size); + else + *len = len_arg; + + return 1; +} + static int do_spi_flash_probe(int argc, char * const argv[]) { unsigned int bus = 0; @@ -135,9 +177,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) { @@ -148,7 +192,7 @@ static int do_spi_flash_erase(int argc, char * const argv[]) return 0;
usage: - puts("Usage: sf erase offset len\n"); + puts("Usage: sf erase offset [+]len\n"); return 1; }
@@ -189,5 +233,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" );
Dear Richard Retanubun,
In message 1297888074-8344-3-git-send-email-RichardRetanubun@RuggedCom.com you wrote:
This patch adds [+]len handler for the erase command that will automatically round up the requested erase length to the flash's sector_size.
common/cmd_sf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49 insertions(+), 4 deletions(-)
Please run your patches through checkpatch - this helps avoiding the frustration of seeing rejects due to simple issues like these:
ERROR: Missing Signed-off-by: line(s)
- if (*arg == '+'){
ERROR: space required before the open brace '{'
- if (ret != 1) { goto usage;
- }
WARNING: braces {} are not necessary for single statement blocks
Best regards,
Wolfgang Denk
This patch adds a new member to struct spi_flash (u16 sector_size) and updates the spi flash drivers to start populating it.
This parameter can be used by spi flash commands that need to round up units of operation to the flash's sector_size.
Signed-off-by: Richard Retanubun RichardRetanubun@RuggedCom.com --- v2: scrubbed via checkpatch, thanks WD!
drivers/mtd/spi/atmel.c | 1 + drivers/mtd/spi/macronix.c | 1 + drivers/mtd/spi/spansion.c | 4 ++-- drivers/mtd/spi/sst.c | 3 ++- drivers/mtd/spi/stmicro.c | 4 ++-- drivers/mtd/spi/winbond.c | 5 +++-- include/spi_flash.h | 2 ++ 7 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/spi/atmel.c b/drivers/mtd/spi/atmel.c index a9910b1..180a52b 100644 --- a/drivers/mtd/spi/atmel.c +++ b/drivers/mtd/spi/atmel.c @@ -498,6 +498,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.sector_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 4155d4d..4a8e17f 100644 --- a/drivers/mtd/spi/macronix.c +++ b/drivers/mtd/spi/macronix.c @@ -217,6 +217,7 @@ struct spi_flash *spi_flash_probe_macronix(struct spi_slave *spi, u8 *idcode) mcx->flash.read = spi_flash_cmd_read_fast; mcx->flash.size = params->page_size * params->pages_per_sector * params->sectors_per_block * params->nr_blocks; + mcx->flash.sector_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 d54a5fa..c88457b 100644 --- a/drivers/mtd/spi/spansion.c +++ b/drivers/mtd/spi/spansion.c @@ -199,8 +199,7 @@ static int spansion_write(struct spi_flash *flash, int spansion_erase(struct spi_flash *flash, u32 offset, size_t len) { struct spansion_spi_flash *spsn = to_spansion_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_S25FLXX_SE, - spsn->params->page_size * spsn->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_S25FLXX_SE, flash->sector_size, offset, len); }
@@ -242,6 +241,7 @@ struct spi_flash *spi_flash_probe_spansion(struct spi_slave *spi, u8 *idcode) spsn->flash.read = spi_flash_cmd_read_fast; spsn->flash.size = params->page_size * params->pages_per_sector * params->nr_sectors; + spsn->flash.sector_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 792d04d..15de12b 100644 --- a/drivers/mtd/spi/sst.c +++ b/drivers/mtd/spi/sst.c @@ -201,7 +201,7 @@ sst_write(struct spi_flash *flash, u32 offset, size_t len, const void *buf)
int sst_erase(struct spi_flash *flash, u32 offset, size_t len) { - return spi_flash_cmd_erase(flash, CMD_SST_SE, SST_SECTOR_SIZE, + return spi_flash_cmd_erase(flash, CMD_SST_SE, flash->sector_size, offset, len); }
@@ -257,6 +257,7 @@ spi_flash_probe_sst(struct spi_slave *spi, u8 *idcode) stm->flash.write = sst_write; stm->flash.erase = sst_erase; stm->flash.size = SST_SECTOR_SIZE * params->nr_sectors; + stm->flash.sector_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 7ef690d..80d838e 100644 --- a/drivers/mtd/spi/stmicro.c +++ b/drivers/mtd/spi/stmicro.c @@ -200,8 +200,7 @@ static int stmicro_write(struct spi_flash *flash, int stmicro_erase(struct spi_flash *flash, u32 offset, size_t len) { struct stmicro_spi_flash *stm = to_stmicro_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_M25PXX_SE, - stm->params->page_size * stm->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_M25PXX_SE, flash->sector_size, offset, len); }
@@ -251,6 +250,7 @@ struct spi_flash *spi_flash_probe_stmicro(struct spi_slave *spi, u8 * idcode) stm->flash.read = spi_flash_cmd_read_fast; stm->flash.size = params->page_size * params->pages_per_sector * params->nr_sectors; + stm->flash.sector_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 e88802f..ec6fb79 100644 --- a/drivers/mtd/spi/winbond.c +++ b/drivers/mtd/spi/winbond.c @@ -173,8 +173,7 @@ out: int winbond_erase(struct spi_flash *flash, u32 offset, size_t len) { struct winbond_spi_flash *stm = to_winbond_spi_flash(flash); - return spi_flash_cmd_erase(flash, CMD_W25_SE, - (1 << stm->params->l2_page_size) * stm->params->pages_per_sector, + return spi_flash_cmd_erase(flash, CMD_W25_SE, flash->sector_size, offset, len); }
@@ -216,6 +215,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.sector_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..039b94e 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -38,6 +38,8 @@ struct spi_flash {
u32 size;
+ u16 sector_size; + int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf); int (*write)(struct spi_flash *flash, u32 offset,
these patches have been integrated/superseded by my recent sf patchset -mike
This patch adds [+]len handler for the erase command that will automatically round up the requested erase length to the flash's sector_size.
Signed-off-by: Richard Retanubun RichardRetanubun@RuggedCom.com --- v2: scrubbed via checkpatch, thanks WD!
common/cmd_sf.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 6e7be81..17a4dd8 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -19,6 +19,48 @@
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 len_arg; + + + round_up_len = 0; + if (*arg == '+') { + round_up_len = 1; + ++arg; + } + + len_arg = simple_strtoul(arg, &ep, 16); + if (ep == arg || *ep != '\0') + return -1; + + if (round_up_len && flash->sector_size > 0) + *len = DIV_ROUND_UP(len_arg, flash->sector_size); + else + *len = len_arg; + + return 1; +} + static int do_spi_flash_probe(int argc, char * const argv[]) { unsigned int bus = 0; @@ -135,8 +177,9 @@ 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); @@ -148,7 +191,7 @@ static int do_spi_flash_erase(int argc, char * const argv[]) return 0;
usage: - puts("Usage: sf erase offset len\n"); + puts("Usage: sf erase offset [+]len\n"); return 1; }
@@ -189,5 +232,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" );
On Wed, 16 Feb 2011 15:27:54 -0500 Richard Retanubun RichardRetanubun@ruggedcom.com wrote:
This patch adds [+]len handler for the erase command that will automatically round up the requested erase length to the flash's sector_size.
common/cmd_sf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 6e7be81..bbd4842 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -19,6 +19,48 @@
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:
- <cmd> offset len - operate on <'offset', 'len')
- <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).
On NOR, + is used to indicate that the second argument is a length, as opposed to an ending address. Rounding seems like a side effect of length mode.
On NAND we unconditionally round up erase lengths, as we don't support ending-address mode (looks like SPI doesn't either).
-Scott
participants (4)
-
Mike Frysinger -
Richard Retanubun -
Scott Wood -
Wolfgang Denk