Message ID | 20200418194959.1017197-1-boris.brezillon@collabora.com |
---|---|
State | Changes Requested |
Delegated to: | Miquel Raynal |
Headers | show |
Series | [1/2] mtd: rawnand: Propage CS selection to sub operations | expand |
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
On Mon, 27 Apr 2020 17:00:02 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > 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: > 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@bootlin.com> > > Thanks, > Miquèl
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, .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;
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(+)