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 Gardinerbengardiner@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:
- no patch: VID offset <page size> required
- chip NAND_NO_SUBPAGE_WRITE patch
- 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
- 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