2018-09-13 06:01:27

by Masahiro Yamada

Subject: [PATCH] mtd: rawnand: denali: fix a race condition when DMA is kicked

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 <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

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);
--
2.7.4

2018-09-14 14:30:05

by Miquel Raynal

Subject: Re: [PATCH] mtd: rawnand: denali: fix a race condition when DMA is kicked

Hello,

Masahiro Yamada <[email protected]> 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 <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>

Reviewed-by: Miquel Raynal <[email protected]>


Thanks,
Miquèl

2018-09-17 12:33:27

by Boris Brezillon

Subject: Re: [PATCH] mtd: rawnand: denali: fix a race condition when DMA is kicked

On Thu, 13 Sep 2018 14:58:49 +0900
Masahiro Yamada <[email protected]> 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 <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>

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);