| Message ID | 20181111075524.13123-16-boris.brezillon@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: rawnand: 3rd batch of cleanup | expand |
On Sun, Nov 11, 2018 at 8:59 AM Boris Brezillon <boris.brezillon@bootlin.com> 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 <boris.brezillon@bootlin.com> 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 : [<c0361f18>] lr : [<c0361edc>] 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)) (...) [<c0361f18>] (fsmc_exec_op) from [<c0355834>] (nand_lp_exec_read_page_op+0x1cc/0x224) [<c0355834>] (nand_lp_exec_read_page_op) from [<c0355968>] (nand_read_page_op+0xdc/0x2f0) [<c0355968>] (nand_read_page_op) from [<c0355c48>] (nand_read_oob_std+0x1c/0x24) [<c0355c48>] (nand_read_oob_std) from [<c0359444>] (nand_read_oob+0x504/0x708) [<c0359444>] (nand_read_oob) from [<c0347548>] (mtd_read_oob+0x54/0xcc) [<c0347548>] (mtd_read_oob) from [<c035cea8>] (create_bbt+0x120/0x290) [<c035cea8>] (create_bbt) from [<c035e87c>] (nand_create_bbt+0x4f8/0x69c) [<c035e87c>] (nand_create_bbt) from [<c035b00c>] (nand_scan_tail+0x9c4/0xb04) [<c035b00c>] (nand_scan_tail) from [<c035b8ac>] (nand_scan_with_ids+0x760/0xab4) [<c035b8ac>] (nand_scan_with_ids) from [<c06ca508>] (fsmc_nand_probe+0x454/0x578) [<c06ca508>] (fsmc_nand_probe) from [<c03071d8>] (platform_drv_probe+0x48/0x98) [<c03071d8>] (platform_drv_probe) from [<c0305620>] (really_probe+0x224/0x2d4) [<c0305620>] (really_probe) from [<c0305830>] (driver_probe_device+0x5c/0x16c) [<c0305830>] (driver_probe_device) from [<c0305a10>] (__driver_attach+0xd0/0xd4) [<c0305a10>] (__driver_attach) from [<c0303798>] (bus_for_each_dev+0x70/0xb4) [<c0303798>] (bus_for_each_dev) from [<c0304a74>] (bus_add_driver+0x170/0x204) [<c0304a74>] (bus_add_driver) from [<c03063ac>] (driver_register+0x74/0x108) [<c03063ac>] (driver_register) from [<c0307390>] (__platform_driver_probe+0x58/0x124) [<c0307390>] (__platform_driver_probe) from [<c000a604>] (do_one_initcall+0x48/0x1a0) [<c000a604>] (do_one_initcall) from [<c06b3dd4>] (kernel_init_freeable+0x104/0x1c8) [<c06b3dd4>] (kernel_init_freeable) from [<c04fcd24>] (kernel_init+0x8/0xf4) [<c04fcd24>] (kernel_init) from [<c00090e0>] (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! Yours, Linus Walleij
On Wed, 9 Jan 2019 19:18:58 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Sun, Nov 11, 2018 at 8:59 AM Boris Brezillon > <boris.brezillon@bootlin.com> 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 <boris.brezillon@bootlin.com> > > 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 : [<c0361f18>] lr : [<c0361edc>] 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)) > (...) > [<c0361f18>] (fsmc_exec_op) from [<c0355834>] > (nand_lp_exec_read_page_op+0x1cc/0x224) > [<c0355834>] (nand_lp_exec_read_page_op) from [<c0355968>] > (nand_read_page_op+0xdc/0x2f0) > [<c0355968>] (nand_read_page_op) from [<c0355c48>] (nand_read_oob_std+0x1c/0x24) > [<c0355c48>] (nand_read_oob_std) from [<c0359444>] (nand_read_oob+0x504/0x708) > [<c0359444>] (nand_read_oob) from [<c0347548>] (mtd_read_oob+0x54/0xcc) > [<c0347548>] (mtd_read_oob) from [<c035cea8>] (create_bbt+0x120/0x290) > [<c035cea8>] (create_bbt) from [<c035e87c>] (nand_create_bbt+0x4f8/0x69c) > [<c035e87c>] (nand_create_bbt) from [<c035b00c>] (nand_scan_tail+0x9c4/0xb04) > [<c035b00c>] (nand_scan_tail) from [<c035b8ac>] (nand_scan_with_ids+0x760/0xab4) > [<c035b8ac>] (nand_scan_with_ids) from [<c06ca508>] > (fsmc_nand_probe+0x454/0x578) > [<c06ca508>] (fsmc_nand_probe) from [<c03071d8>] (platform_drv_probe+0x48/0x98) > [<c03071d8>] (platform_drv_probe) from [<c0305620>] (really_probe+0x224/0x2d4) > [<c0305620>] (really_probe) from [<c0305830>] (driver_probe_device+0x5c/0x16c) > [<c0305830>] (driver_probe_device) from [<c0305a10>] (__driver_attach+0xd0/0xd4) > [<c0305a10>] (__driver_attach) from [<c0303798>] (bus_for_each_dev+0x70/0xb4) > [<c0303798>] (bus_for_each_dev) from [<c0304a74>] (bus_add_driver+0x170/0x204) > [<c0304a74>] (bus_add_driver) from [<c03063ac>] (driver_register+0x74/0x108) > [<c03063ac>] (driver_register) from [<c0307390>] > (__platform_driver_probe+0x58/0x124) > [<c0307390>] (__platform_driver_probe) from [<c000a604>] > (do_one_initcall+0x48/0x1a0) > [<c000a604>] (do_one_initcall) from [<c06b3dd4>] > (kernel_init_freeable+0x104/0x1c8) > [<c06b3dd4>] (kernel_init_freeable) from [<c04fcd24>] (kernel_init+0x8/0xf4) > [<c04fcd24>] (kernel_init) from [<c00090e0>] (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(); + } } /*
On Wed, Jan 9, 2019 at 8:45 PM Boris Brezillon <bbrezillon@kernel.org> wrote: > [Me] > > 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? Tried it, but it sadly does not help :/ The machine crashes at the same place when doing ther first 64 bytes read. I suppose the old code would hold CE enabled across all the commands? Since the old code contained this: /* Support only one CS */ if (chipnr > 0) return; I guess that is normal for FSMC: only one CS. It seems a bit aggressive to toggle CS on/off between every command like this, I suspect the FSMC isn't really built for that but shakes apart or something. I will send a version of your patch that keeps CS enabled for your consideration. (worksforme). Yours, Linus Walleij
On Wed, 9 Jan 2019 21:41:24 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Jan 9, 2019 at 8:45 PM Boris Brezillon <bbrezillon@kernel.org> wrote: > > [Me] > > > 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? > > Tried it, but it sadly does not help :/ > > The machine crashes at the same place when doing ther first > 64 bytes read. > > I suppose the old code would hold CE enabled across all the > commands? For a read/write page accesses, yes. > > Since the old code contained this: > > /* Support only one CS */ > if (chipnr > 0) > return; > > I guess that is normal for FSMC: only one CS. It seems a bit > aggressive to toggle CS on/off between every command like this, > I suspect the FSMC isn't really built for that but shakes apart > or something. Hm, that would be weird. There's indeed timing constraints on the NAND chip side, but none infringing those constraints should not trigger an external abort exception. Can you check which phys range is remapped at 0xcc960000? > > I will send a version of your patch that keeps CS enabled > for your consideration. (worksforme). I guess that's fine to keep CE enabled if the bus is not shared with other memories.
On Wed, Jan 9, 2019 at 9:54 PM Boris Brezillon <bbrezillon@kernel.org> wrote: > [Me] > > I guess that is normal for FSMC: only one CS. It seems a bit > > aggressive to toggle CS on/off between every command like this, > > I suspect the FSMC isn't really built for that but shakes apart > > or something. > > Hm, that would be weird. There's indeed timing constraints on the NAND > chip side, but none infringing those constraints should not trigger an > external abort exception. The manual contains this (the same bit, just another name than in the driver): PBKEN PC-card/NAND-Flash chip-select enable. Enables the corresponding chip-select. If a disabled chip-select is accessed, an HRESP = ERROR is generated on the AHB bus. 0: disabled (default after reset) 1: enabled > Can you check which phys range is remapped at > 0xcc960000? No idea how to do that but I'll see if I can figure it out... Yours, Linus Walleij
On Wed, 9 Jan 2019 22:30:18 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Jan 9, 2019 at 9:54 PM Boris Brezillon <bbrezillon@kernel.org> wrote: > > [Me] > > > I guess that is normal for FSMC: only one CS. It seems a bit > > > aggressive to toggle CS on/off between every command like this, > > > I suspect the FSMC isn't really built for that but shakes apart > > > or something. > > > > Hm, that would be weird. There's indeed timing constraints on the NAND > > chip side, but none infringing those constraints should not trigger an > > external abort exception. > > The manual contains this (the same bit, just another name than > in the driver): > > PBKEN PC-card/NAND-Flash chip-select enable. > Enables the corresponding chip-select. > If a disabled chip-select is accessed, an HRESP = ERROR is generated > on the AHB bus. > 0: disabled (default after reset) > 1: enabled It's still not clear what this bit does. Do you have access to the CS pin on the board? > > > Can you check which phys range is remapped at > > 0xcc960000? > > No idea how to do that but I'll see if I can figure it out... print the __iomem values returned by devm_ioremap_resource() and find the one that matches this address.
On Wed, Jan 9, 2019 at 10:52 PM Boris Brezillon <bbrezillon@kernel.org> wrote: > [Me] > > The manual contains this (the same bit, just another name than > > in the driver): > > > > PBKEN PC-card/NAND-Flash chip-select enable. > > Enables the corresponding chip-select. > > If a disabled chip-select is accessed, an HRESP = ERROR is generated > > on the AHB bus. > > 0: disabled (default after reset) > > 1: enabled > > It's still not clear what this bit does. Do you have access to the CS > pin on the board? Sadly no... > > > Can you check which phys range is remapped at > > > 0xcc960000? > > > > No idea how to do that but I'll see if I can figure it out... > > print the __iomem values returned by devm_ioremap_resource() and find > the one that matches this address. Ah of course. This is the first range, host->data_va "nand_data" I.e. the NAND memory itself. Yours, Linus Walleij
On 09/01/2019 20:44, Boris Brezillon wrote: > 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(); > + } + Will Deacon, an expert on arm mmio shenanigans. e.g. https://elinux.org/images/a/a8/Uh-oh-Its-IO-Ordering-Will-Deacon-Arm.pdf As far as I understand (which isn't very far), a memory barrier (however strong) cannot guarantee that all (posted) writes have reached a device (unless the underlying bus explicitly provides such guarantees?) At best, a memory barrier guarantees that the mmio writes have "left" the CPU and its "coherency fabric" (shared LLC). Thus subsequent accesses are guaranteed to appear "later" on the bus. (Much hand-waving sorry) I've been told that the one true way to ensure that a posted write has reached a device is simply to read the value back, i.e. writel+readl. If I'm wrong, I'd like to be schooled :-) Regards.
On Wed, 9 Jan 2019 23:20:40 +0100 Marc Gonzalez <marc.w.gonzalez@free.fr> wrote: > On 09/01/2019 20:44, Boris Brezillon wrote: > > > 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(); > > + } > > + Will Deacon, an expert on arm mmio shenanigans. > e.g. https://elinux.org/images/a/a8/Uh-oh-Its-IO-Ordering-Will-Deacon-Arm.pdf > > As far as I understand (which isn't very far), a memory barrier (however > strong) cannot guarantee that all (posted) writes have reached a device > (unless the underlying bus explicitly provides such guarantees?) > > At best, a memory barrier guarantees that the mmio writes have "left" the CPU > and its "coherency fabric" (shared LLC). Thus subsequent accesses are > guaranteed to appear "later" on the bus. (Much hand-waving sorry) Yes, that's what I was trying to enforce.
diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c index 41ba76efd6c8..ea69ac6e6d7a 100644 --- a/drivers/mtd/nand/raw/fsmc_nand.c +++ b/drivers/mtd/nand/raw/fsmc_nand.c @@ -609,22 +609,19 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf, } /* fsmc_select_chip - assert or deassert nCE */ -static void fsmc_select_chip(struct nand_chip *chip, int chipnr) +static void fsmc_ce_ctrl(struct fsmc_nand_data *host, bool assert) { - struct fsmc_nand_data *host = mtd_to_fsmc(nand_to_mtd(chip)); - u32 pc; - - /* Support only one CS */ - if (chipnr > 0) - return; + u32 pc = readl(host->regs_va + FSMC_PC); - pc = readl(host->regs_va + FSMC_PC); - if (chipnr < 0) + if (!assert) writel_relaxed(pc & ~FSMC_ENABLE, host->regs_va + FSMC_PC); else writel_relaxed(pc | FSMC_ENABLE, host->regs_va + FSMC_PC); - /* nCE line must be asserted before starting any operation */ + /* + * nCE line changes must be applied before returning from this + * function. + */ mb(); } @@ -645,6 +642,9 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op, int i; pr_debug("Executing operation [%d instructions]:\n", op->ninstrs); + + fsmc_ce_ctrl(host, true); + for (op_id = 0; op_id < op->ninstrs; op_id++) { instr = &op->instrs[op_id]; @@ -701,6 +701,8 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op, } } + fsmc_ce_ctrl(host, false); + return ret; } @@ -1081,7 +1083,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev) mtd->dev.parent = &pdev->dev; nand->exec_op = fsmc_exec_op; - nand->select_chip = fsmc_select_chip; /* * Setup default ECC mode. nand_dt_init() called from nand_scan_ident()
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 <boris.brezillon@bootlin.com> --- Changes in v3: - None Changes in v2: - None --- drivers/mtd/nand/raw/fsmc_nand.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)