| Message ID | 1536818329-11068-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: rawnand: denali: fix a race condition when DMA is kicked | expand |
Hello, Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Thu, 13 Sep 2018 14:58:49 +0900: > I thought the read-back of the DMA_ENABLE register was unnecessary > (at least it is working on my boards), then deleted it in commit > 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper > into caller"). Sorry, I was wrong - it caused a timing issue on > Cyclone5 SoCFPGAs. > > Revive the register read-back, commenting why this is necessary. > > Fixes: 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper into caller") > Reported-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
On Thu, 13 Sep 2018 14:58:49 +0900 Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > I thought the read-back of the DMA_ENABLE register was unnecessary > (at least it is working on my boards), then deleted it in commit > 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper > into caller"). Sorry, I was wrong - it caused a timing issue on > Cyclone5 SoCFPGAs. > > Revive the register read-back, commenting why this is necessary. > > Fixes: 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper into caller") > Reported-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Queued to master. Thanks, Boris > --- > > drivers/mtd/nand/raw/denali.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c > index 67b2065..b864b93 100644 > --- a/drivers/mtd/nand/raw/denali.c > +++ b/drivers/mtd/nand/raw/denali.c > @@ -596,6 +596,12 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf, > } > > iowrite32(DMA_ENABLE__FLAG, denali->reg + DMA_ENABLE); > + /* > + * The ->setup_dma() hook kicks DMA by using the data/command > + * interface, which belongs to a different AXI port from the > + * register interface. Read back the register to avoid a race. > + */ > + ioread32(denali->reg + DMA_ENABLE); > > denali_reset_irq(denali); > denali->setup_dma(denali, dma_addr, page, write);
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 67b2065..b864b93 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -596,6 +596,12 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf, } iowrite32(DMA_ENABLE__FLAG, denali->reg + DMA_ENABLE); + /* + * The ->setup_dma() hook kicks DMA by using the data/command + * interface, which belongs to a different AXI port from the + * register interface. Read back the register to avoid a race. + */ + ioread32(denali->reg + DMA_ENABLE); denali_reset_irq(denali); denali->setup_dma(denali, dma_addr, page, write);
I thought the read-back of the DMA_ENABLE register was unnecessary (at least it is working on my boards), then deleted it in commit 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper into caller"). Sorry, I was wrong - it caused a timing issue on Cyclone5 SoCFPGAs. Revive the register read-back, commenting why this is necessary. Fixes: 586a2c52909d ("mtd: nand: denali: squash denali_enable_dma() helper into caller") Reported-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- drivers/mtd/nand/raw/denali.c | 6 ++++++ 1 file changed, 6 insertions(+)