| Message ID | 1302372335-30232-6-git-send-email-sbabic@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Sandeep Paulraj |
| Headers | show |
Hi Stefano, Thanks for sharing this patch -- I have been using the "-O 2048" (VID header offset) option to prevent subpages here. On Sat, Apr 9, 2011 at 2:05 PM, Stefano Babic <sbabic@denx.de> wrote: > The NAND controller does not support subpage accessing. This is > not used at all for MLC NAND, but it is set for SLC NAND. UBI tries > to access to subpages, and because it fails, it starts "torture tests" "tries to access subpages" is maybe a little too vague; I think Jon Povey described the problem quite succinctly: "the problem is that the ECC generated for an all-FFs page is not all-FFs, and subpage writes are handled by nand_do_write_ops() by writing a full page with FFs in the unset data areas." [1] > on the whole page that are always successful, making an endless loop > with the UBI background task taking most CPU time. > On the console, the issue is recognized by the messages: > > UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from > PEB 37:512, read 512 bytes > UBI: run torture test for PEB 37 > UBI: PEB 37 passed torture test, do not mark it a bad > > Signed-off-by: Stefano Babic <sbabic@denx.de> > CC: Ben Gardiner<bengardiner@nanometrics.ca> > CC: Sandeep Paulraj <s-paulraj@ti.com> > CC: Scott Wood <scottwood@freescale.com> Is it worth mentioning that this will create UBI partitions that are not usable in Linux with a similar patch or a VID header offset workaround? > --- > README | 4 ++++ > drivers/mtd/nand/davinci_nand.c | 3 +++ > include/configs/ea20.h | 1 + > include/linux/mtd/nand.h | 3 ++- > 4 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/README b/README > index 21cd71b..8d664eb 100644 > --- a/README > +++ b/README > @@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options: > that is executed before the actual U-Boot. E.g. when > compiling a NAND SPL. > > +- CONFIG_SYS_NAND_NO_SUBPAGE > + Some drivers (davinci) do not support access to NAND subpage. > + > + > Building the Software: > ====================== > > diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c > index d41579c..f6c7d09 100644 > --- a/drivers/mtd/nand/davinci_nand.c > +++ b/drivers/mtd/nand/davinci_nand.c > @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand) > #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT > nand->options |= NAND_USE_FLASH_BBT; > #endif > +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE > + nand->options |= NAND_NO_SUBPAGE_WRITE; > +#endif > #ifdef CONFIG_SYS_NAND_HW_ECC > nand->ecc.mode = NAND_ECC_HW; > nand->ecc.size = 512; > diff --git a/include/configs/ea20.h b/include/configs/ea20.h > index 1843dae..9e5dda4 100644 > --- a/include/configs/ea20.h > +++ b/include/configs/ea20.h > @@ -171,6 +171,7 @@ > > #define CONFIG_NAND_DAVINCI > #define CONFIG_SYS_NAND_PAGE_2K > +#define CONFIG_SYS_NAND_NO_SUBPAGE > #define CONFIG_SYS_NAND_CS 2 > #define CONFIG_SYS_NAND_BASE DAVINCI_ASYNC_EMIF_DATA_CE2_BASE > #undef CONFIG_SYS_NAND_HW_ECC > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 987a2ec..215e781 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -193,7 +193,8 @@ typedef enum { > && (chip->page_shift > 9)) > > /* Mask to zero out the chip options, which come from the id table */ > -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR) > +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \ > + ~NAND_NO_SUBPAGE_WRITE) First let me say that I would be happy to have a fix for this merged -- so I think the change is "good enough" since it makes UBI work without specifying a VID header offset equal to the NAND page size. What follows is more topical musings that any particular criticisms that should be considered blockers of your patch. I'm wondering about the long-term path for davinci NAND and subpage writes... But the sentiment I've heard about adding an exception to NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the NAND chip and features of the NAND controller (If you have a modern SLC NAND then your chip probably supports subpage writes whereas it is the controller that needs to be limited). What's more is that the davinci nand controller could do subpage writing if the writing operation were informed of the extents of the subpage being written instead of being handed a buffer with 0xFF in the non-target page areas. Which, I believe is Artem's primary motivation for the introduction of nand_write_subpage() to the davinci NAND controller driver [2]. So if the nand_write_subpage family of functions was introduced to the Linux kernel instead of adding another exception to NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci NAND: 1) no patch: VID offset <page size> required 2) chip NAND_NO_SUBPAGE_WRITE patch 3) subpage writes support with nand_write_subpage() systems with 2) or 3) could always use UBI as in 1) and a UBI volume made with 2) would need to be attached as in 1) on systems with 2) or 3) ; but a UBI volume made with 3) could not be used on systems with 1) or 2) which means that we could not make use of the efficiency benefits of nand_write_subpage() if/when it is available on systems which need access to UBI from U-boot. I've cc'd Artem and Jon in the hopes that they can correct any of my leaps above -- what I'd really like is to understand a migration strategy to actually using subpage writes with davinci NAND in the future. Best Regards, Ben Gardiner [1] http://article.gmane.org/gmane.linux.davinci/19853 [2] http://article.gmane.org/gmane.linux.drivers.mtd/31581 --- Nanometrics Inc. http://www.nanometrics.ca
On Sat, Apr 09, 2011 at 08:05:35PM +0200, Stefano Babic wrote: > diff --git a/README b/README > index 21cd71b..8d664eb 100644 > --- a/README > +++ b/README > @@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options: > that is executed before the actual U-Boot. E.g. when > compiling a NAND SPL. > > +- CONFIG_SYS_NAND_NO_SUBPAGE > + Some drivers (davinci) do not support access to NAND subpage. This only controls the davinci driver, so it should be CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE. Is this really board-specific? Does the davinci driver ever support subpage? > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 987a2ec..215e781 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -193,7 +193,8 @@ typedef enum { > && (chip->page_shift > 9)) > > /* Mask to zero out the chip options, which come from the id table */ > -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR) > +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \ > + ~NAND_NO_SUBPAGE_WRITE) I wonder what we really need CHIPOPTIONS_MSK for? Silently ignoring what the driver asked for doesn't seem right. Can't we just OR the two sources? And it would be a driver error to specify a flag that doesn't make sense to be set this way (i.e. only do it with the "doesn't support X" flags). -Scott
Ben Gardiner wrote: > "tries to access subpages" is maybe a little too vague; I think Jon > Povey described the problem quite succinctly: That's me, I better wake up! > What's more is that the davinci nand controller could do subpage > writing if the writing operation were informed of the extents of the > subpage being written instead of being handed a buffer with 0xFF in > the non-target page areas. Which, I believe is Artem's primary > motivation for the introduction of nand_write_subpage() to the > davinci NAND controller driver [2]. I can't remember if I posted about this at the time, but what I ended up doing was propagating the subpage start offset and length through to all the write_page() functions. At the appropriate place the ECC for any "unwritten" subpages is FF'd out. It's a fairly ugly platform-specific hack but it works, if anyone wants to see it I can dig out the patch. -- Jon Povey jon.povey@racelogic.co.uk Racelogic is a limited company registered in England. Registered number 2743719 . Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB . The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network
On 04/11/2011 04:04 PM, Ben Gardiner wrote: > Hi Stefano, Hi Ben, > > Thanks for sharing this patch -- I have been using the "-O 2048" (VID > header offset) option to prevent subpages here. Yes, this works too, at least with Linux. > > On Sat, Apr 9, 2011 at 2:05 PM, Stefano Babic <sbabic@denx.de> wrote: >> The NAND controller does not support subpage accessing. This is >> not used at all for MLC NAND, but it is set for SLC NAND. UBI tries >> to access to subpages, and because it fails, it starts "torture tests" > > "tries to access subpages" is maybe a little too vague; I think Jon > Povey described the problem quite succinctly: > > "the problem is that the ECC generated for an all-FFs page is not > all-FFs, and subpage writes are handled by > nand_do_write_ops() by writing a full page with FFs in the unset data > areas." [1] Thanks - I have not found this link. I will merge Jon's comment. >> on the whole page that are always successful, making an endless loop >> with the UBI background task taking most CPU time. >> On the console, the issue is recognized by the messages: >> >> UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from >> PEB 37:512, read 512 bytes >> UBI: run torture test for PEB 37 >> UBI: PEB 37 passed torture test, do not mark it a bad >> >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> CC: Ben Gardiner<bengardiner@nanometrics.ca> >> CC: Sandeep Paulraj <s-paulraj@ti.com> >> CC: Scott Wood <scottwood@freescale.com> > > Is it worth mentioning that this will create UBI partitions that are > not usable in Linux with a similar patch or a VID header offset > workaround? Yes, better to explain ;-) > First let me say that I would be happy to have a fix for this merged > -- so I think the change is "good enough" since it makes UBI work > without specifying a VID header offset equal to the NAND page size. > What follows is more topical musings that any particular criticisms > that should be considered blockers of your patch. > > I'm wondering about the long-term path for davinci NAND and subpage writes... > > But the sentiment I've heard about adding an exception to > NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the > NAND chip and features of the NAND controller (If you have a modern > SLC NAND then your chip probably supports subpage writes whereas it is > the controller that needs to be limited). That is true. With this patch I constrained the SLC NAND on my board to be considered as MLC, as a MLC NAND cannot be accessed in subpage mode. However, I set also a CONFIG_SYS_NAND_NO_SUBPAGE, and instead of dropping the option in the mask we could protect the code where the option is checked. In other words, we could change nand_base.c in this way: #ifndef CONFIG_SYS_NAND_NO_SUBPAGE if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) { switch(chip->ecc.steps) { case 2: mtd->subpage_sft = 1; break; case 4: case 8: case 16: mtd->subpage_sft = 2; break; } } #endif Then it could be clearer that the restriction is due to the NAND controller, and not to the chip itself. > > What's more is that the davinci nand controller could do subpage > writing if the writing operation were informed of the extents of the > subpage being written instead of being handed a buffer with 0xFF in > the non-target page areas. Which, I believe is Artem's primary > motivation for the introduction of nand_write_subpage() to the davinci > NAND controller driver [2]. Well, of course, if the davinci NAND can handle subpages, we can remove the limitation. What do you think to add this patch in the way I suggest now (without touching NAND_CHIPOPTIONS_MSK, that makes the MTD inconsistent with Linux) until a subpage_write is added to davinci ? > > So if the nand_write_subpage family of functions was introduced to the > Linux kernel instead of adding another exception to > NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci > NAND: > 1) no patch: VID offset <page size> required > 2) chip NAND_NO_SUBPAGE_WRITE patch > 3) subpage writes support with nand_write_subpage() > > systems with 2) or 3) could always use UBI as in 1) and a UBI volume > made with 2) would need to be attached as in 1) on systems with 2) or > 3) ; but a UBI volume made with 3) could not be used on systems with > 1) or 2) which means that we could not make use of the efficiency > benefits of nand_write_subpage() if/when it is available on systems > which need access to UBI from U-boot. I agree with your analyses. As personal feeling, solution one has the drawback that it cannot be used in u-boot, and it seems to me very easy to have a wrong UBI on the target. We can consider 2) as temporary solution, until 3) is available. Best regards, Stefano Babic
On 04/11/2011 09:16 PM, Scott Wood wrote: > This only controls the davinci driver, so it should be > CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE. > > Is this really board-specific? No, really not. > Does the davinci driver ever support > subpage? No, it does not. This problem affects all boards using the davinci NAND controller. At least with 850 or OMAP-l1 processors, that I have tested. >> @@ -193,7 +193,8 @@ typedef enum { >> && (chip->page_shift > 9)) >> >> /* Mask to zero out the chip options, which come from the id table */ >> -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR) >> +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \ >> + ~NAND_NO_SUBPAGE_WRITE) > > I wonder what we really need CHIPOPTIONS_MSK for? Silently ignoring what > the driver asked for doesn't seem right. Can't we just OR the two > sources? And it would be a driver error to specify a flag that doesn't > make sense to be set this way (i.e. only do it with the "doesn't support > X" flags). Ben reports quite the same issue, as this patch mixes chip options with NAND controller capabilities. Probably it is better as I answered to Ben to use CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE in nand_base.c to switch off subpage access instead of setting the chip as not able to handle subpages, changing the NAND_CHIPOPTIONS_MSK. Best regards, Stefano Babic
On Tue, Apr 12, 2011 at 5:08 AM, Stefano Babic <sbabic@denx.de> wrote: > Ben Gardiner wrote: >> Thanks for sharing this patch -- I have been using the "-O 2048" (VID >> header offset) option to prevent subpages here. > > Yes, this works too, at least with Linux. (being picky / for archival purposes) In u-boot, specifying the VID header offset as an argument to the ubi attach command -- i.e. 'ubi part <mtd_partition> 2048' -- also works (on da850evm, at least). > [...] >> First let me say that I would be happy to have a fix for this merged >> -- so I think the change is "good enough" since it makes UBI work >> without specifying a VID header offset equal to the NAND page size. >> What follows is more topical musings that any particular criticisms >> that should be considered blockers of your patch. >> >> I'm wondering about the long-term path for davinci NAND and subpage writes... >> >> But the sentiment I've heard about adding an exception to >> NAND_CHIPOPTIONS_MSK as above is that we are mixing features of the >> NAND chip and features of the NAND controller (If you have a modern >> SLC NAND then your chip probably supports subpage writes whereas it is >> the controller that needs to be limited). > > That is true. With this patch I constrained the SLC NAND on my board to > be considered as MLC, as a MLC NAND cannot be accessed in subpage mode. > > However, I set also a CONFIG_SYS_NAND_NO_SUBPAGE, and instead of > dropping the option in the mask we could protect the code where the > option is checked. In other words, we could change nand_base.c in this way: > > #ifndef CONFIG_SYS_NAND_NO_SUBPAGE > if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && > !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) { > switch(chip->ecc.steps) { > case 2: > mtd->subpage_sft = 1; > break; > case 4: > case 8: > case 16: > mtd->subpage_sft = 2; > break; > } > } > #endif > > Then it could be clearer that the restriction is due to the NAND > controller, and not to the chip itself. I agree -- it keeps the chip and controller options separated. >> What's more is that the davinci nand controller could do subpage >> writing if the writing operation were informed of the extents of the >> subpage being written instead of being handed a buffer with 0xFF in >> the non-target page areas. Which, I believe is Artem's primary >> motivation for the introduction of nand_write_subpage() to the davinci >> NAND controller driver [2]. > > Well, of course, if the davinci NAND can handle subpages, we can remove > the limitation. What do you think to add this patch in the way I suggest > now (without touching NAND_CHIPOPTIONS_MSK, that makes the MTD > inconsistent with Linux) until a subpage_write is added to davinci ? Yes, I like the way your proposed changes disabled subpage initialization instead of adding an exception to the chip options mask. I'm still OK with merging a patch to disable subpage writes on da850 -- as an interim solution. i.e. until a subpage_write is added to the mtd subsystem, and davinci-nand has an override and u-boot gets a similar change. >> So if the nand_write_subpage family of functions was introduced to the >> Linux kernel instead of adding another exception to >> NAND_CHIPOPTIONS_MSK then we would have 3 ways to use UBI on davinci >> NAND: >> 1) no patch: VID offset <page size> required >> 2) chip NAND_NO_SUBPAGE_WRITE patch >> 3) subpage writes support with nand_write_subpage() >> >> systems with 2) or 3) could always use UBI as in 1) and a UBI volume >> made with 2) would need to be attached as in 1) on systems with 2) or >> 3) ; but a UBI volume made with 3) could not be used on systems with >> 1) or 2) which means that we could not make use of the efficiency >> benefits of nand_write_subpage() if/when it is available on systems >> which need access to UBI from U-boot. > > I agree with your analyses. As personal feeling, solution one has the > drawback that it cannot be used in u-boot, and it seems to me very easy > to have a wrong UBI on the target. We can consider 2) as temporary > solution, until 3) is available. Thanks for confirming, Stefano. I'm not intimately familiar with the ea20 so there could be additional quirks in the way -- but I can say that option 1) works in u-boot on the da850evm (+UI board). I use 'ubi part ubi_part 2048' -- ubi_part is the mtdpart name of my ubi part :) Best Regards, Ben Gardiner --- Nanometrics Inc. http://www.nanometrics.ca
On Tue, 12 Apr 2011 11:44:26 +0200 Stefano Babic <sbabic@denx.de> wrote: > On 04/11/2011 09:16 PM, Scott Wood wrote: > > This only controls the davinci driver, so it should be > > CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE. > > > > Is this really board-specific? > > No, really not. > > > Does the davinci driver ever support > > subpage? > > No, it does not. This problem affects all boards using the davinci NAND > controller. At least with 850 or OMAP-l1 processors, that I have tested. > > >> @@ -193,7 +193,8 @@ typedef enum { > >> && (chip->page_shift > 9)) > >> > >> /* Mask to zero out the chip options, which come from the id table */ > >> -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR) > >> +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \ > >> + ~NAND_NO_SUBPAGE_WRITE) > > > > I wonder what we really need CHIPOPTIONS_MSK for? Silently ignoring what > > the driver asked for doesn't seem right. Can't we just OR the two > > sources? And it would be a driver error to specify a flag that doesn't > > make sense to be set this way (i.e. only do it with the "doesn't support > > X" flags). > > Ben reports quite the same issue, as this patch mixes chip options with > NAND controller capabilities. > > Probably it is better as I answered to Ben to use > CONFIG_SYS_DAVINCI_NAND_NO_SUBPAGE in nand_base.c to switch off subpage > access instead of setting the chip as not able to handle subpages, > changing the NAND_CHIPOPTIONS_MSK. Davinci-specific #defines do not belong in nand_base.c[1]. The controller driver should be able to set "this isn't supported" options just as well as the chip data -- I just don't think it should be limited to this specific one. For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR. Before this thread, I didn't realize it they were getting ignored. Things work anyway because the former is an optimization, and the latter is getting forced on after the masking, for some reason -- does autoincr simply not work? Can we remove the code? :-) -Scott [1] Nor should it be turned back into a non-davinci define -- what if there are multiple NAND controllers supported, and only one requires this? It's not so bad in U-Boot (I'd still rather avoid it though), but this approach is not going to go over well in Linux. How is Linux handling this?
On 04/13/2011 06:24 PM, Scott Wood wrote: Hi Scott, > Davinci-specific #defines do not belong in nand_base.c[1]. The controller > driver should be able to set "this isn't supported" options just as well as > the chip data -- I just don't think it should be limited to this specific > one. surely, but it is not clear to me how. There is no entry for a write_subpage function, as this issue does not happen with other controllers, and I do not see a callback for the driver after the nand_scan() function, where I thought the driver could change the options according to its capabilities. > > For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR. > Before this thread, I didn't realize it they were getting ignored. > Things > work anyway because the former is an optimization, and the latter is getting > forced on after the masking, for some reason -- does autoincr simply not > work? Can we remove the code? :-) Well, the options are simply ignored, I agree about removing them. > > -Scott > > [1] Nor should it be turned back into a non-davinci define -- what if there > are multiple NAND controllers supported, and only one requires this? It's > not so bad in U-Boot (I'd still rather avoid it though), but this approach > is not going to go over well in Linux. > > How is Linux handling this? What I have seen (kernel 2.6.38), the options are ignored in Linux as well, and this issue is open for the davinci processors. Best regards, Stefano Babic
On Fri, 15 Apr 2011 19:34:48 +0200 Stefano Babic <sbabic@denx.de> wrote: > On 04/13/2011 06:24 PM, Scott Wood wrote: > > Hi Scott, > > > Davinci-specific #defines do not belong in nand_base.c[1]. The controller > > driver should be able to set "this isn't supported" options just as well as > > the chip data -- I just don't think it should be limited to this specific > > one. > > surely, but it is not clear to me how. There is no entry for a > write_subpage function, as this issue does not happen with other > controllers, I don't see where subpage writes are done at all, actually. > and I do not see a callback for the driver after the > nand_scan() function, where I thought the driver could change the > options according to its capabilities. nand_scan() is broken into head and tail functions. In Linux, the driver calls these, and can look at the chip info before tail is called. In U-boot, common code drives this, and the controller driver is not involved -- but it would be good to change this. > > For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR. > > Before this thread, I didn't realize it they were getting ignored. > > Things > > work anyway because the former is an optimization, and the latter is getting > > forced on after the masking, for some reason -- does autoincr simply not > > work? Can we remove the code? :-) > > Well, the options are simply ignored, I agree about removing them. I think it can be enabled by the controller driver between head and tail (at least on Linux), though I don't see any drivers that do this as far as a quick grep shows. -Scott
On 04/15/2011 10:29 PM, Scott Wood wrote: > > nand_scan() is broken into head and tail functions. In Linux, the driver > calls these, and can look at the chip info before tail is called. In > U-boot, common code drives this, and the controller driver is not involved > -- but it would be good to change this. Agree, we can do in this way. > >>> For example, fsl_elbc_nand.c sets NAND_NO_READRDY and NAND_NO_AUTOINCR. >>> Before this thread, I didn't realize it they were getting ignored. >>> Things >>> work anyway because the former is an optimization, and the latter is getting >>> forced on after the masking, for some reason -- does autoincr simply not >>> work? Can we remove the code? :-) >> >> Well, the options are simply ignored, I agree about removing them. > > I think it can be enabled by the controller driver between head and tail > (at least on Linux), though I don't see any drivers that do this as far as > a quick grep shows. The only point is we need that the controller checks the parameters after the tail part, as the tail part scans the chip and sets its options, as the SUBPAGE flag. We could add a pre_adjust() and post_adjust() functions to be called after the head and tail part of the nand_scan(). Best regards, Stefano
On Fri, 22 Apr 2011 09:13:34 +0200 Stefano Babic <sbabic@denx.de> wrote: > The only point is we need that the controller checks the parameters > after the tail part, as the tail part scans the chip and sets its > options, as the SUBPAGE flag. We could add a pre_adjust() and > post_adjust() functions to be called after the head and tail part of the > nand_scan(). This is what the tail function is supposed to be for. I don't see where NAND_NO_AUTOINCR or NAND_NO_SUBPAGE_WRITE are called in the head (nand_scan_ident/nand_get_flash_type) functions. I only see NAND_BUSWIDTH_16 and NAND_4PAGE_ARRAY checked there. -Scott
diff --git a/README b/README index 21cd71b..8d664eb 100644 --- a/README +++ b/README @@ -2907,6 +2907,10 @@ Low Level (hardware related) configuration options: that is executed before the actual U-Boot. E.g. when compiling a NAND SPL. +- CONFIG_SYS_NAND_NO_SUBPAGE + Some drivers (davinci) do not support access to NAND subpage. + + Building the Software: ====================== diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index d41579c..f6c7d09 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand) #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT nand->options |= NAND_USE_FLASH_BBT; #endif +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE + nand->options |= NAND_NO_SUBPAGE_WRITE; +#endif #ifdef CONFIG_SYS_NAND_HW_ECC nand->ecc.mode = NAND_ECC_HW; nand->ecc.size = 512; diff --git a/include/configs/ea20.h b/include/configs/ea20.h index 1843dae..9e5dda4 100644 --- a/include/configs/ea20.h +++ b/include/configs/ea20.h @@ -171,6 +171,7 @@ #define CONFIG_NAND_DAVINCI #define CONFIG_SYS_NAND_PAGE_2K +#define CONFIG_SYS_NAND_NO_SUBPAGE #define CONFIG_SYS_NAND_CS 2 #define CONFIG_SYS_NAND_BASE DAVINCI_ASYNC_EMIF_DATA_CE2_BASE #undef CONFIG_SYS_NAND_HW_ECC diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 987a2ec..215e781 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -193,7 +193,8 @@ typedef enum { && (chip->page_shift > 9)) /* Mask to zero out the chip options, which come from the id table */ -#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR) +#define NAND_CHIPOPTIONS_MSK (0x0000ffff & ~NAND_NO_AUTOINCR & \ + ~NAND_NO_SUBPAGE_WRITE) /* Non chip related options */ /* Use a flash based bad block table. This option is passed to the
The NAND controller does not support subpage accessing. This is not used at all for MLC NAND, but it is set for SLC NAND. UBI tries to access to subpages, and because it fails, it starts "torture tests" on the whole page that are always successful, making an endless loop with the UBI background task taking most CPU time. On the console, the issue is recognized by the messages: UBI error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from PEB 37:512, read 512 bytes UBI: run torture test for PEB 37 UBI: PEB 37 passed torture test, do not mark it a bad Signed-off-by: Stefano Babic <sbabic@denx.de> CC: Ben Gardiner<bengardiner@nanometrics.ca> CC: Sandeep Paulraj <s-paulraj@ti.com> CC: Scott Wood <scottwood@freescale.com> --- README | 4 ++++ drivers/mtd/nand/davinci_nand.c | 3 +++ include/configs/ea20.h | 1 + include/linux/mtd/nand.h | 3 ++- 4 files changed, 10 insertions(+), 1 deletions(-)