Hello.

On 28-01-2012 20:08, Gary Thomas wrote:

The use of pdev->id as a channel select is problematic, at least
on the OMAP-L13x when using 4 bit hardware assisted ECC. On this
hardware, only EMiF channels (chip select) 2 through 5 are available.
Specifying platform device id 1 when using chip select 3 is obscure
at best.

If the core_chipsel value used by the driver is incorrect, then
the ECC hardware is not set up properly and ends up always reading
ECC values of zero. This is totally broken as no ECC errors will
be detected on reads and incorrect ECC values will be stored on writes.

The best way to fix this is to pass the actual chip select in
the platform data, rather than have it be inferred from the
platform device id. The attached patch provides for this. Also,
this patch is a no-op for target platforms that do not specify
the core_chipsel value in the platform data. This patch was
generated against the mainline 3.0 kernel, but applies to 3.2
as well.

Note: there does not seem to be an identified maintainer of this
code, so this list was my first guess at where to post.

Pelase send your patches inline, not as attachments.

From 9695c4d14bacf12dce8b8ab027f1658d6ecea6f0 Mon Sep 17 00:00:00 2001
From: Gary Thomas <g...@mlbassoc.com>
Date: Fri, 27 Jan 2012 14:44:08 -0700
Subject: [PATCH] davinci_nand: Fix 4 bit ECC on OMAP-L138

Don't include this header in the patches.

NAND chip selects restricted to 2-5 when using HW 4BIT ECC.
Using the platform device id to specify this is problematic
at best and confusing at least.

Let the platform data specify the actual chip select (2..5)

Signed-off-by: Gary Thomas <g...@mlbassoc.com>
---
arch/arm/mach-davinci/include/mach/nand.h | 5 ++++-
drivers/mtd/nand/davinci_nand.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/nand.h
b/arch/arm/mach-davinci/include/mach/nand.h
index 0251510..2d71548 100644
--- a/arch/arm/mach-davinci/include/mach/nand.h
+++ b/arch/arm/mach-davinci/include/mach/nand.h
@@ -57,7 +57,10 @@ struct davinci_nand_pdata { /* platform_data */
uint32_t mask_cle;

/* for packages using two chipselects */
- uint32_t mask_chipsel;
+ uint32_t mask_chipsel;

You're adding trailing whitespace here.

+
+ /* Old way using pdev->id is confusing */

Shaould indent this with a tab, not spaces.

+ uint32_t core_chipsel;

/* board's default static partition info */
struct mtd_partition *parts;
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 1f34951..2072da5 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -590,7 +590,10 @@ static int __init nand_davinci_probe(struct
platform_device *pdev)
info->ioaddr = (uint32_t __force) vaddr;

info->current_cs = info->ioaddr;
- info->core_chipsel = pdev->id;
+ if (pdata->core_chipsel)
+ info->core_chipsel = pdata->core_chipsel-2;
+ else
+ info->core_chipsel = pdev->id;

Your code is whitespace damaged here, i.e. all tabs replaced by spaces. And
that's despite the patch is in attachament...

WBR, Sergei

Fair enough, I can clean up the cosmetics. What about the
content of the patch itself??

   I like it, if you ask me.

WBR, Sergei
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to