From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1ghJmj-0007D0-E9 for linux-mtd@lists.infradead.org; Wed, 09 Jan 2019 19:45:07 +0000 Date: Wed, 9 Jan 2019 20:44:51 +0100 From: Boris Brezillon To: Linus Walleij Cc: Boris Brezillon , Maxim Levitsky , Tudor Ambarus , Masahiro Yamada , Richard Weinberger , Marc Gonzalez , Janusz Krzysztofik , Stefan Agner , Marek Vasut , Harvey Hunt , linux-mtd@lists.infradead.org, Miquel Raynal , Han Xu , Xiaolei Li , Brian Norris , David Woodhouse Subject: Re: [PATCH v3 15/22] mtd: rawnand: fsmc: Stop implementing ->select_chip() Message-ID: <20190109204441.6aeab463@bbrezillon> In-Reply-To: References: <20181111075524.13123-1-boris.brezillon@bootlin.com> <20181111075524.13123-16-boris.brezillon@bootlin.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, 9 Jan 2019 19:18:58 +0100 Linus Walleij wrote: > On Sun, Nov 11, 2018 at 8:59 AM Boris Brezillon > wrote: > > > Now that the CS line to assert is directly passed through the > > nand_operation struct we can replace the fsmc_select_chip() > > implementation by an internal fsmc_ce_ctrl() function which is > > directly called from fsmc_exec_op() > > > > Signed-off-by: Boris Brezillon > > This commit regresses the Nomadik NHK15 in a curious way. > The code seems fine but after a few reads to the chip it crashes > (trace with debug prints enabled): > > fsmc-nand 10100000.flash: FSMC device partno 090, manufacturer 80, > revision 00, config 00 > Executing operation [2 instructions]: > ->CMD [0xff] > ->WAITRDY [max 250 ms] > Executing operation [1 instructions]: > ->CMD [0x70] > Executing operation [1 instructions]: > ->DATA_IN [1 B, force 8-bit] > Executing operation [1 instructions]: > ->CMD [0x00] > Executing operation [3 instructions]: > ->CMD [0x90] > ->ADDR [1 cyc] > ->DATA_IN [2 B, force 8-bit] > Executing operation [3 instructions]: > ->CMD [0x90] > ->ADDR [1 cyc] > ->DATA_IN [8 B, force 8-bit] > Executing operation [3 instructions]: > ->CMD [0x90] > ->ADDR [1 cyc] > ->DATA_IN [4 B, force 8-bit] > Executing operation [3 instructions]: > ->CMD [0x90] > ->ADDR [1 cyc] > ->DATA_IN [5 B, force 8-bit] > nand: device found, Manufacturer ID: 0x20, Chip ID: 0xa1 > nand: ST Micro NAND 128MiB 1,8V 8-bit > nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 > fsmc-nand 10100000.flash: Using 1-bit HW ECC scheme > Scanning device for bad blocks > Executing operation [5 instructions]: > ->CMD [0x00] > ->ADDR [4 cyc] > ->CMD [0x30] > ->WAITRDY [max 200000 ms] > Executing operation [1 instructions]: > ->CMD [0x70] > Executing operation [1 instructions]: > ->DATA_IN [1 B, force 8-bit] > Executing operation [1 instructions]: > ->CMD [0x00] > ->DATA_IN [64 B] > Unhandled fault: external abort on non-linefetch (0x008) at 0xcc960000 > pgd = (ptrval) > [cc960000] *pgd=0b808811, *pte=40000653, *ppte=40000552 > Internal error: : 8 [#1] PREEMPT ARM > Modules linked in: > CPU: 0 PID: 1 Comm: swapper Not tainted 5.0.0-rc1+ #84 > Hardware name: Nomadik STn8815 > PC is at fsmc_exec_op+0x180/0x284 > LR is at fsmc_exec_op+0x144/0x284 > pc : [] lr : [] psr: 20000013 > sp : cb82ba88 ip : c092e6f8 fp : c0644078 > r10: c0615ae8 r9 : cb9d5020 r8 : 00000000 > r7 : cb9d5038 r6 : cb82bac4 r5 : 00000004 r4 : cb82bb20 > r3 : cb8807fc r2 : cb88083c r1 : cc960000 r0 : 00000013 > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 0005317f Table: 0b334000 DAC: 00000053 > Process swapper (pid: 1, stack limit = 0x(ptrval)) > (...) > [] (fsmc_exec_op) from [] > (nand_lp_exec_read_page_op+0x1cc/0x224) > [] (nand_lp_exec_read_page_op) from [] > (nand_read_page_op+0xdc/0x2f0) > [] (nand_read_page_op) from [] (nand_read_oob_std+0x1c/0x24) > [] (nand_read_oob_std) from [] (nand_read_oob+0x504/0x708) > [] (nand_read_oob) from [] (mtd_read_oob+0x54/0xcc) > [] (mtd_read_oob) from [] (create_bbt+0x120/0x290) > [] (create_bbt) from [] (nand_create_bbt+0x4f8/0x69c) > [] (nand_create_bbt) from [] (nand_scan_tail+0x9c4/0xb04) > [] (nand_scan_tail) from [] (nand_scan_with_ids+0x760/0xab4) > [] (nand_scan_with_ids) from [] > (fsmc_nand_probe+0x454/0x578) > [] (fsmc_nand_probe) from [] (platform_drv_probe+0x48/0x98) > [] (platform_drv_probe) from [] (really_probe+0x224/0x2d4) > [] (really_probe) from [] (driver_probe_device+0x5c/0x16c) > [] (driver_probe_device) from [] (__driver_attach+0xd0/0xd4) > [] (__driver_attach) from [] (bus_for_each_dev+0x70/0xb4) > [] (bus_for_each_dev) from [] (bus_add_driver+0x170/0x204) > [] (bus_add_driver) from [] (driver_register+0x74/0x108) > [] (driver_register) from [] > (__platform_driver_probe+0x58/0x124) > [] (__platform_driver_probe) from [] > (do_one_initcall+0x48/0x1a0) > [] (do_one_initcall) from [] > (kernel_init_freeable+0x104/0x1c8) > [] (kernel_init_freeable) from [] (kernel_init+0x8/0xf4) > [] (kernel_init) from [] (ret_from_fork+0x14/0x34) > (...) > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > After looking at it I realized that if I uncomment this line: > > // fsmc_ce_ctrl(host, false); > > The driver works fine, so just holding CE enabled all the time makes > everything work. > > I suspect it is some kind of timing issue maybe platform- or electronics > dependent where you need to hold CE enabled for a while before > reading pages from the NAND. This would explain why it is not seen > in the platform this was developed on. > > I will experiment with some delay valued and try to read some data > sheets but if you already have hints on how to deal with this I'd > like to hear! Might be caused by a missing barrier: when de-asserting the CE line, we must make sure all accesses to the ->data_va range have been done. Can you try with the following diff applied? --->8--- diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c index 325b4414dccc..264d809c2d37 100644 --- a/drivers/mtd/nand/raw/fsmc_nand.c +++ b/drivers/mtd/nand/raw/fsmc_nand.c @@ -598,16 +598,21 @@ static void fsmc_ce_ctrl(struct fsmc_nand_data *host, bool assert) { u32 pc = readl(host->regs_va + FSMC_PC); - if (!assert) + if (!assert) { + /* + * Make sure all previous read/write have been done before + * de-asserting the CE line. + */ + mb(); writel_relaxed(pc & ~FSMC_ENABLE, host->regs_va + FSMC_PC); - else + } else { writel_relaxed(pc | FSMC_ENABLE, host->regs_va + FSMC_PC); - - /* - * nCE line changes must be applied before returning from this - * function. - */ - mb(); + /* + * nCE assertion must be applied before returning from this + * function. + */ + mb(); + } } /*