| Message ID | 1433505164-24112-3-git-send-email-r.spliet@ultimaker.com |
|---|---|
| State | RFC |
| Delegated to: | Scott Wood |
| Headers | show |
On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: > From: yassin <yassinjaffer@gmail.com> > > Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> > --- > drivers/mtd/nand/Makefile | 2 +- > drivers/mtd/nand/nand_timings.c | 252 > ++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/nand.h | 3 + > 3 files changed, 256 insertions(+), 1 deletion(-) > create mode 100644 drivers/mtd/nand/nand_timings.c This code comes from Linux and yet I see no acknowledgement of that, much less a statement of which version of Linux this was pulled from. It would probably be better to handle this as part of a general sync with the Linux mtd code. -Scott
Hello Scott et al., Op 06-06-15 om 00:02 schreef Scott Wood: > On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: >> From: yassin <yassinjaffer@gmail.com> >> >> Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> >> --- >> drivers/mtd/nand/Makefile | 2 +- >> drivers/mtd/nand/nand_timings.c | 252 >> ++++++++++++++++++++++++++++++++++++++++ >> include/linux/mtd/nand.h | 3 + >> 3 files changed, 256 insertions(+), 1 deletion(-) >> create mode 100644 drivers/mtd/nand/nand_timings.c > This code comes from Linux and yet I see no acknowledgement of that, > much less a statement of which version of Linux this was pulled from. Correct, my apologies as I should have clarified that "work by Boris Brezillon" does not mean "upsteam work". The code comes from Boris' github tree[1] and contains work that was not yet brought upstream, yet is required for NAND on sunxi to work. > It would probably be better to handle this as part of a general sync > with the Linux mtd code. Ideally yes, but. In upstream Linux MTD we have a few issues to address as shown by Boris' patch-set, most importantly: - NAND chip timings (patch 1 to 3) - Randomisation support (patch 6) - Per-partition settings for ECC and randomisation (left out of my RFC, highly desirable but not strictly required for U-boot booting) - OF definition of all the above U-Boot has the additional challenge - Partitioning is not done the upstream way, but rather in/around cmd_mtdparts We started discussion on these topics in the #mtd IRC channel (OFTC), and several ideas have come up. The way I see it these issues can currently either be addressed by bolting/duct-taping/tie-wrapping more layers of indirection on top of upstream MTD framework, but really I feel that it might require a bit more of a structural approach that may or may not break existing drivers. The code I posted as RFC is functional. If you wish to steer towards an MTD sync-up, I highly suggest first getting these issues tackled, and then making sure that we extend the U-boot side of MTD with OF support for everything. This trajectory should include deprecating the current implementation of cmd_mtdparts and hook that command up to the "new" upstream way. In other words: now is probably not the right moment. I'm afraid that as much as I'd like to, I will not have time to address every single one of these issues (not to mention I am hardly the expert in MTD/NAND). I'm happy to think along with designing a sustainable solution, but I will need someone to chime in when it comes to carrying solutions upstream. Roy > -Scott [1] https://github.com/bbrezillon/linux-sunxi/commits/sunxi-nand
On 8 June 2015 at 10:11, Roy Spliet <r.spliet@ultimaker.com> wrote: > Hello Scott et al., > > Op 06-06-15 om 00:02 schreef Scott Wood: > > On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: > > From: yassin <yassinjaffer@gmail.com> > > Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> > --- > drivers/mtd/nand/Makefile | 2 +- > drivers/mtd/nand/nand_timings.c | 252 > ++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/nand.h | 3 + > 3 files changed, 256 insertions(+), 1 deletion(-) > create mode 100644 drivers/mtd/nand/nand_timings.c > > This code comes from Linux and yet I see no acknowledgement of that, > much less a statement of which version of Linux this was pulled from. > > Correct, my apologies as I should have clarified that "work by Boris > Brezillon" does not > mean "upsteam work". The code comes from Boris' github tree[1] and contains > work > that was not yet brought upstream, yet is required for NAND on sunxi to > work. > > It would probably be better to handle this as part of a general sync > with the Linux mtd code. > > Ideally yes, but. In upstream Linux MTD we have a few issues to address as > shown > by Boris' patch-set, most importantly: > - NAND chip timings (patch 1 to 3) > - Randomisation support (patch 6) > - Per-partition settings for ECC and randomisation (left out of my RFC, > highly desirable > but not strictly required for U-boot booting) Hello, as I understand it the ECC and randomisation settings for the bootloader part of the nand are suboptimal or unusable for ubifs so if u-boot SPL is to read the u-boot binary and later u-boot the kernel from an ubifs volume it has to support non-uniform settings. Alternatively the bootloader part can be extended to contain partitions for u-boot binary and kernel image written to raw partition without filesystem much like what Andriod usually does. Thanks Michal
Hello Michal, Op 08-06-15 om 10:34 schreef Michal Suchanek: > On 8 June 2015 at 10:11, Roy Spliet <r.spliet@ultimaker.com> wrote: >> Hello Scott et al., >> >> Op 06-06-15 om 00:02 schreef Scott Wood: >> >> On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: >> >> From: yassin <yassinjaffer@gmail.com> >> >> Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> >> --- >> drivers/mtd/nand/Makefile | 2 +- >> drivers/mtd/nand/nand_timings.c | 252 >> ++++++++++++++++++++++++++++++++++++++++ >> include/linux/mtd/nand.h | 3 + >> 3 files changed, 256 insertions(+), 1 deletion(-) >> create mode 100644 drivers/mtd/nand/nand_timings.c >> >> This code comes from Linux and yet I see no acknowledgement of that, >> much less a statement of which version of Linux this was pulled from. >> >> Correct, my apologies as I should have clarified that "work by Boris >> Brezillon" does not >> mean "upsteam work". The code comes from Boris' github tree[1] and contains >> work >> that was not yet brought upstream, yet is required for NAND on sunxi to >> work. >> >> It would probably be better to handle this as part of a general sync >> with the Linux mtd code. >> >> Ideally yes, but. In upstream Linux MTD we have a few issues to address as >> shown >> by Boris' patch-set, most importantly: >> - NAND chip timings (patch 1 to 3) >> - Randomisation support (patch 6) >> - Per-partition settings for ECC and randomisation (left out of my RFC, >> highly desirable >> but not strictly required for U-boot booting) > Hello, > > as I understand it the ECC and randomisation settings for the > bootloader part of the nand are suboptimal or unusable for ubifs so if > u-boot SPL is to read the u-boot binary and later u-boot the kernel > from an ubifs volume it has to support non-uniform settings. > Alternatively the bootloader part can be extended to contain > partitions for u-boot binary and kernel image written to raw partition > without filesystem much like what Andriod usually does. SPL does not read U-boot from UBI. The SPL driver is separate, much smaller and only reads in the same way BROM does. U-boot itself is not bound by the limitations imposed by BROM, which means proper randomisation and ECC settings can be used for "the UBI partition". Yours, Roy > > Thanks > > Michal
On Mon, 2015-06-08 at 10:11 +0200, Roy Spliet wrote: > Hello Scott et al., > > Op 06-06-15 om 00:02 schreef Scott Wood: > > On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: > > > From: yassin <yassinjaffer@gmail.com> > > > > > > Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> > > > --- > > > drivers/mtd/nand/Makefile | 2 +- > > > drivers/mtd/nand/nand_timings.c | 252 > > > ++++++++++++++++++++++++++++++++++++++++ > > > include/linux/mtd/nand.h | 3 + > > > 3 files changed, 256 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/mtd/nand/nand_timings.c > > This code comes from Linux and yet I see no acknowledgement of > > that, > > much less a statement of which version of Linux this was pulled > > from. > Correct, my apologies as I should have clarified that "work by Boris > Brezillon" does not > mean "upsteam work". The code comes from Boris' github tree[1] and > contains work > that was not yet brought upstream, yet is required for NAND on sunxi > to work. > > It would probably be better to handle this as part of a general > > sync > > with the Linux mtd code. > Ideally yes, but. In upstream Linux MTD we have a few issues to > address as shown > by Boris' patch-set, most importantly: > - NAND chip timings (patch 1 to 3) Patches 1-3 are already in Linux (if there are subtle differences needed those should be followup patches, with an explanation of why it needs to differ from Linux), except that patch 3 for some reason has "onfi_timing_mode_ds" where Linux has "onfi_timing_mode_default". I will try to do a sync soon. > U-Boot has the additional challenge > - Partitioning is not done the upstream way, but rather in/around > cmd_mtdparts > > We started discussion on these topics in the #mtd IRC channel > (OFTC), and several ideas > have come up. The way I see it these issues can currently either be > addressed by > bolting/duct-taping/tie-wrapping more layers of indirection on top > of upstream MTD > framework, but really I feel that it might require a bit more of a > structural approach > that may or may not break existing drivers. > The code I posted as RFC is functional. If you wish to steer towards > an MTD sync-up, I > highly suggest first getting these issues tackled, and then making > sure that we extend > the U-boot side of MTD with OF support for everything. This > trajectory should include > deprecating the current implementation of cmd_mtdparts and hook that > command up > to the "new" upstream way. In other words: now is probably not the > right moment. I don't see why those things are a prerequisite for syncing MTD from Linux to U-Boot. If there are further changes in Linux after a sync that we want to bring in, we can sync again and it will be easier because the sync will contain fewer changes. -Scott
Hi Scott, On June 8, 2015, 8:24 p.m., Scott Wood wrote: <snip> >> Correct, my apologies as I should have clarified that "work by Boris >> Brezillon" does not >> mean "upsteam work". The code comes from Boris' github tree[1] and >> contains work >> that was not yet brought upstream, yet is required for NAND on sunxi >> to work. >>> It would probably be better to handle this as part of a general >>> sync >>> with the Linux mtd code. >> Ideally yes, but. In upstream Linux MTD we have a few issues to >> address as shown >> by Boris' patch-set, most importantly: >> - NAND chip timings (patch 1 to 3) > > Patches 1-3 are already in Linux (if there are subtle differences > needed those should be followup patches, with an explanation of why it > needs to differ from Linux), except that patch 3 for some reason has > "onfi_timing_mode_ds" where Linux has "onfi_timing_mode_default". > > I will try to do a sync soon. If you could do that that would be great, that should at least shrink the size of this patch-set by 3 patches. I assume that you will not be sending a pull-req for this sync for v2015.07, if you can make a git branch with the sync available somewhere for us to build on top of, then that would be great. Thanks & Regards, Hans
On Wed, 2015-06-10 at 10:33 +0200, Hans de Goede wrote: > Hi Scott, > > On June 8, 2015, 8:24 p.m., Scott Wood wrote: > > <snip> > > >> Correct, my apologies as I should have clarified that "work by > Boris > >> Brezillon" does not > >> mean "upsteam work". The code comes from Boris' github tree[1] > and > >> contains work > >> that was not yet brought upstream, yet is required for NAND on > sunxi > >> to work. > >>> It would probably be better to handle this as part of a general > >>> sync > >>> with the Linux mtd code. > >> Ideally yes, but. In upstream Linux MTD we have a few issues to > >> address as shown > >> by Boris' patch-set, most importantly: > >> - NAND chip timings (patch 1 to 3) > > > > Patches 1-3 are already in Linux (if there are subtle differences > > needed those should be followup patches, with an explanation of > why it > > needs to differ from Linux), except that patch 3 for some reason > has > > "onfi_timing_mode_ds" where Linux has "onfi_timing_mode_default". > > > > I will try to do a sync soon. > > If you could do that that would be great, that should at least shrink > the size of this patch-set by 3 patches. > > I assume that you will not be sending a pull-req for this sync for > v2015.07, No, it's too late for that even if I had it ready now. > if you can make a git branch with the sync available > somewhere for us to build on top of, then that would be great. OK. -Scott
On Mon, 08 Jun 2015 10:41:28 +0200 Roy Spliet <r.spliet@ultimaker.com> wrote: > Hello Michal, > > Op 08-06-15 om 10:34 schreef Michal Suchanek: > > On 8 June 2015 at 10:11, Roy Spliet <r.spliet@ultimaker.com> wrote: > >> Hello Scott et al., > >> > >> Op 06-06-15 om 00:02 schreef Scott Wood: > >> > >> On Fri, 2015-06-05 at 13:52 +0200, Roy Spliet wrote: > >> > >> From: yassin <yassinjaffer@gmail.com> > >> > >> Signed-off-by: Roy Spliet <r.spliet@ultimaker.com> > >> --- > >> drivers/mtd/nand/Makefile | 2 +- > >> drivers/mtd/nand/nand_timings.c | 252 > >> ++++++++++++++++++++++++++++++++++++++++ > >> include/linux/mtd/nand.h | 3 + > >> 3 files changed, 256 insertions(+), 1 deletion(-) > >> create mode 100644 drivers/mtd/nand/nand_timings.c > >> > >> This code comes from Linux and yet I see no acknowledgement of that, > >> much less a statement of which version of Linux this was pulled from. > >> > >> Correct, my apologies as I should have clarified that "work by Boris > >> Brezillon" does not > >> mean "upsteam work". The code comes from Boris' github tree[1] and contains > >> work > >> that was not yet brought upstream, yet is required for NAND on sunxi to > >> work. > >> > >> It would probably be better to handle this as part of a general sync > >> with the Linux mtd code. > >> > >> Ideally yes, but. In upstream Linux MTD we have a few issues to address as > >> shown > >> by Boris' patch-set, most importantly: > >> - NAND chip timings (patch 1 to 3) > >> - Randomisation support (patch 6) > >> - Per-partition settings for ECC and randomisation (left out of my RFC, > >> highly desirable > >> but not strictly required for U-boot booting) > > Hello, > > > > as I understand it the ECC and randomisation settings for the > > bootloader part of the nand are suboptimal or unusable for ubifs so if > > u-boot SPL is to read the u-boot binary and later u-boot the kernel > > from an ubifs volume it has to support non-uniform settings. > > Alternatively the bootloader part can be extended to contain > > partitions for u-boot binary and kernel image written to raw partition > > without filesystem much like what Andriod usually does. > SPL does not read U-boot from UBI. The SPL driver is separate, much > smaller and > only reads in the same way BROM does. U-boot itself is not bound by the > limitations > imposed by BROM, which means proper randomisation and ECC settings can > be used > for "the UBI partition". Yep, I agree with Roy here: we shouldn't use the BROM config except for the SPL partition. Defining the same randomizer seed for all the pages in a given block is pretty much useless since it won't prevent the reproducible pattern issue, which is one of the cause of excessive bitblits in MLC NANDs. The same goes for the ECC config used by the BROM, because in some cases it might prevent using the whole page (ECC config is stronger than required, and thus ECC bytes take more than what's available in the OOB area).
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index a0cf4d5..f194493 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -32,7 +32,7 @@ obj-y += nand_bbt.o obj-y += nand_ids.o obj-y += nand_util.o obj-y += nand_ecc.o -obj-y += nand_base.o +obj-y += nand_base.o nand_timings.o endif # not spl diff --git a/drivers/mtd/nand/nand_timings.c b/drivers/mtd/nand/nand_timings.c new file mode 100644 index 0000000..9e8b0a5 --- /dev/null +++ b/drivers/mtd/nand/nand_timings.c @@ -0,0 +1,252 @@ +/* + * Copyright (C) 2014 Free Electrons + * + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> + * + * 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 <common.h> +#include <linux/err.h> +#include <linux/mtd/nand.h> + +static const struct nand_sdr_timings onfi_sdr_timings[] = { + /* Mode 0 */ + { + .tADL_min = 200000, + .tALH_min = 20000, + .tALS_min = 50000, + .tAR_min = 25000, + .tCEA_max = 100000, + .tCEH_min = 20000, + .tCH_min = 20000, + .tCHZ_max = 100000, + .tCLH_min = 20000, + .tCLR_min = 20000, + .tCLS_min = 50000, + .tCOH_min = 0, + .tCS_min = 70000, + .tDH_min = 20000, + .tDS_min = 40000, + .tFEAT_max = 1000000, + .tIR_min = 10000, + .tITC_max = 1000000, + .tRC_min = 100000, + .tREA_max = 40000, + .tREH_min = 30000, + .tRHOH_min = 0, + .tRHW_min = 200000, + .tRHZ_max = 200000, + .tRLOH_min = 0, + .tRP_min = 50000, + .tRST_max = 250000000000ULL, + .tWB_max = 200000, + .tRR_min = 40000, + .tWC_min = 100000, + .tWH_min = 30000, + .tWHR_min = 120000, + .tWP_min = 50000, + .tWW_min = 100000, + }, + /* Mode 1 */ + { + .tADL_min = 100000, + .tALH_min = 10000, + .tALS_min = 25000, + .tAR_min = 10000, + .tCEA_max = 45000, + .tCEH_min = 20000, + .tCH_min = 10000, + .tCHZ_max = 50000, + .tCLH_min = 10000, + .tCLR_min = 10000, + .tCLS_min = 25000, + .tCOH_min = 15000, + .tCS_min = 35000, + .tDH_min = 10000, + .tDS_min = 20000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 50000, + .tREA_max = 30000, + .tREH_min = 15000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRP_min = 25000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 45000, + .tWH_min = 15000, + .tWHR_min = 80000, + .tWP_min = 25000, + .tWW_min = 100000, + }, + /* Mode 2 */ + { + .tADL_min = 100000, + .tALH_min = 10000, + .tALS_min = 15000, + .tAR_min = 10000, + .tCEA_max = 30000, + .tCEH_min = 20000, + .tCH_min = 10000, + .tCHZ_max = 50000, + .tCLH_min = 10000, + .tCLR_min = 10000, + .tCLS_min = 15000, + .tCOH_min = 15000, + .tCS_min = 25000, + .tDH_min = 5000, + .tDS_min = 15000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 35000, + .tREA_max = 25000, + .tREH_min = 15000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tRP_min = 17000, + .tWC_min = 35000, + .tWH_min = 15000, + .tWHR_min = 80000, + .tWP_min = 17000, + .tWW_min = 100000, + }, + /* Mode 3 */ + { + .tADL_min = 100000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 50000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 25000, + .tDH_min = 5000, + .tDS_min = 10000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 30000, + .tREA_max = 20000, + .tREH_min = 10000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRP_min = 15000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 30000, + .tWH_min = 10000, + .tWHR_min = 80000, + .tWP_min = 15000, + .tWW_min = 100000, + }, + /* Mode 4 */ + { + .tADL_min = 70000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 30000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 20000, + .tDH_min = 5000, + .tDS_min = 10000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 25000, + .tREA_max = 20000, + .tREH_min = 10000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 5000, + .tRP_min = 12000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 25000, + .tWH_min = 10000, + .tWHR_min = 80000, + .tWP_min = 12000, + .tWW_min = 100000, + }, + /* Mode 5 */ + { + .tADL_min = 70000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 30000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 15000, + .tDH_min = 5000, + .tDS_min = 7000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 20000, + .tREA_max = 16000, + .tREH_min = 7000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 5000, + .tRP_min = 10000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 20000, + .tWH_min = 7000, + .tWHR_min = 80000, + .tWP_min = 10000, + .tWW_min = 100000, + }, +}; + +/** + * onfi_async_timing_mode_to_sdr_timings - [NAND Interface] Retrieve NAND + * timings according to the given ONFI timing mode + * @mode: ONFI timing mode + */ +const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode) +{ + if (mode < 0 || mode >= ARRAY_SIZE(onfi_sdr_timings)) + return ERR_PTR(-EINVAL); + + return &onfi_sdr_timings[mode]; +} +EXPORT_SYMBOL(onfi_async_timing_mode_to_sdr_timings); diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index b026110..abda5c3 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -1057,6 +1057,9 @@ struct nand_sdr_timings { u32 tWW_min; }; +/* get timing characteristics from ONFI timing mode. */ +const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); + #ifdef __UBOOT__ /* Standard NAND functions from nand_base.c */ void nand_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len);