Messages in this thread | | | Date | Mon, 13 Mar 2023 12:23:12 +0100 | From | Neil Armstrong <> | Subject | Re: [RFC PATCH v1] mtd: rawnand: meson: invalidate cache on polling ECC bit |
| |
On 13/03/2023 12:18, Miquel Raynal wrote: > Hi Arseniy, > > avkrasnov@sberdevices.ru wrote on Mon, 13 Mar 2023 10:36:11 +0300: > >> Hello, >> >> we reproduced this problem on one of our boards. It triggers very rare >> when 'usleep_range()' is present, but when sleeping is removed - it fires >> always. I suppose problem is with caching, as 'info_buf' memory is mapped by >> 'dma_map_single()'. > > The fix looks really legitimate, indeed I get that the usleep_range() > might make it work most of the time but not always. Having this bit in > a DMA buf area is a bit strange. Well, the fix LGTM anyway.
Yep it looks legitimate!
LGTM
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> >> >> Thanks, Arseniy >> >> On 13.03.2023 10:32, Arseniy Krasnov wrote: >>> 'info_buf' memory is cached and driver polls ECC bit in it. This bit >>> is set by the NAND controller. If 'usleep_range()' returns before device >>> sets this bit, 'info_buf' will be cached and driver won't see update of >>> this bit and will loop forever. >>> >>> Fixes: 8fae856c5350 ("mtd: rawnand: meson: add support for Amlogic NAND flash controller") >>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>> --- >>> drivers/mtd/nand/raw/meson_nand.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c >>> index 5ee01231ac4c..2c05c08a0eaf 100644 >>> --- a/drivers/mtd/nand/raw/meson_nand.c >>> +++ b/drivers/mtd/nand/raw/meson_nand.c >>> @@ -176,6 +176,7 @@ struct meson_nfc { >>> >>> dma_addr_t daddr; >>> dma_addr_t iaddr; >>> + u32 info_bytes; >>> >>> unsigned long assigned_cs; >>> }; >>> @@ -503,6 +504,7 @@ static int meson_nfc_dma_buffer_setup(struct nand_chip *nand, void *databuf, >>> nfc->daddr, datalen, dir); >>> return ret; >>> } >>> + nfc->info_bytes = infolen; >>> cmd = GENCMDIADDRL(NFC_CMD_AIL, nfc->iaddr); >>> writel(cmd, nfc->reg_base + NFC_REG_CMD); >>> >>> @@ -520,8 +522,10 @@ static void meson_nfc_dma_buffer_release(struct nand_chip *nand, >>> struct meson_nfc *nfc = nand_get_controller_data(nand); >>> >>> dma_unmap_single(nfc->dev, nfc->daddr, datalen, dir); >>> - if (infolen) >>> + if (infolen) { >>> dma_unmap_single(nfc->dev, nfc->iaddr, infolen, dir); >>> + nfc->info_bytes = 0; >>> + } >>> } >>> >>> static int meson_nfc_read_buf(struct nand_chip *nand, u8 *buf, int len) >>> @@ -710,6 +714,8 @@ static void meson_nfc_check_ecc_pages_valid(struct meson_nfc *nfc, >>> usleep_range(10, 15); >>> /* info is updated by nfc dma engine*/ >>> smp_rmb(); >>> + dma_sync_single_for_cpu(nfc->dev, nfc->iaddr, nfc->info_bytes, >>> + DMA_FROM_DEVICE); >>> ret = *info & ECC_COMPLETE; >>> } while (!ret); >>> } > > > Thanks, > Miquèl
| |