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

[Gupta, Pekon]

>From: Scott Wood [mailto:scottwood at freescale.com]
>>On Wed, 2014-04-09 at 10:33 +0000, Gupta, Pekon wrote:
>> >From: Helmut Schaa [mailto:helmut.schaa at googlemail.com]
>> >>On Wed, Apr 9, 2014 at 11:38 AM, Gupta, Pekon <pekon at ti.com> wrote:
>> [...]
>> >>
>> >> (1) if chip->ecc.hwctl() and chip->ecc.calculate are not implemented but you
>> >>  still want to use subpage write feature, then you need to provide custom
>> >>  implementation for chip->ecc.write_subpage().
>> >>  that's same for other interfaces of nand_chip like chip->ecc.write_page().
>> >
>> >But these don't cause panics :)
>> >
>> because fsl_elbc_nand.c uses custom implementations of chip->ecc.write_page()
>> 	@@ fsl_elbc_chip_init(...)
>> 		chip->ecc.write_page = fsl_elbc_write_page;
>>
>> Same needs to be done if subpage write is needed. However, as this is
>> a regression so please check if following patch solves your problem.[1]
>>
>>
>> >> (2) If you don't want to use subpage write feature then just disable it using
>> >>         chip->options |= NAND_NO_SUBPAGE_WRITE;
>> >>
>> >> Can you please tell which NAND controller driver is causing this ?
>> >> We need to fix that..
>> >
>> >This happens with fsl_elbc_nand (while trying to run ubiformat on a
>> >mtd dev) but the
>> >crash was caused by the introduction of nand_write_subpage_hwecc. So, in this
>> >case I think instead of trying to fix every possible driver we should
>> >let the nand core
>> >code handle this issue gracefully. Maybe we could add a WARN_ON_ONCE to
>> >notice which drivers require adjustments.
>> >
>> Yes agree. May be good to keep subpage write disabled by default,
>> as only handful drivers possibly use that.
>>
>>
>> [1] ## <not compile tested>
>> ------------
>> From bfd39102ed6aa99b7ac2b8394a2d12b879fbb4b7 Mon Sep 17 00:00:00 2001
>> 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).

Are you sure subpage write is supported on this driver ? I couldn't find any
custom implementation for chip->ecc.write_subpage() in fsl_elbc_nand.c.

And when fsl_elbc_nand.c uses default implementation of
chip->ecc.write_subpage() = nand_write_subpage_hwecc then it crashes
because it does not implement chip->ecc.hwctl().  (refer [1])

However above is just a fix for this regression, as it needs to be back ported
till kernel 3.10.x+. I'll send another patch to  'disable' subpage write support
by default for drivers, and only the ones which need it should explicitly enable it.


[1] http://lists.infradead.org/pipermail/linux-mtd/2014-April/053182.html

with regards, pekon