Re: [PATCH v4 7/8] mtd: rawnand: arasan: Add new Arasan NAND controller

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 10 May
2020 09:03:14 +0200:

> On Fri,  8 May 2020 19:13:38 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > +static int anfc_exec_op(struct nand_chip *chip,
> > +			const struct nand_operation *op,
> > +			bool check_only)
> > +{
> > +	int ret;
> > +
> > +	if (check_only)
> > +		return nand_op_parser_exec_op(chip, &anfc_op_parser, op,
> > +					      check_only);  
> 
> You should also check the DATA_IN/OUT size here ^.

Here is my proposal:

---8<---

+static int anfc_check_op(struct nand_chip *chip,
+                        const struct nand_operation *op)
+{
+       int op_id;
+
+       /*
+        * The controller abstracts all the NAND operations and do not support
+        * data only operations.
+        */
+       for (op_id = 0; op_id < op->ninstrs; op_id++) {
+               instr = &op->instrs[op_id];
+
+               switch (instr->type) {
+               case NAND_OP_ADDR_INSTR:
+                       if (instr->ctx.addr.naddrs > ANFC_MAX_ADDR_CYC)
+                               return -ENOTSUPP;
+                       break;
+               case NAND_OP_DATA_IN_INSTR:
+               case NAND_OP_DATA_OUT_INSTR:
+                       if (instr->ctx.data.len > ANFC_MAX_CHUNK_SIZE)
+                               return -ENOTSUPP;
+                       break;
+               default:
+               }
+       }
+
+       /*
+        * The controller does not allow to proceed with a CMD+DATA_IN cycle
+        * manually on the bus by reading data from the data register. Instead,
+        * the controller abstract a status read operation with its own status
+        * register after ordering a read status operation. Hence, we cannot
+        * support any CMD+DATA_IN operation other than a READ STATUS.
+        */
+       if (op->ninstrs == 2 &&
+           op->instrs[0].type == NAND_OP_CMD_INSTR &&
+           op->instrs[0].ctx.cmd.opcode != NAND_CMD_STATUS &&
+           op->instrs[1].type == NAND_OP_DATA_IN_INSTR)
+               return -ENOTSUPP;
+
+       return nand_op_parser_exec_op(chip, &anfc_op_parser, op,
+                                     check_only);
+}
+
 static int anfc_exec_op(struct nand_chip *chip,
                        const struct nand_operation *op,
                        bool check_only)
@@ -774,8 +813,7 @@ static int anfc_exec_op(struct nand_chip *chip,
        int ret;
 
        if (check_only)
-               return nand_op_parser_exec_op(chip, &anfc_op_parser, op,
-                                             check_only);
+               return anfc_check_op(chip, op);
 
        ret = anfc_select_target(chip, op->cs);
        if (ret)

--->8---

What do you think?