mtd: rawnand: use longest matching pattern
authorStefan Agner <stefan@agner.ch>
Fri, 19 Apr 2019 07:47:17 +0000 (09:47 +0200)
committerMiquel Raynal <miquel.raynal@bootlin.com>
Thu, 27 Jun 2019 18:05:22 +0000 (20:05 +0200)
Sometimes the exec_op parser does not choose the optimal pattern if
multiple patterns with optional elements are available. Since the stack
automatically splits operations in multiple exec_op calls, a non-optimal
pattern gets broken up into multiple calls. E.g. an OOB read using the
vf610 driver:
  nand: executing subop:
  nand:     ->CMD      [0x00]
  nand:     ->ADDR     [5 cyc: 00 08 ea 94 02]
  nand:     ->CMD      [0x30]
  nand:     ->WAITRDY  [max 200000 ms]
  nand:       DATA_IN  [64 B]
  nand: executing subop:
  nand:       CMD      [0x00]
  nand:       ADDR     [5 cyc: 00 08 ea 94 02]
  nand:       CMD      [0x30]
  nand:       WAITRDY  [max 200000 ms]
  nand:     ->DATA_IN  [64 B]

However, the vf610 driver has a pattern which can execute the complete
command in a single go...

This patch makes sure that the longest matching pattern is chosen
instead of the first (potentially only partial) match. With this
change the vf610 reads the OOB in a single exec_op call:
  nand: executing subop:
  nand:     ->CMD      [0x00]
  nand:     ->ADDR     [5 cyc: 00 08 c0 1d 00]
  nand:     ->CMD      [0x30]
  nand:     ->WAITRDY  [max 200000 ms]
  nand:     ->DATA_IN  [64 B]

Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
drivers/mtd/nand/raw/nand_base.c

index 2cf71060d6f8459c7377ee2c29303efd5c23ec45..d565b4a9dce1a93026fc37452ba1463009b4ba27 100644 (file)
@@ -2156,6 +2156,22 @@ static void nand_op_parser_trace(const struct nand_op_parser_ctx *ctx)
 }
 #endif
 
+static int nand_op_parser_cmp_ctx(const struct nand_op_parser_ctx *a,
+                                 const struct nand_op_parser_ctx *b)
+{
+       if (a->subop.ninstrs < b->subop.ninstrs)
+               return -1;
+       else if (a->subop.ninstrs > b->subop.ninstrs)
+               return 1;
+
+       if (a->subop.last_instr_end_off < b->subop.last_instr_end_off)
+               return -1;
+       else if (a->subop.last_instr_end_off > b->subop.last_instr_end_off)
+               return 1;
+
+       return 0;
+}
+
 /**
  * nand_op_parser_exec_op - exec_op parser
  * @chip: the NAND chip
@@ -2190,32 +2206,40 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
        unsigned int i;
 
        while (ctx.subop.instrs < op->instrs + op->ninstrs) {
-               int ret;
+               const struct nand_op_parser_pattern *pattern;
+               struct nand_op_parser_ctx best_ctx;
+               int ret, best_pattern = -1;
 
                for (i = 0; i < parser->npatterns; i++) {
-                       const struct nand_op_parser_pattern *pattern;
+                       struct nand_op_parser_ctx test_ctx = ctx;
 
                        pattern = &parser->patterns[i];
-                       if (!nand_op_parser_match_pat(pattern, &ctx))
+                       if (!nand_op_parser_match_pat(pattern, &test_ctx))
                                continue;
 
-                       nand_op_parser_trace(&ctx);
-
-                       if (check_only)
-                               break;
-
-                       ret = pattern->exec(chip, &ctx.subop);
-                       if (ret)
-                               return ret;
+                       if (best_pattern >= 0 &&
+                           nand_op_parser_cmp_ctx(&test_ctx, &best_ctx) <= 0)
+                               continue;
 
-                       break;
+                       best_pattern = i;
+                       best_ctx = test_ctx;
                }
 
-               if (i == parser->npatterns) {
+               if (best_pattern < 0) {
                        pr_debug("->exec_op() parser: pattern not found!\n");
                        return -ENOTSUPP;
                }
 
+               ctx = best_ctx;
+               nand_op_parser_trace(&ctx);
+
+               if (!check_only) {
+                       pattern = &parser->patterns[best_pattern];
+                       ret = pattern->exec(chip, &ctx.subop);
+                       if (ret)
+                               return ret;
+               }
+
                /*
                 * Update the context structure by pointing to the start of the
                 * next subop.