| Message ID | 20200502163432.1543243-2-boris.brezillon@collabora.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: brcmnand: Convert to exec_op() | expand |
On Sat, 2 May 2020 18:34:30 +0200 Boris Brezillon <boris.brezillon@collabora.com> wrote: > Erase and program operations need the WP (Write Protect) pin to be > de-asserted to take effect. Let's add the concept of destructive > operation and pass the information to exec_op() so controllers know > when they should de-assert this pin without having to guess it from > the command opcode. > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > include/linux/mtd/rawnand.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index c47cbcb86b71..6014e7389507 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -854,6 +854,8 @@ struct nand_op_parser { > /** > * struct nand_operation - NAND operation descriptor > * @cs: the CS line to select for this NAND operation > + * @deassert_wp: set to true when the operation requires the WP pin to be > + * de-asserted (ERASE, PROG, ...) > * @instrs: array of instructions to execute > * @ninstrs: length of the @instrs array > * > @@ -861,6 +863,7 @@ struct nand_op_parser { > */ > struct nand_operation { > unsigned int cs; > + bool deassert_wp; > const struct nand_op_instr *instrs; > unsigned int ninstrs; > }; > @@ -872,6 +875,14 @@ struct nand_operation { > .ninstrs = ARRAY_SIZE(_instrs), \ > } > > +#define NAND_DESTRUCTIVE_OPERATION(_cs, _instrs) \ > + { \ > + .cs = _cs, \ > + .deassert_wp = true, \ > + .instrs = _instrs, \ > + .ninstrs = ARRAY_SIZE(_instrs), \ > + } > + > int nand_op_parser_exec_op(struct nand_chip *chip, > const struct nand_op_parser *parser, > const struct nand_operation *op, bool check_only); The following diff should be part of this patch, otherwise none of the operations are flagged as destructive. I'll fix that in v2, but I'd still like to get feedback before sending a new version. --->8--- diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index fb8addf7637e..4111e7ac0834 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -1308,7 +1308,8 @@ static int nand_exec_prog_page_op(struct nand_chip *chip, unsigned int page, NAND_OP_CMD(NAND_CMD_PAGEPROG, PSEC_TO_NSEC(sdr->tWB_max)), NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tPROG_max), 0), }; - struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); + struct nand_operation op = NAND_DESTRUCTIVE_OPERATION(chip->cur_cs, + instrs); int naddrs = nand_fill_column_cycles(chip, addrs, offset_in_page); int ret; u8 status; @@ -1695,7 +1696,8 @@ int nand_erase_op(struct nand_chip *chip, unsigned int eraseblock) PSEC_TO_MSEC(sdr->tWB_max)), NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tBERS_max), 0), }; - struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); + struct nand_operation op = NAND_DESTRUCTIVE_OPERATION(chip->cur_cs, + instrs); if (chip->options & NAND_ROW_ADDR_3) instrs[1].ctx.addr.naddrs++;
Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 3 May 2020 10:00:18 +0200: > On Sat, 2 May 2020 18:34:30 +0200 > Boris Brezillon <boris.brezillon@collabora.com> wrote: > > > Erase and program operations need the WP (Write Protect) pin to be > > de-asserted to take effect. Let's add the concept of destructive > > operation and pass the information to exec_op() so controllers know > > when they should de-assert this pin without having to guess it from > > the command opcode. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > --- > > include/linux/mtd/rawnand.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > index c47cbcb86b71..6014e7389507 100644 > > --- a/include/linux/mtd/rawnand.h > > +++ b/include/linux/mtd/rawnand.h > > @@ -854,6 +854,8 @@ struct nand_op_parser { > > /** > > * struct nand_operation - NAND operation descriptor > > * @cs: the CS line to select for this NAND operation > > + * @deassert_wp: set to true when the operation requires the WP pin to be > > + * de-asserted (ERASE, PROG, ...) > > * @instrs: array of instructions to execute > > * @ninstrs: length of the @instrs array > > * > > @@ -861,6 +863,7 @@ struct nand_op_parser { > > */ > > struct nand_operation { > > unsigned int cs; > > + bool deassert_wp; > > const struct nand_op_instr *instrs; > > unsigned int ninstrs; > > }; > > @@ -872,6 +875,14 @@ struct nand_operation { > > .ninstrs = ARRAY_SIZE(_instrs), \ > > } > > > > +#define NAND_DESTRUCTIVE_OPERATION(_cs, _instrs) \ > > + { \ > > + .cs = _cs, \ > > + .deassert_wp = true, \ > > + .instrs = _instrs, \ > > + .ninstrs = ARRAY_SIZE(_instrs), \ > > + } > > + > > int nand_op_parser_exec_op(struct nand_chip *chip, > > const struct nand_op_parser *parser, > > const struct nand_operation *op, bool check_only); > > The following diff should be part of this patch, otherwise none of the > operations are flagged as destructive. I'll fix that in v2, but I'd still > like to get feedback before sending a new version. > > --->8--- > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index fb8addf7637e..4111e7ac0834 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -1308,7 +1308,8 @@ static int nand_exec_prog_page_op(struct nand_chip *chip, unsigned int page, > NAND_OP_CMD(NAND_CMD_PAGEPROG, PSEC_TO_NSEC(sdr->tWB_max)), > NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tPROG_max), 0), > }; > - struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); > + struct nand_operation op = NAND_DESTRUCTIVE_OPERATION(chip->cur_cs, > + instrs); > int naddrs = nand_fill_column_cycles(chip, addrs, offset_in_page); > int ret; > u8 status; > @@ -1695,7 +1696,8 @@ int nand_erase_op(struct nand_chip *chip, unsigned int eraseblock) > PSEC_TO_MSEC(sdr->tWB_max)), > NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tBERS_max), 0), > }; > - struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); > + struct nand_operation op = NAND_DESTRUCTIVE_OPERATION(chip->cur_cs, > + instrs); > > if (chip->options & NAND_ROW_ADDR_3) > instrs[1].ctx.addr.naddrs++; What about nand_prog_page_end_op() or even nand_write_change_column_op()?
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index c47cbcb86b71..6014e7389507 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -854,6 +854,8 @@ struct nand_op_parser { /** * struct nand_operation - NAND operation descriptor * @cs: the CS line to select for this NAND operation + * @deassert_wp: set to true when the operation requires the WP pin to be + * de-asserted (ERASE, PROG, ...) * @instrs: array of instructions to execute * @ninstrs: length of the @instrs array * @@ -861,6 +863,7 @@ struct nand_op_parser { */ struct nand_operation { unsigned int cs; + bool deassert_wp; const struct nand_op_instr *instrs; unsigned int ninstrs; }; @@ -872,6 +875,14 @@ struct nand_operation { .ninstrs = ARRAY_SIZE(_instrs), \ } +#define NAND_DESTRUCTIVE_OPERATION(_cs, _instrs) \ + { \ + .cs = _cs, \ + .deassert_wp = true, \ + .instrs = _instrs, \ + .ninstrs = ARRAY_SIZE(_instrs), \ + } + int nand_op_parser_exec_op(struct nand_chip *chip, const struct nand_op_parser *parser, const struct nand_operation *op, bool check_only);
Erase and program operations need the WP (Write Protect) pin to be de-asserted to take effect. Let's add the concept of destructive operation and pass the information to exec_op() so controllers know when they should de-assert this pin without having to guess it from the command opcode. Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- include/linux/mtd/rawnand.h | 11 +++++++++++ 1 file changed, 11 insertions(+)