Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Mon, 2010-01-11 at 12:00 +0530, Vimal Singh wrote:
> On Sun, Jan 10, 2010 at 2:16 PM, Artem Bityutskiy wrote:
> > On Thu, 2009-12-31 at 17:50 +0530, Vimal Singh wrote:
> >> 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);
> >
> > Pushed this patch to my l2-mtd-2.6.git/dunno.
> >
>
> I have posted this patch formally in patch series:
> [PATCH 0/3][NAND][OMAP]: fixing omap nand driver issues
>
> Corresponding patch is:
> [PATCH 3/3][NAND][OMAP] : Fixing issue in oamp nand driver in prefetch mode
> read
>
> Can you please give a look for this patch series?
Just pushed them to my l2-mtd-2.6.git/dunno
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Sun, Jan 10, 2010 at 2:16 PM, Artem Bityutskiy wrote:
> On Thu, 2009-12-31 at 17:50 +0530, Vimal Singh wrote:
>> 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);
>
> Pushed this patch to my l2-mtd-2.6.git/dunno.
>
I have posted this patch formally in patch series:
[PATCH 0/3][NAND][OMAP]: fixing omap nand driver issues
Corresponding patch is:
[PATCH 3/3][NAND][OMAP] : Fixing issue in oamp nand driver in prefetch mode read
Can you please give a look for this patch series?
--
Regards,
Vimal Singh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Thu, 2009-12-31 at 17:50 +0530, Vimal Singh wrote:
> 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);
Pushed this patch to my l2-mtd-2.6.git/dunno.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Thu, Dec 31, 2009 at 4:20 AM, Vimal Singh wrote:
> On Wed, Dec 30, 2009 at 3:17 AM, Steve Sakoman wrote:
>> On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman 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);
I tested this on Overo and can confirm that it fixes the issue.
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Wed, Dec 30, 2009 at 3:17 AM, Steve Sakoman wrote:
> On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Tue, Dec 29, 2009 at 12:38 PM, Steve Sakoman 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 . . . Steve -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
On Wed, Dec 23, 2009 at 9:44 AM, Tony Lindgren wrote:
> * Vimal Singh [091223 01:32]:
>> In omap nand driver we used to do 32-bit reads (in function
>> omap_read_buf_pref, using 'ioread32_rep' i.e.: __raw_readsl), and it
>> was working fine till some time back. But now, somehow, this has got
>> some problem. Reads are not being done successfully, and driver gives
>> lots for error reprot like below:
>>
>> td->read(0x604 bytes from 0x1fc) returned ECC error
>> uncorrectable error :
>> uncorrectable error :
>> uncorrectable error :
>> uncorrectable error :
>> uncorrectable error :
>> uncorrectable error :
>> mtd->read(0x520 bytes from 0x2e0) returned ECC error
>>
>> While if we change 32-bit reads to 16-bit reads (using 'ioread16_rep'
>> i.e.: __raw_readwl) makes it working fine. (A patch to do this is
>> listed below at the bottom of this mail.)
>>
>> I have not seen any change in routine '__raw_readsl'.
>> So, could someone help me find out the root cause the issue?
>>
>> Thanks for any input.
>
> Maybe an issue with the GPMC timings?
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.
Steve
> Regards,
>
> Tony
>
>>
>> --
>> Regards,
>> Vimal Singh
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index aaef170..dc493f5 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -288,14 +288,14 @@ static void omap_read_buf_pref(struct mtd_info
>> struct omap_nand_info, mtd);
>> uint32_t pfpw_status = 0, r_count = 0;
>> int ret = 0;
>> - u32 *p = (u32 *)buf;
>> + u16 *p = (u16 *)buf;
>>
>> /* take care of subpage reads */
>> - for (; len % 4 != 0; ) {
>> + for (; len % 2 != 0; ) {
>> *buf++ = __raw_readb(info->nand.IO_ADDR_R);
>> len--;
>> }
>> - p = (u32 *) buf;
>> + p = (u16 *) buf;
>>
>> /* configure and start prefetch transfer */
>> ret = gpmc_prefetch_enable(info->gpmc_cs, 0x0, len, 0x0);
>> @@ -308,10 +308,10 @@ static void omap_read_buf_pref(struct mtd_info
>> *mtd, u_char *buf, int len)
>> } else {
>> do {
>> pfpw_status = gpmc_prefetch_status();
>> - r_count = ((pfpw_status >> 24) & 0x7F) >> 2;
>> - ioread32_rep(info->nand_pref_fifo_add, p, r_count);
>> + r_count = ((pfpw_status >> 24) & 0x7F) >> 1;
>> + ioread16_rep(info->nand_pref_fifo_add, p, r_count);
>> p += r_count;
>> - len -= r_count << 2;
>> + len -= r_count << 1;
>> } while (len);
>>
>> /* disable and stop the PFPW engine */
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue in oamp nand driver with 32-bit reads in prefetch mode
* Vimal Singh [091223 01:32]:
> In omap nand driver we used to do 32-bit reads (in function
> omap_read_buf_pref, using 'ioread32_rep' i.e.: __raw_readsl), and it
> was working fine till some time back. But now, somehow, this has got
> some problem. Reads are not being done successfully, and driver gives
> lots for error reprot like below:
>
> td->read(0x604 bytes from 0x1fc) returned ECC error
> uncorrectable error :
> uncorrectable error :
> uncorrectable error :
> uncorrectable error :
> uncorrectable error :
> uncorrectable error :
> mtd->read(0x520 bytes from 0x2e0) returned ECC error
>
> While if we change 32-bit reads to 16-bit reads (using 'ioread16_rep'
> i.e.: __raw_readwl) makes it working fine. (A patch to do this is
> listed below at the bottom of this mail.)
>
> I have not seen any change in routine '__raw_readsl'.
> So, could someone help me find out the root cause the issue?
>
> Thanks for any input.
Maybe an issue with the GPMC timings?
Regards,
Tony
>
> --
> Regards,
> Vimal Singh
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index aaef170..dc493f5 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -288,14 +288,14 @@ static void omap_read_buf_pref(struct mtd_info
> struct omap_nand_info, mtd);
> uint32_t pfpw_status = 0, r_count = 0;
> int ret = 0;
> - u32 *p = (u32 *)buf;
> + u16 *p = (u16 *)buf;
>
> /* take care of subpage reads */
> - for (; len % 4 != 0; ) {
> + for (; len % 2 != 0; ) {
> *buf++ = __raw_readb(info->nand.IO_ADDR_R);
> len--;
> }
> - p = (u32 *) buf;
> + p = (u16 *) buf;
>
> /* configure and start prefetch transfer */
> ret = gpmc_prefetch_enable(info->gpmc_cs, 0x0, len, 0x0);
> @@ -308,10 +308,10 @@ static void omap_read_buf_pref(struct mtd_info
> *mtd, u_char *buf, int len)
> } else {
> do {
> pfpw_status = gpmc_prefetch_status();
> - r_count = ((pfpw_status >> 24) & 0x7F) >> 2;
> - ioread32_rep(info->nand_pref_fifo_add, p, r_count);
> + r_count = ((pfpw_status >> 24) & 0x7F) >> 1;
> + ioread16_rep(info->nand_pref_fifo_add, p, r_count);
> p += r_count;
> - len -= r_count << 2;
> + len -= r_count << 1;
> } while (len);
>
> /* disable and stop the PFPW engine */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
