Skip to content

Commit

Permalink
mtd: rawnand: nandsim: Artificially prevent sequential page reads
Browse files Browse the repository at this point in the history
The continuous read support added recently makes nandsim
unhappy. Indeed, all the supported commands should be re-encoded into
internal commands, so of course there is currently no support for the
commands and patterns needed for continuous reads to work.

I tried to add support for them but nandsim (which is more a tool to
develop/debug upper layers rather than the raw NAND core) suffers from a
big limitation: it's internal parser needs to know what exact operation
is happening when the address cycles are performed. The research is then
sequential from the start up to the address cycles, but does not check
what's coming next even though the information is available. This is a
limitation which is related to the old API used by the core which kind
of forced the controllers to guess what operation was being performed
rather early. Today the core uses a more transparent API called
->exec_op() which no longer requires controller drivers to do any more
guessing, but despite being updated to ->exec_op(), nandsim is still a
bit constrained on this regard and thus cannot handle sequential page
reads because the start sequence beginning is identical to a regular
page read.

If the internal algorithm is updated some day, it should be possible to
make it support sequential page reads by adding something like:

       /* Large page devices continuous read page start */
       {OPT_LARGEPAGE, {STATE_CMD_READ0, STATE_ADDR_PAGE, STATE_CMD_READSTART,
                        STATE_CMD_READCACHESEQ | ACTION_CPY, STATE_DATAOUT,
                        STATE_READY}},
       /* Large page devices continuous read page continue */
       {OPT_LARGEPAGE, {STATE_CMD_READCACHESEQ | ACTION_CPY_NEXT, STATE_DATAOUT,
                        STATE_READY}},
       /* Large page devices continuous read page end */
       {OPT_LARGEPAGE, {STATE_CMD_READCACHEEND | ACTION_CPY_NEXT, STATE_DATAOUT,
                        STATE_READY}},

For now, we just return -EOPNOTSUPP when the core asks controller
drivers if they support the feature in order to prevent any further use
of these opcodes.

Note: This is a hack, ->exec_op() is not supposed to check against the
COMMAND opcodes unless _really_ needed.

Fixes: 003fe4b ("mtd: rawnand: Support for sequential cache reads")
Reported-by: Zhihao Cheng <chengzhihao1@huawei.com>
Link: https://lore.kernel.org/linux-mtd/fd34fe55-7f4a-030d-8653-9bb9cf08410d@huawei.com/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Zhihao Cheng <chengzhihao1@huawei.com>
Acked-by: Richard Weinberger <richard@nod.at>
Link: https://lore.kernel.org/linux-mtd/20230310085452.1368716-1-miquel.raynal@bootlin.com
  • Loading branch information
Miquel Raynal committed Mar 22, 2023
1 parent a56cde4 commit 9b043c6
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion drivers/mtd/nand/raw/nandsim.c
Original file line number Diff line number Diff line change
Expand Up @@ -2160,8 +2160,23 @@ static int ns_exec_op(struct nand_chip *chip, const struct nand_operation *op,
const struct nand_op_instr *instr = NULL;
struct nandsim *ns = nand_get_controller_data(chip);

if (check_only)
if (check_only) {
/* The current implementation of nandsim needs to know the
* ongoing operation when performing the address cycles. This
* means it cannot make the difference between a regular read
* and a continuous read. Hence, this hack to manually refuse
* supporting sequential cached operations.
*/
for (op_id = 0; op_id < op->ninstrs; op_id++) {
instr = &op->instrs[op_id];
if (instr->type == NAND_OP_CMD_INSTR &&
(instr->ctx.cmd.opcode == NAND_CMD_READCACHEEND ||
instr->ctx.cmd.opcode == NAND_CMD_READCACHESEQ))
return -EOPNOTSUPP;
}

return 0;
}

ns->lines.ce = 1;

Expand Down

0 comments on commit 9b043c6

Please sign in to comment.