Re: [PATCH 1/2] mtd: rawnand: Propage CS selection to sub operations

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon, 27 Apr 2020 17:00:02 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Sat, 18 Apr
> 2020 21:49:58 +0200:
> 
> > Some controller using the instruction parse infrastructure might need
> > to know which CS a specific sub-operation is targeting. Let's propagate
> > this information.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 1 +
> >  include/linux/mtd/rawnand.h      | 2 ++
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index c24e5e2ba130..fa9ac18e97a1 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -2176,6 +2176,7 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
> >  			   const struct nand_operation *op, bool check_only)
> >  {
> >  	struct nand_op_parser_ctx ctx = {
> > +		.subop.cs = op->cs,  
> 
> I think this information might become important, could you as well add
> it to the parser tracer? Something like this would do the trick:
> 

Sure, I'll do that. Thanks for the diff.

> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -2112,7 +2112,7 @@ static void nand_op_parser_trace(const struct nand_op_parser_ctx *ctx)
>         char *prefix = "      ";
>         unsigned int i;
>  
> -       pr_debug("executing subop:\n");
> +       pr_debug("executing subop (CS%d):\n", ctx->subop.cs);
>  
>         for (i = 0; i < ctx->ninstrs; i++) {
>                 instr = &ctx->instrs[i];
> 
> >  		.subop.instrs = op->instrs,
> >  		.instrs = op->instrs,
> >  		.ninstrs = op->ninstrs,
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 1e76196f9829..8e8d1a61e2fb 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -694,6 +694,7 @@ struct nand_op_instr {
> >  
> >  /**
> >   * struct nand_subop - a sub operation
> > + * @cs: the CS line to select for this NAND sub-operation
> >   * @instrs: array of instructions
> >   * @ninstrs: length of the @instrs array
> >   * @first_instr_start_off: offset to start from for the first instruction
> > @@ -709,6 +710,7 @@ struct nand_op_instr {
> >   * controller driver.
> >   */
> >  struct nand_subop {
> > +	unsigned int cs;
> >  	const struct nand_op_instr *instrs;
> >  	unsigned int ninstrs;
> >  	unsigned int first_instr_start_off;  
> 
> With this small addition:
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> 
> Thanks,
> Miquèl


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