Re: [RFC PATCH] mtd: spi-nor: add support to non-uniform SPI NOR flash memories
From: Tudor Ambarus
Date: Tue May 22 2018 - 00:07:08 EST
Hi, Marek,
On 05/21/2018 07:59 PM, Marek Vasut wrote:
On 05/21/2018 06:42 PM, Tudor Ambarus wrote:
Hi, Marek,
[...]
This is a transitional patch: non-uniform erase maps will be used later
when initialized based on the SFDP data.
What about non-SFDP non-linear flashes ?
Non-SFDP non-uniform flashes support is not addressed with this
proposal, I should have told this in the commit message, thanks. But we
are backward compatible, if non-SFDP, the flashes are considered
uniform.
OK. btw wall-of-text description of patch isn't my fav thing.
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxxxxxx>
[tudor.ambarus@xxxxxxxxxxxxx:
- add improvements on how the erase map is handled. The map is an array
describing the boundaries of the erase regions. LSB bits of the region's
offset are used to describe the supported erase types, to indicate if
that specific region is the last region in the map and to mark if the
region is overlaid or not. When one sends an addr and len to erase a
chunk of memory, we identify in which region the address fits, we start
erasing with the best fitted erase commands and when the region ends,
continue to erase from the next region. The erase is optimal: identify
the start offset (once), then erase with the best erase command,
move forward and repeat.
Is that like an R-tree ?
Not really. I find this RFC proposal faster and neat, but I'm open for
suggestions and guidance.
One wants to erase a contiguous chunk of memory and sends us the
starting address and the total length. The algorithm of finding the best
sequence of erase commands can be summarized in four steps:
1. Find in which region the address fits.
This step is done only once, at the beginning. For the non-uniform
SFDP-defined flashes, usually there are two or three regions defined.
Nevertheless, in the worst case, the maximum number of regions that can
be defined is on eight bits, so 255. Linear search for just 255 elements
in the worst case looks good for me, especially that we do this search
once.
2. Find the *best* erase command that is defined in that region.
Each region can define maximum 4 erase commands. *Best* is defined as
the largest/biggest supported erase command with which the provided
address is aligned and which does not erase more that what the user has
asked for. In case of overlaid regions, alignment does not matter. The
largest command will erase the remaining of the overlaid region without
touching the region with which it overlaps (see S25FS512S). The
supported erase commands are ordered by size with the biggest queried
first. It is desirable to erase with large erase commands so that we
erase as much as we can in one shoot, minimizing the erase() calls.
3. Erase sector with the *best* erase command and move forward in a
linear fashion.
addr += cmd->size;
len -= cmd->size;
If the new address exceeds the end of this region, move to the next.
4. While (len) goto step2.
That's all. Linearity is an advantage. We find the starting region and
then we traverse each region in order without other queries.
- order erase types by size, with the biggest erase type at BIT(0). With
this, we can iterate from the biggest supported erase type to the
smallest,
and when find one that meets all the required conditions, break the
loop.
This saves time in determining the best erase cmd.
- minimize the amount of erase() calls by using the best sequence of
erase
type commands depending on alignment.
Nice, this was long overdue
- replace spi_nor_find_uniform_erase() with
spi_nor_select_uniform_erase().
Even for the SPI NOR memories with non-uniform erase types, we can
determine
at init if there are erase types that can erase the entire memory.
Fill at
init the uniform_erase_type bitmask, to encode the erase type
commands that
can erase the entire memory.
- clarify support for overlaid regions. Considering one of the erase
maps
of the S25FS512S memory:
Bottom: 8x 4KB sectors at bottom (only 4KB erase supported),
1x overlaid 224KB sector at bottom (only 256KB erase
supported),
255x 256KB sectors (only 256KB erase supported)
S25FS512S states that 'if a sector erase command is applied to a
256KB range
that is overlaid by 4KB secors, the overlaid 4kB sectors are not
affected by
the erase'. When at init, the overlaid region size should be set to
region->size = erase_size - count; in order to not miss chunks of data
when traversing the regions.
- backward compatibility test done on MX25L25673G.
The 'erase with the best command, move forward and repeat' approach was
suggested by Cristian Birsan in a brainstorm session, so:
]
Suggested-by: Cristian Birsan <cristian.birsan@xxxxxxxxxxxxx>
Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
---
drivers/mtd/spi-nor/spi-nor.c | 281
+++++++++++++++++++++++++++++++++++++++---
include/linux/mtd/spi-nor.h | 89 +++++++++++++
2 files changed, 356 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c
index 494b7a2..bb70664 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -260,6 +260,17 @@ static void spi_nor_set_4byte_opcodes(struct
spi_nor *nor,
nor->read_opcode = spi_nor_convert_3to4_read(nor->read_opcode);
nor->program_opcode =
spi_nor_convert_3to4_program(nor->program_opcode);
nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
+
+ if (!spi_nor_has_uniform_erase(nor)) {
+ struct spi_nor_erase_map *map = &nor->erase_map;
+ struct spi_nor_erase_command *cmd;
+ int i;
+
+ for (i = 0; i < SNOR_CMD_ERASE_MAX; i++) {
+ cmd = &map->commands[i];
+ cmd->opcode = spi_nor_convert_3to4_erase(cmd->opcode);
+ }
+ }
}
/* Enable/disable 4-byte addressing mode. */
@@ -497,6 +508,131 @@ static int spi_nor_erase_sector(struct spi_nor
*nor, u32 addr)
return nor->write_reg(nor, nor->erase_opcode, buf,
nor->addr_width);
}
+/* JEDEC JESD216B Standard imposes erase sizes to be power of 2. */
+static inline u64
+spi_nor_div_by_erase_size(const struct spi_nor_erase_command *cmd,
+ u64 dividend, u32 *remainder)
+{
+ *remainder = (u32)dividend & cmd->size_mask;
+ return dividend >> cmd->size_shift;
+}
+
+static const struct spi_nor_erase_command *
+spi_nor_find_best_erase_cmd(const struct spi_nor_erase_map *map,
+ const struct spi_nor_erase_region *region, u64 addr,
+ u32 len)
+{
+ const struct spi_nor_erase_command *cmd;
+ u32 rem;
+ int i;
+ u8 cmd_mask = region->offset & SNOR_CMD_ERASE_MASK;
+
+ /*
+ * Commands are ordered by size, with the biggest erase type at
+ * index 0.
+ */
+ for (i = 0; i < SNOR_CMD_ERASE_MAX; i++) {
+ /* Does the erase region support the tested erase command? */
+ if (!(cmd_mask & BIT(i)))
+ continue;
+
+ cmd = &map->commands[i];
+
+ /* Don't erase more than what the user has asked for. */
+ if (cmd->size > len)
+ continue;
Are you sure checking for the full erase block length first and then
checking if you can sub-erase the block is OK ?
will respond in the next comment.
+ if (!(region->offset & SNOR_OVERLAID_REGION)) {
+ /* 'addr' must be aligned to the erase size. */
+ spi_nor_div_by_erase_size(cmd, addr, &rem);
oh, I missed the if here, this should have been confusing.
if (rem)
continue;
else
return cmd;
The else case can be merged with the one from below.
Returning to your previous question. I iterate from the biggest erase
command to the smallest, because bigger is preferred, it will minimize
the amount of erase() calls. The biggest erase command that doesn't
erase more that what the user has asked for, will do. If the region is
not-overlaid the address must also be aligned with the erase size.
You can have a flash with 4k sectors which also supports 64k erase and
try to erase ie. 128k at offset +4k. That means you need to first erase
small chunks, then big chunk, then small chunks again. So I don't think
you can start with large chunk to see if you can erase it, since on such
a setup the erase will degrade to massive amount of 4k erase ops.
I'm looking for the biggest supported erase command with which the
provided address is aligned and which does not erase more that what the
user has asked for. In your example, 4k erase type will be used until
reaching the 64k offset, then a 64k erase type, then a 4k type.
[...]
+ while (len) {
+ cmd = spi_nor_find_best_erase_cmd(map, region, addr, len);
+ if (!cmd)
+ return -EINVAL;
What would happen if you realize mid-way that you cannot erase some
sector , do you end up with partial erase ?
Is this possible? In non-overlaid regions, the address is aligned with
at least one of the erase commands, else -EINVAL. For overlaid regions
alignment doesn't matter. But yes, if this is possible, in this case,
this proposal will do a partial erase.
Shouldn't we fail up front instead ?
It will be great if we can do this without having performance penalties.
Can we loose the conditions for the last erase command? If one wants to
erase 80k chunk starting from offset 0 and only 32k and 64k erase type
are supported, can we erase 96k?
Thanks,
ta