2015-02-03 22:15:10

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver

Hi Brian,

On Wed, Jan 14, 2015 at 9:38 AM, <[email protected]> wrote:
> From: Graham Moore <[email protected]>
>
> The Denali Controller IP does not support sub-page writes.
>
> Signed-off-by: Graham Moore <[email protected]>
> Signed-off-by: Dinh Nguyen <[email protected]>
> ---
> 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


2015-02-03 22:23:41

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver

On Tue, Feb 3, 2015 at 11:15 PM, Dinh Nguyen <[email protected]> wrote:
> Hi Brian,
>
> On Wed, Jan 14, 2015 at 9:38 AM, <[email protected]> wrote:
>> From: Graham Moore <[email protected]>
>>
>> The Denali Controller IP does not support sub-page writes.
>>
>> Signed-off-by: Graham Moore <[email protected]>
>> Signed-off-by: Dinh Nguyen <[email protected]>
>> ---
>> 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.

--
Thanks,
//richard

2015-02-05 16:35:29

by Graham Moore

[permalink] [raw]
Subject: Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver



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

2015-02-06 08:29:45

by Ricard Wanderlof

[permalink] [raw]
Subject: Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver


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
--
Ricard Wolf Wanderl?f ricardw(at)axis.com
Axis Communications AB, Lund, Sweden http://www.axis.com
Phone +46 46 272 2016 Fax +46 46 13 61 30

2015-02-06 09:29:06

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver

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