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

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

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> 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@collabora.com>
> ---
>  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:

--- 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@bootlin.com>

Thanks,
Miquèl

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