[U-Boot] [PATCH 6/6] EA20: do not use subpage write for NAND

[Stefano Babic <sbabic@denx.de>]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================