LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
@ 2015-01-14 15:38 dinguyen
  2015-02-03 22:15 ` Dinh Nguyen
  2015-03-31  1:33 ` Brian Norris
  0 siblings, 2 replies; 7+ messages in thread
From: dinguyen @ 2015-01-14 15:38 UTC (permalink / raw)
  To: computersforpeace
  Cc: dinh.linux, dwmw2, yamada.m, josh, linux-mtd, linux-kernel,
	Graham Moore, Dinh Nguyen

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-01-14 15:38 [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver dinguyen
@ 2015-02-03 22:15 ` Dinh Nguyen
  2015-02-03 22:23   ` Richard Weinberger
  2015-03-31  1:33 ` Brian Norris
  1 sibling, 1 reply; 7+ messages in thread
From: Dinh Nguyen @ 2015-02-03 22:15 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: computersforpeace, dwmw2, yamada.m, josh, linux-mtd, Linux List,
	Graham Moore

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-02-03 22:15 ` Dinh Nguyen
@ 2015-02-03 22:23   ` Richard Weinberger
  2015-02-05 16:18     ` Graham Moore
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Weinberger @ 2015-02-03 22:23 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Dinh Nguyen, Brian Norris, David Woodhouse, yamada.m,
	Josh Triplett, linux-mtd, Linux List, Graham Moore

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.

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-02-03 22:23   ` Richard Weinberger
@ 2015-02-05 16:18     ` Graham Moore
  2015-02-06  8:29       ` Ricard Wanderlof
  0 siblings, 1 reply; 7+ messages in thread
From: Graham Moore @ 2015-02-05 16:18 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Dinh Nguyen, Dinh Nguyen, Brian Norris, David Woodhouse,
	yamada.m, Josh Triplett, linux-mtd, Linux List



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-02-05 16:18     ` Graham Moore
@ 2015-02-06  8:29       ` Ricard Wanderlof
  2015-02-06  9:28         ` Richard Weinberger
  0 siblings, 1 reply; 7+ messages in thread
From: Ricard Wanderlof @ 2015-02-06  8:29 UTC (permalink / raw)
  To: Graham Moore
  Cc: Richard Weinberger, Dinh Nguyen, Josh Triplett, Linux List,
	yamada.m, linux-mtd, Dinh Nguyen, Brian Norris, David Woodhouse


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            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-02-06  8:29       ` Ricard Wanderlof
@ 2015-02-06  9:28         ` Richard Weinberger
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Weinberger @ 2015-02-06  9:28 UTC (permalink / raw)
  To: Ricard Wanderlof, Graham Moore
  Cc: Dinh Nguyen, Josh Triplett, Linux List, yamada.m, linux-mtd,
	Dinh Nguyen, Brian Norris, David Woodhouse

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver
  2015-01-14 15:38 [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver dinguyen
  2015-02-03 22:15 ` Dinh Nguyen
@ 2015-03-31  1:33 ` Brian Norris
  1 sibling, 0 replies; 7+ messages in thread
From: Brian Norris @ 2015-03-31  1:33 UTC (permalink / raw)
  To: dinguyen
  Cc: dinh.linux, dwmw2, yamada.m, josh, linux-mtd, linux-kernel,
	Graham Moore, Ricard Wanderlof, Richard Weinberger

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-03-31  1:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 15:38 [PATCH] mtd: denali: Disable sub-page writes in Denali NAND driver dinguyen
2015-02-03 22:15 ` Dinh Nguyen
2015-02-03 22:23   ` Richard Weinberger
2015-02-05 16:18     ` Graham Moore
2015-02-06  8:29       ` Ricard Wanderlof
2015-02-06  9:28         ` Richard Weinberger
2015-03-31  1:33 ` Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).