[PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[Lars-Peter Clausen]

From: Holger Freyther <zecke@openmoko.org>

This patch adds a flag to the s3c2410_nand platform data, which configures
whether hardware ECC is used.

Currently it is only possible to decide whether hw ECC should be used or not at
compile time through a config option. But if you want to build a kernel which
runs on multiple devices you might have a configuration where some devices
require hw ECC and some devices which want software ECC.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 arch/arm/plat-samsung/include/plat/nand.h |    1 +
 drivers/mtd/nand/s3c2410.c                |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/nand.h b/arch/arm/plat-samsung/include/plat/nand.h
index b64115f..cbdc8f0 100644
--- a/arch/arm/plat-samsung/include/plat/nand.h
+++ b/arch/arm/plat-samsung/include/plat/nand.h
@@ -49,6 +49,7 @@ struct s3c2410_platform_nand {
 	int	twrph1;	/* time for release CLE/ALE from nWE/nOE inactive */
 
 	unsigned int	ignore_unset_ecc:1;
+	unsigned int	software_ecc:1; /* force software ecc at runtime */
 
 	int			nr_sets;
 	struct s3c2410_nand_set *sets;
diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
index cea775a..4666b5e 100644
--- a/drivers/mtd/nand/s3c2410.c
+++ b/drivers/mtd/nand/s3c2410.c
@@ -842,7 +842,7 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
 	nmtd->mtd.owner    = THIS_MODULE;
 	nmtd->set	   = set;
 
-	if (hardware_ecc) {
+	if (!(info->platform && info->platform->software_ecc) && hardware_ecc) {
 		chip->ecc.calculate = s3c2410_nand_calculate_ecc;
 		chip->ecc.correct   = s3c2410_nand_correct_data;
 		chip->ecc.mode	    = NAND_ECC_HW;
-- 
1.7.2.5

[PATCH 2/2] ARM: s3c2440: GTA02: Disable hardware ECC by default

[Lars-Peter Clausen]

Early versions of uboot used for the GTA02 flashed the NAND with ECC information
incompatible to s3c2440 hardware ECC. Disable hardware error correction by
default, unless the bootloader explicitly enables it.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 arch/arm/mach-s3c2440/mach-gta02.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s3c2440/mach-gta02.c b/arch/arm/mach-s3c2440/mach-gta02.c
index 0db2411..b14d91f 100644
--- a/arch/arm/mach-s3c2440/mach-gta02.c
+++ b/arch/arm/mach-s3c2440/mach-gta02.c
@@ -445,6 +445,7 @@ static struct s3c2410_platform_nand __initdata gta02_nand_info = {
 	.twrph1		= 15,
 	.nr_sets	= ARRAY_SIZE(gta02_nand_sets),
 	.sets		= gta02_nand_sets,
+	.software_ecc	= 1,
 };
 
 
@@ -563,6 +564,14 @@ static void gta02_poweroff(void)
 	pcf50633_reg_set_bit_mask(gta02_pcf, PCF50633_REG_OOCSHDWN, 1, 1);
 }
 
+static int __init hardware_ecc_setup(char *str)
+{
+	if (str && str[0] == '1')
+		gta02_nand_info.software_ecc = 0;
+	return 1;
+}
+__setup("hardware_ecc=", hardware_ecc_setup);
+
 static void __init gta02_machine_init(void)
 {
 	/* Set the panic callback to turn AUX LED on or off. */
-- 
1.7.2.5

Re: [PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[Artem Bityutskiy]

On Tue, 2011-04-12 at 21:47 +0200, Lars-Peter Clausen wrote:
> From: Holger Freyther <zecke@openmoko.org>
> 
> This patch adds a flag to the s3c2410_nand platform data, which configures
> whether hardware ECC is used.
> 
> Currently it is only possible to decide whether hw ECC should be used or not at
> compile time through a config option. But if you want to build a kernel which
> runs on multiple devices you might have a configuration where some devices
> require hw ECC and some devices which want software ECC.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Extending platform data is kind of vetoed in arm tree, I do not think
the MTD tree can take these changes.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[Lars-Peter Clausen]

On 04/14/2011 02:08 PM, Artem Bityutskiy wrote:
> On Tue, 2011-04-12 at 21:47 +0200, Lars-Peter Clausen wrote:
>> From: Holger Freyther <zecke@openmoko.org>
>>
>> This patch adds a flag to the s3c2410_nand platform data, which configures
>> whether hardware ECC is used.
>>
>> Currently it is only possible to decide whether hw ECC should be used or not at
>> compile time through a config option. But if you want to build a kernel which
>> runs on multiple devices you might have a configuration where some devices
>> require hw ECC and some devices which want software ECC.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Extending platform data is kind of vetoed in arm tree, I do not think
> the MTD tree can take these changes.
> 
That is not my understanding of the situation. But what do you suggest as an
alternative for fixing this issue?

- Lars

Re: [PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[Artem Bityutskiy]

On Thu, 2011-04-14 at 16:48 +0200, Lars-Peter Clausen wrote:
> On 04/14/2011 02:08 PM, Artem Bityutskiy wrote:
> > On Tue, 2011-04-12 at 21:47 +0200, Lars-Peter Clausen wrote:
> >> From: Holger Freyther <zecke@openmoko.org>
> >>
> >> This patch adds a flag to the s3c2410_nand platform data, which configures
> >> whether hardware ECC is used.
> >>
> >> Currently it is only possible to decide whether hw ECC should be used or not at
> >> compile time through a config option. But if you want to build a kernel which
> >> runs on multiple devices you might have a configuration where some devices
> >> require hw ECC and some devices which want software ECC.
> >>
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > 
> > Extending platform data is kind of vetoed in arm tree, I do not think
> > the MTD tree can take these changes.
> > 
> That is not my understanding of the situation. But what do you suggest as an
> alternative for fixing this issue?

Well, I got this understanding by talking to the OMAP maintainer and by
chatting with rmk. But I might be wrong. So the understanding is that
board data extending is banned, at least for a while. And the arm world
has to consolidate and probably switch to the DT tree approach. This
would be painful, this would make some vendors to return to behind the
curtains, so this would be a step back in the short run.

But in a couple of years this would be resolved, may be under Linaro's
aegis, and then the arm world would make 2 steps forward, so that
previous step back would be compensated in the longer run.

But! I'm just an small MTD guy, so I might be mistaken. Of course if you
make your board file changes be merged via the corresponding arm
sub-tree - go ahead send your MTD driver updates! This is I guess the
answer to your "what do you suggest" question.

CCing the arm lists so that people could correct me.

Here is the beginning of the thread:
http://lists.infradead.org/pipermail/linux-mtd/2011-April/034866.html

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH 1/2] MTD: s3c2410_nand: Add option to disable hw ECC at runtime

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 05:01:11PM +0300, Artem Bityutskiy wrote:
> Well, I got this understanding by talking to the OMAP maintainer and by
> chatting with rmk. But I might be wrong. So the understanding is that
> board data extending is banned, at least for a while. And the arm world
> has to consolidate and probably switch to the DT tree approach. This
> would be painful, this would make some vendors to return to behind the
> curtains, so this would be a step back in the short run.

Well, what Linus desires from ARM folk is a concerted effort to get the
ARM tree 'under control' and stop the thing forever increasing without
any apparant control.

Linus has said is that if arch/arm/mach* show up with significant numbers
of net additional lines of code in the diffstat, he'll refuse to pull the
git tree.  (See various mails from Linus on linux-arm-kernel.)

My preference is to limit this merge window to just consolidation and
bug fixes so that we can show that we're taking the issue seriously and
are putting a decent amount of effort into addressing the problem.
However, the problem with adding new stuff is that it dilutes the
diffstat, and makes it much harder to show that we're doing what's
required.

Eg, lets say we end up removing 1000 LOC, but end up adding 1000 LOC of
new platform data via other trees.  I don't think Linus will be pleased
when he comes to the end of the merge window and arch/arm/ again shows
up heavily in the statistics without a significant reduction.  I suspect
if that were to happen, things will get much harder for us.

When you bear in mind that in total, arch/arm was pusing 60k lines of
new code into the kernel _each_ merge window.  If we manage to reduce
the size of arch/arm by only 10k, it is really just a small drop in the
ocean in comparison - we need much more effort than that...