| Message ID | 20140722120346.0f2d0e77@parrot.com |
|---|---|
| State | RFC |
| Headers | show |
Hi Matthieu On Tue, 22 Jul 2014 12:03:46 +0200 Matthieu CASTET <matthieu.castet@parrot.com> wrote: > Hi, > > > Do you know if all these timings will be used by the nand drivers ? I don't know (it depends on each NAND controller), and this is exactly why I decided to define all the timings described in the ONFI specification [1]. > > I did a similar patch [1] (that wasn't merged :( ), and I used reduced > timing info. I'm sorry it didn't make it to mainline, do you know why ? Could you point out where "reduced timing info" is defined in the ONFI specification ? > > I also have support for the omap driver > (http://article.gmane.org/gmane.linux.ports.arm.omap/88606/match=) and > a controller we use in our chip (not upstream). It should be pretty easy to convert the full timings list into a reduced one (actually, that's what your patch is doing), and you can thus port your work on top of these patches. Anyway, I'm not sure what you have in mind, but unless there is a strong reason to drop full timings in favor of reduced ones I'd like to keep them (even if this implies adding a new converter to get reduced timings list). Best Regards, Boris
Hi Boris, Le Tue, 22 Jul 2014 14:12:19 +0200, Boris BREZILLON <boris.brezillon@free-electrons.com> a écrit : > Hi Matthieu > > On Tue, 22 Jul 2014 12:03:46 +0200 > Matthieu CASTET <matthieu.castet@parrot.com> wrote: > > > Hi, > > > > > > > > I did a similar patch [1] (that wasn't merged :( ), and I used reduced > > timing info. > > I'm sorry it didn't make it to mainline, do you know why ? For the omap part there was a gpmc code rewrite that conflict with the patch. For the mtd stuff, I don't know/remember (I think for was no reply). > Could you point out where "reduced timing info" is defined in the ONFI > specification ? It is not defined on onfi. This was more a simplification of timings in order to simplify the driver side (avoid code duplication). Most controller allow to control nRE and nWE pulse and the setup time. Do you have drivers that use onfi timings ? > > > > > I also have support for the omap driver > > (http://article.gmane.org/gmane.linux.ports.arm.omap/88606/match=) and > > a controller we use in our chip (not upstream). > > It should be pretty easy to convert the full timings list into a > reduced one (actually, that's what your patch is doing), and you can > thus port your work on top of these patches. Yes I think an helper will be useful in order to help driver to use these timings. It can be a function that return the reduced version for a onfi mode and edo support. > > Anyway, I'm not sure what you have in mind, but unless there is a strong > reason to drop full timings in favor of reduced ones I'd like to > keep them (even if this implies adding a new converter to get reduced > timings list). > No problem. I have nothing special in mind. I hope this could give ideas how to use the onfi timings in mtd drivers (understanding how to use ONFI timings can be tricky). Matthieu
Hi Matthieu, On Thu, 24 Jul 2014 11:56:50 +0200 Matthieu CASTET <matthieu.castet@parrot.com> wrote: [...] > > > > > > I did a similar patch [1] (that wasn't merged :( ), and I used reduced > > > timing info. > > > > I'm sorry it didn't make it to mainline, do you know why ? > For the omap part there was a gpmc code rewrite that conflict with the > patch. > For the mtd stuff, I don't know/remember (I think for was no reply). Okay. > > > Could you point out where "reduced timing info" is defined in the ONFI > > specification ? > It is not defined on onfi. > This was more a simplification of timings in order to simplify the > driver side (avoid code duplication). Most controller allow to control > nRE and nWE pulse and the setup time. > > Do you have drivers that use onfi timings ? Hopefully, I will have one soon, if I manage to get some time to respin my sunxi NAND series [1] (I'm using nand timings in the sunxi_nand_chip_set_timings function) :-) IIRC, the atmel NAND driver configure its timings through the SMC (Static Memory Controller) block and this one use the timings you described. > > > > > > > > I also have support for the omap driver > > > (http://article.gmane.org/gmane.linux.ports.arm.omap/88606/match=) and > > > a controller we use in our chip (not upstream). > > > > It should be pretty easy to convert the full timings list into a > > reduced one (actually, that's what your patch is doing), and you can > > thus port your work on top of these patches. > Yes I think an helper will be useful in order to help driver to use > these timings. > It can be a function that return the reduced version for a onfi mode > and edo support. I'm not against the helper function (Brian, any opinion), but I'm not sure we should call these timings "reduced onfi timings" as they are not defined in the ONFI spec. > > > > > Anyway, I'm not sure what you have in mind, but unless there is a strong > > reason to drop full timings in favor of reduced ones I'd like to > > keep them (even if this implies adding a new converter to get reduced > > timings list). > > > No problem. I have nothing special in mind. I hope this could give ideas > how to use the onfi timings in mtd drivers (understanding how to use > ONFI timings can be tricky). > Okay, as I said I'm not opposed to a new helper function that provide what you called "reduced timings", so feel free to propose something ;-). Best Regards, Boris [1]http://lists.infradead.org/pipermail/linux-mtd/2014-March/052558.html
On Thu, Jul 24, 2014 at 12:16:07PM +0200, Boris BREZILLON wrote: > On Thu, 24 Jul 2014 11:56:50 +0200 Matthieu CASTET <matthieu.castet@parrot.com> wrote: > [...] > > > > > > > > I did a similar patch [1] (that wasn't merged :( ), and I used reduced > > > > timing info. > > > > > > I'm sorry it didn't make it to mainline, do you know why ? > > For the omap part there was a gpmc code rewrite that conflict with the > > patch. > > For the mtd stuff, I don't know/remember (I think for was no reply). Likely. Even today (with me spending more time on MTD), things can still fall through the cracks :( > > > Could you point out where "reduced timing info" is defined in the ONFI > > > specification ? > > It is not defined on onfi. > > This was more a simplification of timings in order to simplify the > > driver side (avoid code duplication). Most controller allow to control > > nRE and nWE pulse and the setup time. > > > > Do you have drivers that use onfi timings ? [...] I have hardware which can use a few timings (tWP, tWH, tRP, tREH, tCS, tCLH, tALH, tADL, tCCS, tWB, tWHR, tREAD). I don't currently optimize the timings in my driver, but this work could be useful. > > > > I also have support for the omap driver > > > > (http://article.gmane.org/gmane.linux.ports.arm.omap/88606/match=) and > > > > a controller we use in our chip (not upstream). > > > > > > It should be pretty easy to convert the full timings list into a > > > reduced one (actually, that's what your patch is doing), and you can > > > thus port your work on top of these patches. > > Yes I think an helper will be useful in order to help driver to use > > these timings. > > It can be a function that return the reduced version for a onfi mode > > and edo support. > > I'm not against the helper function (Brian, any opinion), but I'm not > sure we should call these timings "reduced onfi timings" as they are > not defined in the ONFI spec. No argument against a simplifying helper. I think it's good to get the full timings, but if drivers need some similar computations to calculate more helpful timings, then that sounds like a good idea. I agree we should avoid the 'ONFI' name on things that aren't exactly ONFI. > > > Anyway, I'm not sure what you have in mind, but unless there is a strong > > > reason to drop full timings in favor of reduced ones I'd like to > > > keep them (even if this implies adding a new converter to get reduced > > > timings list). > > > > > No problem. I have nothing special in mind. I hope this could give ideas > > how to use the onfi timings in mtd drivers (understanding how to use > > ONFI timings can be tricky). > > > > Okay, as I said I'm not opposed to a new helper function that provide > what you called "reduced timings", so feel free to propose > something ;-). Yes, please! Sometime I'll take a closer look at Matthieu's old patch, as well as Boris's initial sunxi NAND driver and my own hardware and see if things align well enough. But patches are welcome. I just merged Boris's stuff as a starting point (no one uses it yet), since similar questions were coming up in another NAND driver submission (Lee Jone's STMicro NANDi (?) driver). Brian
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2cbd091..2fc1a99 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -54,4 +54,4 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o -nand-objs := nand_base.o nand_bbt.o +nand-objs := nand_base.o nand_bbt.o nand_timing.o diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 8916bc6..0d6bd88 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3238,6 +3238,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd, if (*maf_id != NAND_MFR_SAMSUNG && !type->pagesize) chip->options &= ~NAND_SAMSUNG_LP_OPTIONS; ident_done: + nand_select_speed(chip, *maf_id, *dev_id); /* Try to identify manufacturer */ for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) { diff --git a/drivers/mtd/nand/nand_timing.c b/drivers/mtd/nand/nand_timing.c new file mode 100644 index 0000000..7211c9c --- /dev/null +++ b/drivers/mtd/nand/nand_timing.c @@ -0,0 +1,170 @@ +/* + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#include <linux/module.h> +#include <linux/types.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> + +/* + * this table is precomputed from onfi timings with the following program + */ +#if 0 +int print_timing(const struct onfi_timings *timings, int edo_off) +{ + struct reduced_onfi t; + int tmp; + int edo = timings->tRC < 30 && !edo_off; + + /* nWE low */ + t.twp = max(timings->tWP, timings->tDS); + + /* nCS low to nWE low */ + tmp = max(timings->tCLS, timings->tCS); + tmp = max(tmp, timings->tALS); + t.twsetup = tmp - t.twp; + assert(t.twsetup >= 0); + + /* nWE high */ + tmp = max(timings->tWH, timings->tDH); /* nWE high & data hold */ + tmp = max(tmp, timings->tCH); /* cs hold */ + tmp = max(tmp, timings->tCLH); /* cmd hold */ + t.twh = max(tmp, timings->tALH); /* addr hold */ + + assert(t.twp + t.twh <= timings->tWC); + t.twc = timings->tWC; + + t.edo = edo; + if (edo == 0) { + + /* nRE low */ + t.trp = max(timings->tRP, timings->tREA); + + /* nRE high */ + t.treh = timings->tREH; + t.trc = max(timings->tRC, t.trp + t.treh); + } + else { + /* nRE low */ + t.trp = timings->tRP; + + /* nRE high */ + t.treh = timings->tREH; + + t.trc = max(timings->tRC, timings->tREA); + } + + /* nCS low to nRE low */ + t.trsetup = max(timings->tCEA - timings->tREA, timings->tCLR); + + /* Min time from rdn rising edge to output hi-Z */ + t.bta = timings->tRHZ; + + /* Min time from busy rising edge and rdn falling edge (read).*/ + t.tbusy = timings->tRR; + + /* Min time from wrn rising edge to rdn falling edge. */ + t.twhr = timings->tWHR; + assert(t.twhr >= 0); + + t.tceh = 0; + + printf("{\n"); + printf("/* %s edo=%d */\n", timings->name, t.edo); + printf(".twp = %3d, .twh = %3d, .twc = %3d, .twsetup = %d,\n", + t.twp, t.twh, t.twc, t.twsetup); + printf(".trp = %3d, .treh = %3d, .trc = %3d, .trsetup = %d,\n", + t.trp, t.treh, t.trc, t.trsetup); + printf(".twhr = %d, .tceh = %d, .bta = %d, .tbusy = %d, .edo = %d,\n", + t.twhr, t.tceh, t.bta, t.tbusy, t.edo); + printf("},\n"); +#endif + +static struct reduced_onfi nand_timing[] = +{ + { + /* onfi mode 0 edo=0 */ + .twp = 50, .twh = 30, .twc = 100, .twsetup = 20, + .trp = 50, .treh = 30, .trc = 100, .trsetup = 60, + .twhr = 120, .tceh = 0, .bta = 200, .tbusy = 39, .edo = 0, + }, + { + /* onfi mode 1 edo=0 */ + .twp = 25, .twh = 15, .twc = 45, .twsetup = 10, + .trp = 30, .treh = 15, .trc = 50, .trsetup = 15, + .twhr = 80, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 0, + }, + { + /* onfi mode 2 edo=0 */ + .twp = 17, .twh = 15, .twc = 35, .twsetup = 8, + .trp = 25, .treh = 15, .trc = 40, .trsetup = 10, + .twhr = 80, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 0, + }, + { + /* onfi mode 3 edo=0 */ + .twp = 15, .twh = 10, .twc = 30, .twsetup = 10, + .trp = 20, .treh = 10, .trc = 30, .trsetup = 10, + .twhr = 60, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 0, + }, + { + /* onfi mode 4 edo=1 */ + .twp = 12, .twh = 10, .twc = 25, .twsetup = 8, + .trp = 12, .treh = 10, .trc = 25, .trsetup = 10, + .twhr = 60, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 1, + }, + { + /* onfi mode 5 edo=1 */ + .twp = 10, .twh = 7, .twc = 20, .twsetup = 5, + .trp = 10, .treh = 7, .trc = 20, .trsetup = 10, + .twhr = 60, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 1, + }, + { + /* onfi mode 4 edo=0 */ + .twp = 12, .twh = 10, .twc = 25, .twsetup = 8, + .trp = 20, .treh = 10, .trc = 30, .trsetup = 10, + .twhr = 60, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 0, + }, + { + /* onfi mode 5 edo=0 */ + .twp = 10, .twh = 7, .twc = 20, .twsetup = 5, + .trp = 16, .treh = 7, .trc = 23, .trsetup = 10, + .twhr = 60, .tceh = 0, .bta = 100, .tbusy = 20, .edo = 0, + }, +}; + +void nand_select_speed(struct nand_chip *chip, int maf_id, int dev_id) +{ + int i; + chip->onfi_speed = -1; + if (chip->onfi_version) { + int mode = le16_to_cpu(chip->onfi_params.async_timing_mode); + int max_mode = 0; + for (i = 0; i <= 5; i++) { + if (mode & (1 << i)) + max_mode = i; + } + chip->onfi_speed = max_mode; + } + /* + * For flash that are not ONFI we could use maf_id and dev_id to select a + * speed. But we need to make sure to select a speed compatible with all + * flash generation that share the same ids. + */ +} + +const struct reduced_onfi *nand_get_timing(int mode, int edo) +{ + if (mode < 0) + mode = 0; + if (!edo && mode >= 4) + mode += 2; + if (mode > ARRAY_SIZE(nand_timing)) + mode = 0; + return &(nand_timing[mode]); +} +EXPORT_SYMBOL_GPL(nand_get_timing); diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index c8518d4..95f2871 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -474,6 +474,7 @@ struct nand_buffers { * @pagebuf_bitflips: [INTERN] holds the bitflip count for the page which is * currently in data_buf. * @subpagesize: [INTERN] holds the subpagesize + * @onfi_speed: [INTERN] holds the ONFI speed, -1 if not supported. * @onfi_version: [INTERN] holds the chip ONFI version (BCD encoded), * non 0 if ONFI supported. * @onfi_params: [INTERN] holds the ONFI page parameter when ONFI is @@ -545,6 +546,7 @@ struct nand_chip { int badblockpos; int badblockbits; + int onfi_speed; int onfi_version; struct nand_onfi_params onfi_params; @@ -691,6 +693,60 @@ struct platform_nand_data { struct platform_nand_ctrl ctrl; }; +/** + * timing in ns, + * there are computed from onfi table : + * twsetup = max(tCLS, tCS, tALS) - twp + * twp = max(tWP, tDS) + * twh = max(tCLH, tCH, tALH, tDH, tWH) + * twc = tWC + * trsetup = max(tCEA-tREA, tCLR) + * treh = tREH + * if (edo == 0) { + * trp = max(tRP, tREA) + * trc = max(tRC, trp + treh) + * } + * else { + * trp = tRP + * trc = max(tRC, tREA) + * } + * + * bta = max(tRHZ, tCHZ) + * busy = tRR + * twhr = tWHR + * + * Note that twp + twh can be smaller than twc, so you should do : + * twp_clk = ns_to_ticks(twp) + * twh_clk = ns_to_ticks(max(twh, twc - ticks_to_ns(twp_clk))) + * or + * twh_clk = ps_to_ticks(max(ns_to_ps(twh), ns_to_ps(twc) - ticks_to_ps(twp_clk))) + * using picosecond can help for rounding. For example a 156Mhz, mode=4 edo=1 + * with ps we got twp_clk=twh_clk=2 (12820 ps) + * without ps we got twp_clk = 2 (12 ns) and twh_clk=3 (18 ns) + * This is because 12820 + 12820 > 15000 but 12 + 12 < 15. + */ +struct reduced_onfi { + u8 twsetup; /* nCS low to nWE low */ + u8 twp; /* nWE low */ + u8 twh; /* nWE high */ + u8 twc; /* write cyle */ + + u8 trsetup; /* nCS low to nRE low */ + u8 trp; /* nRE low */ + u8 treh; /* nRE high */ + u8 trc; /* read cycle */ + + u8 bta; /* Min time from nRE rising edge to output hi-Z */ + u8 tbusy; /* Min time from busy rising edge and nRE falling edge */ + u8 twhr; /* Min time from nWE rising edge to nRE falling edge. */ + u8 tceh; /* Min time for nCE high */ + + u8 edo; /* edo mode */ +}; + +void nand_select_speed(struct nand_chip *chip, int maf_id, int dev_id); +const struct reduced_onfi *nand_get_timing(int mode, int edo); + /* Some helpers to access the data structures */ static inline struct platform_nand_chip *get_platform_nandchip(struct mtd_info *mtd)
Hi, Do you know if all these timings will be used by the nand drivers ? I did a similar patch [1] (that wasn't merged :( ), and I used reduced timing info. I also have support for the omap driver (http://article.gmane.org/gmane.linux.ports.arm.omap/88606/match=) and a controller we use in our chip (not upstream). Matthieu [1] [PATCH 2/3] mtd nand : get timings from onfi We get from onfi param the max speed supported by the chip. A precomputed table for ONFI timings is generated. Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com> --- drivers/mtd/nand/Makefile | 2 +- drivers/mtd/nand/nand_base.c | 1 + drivers/mtd/nand/nand_timing.c | 170 ++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 56 +++++++++++++ 4 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/nand_timing.c