mtd: denali: Disable sub-page writes in Denali NAND driver

From: Graham Moore <grmoore@opensource.altera.com>

The Denali Controller IP does not support sub-page writes.

Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
 drivers/mtd/nand/denali.c | 3 +++
 1 file changed, 3 insertions(+)

Hi Brian,

On Wed, Jan 14, 2015 at 9:38 AM,  <dinguyen@opensource.altera.com> wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> The Denali Controller IP does not support sub-page writes.
>
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
>  drivers/mtd/nand/denali.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index b3b7ca1..b16b040 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1565,6 +1565,9 @@ int denali_init(struct denali_nand_info *denali)
>         denali->nand.options |= NAND_SKIP_BBTSCAN;
>         denali->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
>
> +       /* no subpage writes on denali */
> +       denali->nand.options |= NAND_NO_SUBPAGE_WRITE;
> +
>         /*
>          * Denali Controller only support 15bit and 8bit ECC in MRST,
>          * so just let controller do 15bit ECC for MLC and 8bit ECC for
> --
> 2.2.1
>

Was wondering if you got a chance to look at this patch?

Thanks,
Dinh

On Tue, Feb 3, 2015 at 11:15 PM, Dinh Nguyen <dinh.linux@gmail.com> wrote:
> Hi Brian,
>
> On Wed, Jan 14, 2015 at 9:38 AM,  <dinguyen@opensource.altera.com> wrote:
>> From: Graham Moore <grmoore@opensource.altera.com>
>>
>> The Denali Controller IP does not support sub-page writes.
>>
>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
>> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
>> ---
>>  drivers/mtd/nand/denali.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
>> index b3b7ca1..b16b040 100644
>> --- a/drivers/mtd/nand/denali.c
>> +++ b/drivers/mtd/nand/denali.c
>> @@ -1565,6 +1565,9 @@ int denali_init(struct denali_nand_info *denali)
>>         denali->nand.options |= NAND_SKIP_BBTSCAN;
>>         denali->nand.ecc.mode = NAND_ECC_HW_SYNDROME;
>>
>> +       /* no subpage writes on denali */
>> +       denali->nand.options |= NAND_NO_SUBPAGE_WRITE;
>> +
>>         /*
>>          * Denali Controller only support 15bit and 8bit ECC in MRST,
>>          * so just let controller do 15bit ECC for MLC and 8bit ECC for
>> --
>> 2.2.1
>>
>
> Was wondering if you got a chance to look at this patch?

So this driver never worked?
If it worked for some users we have to make sure that your change does not break
existing setups.
/me thinks of UBI.

On 02/03/2015 04:23 PM, Richard Weinberger wrote:
...

>
> So this driver never worked?
> If it worked for some users we have to make sure that your change does not break
> existing setups.
> /me thinks of UBI.
>

Actually, we made this change to make UBIFS work.  So, yes, the driver 
never worked for UBI.  Worked fine for JFFS2, raw data.

A customer reported an issue with ECC errors when using UBIFS on NAND 
flash with Altera SoC.

We debugged it and found the ECC errors occur because the UBI subsystem 
is trying to write sub-pages in the NAND, but neither the NAND chip 
itself nor the Denali NAND controller support sub-page writes.

In more detail, the UBIFS tries to write a sub-page, but the controller 
writes a whole page instead.  The controller's buffer is initialized to 
FF, so all the data outside the subpage is FF.

But in this case, part of the page in the device is already non-FF.  And 
that data does not change when FF is written to it.

The Denali controller calculates ECC on the data written, but that data 
will not match the data in the device, and so the ECC is incorrect.

When a subsequent read occurs, the ECC will not match and an error will 
occur.

Thanks,
Graham Moore

On Thu, 5 Feb 2015, Graham Moore wrote:

> Actually, we made this change to make UBIFS work.  So, yes, the driver 
> never worked for UBI.  Worked fine for JFFS2, raw data.
> 
> A customer reported an issue with ECC errors when using UBIFS on NAND 
> flash with Altera SoC.
> 
> We debugged it and found the ECC errors occur because the UBI subsystem 
> is trying to write sub-pages in the NAND, but neither the NAND chip 
> itself nor the Denali NAND controller support sub-page writes.

Just a bit curious.

It is not uncommon for controllers or chips not to support sub-page 
writes. In that case however, the partition(s) used by UBI should be 
formatted accordingly, i.e. using the appropriate --sub-page-size argument 
to ubiformat (when formatting partitions on the system itself), or the 
corresponding argument to ubinize (when preparing images offline).

If that is done correctly, then the lack of subpage write capability is 
not a problem per se (of course, the UBI EC and VID headers then take up 
more space so less space is available for user data; on a flash with 2k 
pages it is only 2k bytes per LEB that is lost however).

/Ricard

Am 06.02.2015 um 09:29 schrieb Ricard Wanderlof:
> 
> On Thu, 5 Feb 2015, Graham Moore wrote:
> 
>> Actually, we made this change to make UBIFS work.  So, yes, the driver 
>> never worked for UBI.  Worked fine for JFFS2, raw data.
>>
>> A customer reported an issue with ECC errors when using UBIFS on NAND 
>> flash with Altera SoC.
>>
>> We debugged it and found the ECC errors occur because the UBI subsystem 
>> is trying to write sub-pages in the NAND, but neither the NAND chip 
>> itself nor the Denali NAND controller support sub-page writes.
> 
> Just a bit curious.
> 
> It is not uncommon for controllers or chips not to support sub-page 
> writes. In that case however, the partition(s) used by UBI should be 
> formatted accordingly, i.e. using the appropriate --sub-page-size argument 
> to ubiformat (when formatting partitions on the system itself), or the 
> corresponding argument to ubinize (when preparing images offline).
> 
> If that is done correctly, then the lack of subpage write capability is 
> not a problem per se (of course, the UBI EC and VID headers then take up 
> more space so less space is available for user data; on a flash with 2k 
> pages it is only 2k bytes per LEB that is lost however).

Yeah, but UBI automatically will use subpages unless you specify
use the vid_hdr_offs parameter.
IOW, if the driver advertises subpages UBI will use them.

Thanks,
//richard

On Wed, Jan 14, 2015 at 09:38:50AM -0600, dinguyen@opensource.altera.com wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
> 
> The Denali Controller IP does not support sub-page writes.
> 
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>

Pushed to l2-mtd.git. Thanks!

Brian