[PATCH] mtd: nand: Disable subpage writes for drivers without ecc->hwctl

[Helmut Schaa]

On Thu, Apr 10, 2014 at 9:26 AM, Gupta, Pekon <pekon at ti.com> wrote:
> Hi Helmut Schaa,
>
>>From: Helmut Schaa [mailto:helmut.schaa at googlemail.com]
>>>Scott Wood <scottwood at freescale.com> schrieb:
> [...]
>>>> From: Pekon Gupta <pekon at ti.com>
>>>> Date: Wed, 9 Apr 2014 15:51:25 +0530
>>>> Subject: [PATCH] mtd: eLBC NAND: disable subpage write support
>>>>
>>>> As fsl_elbc_nand do not implement NAND ECC interfaces (like
>>>chip->ecc.hwctl(),
>>>> chip->ecc.calculate, and chip->ecc.correct) So it cannot use default
>>>> implementation of nand_write_subpage_hwecc() as in nand_base.c.
>>>> Hence disabling subpage write support till a custom implementation
>>>for
>>>> chip->ecc_write_subpage is added.
>>>>
>>>> CC: <stable at vger.kernel.org> # 3.10.x+
>>>> Signed-off-by: Pekon Gupta <pekon at ti.com>
>>>> ---
>>>>  drivers/mtd/nand/fsl_elbc_nand.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
>>>b/drivers/mtd/nand/fsl_elbc_nand.c
>>>> index ec549cd..a21252c 100644
>>>> --- a/drivers/mtd/nand/fsl_elbc_nand.c
>>>> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
>>>> @@ -755,6 +755,7 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd
>>>*priv)
>>>>
>>>>         /* set up nand options */
>>>>         chip->bbt_options = NAND_BBT_USE_FLASH;
>>>> +       chip->options |= NAND_NO_SUBPAGE_WRITE;
>>>
>>>Won't this break compatibility with existing UBI volumes?  That's why I
>>>didn't set this flag on eLBC when I set it on IFC (on the latter UBI is
>>>simply broken without that flag, but eLBC gets away with it because of
>>>the ECC algorithm used).
>>
>>Could be. I had to override the VID header offset accordingly to be able to attach to the ubi volume after applying this patch ...
>>
> There could be some misconfiguration in the way you generate your
> UBIFS image. Just check following:
> (1) min-io-size (-m) passed to mkfs.ubifs and ubinize commands?
> (2) vid-hdr-offset (-O) passed to ubinize and ubiattach commands?
> (3) subpage-size (-s) passed to ubinize commands? [optional]
> All of the above should be set to $PAGE_SIZE of your NAND device.
>
> Because if your image itself was generated such a way that vid-hdr was
> offset at page-boundaries then you shouldn't have needed this change.
> ---------------------------
> nand_base.c @@ nand_write_page(..)
>         if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
>                 chip->ecc.write_subpage)
>                 subpage = offset || (data_len < mtd->writesize);   <<-----
>         else
>                 subpage = 0;
>         [...]
>         else if (subpage)  <<--------
>                 status = chip->ecc.write_subpage(mtd, chip, offset, data_len,
>                                                          buf, oob_required);
> ---------------------------
> So, even if the subpage-write is enabled, but all the write accesses are
> page-aligned, (that is offset==0 && data_len % mtd->writesize ==0)
> then also chip->ecc.write_subpage() does _not_ come in path.

Reverting "mtd: nand: subpage write support for hardware based ECC
schemes" and then
using ubiformat (without additional options) to create a volume
results in a VID header offset
of 512. And it was possible to attach to this volume successfully.
Setting NAND_NO_SUBPAGE_WRITE prevents this ubi volume to be attached
without explicitly
setting the VID header offset. This indicates that with this patch
some setups might break.

I don't necessarily want to get my patch in but IMO this is not a regression in
fsl_elbc_nand but in nand_base (also possibly affecting other drivers).

Helmut