| Message ID | 20180503074544.19938-1-boris.brezillon@bootlin.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: rawnand: fsmc: Make sure we wait tWB before polling the STATUS reg | expand |
Hi Boris, On Thu, 3 May 2018 09:45:44 +0200, Boris Brezillon <boris.brezillon@bootlin.com> wrote: > NAND chips require a bit of time to take the NAND operation into account > and set the BUSY bit in the STATUS reg. Make sure we don't poll the > STATUS reg too early. > > Fixes: 4da712e70294 ("mtd: nand: fsmc: use ->exec_op()") > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/nand/raw/fsmc_nand.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c > index 28c48dcc514e..0960665858b7 100644 > --- a/drivers/mtd/nand/raw/fsmc_nand.c > +++ b/drivers/mtd/nand/raw/fsmc_nand.c > @@ -18,6 +18,7 @@ > > #include <linux/clk.h> > #include <linux/completion.h> > +#include <linux/delay.h> > #include <linux/dmaengine.h> > #include <linux/dma-direction.h> > #include <linux/dma-mapping.h> > @@ -695,6 +696,13 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op, > pr_debug(" ->WAITRDY [max %d ms]\n", > instr->ctx.waitrdy.timeout_ms); > > + /* > + * Make sure we wait tWB before polling the STATUS > + * register. > + */ > + if (op_id && op->instrs[op_id - 1].delay_ns) > + ndelay(op->instrs[op_id - 1].delay_ns); > + > ret = nand_soft_waitrdy(chip, > instr->ctx.waitrdy.timeout_ms); > break; I'm afraid that we encounter this exact same issue with all the drivers using nand_soft_waitrdy() whenever the controller does not already wait for tWB. Could we force a tWB_max delay directly inside nand_soft_waitrdy()? Thanks, Miquèl
On Fri, 4 May 2018 11:45:10 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Boris, > > On Thu, 3 May 2018 09:45:44 +0200, Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > > NAND chips require a bit of time to take the NAND operation into account > > and set the BUSY bit in the STATUS reg. Make sure we don't poll the > > STATUS reg too early. > > > > Fixes: 4da712e70294 ("mtd: nand: fsmc: use ->exec_op()") > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/mtd/nand/raw/fsmc_nand.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c > > index 28c48dcc514e..0960665858b7 100644 > > --- a/drivers/mtd/nand/raw/fsmc_nand.c > > +++ b/drivers/mtd/nand/raw/fsmc_nand.c > > @@ -18,6 +18,7 @@ > > > > #include <linux/clk.h> > > #include <linux/completion.h> > > +#include <linux/delay.h> > > #include <linux/dmaengine.h> > > #include <linux/dma-direction.h> > > #include <linux/dma-mapping.h> > > @@ -695,6 +696,13 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op, > > pr_debug(" ->WAITRDY [max %d ms]\n", > > instr->ctx.waitrdy.timeout_ms); > > > > + /* > > + * Make sure we wait tWB before polling the STATUS > > + * register. > > + */ > > + if (op_id && op->instrs[op_id - 1].delay_ns) > > + ndelay(op->instrs[op_id - 1].delay_ns); > > + > > ret = nand_soft_waitrdy(chip, > > instr->ctx.waitrdy.timeout_ms); > > break; > > I'm afraid that we encounter this exact same issue with all the drivers > using nand_soft_waitrdy() whenever the controller does not already > wait for tWB. Could we force a tWB_max delay directly inside > nand_soft_waitrdy()? Yep, I agree. I'll send a new version doing that. Thanks, Boris
diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c index 28c48dcc514e..0960665858b7 100644 --- a/drivers/mtd/nand/raw/fsmc_nand.c +++ b/drivers/mtd/nand/raw/fsmc_nand.c @@ -18,6 +18,7 @@ #include <linux/clk.h> #include <linux/completion.h> +#include <linux/delay.h> #include <linux/dmaengine.h> #include <linux/dma-direction.h> #include <linux/dma-mapping.h> @@ -695,6 +696,13 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op, pr_debug(" ->WAITRDY [max %d ms]\n", instr->ctx.waitrdy.timeout_ms); + /* + * Make sure we wait tWB before polling the STATUS + * register. + */ + if (op_id && op->instrs[op_id - 1].delay_ns) + ndelay(op->instrs[op_id - 1].delay_ns); + ret = nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms); break;
NAND chips require a bit of time to take the NAND operation into account and set the BUSY bit in the STATUS reg. Make sure we don't poll the STATUS reg too early. Fixes: 4da712e70294 ("mtd: nand: fsmc: use ->exec_op()") Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/mtd/nand/raw/fsmc_nand.c | 8 ++++++++ 1 file changed, 8 insertions(+)