All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vimal Singh <vimal.newwork@gmail.com>
To: Steve Sakoman <sakoman@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>,
	linux-omap@vger.kernel.org,
	Linux MTD <linux-mtd@lists.infradead.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
Date: Thu, 31 Dec 2009 17:50:44 +0530	[thread overview]
Message-ID: <ce9ab5790912310420h20beb04csba060b3f5cfeee92@mail.gmail.com> (raw)
In-Reply-To: <5e088bd90912291347j370ab30cid7c6cfc558c2ed18@mail.gmail.com>

On Wed, Dec 30, 2009 at 3:17 AM, Steve Sakoman <sakoman@gmail.com> wrote:
> On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman <sakoman@gmail.com> wrote:
>
>> I can confirm that this issue exists on Overo too.  Sadly, switching
>> to 16 bit reads does not fix the issue for me.  I'll start digging to
>> see if I can find what's broken.
>
> I can also confirm that there are days when I am not even capable of
> applying a patch properly :-(
>
> Switching to 16 bit read accesses does indeed fix the ECC issue on Overo too.
>
> Which still leaves us needing to find the root cause of the breakage . . .

There is a bug in nand prefetch read routine, which comes into effect
only if nand device is a 16-bit device (as we have in zoom boards).
This bug is effective only with below combination of conditions:
1. nand deivce, in use, is a 16 bit device
2. nand driver supports 'subpage' read
3. SW ECC is in use

This was not seen old  kernel (ex: .23), because when, in early days,
we tested this (nand prefetch read in LDP boards) there was no
'subpage read' support.
Later when we had subpage read in (.27) kernel, we had hw ecc enabled
always in our internal tree. So, we missed this bug.

Here is a patch to fix this issue:

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1bb799f..75004fe 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -295,11 +295,14 @@ static void omap_read_buf_pref(struct
 	u32 *p = (u32 *)buf;

 	/* take care of subpage reads */
-	for (; len % 4 != 0; ) {
-		*buf++ = __raw_readb(info->nand.IO_ADDR_R);
-		len--;
+	if (len % 4) {
+		if (info->nand.options & NAND_BUSWIDTH_16)
+			omap_read_buf16(mtd, buf, len % 4);
+		else
+			omap_read_buf8(mtd, buf, len % 4);
+		p = (u32 *) (buf + len % 4);
+		len -= len % 4;
 	}
-	p = (u32 *) buf;

 	/* configure and start prefetch transfer */
 	ret = gpmc_prefetch_enable(info->gpmc_cs, 0x0, len, 0x0);
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Vimal Singh <vimal.newwork@gmail.com>
To: Steve Sakoman <sakoman@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>,
	linux-omap@vger.kernel.org,
	Linux MTD <linux-mtd@lists.infradead.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
Date: Thu, 31 Dec 2009 17:50:44 +0530	[thread overview]
Message-ID: <ce9ab5790912310420h20beb04csba060b3f5cfeee92@mail.gmail.com> (raw)
In-Reply-To: <5e088bd90912291347j370ab30cid7c6cfc558c2ed18@mail.gmail.com>

On Wed, Dec 30, 2009 at 3:17 AM, Steve Sakoman <sakoman@gmail.com> wrote:
> On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman <sakoman@gmail.com> wrote:
>
>> I can confirm that this issue exists on Overo too.  Sadly, switching
>> to 16 bit reads does not fix the issue for me.  I'll start digging to
>> see if I can find what's broken.
>
> I can also confirm that there are days when I am not even capable of
> applying a patch properly :-(
>
> Switching to 16 bit read accesses does indeed fix the ECC issue on Overo too.
>
> Which still leaves us needing to find the root cause of the breakage . . .

There is a bug in nand prefetch read routine, which comes into effect
only if nand device is a 16-bit device (as we have in zoom boards).
This bug is effective only with below combination of conditions:
1. nand deivce, in use, is a 16 bit device
2. nand driver supports 'subpage' read
3. SW ECC is in use

This was not seen old  kernel (ex: .23), because when, in early days,
we tested this (nand prefetch read in LDP boards) there was no
'subpage read' support.
Later when we had subpage read in (.27) kernel, we had hw ecc enabled
always in our internal tree. So, we missed this bug.

Here is a patch to fix this issue:

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1bb799f..75004fe 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -295,11 +295,14 @@ static void omap_read_buf_pref(struct
 	u32 *p = (u32 *)buf;

 	/* take care of subpage reads */
-	for (; len % 4 != 0; ) {
-		*buf++ = __raw_readb(info->nand.IO_ADDR_R);
-		len--;
+	if (len % 4) {
+		if (info->nand.options & NAND_BUSWIDTH_16)
+			omap_read_buf16(mtd, buf, len % 4);
+		else
+			omap_read_buf8(mtd, buf, len % 4);
+		p = (u32 *) (buf + len % 4);
+		len -= len % 4;
 	}
-	p = (u32 *) buf;

 	/* configure and start prefetch transfer */
 	ret = gpmc_prefetch_enable(info->gpmc_cs, 0x0, len, 0x0);

WARNING: multiple messages have this Message-ID (diff)
From: vimal.newwork@gmail.com (Vimal Singh)
To: linux-arm-kernel@lists.infradead.org
Subject: Issue in oamp nand driver with 32-bit reads in prefetch mode
Date: Thu, 31 Dec 2009 17:50:44 +0530	[thread overview]
Message-ID: <ce9ab5790912310420h20beb04csba060b3f5cfeee92@mail.gmail.com> (raw)
In-Reply-To: <5e088bd90912291347j370ab30cid7c6cfc558c2ed18@mail.gmail.com>

On Wed, Dec 30, 2009 at 3:17 AM, Steve Sakoman <sakoman@gmail.com> wrote:
> On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman <sakoman@gmail.com> wrote:
>
>> I can confirm that this issue exists on Overo too. ?Sadly, switching
>> to 16 bit reads does not fix the issue for me. ?I'll start digging to
>> see if I can find what's broken.
>
> I can also confirm that there are days when I am not even capable of
> applying a patch properly :-(
>
> Switching to 16 bit read accesses does indeed fix the ECC issue on Overo too.
>
> Which still leaves us needing to find the root cause of the breakage . . .

There is a bug in nand prefetch read routine, which comes into effect
only if nand device is a 16-bit device (as we have in zoom boards).
This bug is effective only with below combination of conditions:
1. nand deivce, in use, is a 16 bit device
2. nand driver supports 'subpage' read
3. SW ECC is in use

This was not seen old  kernel (ex: .23), because when, in early days,
we tested this (nand prefetch read in LDP boards) there was no
'subpage read' support.
Later when we had subpage read in (.27) kernel, we had hw ecc enabled
always in our internal tree. So, we missed this bug.

Here is a patch to fix this issue:

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1bb799f..75004fe 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -295,11 +295,14 @@ static void omap_read_buf_pref(struct
 	u32 *p = (u32 *)buf;

 	/* take care of subpage reads */
-	for (; len % 4 != 0; ) {
-		*buf++ = __raw_readb(info->nand.IO_ADDR_R);
-		len--;
+	if (len % 4) {
+		if (info->nand.options & NAND_BUSWIDTH_16)
+			omap_read_buf16(mtd, buf, len % 4);
+		else
+			omap_read_buf8(mtd, buf, len % 4);
+		p = (u32 *) (buf + len % 4);
+		len -= len % 4;
 	}
-	p = (u32 *) buf;

 	/* configure and start prefetch transfer */
 	ret = gpmc_prefetch_enable(info->gpmc_cs, 0x0, len, 0x0);

  reply	other threads:[~2009-12-31 12:21 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-23  9:33 Issue in oamp nand driver with 32-bit reads in prefetch mode Vimal Singh
2009-12-23  9:33 ` Vimal Singh
2009-12-23 17:44 ` Tony Lindgren
2009-12-23 17:44   ` Tony Lindgren
2009-12-29 20:38   ` Steve Sakoman
2009-12-29 20:38     ` Steve Sakoman
2009-12-29 20:38     ` Steve Sakoman
2009-12-29 21:47     ` Steve Sakoman
2009-12-29 21:47       ` Steve Sakoman
2009-12-29 21:47       ` Steve Sakoman
2009-12-31 12:20       ` Vimal Singh [this message]
2009-12-31 12:20         ` Vimal Singh
2009-12-31 12:20         ` Vimal Singh
2010-01-02 15:02         ` Steve Sakoman
2010-01-02 15:02           ` Steve Sakoman
2010-01-02 15:02           ` Steve Sakoman
2010-01-10  8:46         ` Artem Bityutskiy
2010-01-10  8:46           ` Artem Bityutskiy
2010-01-10  8:46           ` Artem Bityutskiy
2010-01-11  6:30           ` Vimal Singh
2010-01-11  6:30             ` Vimal Singh
2010-01-11  6:30             ` Vimal Singh
2010-01-16 21:35             ` Artem Bityutskiy
2010-01-16 21:35               ` Artem Bityutskiy
2010-01-16 21:35               ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce9ab5790912310420h20beb04csba060b3f5cfeee92@mail.gmail.com \
    --to=vimal.newwork@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=sakoman@gmail.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.