Re: [PATCH 1/3] mtd: rawnand: Add the concept of destructive operation

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

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()?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/