[PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

changelog v3:

Just a rebase on current l2-mtd.git.

changelog v2:
  - extract the iounmap calls from patch "Add support for error IRQ" into a new patch.
  - swapped patches "Add support for error IRQ" and "Add error IRQ resource"
  - corrected an indentation mistake

This series cleans up the flctl when run in hardware ecc mode. The first 2 patches make sure we catch all errors that result from hardware transmission. The other patches handle how the ecc is layed out, correct some code to write and read it and make sure we propagate the statistics about errors/repairs to the nand base.

Bastian Hecht (10):
  mtd: sh_flctl: Add missing iounmap()
  ARM: sh-mobile: mackerel: Add error IRQ resource
  mtd: sh_flctl: Add support for error IRQ
  mtd: sh_flctl: Use different OOB layout
  mtd: sh_flctl: Fix hardware ECC behaviour
  mtd: sh_flctl: Simplify the hardware ecc page read/write
  mtd: sh_flctl: Group sector accesses into a single transfer
  mtd: sh_flctl: Restructure the hardware ECC handling
  mtd: sh_flctl: Use user oob data in hardware ECC mode
  ARM: sh-mobile: mackerel: Use hardware error correction

 arch/arm/mach-shmobile/board-mackerel.c |    7 +-
 drivers/mtd/nand/sh_flctl.c             |  296 ++++++++++++++++++-------------
 include/linux/mtd/sh_flctl.h            |   23 ++-
 3 files changed, 193 insertions(+), 133 deletions(-)

-- 
1.7.5.4

[PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

changelog v3:

Just a rebase on current l2-mtd.git.

changelog v2:
  - extract the iounmap calls from patch "Add support for error IRQ" into a new patch.
  - swapped patches "Add support for error IRQ" and "Add error IRQ resource"
  - corrected an indentation mistake

This series cleans up the flctl when run in hardware ecc mode. The first 2 patches make sure we catch all errors that result from hardware transmission. The other patches handle how the ecc is layed out, correct some code to write and read it and make sure we propagate the statistics about errors/repairs to the nand base.

Bastian Hecht (10):
  mtd: sh_flctl: Add missing iounmap()
  ARM: sh-mobile: mackerel: Add error IRQ resource
  mtd: sh_flctl: Add support for error IRQ
  mtd: sh_flctl: Use different OOB layout
  mtd: sh_flctl: Fix hardware ECC behaviour
  mtd: sh_flctl: Simplify the hardware ecc page read/write
  mtd: sh_flctl: Group sector accesses into a single transfer
  mtd: sh_flctl: Restructure the hardware ECC handling
  mtd: sh_flctl: Use user oob data in hardware ECC mode
  ARM: sh-mobile: mackerel: Use hardware error correction

 arch/arm/mach-shmobile/board-mackerel.c |    7 +-
 drivers/mtd/nand/sh_flctl.c             |  296 ++++++++++++++++++-------------
 include/linux/mtd/sh_flctl.h            |   23 ++-
 3 files changed, 193 insertions(+), 133 deletions(-)

-- 
1.7.5.4

[PATCH v3 01/10] mtd: sh_flctl: Add missing iounmap()

[Bastian Hecht]

Add the unmapping for the error case and for the driver removal.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/mtd/nand/sh_flctl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 2ee9a1b..ed86a64 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -919,6 +919,7 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 
 err_chip:
 	pm_runtime_disable(&pdev->dev);
+	iounmap(flctl->reg);
 err_iomap:
 	kfree(flctl);
 	return ret;
@@ -930,6 +931,7 @@ static int __devexit flctl_remove(struct platform_device *pdev)
 
 	nand_release(&flctl->mtd);
 	pm_runtime_disable(&pdev->dev);
+	iounmap(flctl->reg);
 	kfree(flctl);
 
 	return 0;
-- 
1.7.5.4

[PATCH v3 01/10] mtd: sh_flctl: Add missing iounmap()

[Bastian Hecht]

Add the unmapping for the error case and for the driver removal.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/mtd/nand/sh_flctl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 2ee9a1b..ed86a64 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -919,6 +919,7 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 
 err_chip:
 	pm_runtime_disable(&pdev->dev);
+	iounmap(flctl->reg);
 err_iomap:
 	kfree(flctl);
 	return ret;
@@ -930,6 +931,7 @@ static int __devexit flctl_remove(struct platform_device *pdev)
 
 	nand_release(&flctl->mtd);
 	pm_runtime_disable(&pdev->dev);
+	iounmap(flctl->reg);
 	kfree(flctl);
 
 	return 0;
-- 
1.7.5.4

[PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Bastian Hecht]

Supply the platform data for the FLSTEI error IRQ.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 arch/arm/mach-shmobile/board-mackerel.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 8758f94..1de1fb7 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -990,7 +990,11 @@ static struct resource nand_flash_resources[] = {
 		.start	= 0xe6a30000,
 		.end	= 0xe6a3009b,
 		.flags	= IORESOURCE_MEM,
-	}
+	},
+	[1] = {
+		.start	= evt2irq(0x0d80), /* flstei: status error irq */
+		.flags	= IORESOURCE_IRQ,
+	},
 };
 
 static struct sh_flctl_platform_data nand_flash_data = {
-- 
1.7.5.4

[PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Bastian Hecht]

Supply the platform data for the FLSTEI error IRQ.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 arch/arm/mach-shmobile/board-mackerel.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 8758f94..1de1fb7 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -990,7 +990,11 @@ static struct resource nand_flash_resources[] = {
 		.start	= 0xe6a30000,
 		.end	= 0xe6a3009b,
 		.flags	= IORESOURCE_MEM,
-	}
+	},
+	[1] = {
+		.start	= evt2irq(0x0d80), /* flstei: status error irq */
+		.flags	= IORESOURCE_IRQ,
+	},
 };
 
 static struct sh_flctl_platform_data nand_flash_data = {
-- 
1.7.5.4

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Rafael J. Wysocki]

On Wednesday, May 02, 2012, Bastian Hecht wrote:
> Supply the platform data for the FLSTEI error IRQ.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

I'm going to take this patch into the renesas.git tree and push it
upstream throuch arm-soc, if that's not a problem.

Thanks,
Rafael


> ---
>  arch/arm/mach-shmobile/board-mackerel.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 8758f94..1de1fb7 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -990,7 +990,11 @@ static struct resource nand_flash_resources[] = {
>  		.start	= 0xe6a30000,
>  		.end	= 0xe6a3009b,
>  		.flags	= IORESOURCE_MEM,
> -	}
> +	},
> +	[1] = {
> +		.start	= evt2irq(0x0d80), /* flstei: status error irq */
> +		.flags	= IORESOURCE_IRQ,
> +	},
>  };
>  
>  static struct sh_flctl_platform_data nand_flash_data = {
> 

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Rafael J. Wysocki]

On Wednesday, May 02, 2012, Bastian Hecht wrote:
> Supply the platform data for the FLSTEI error IRQ.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

I'm going to take this patch into the renesas.git tree and push it
upstream throuch arm-soc, if that's not a problem.

Thanks,
Rafael


> ---
>  arch/arm/mach-shmobile/board-mackerel.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 8758f94..1de1fb7 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -990,7 +990,11 @@ static struct resource nand_flash_resources[] = {
>  		.start	= 0xe6a30000,
>  		.end	= 0xe6a3009b,
>  		.flags	= IORESOURCE_MEM,
> -	}
> +	},
> +	[1] = {
> +		.start	= evt2irq(0x0d80), /* flstei: status error irq */
> +		.flags	= IORESOURCE_IRQ,
> +	},
>  };
>  
>  static struct sh_flctl_platform_data nand_flash_data = {
> 

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Magnus Damm]

2012/5/3 Rafael J. Wysocki <rjw@sisk.pl>:
> On Wednesday, May 02, 2012, Bastian Hecht wrote:
>> Supply the platform data for the FLSTEI error IRQ.
>>
>> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
>
> I'm going to take this patch into the renesas.git tree and push it
> upstream throuch arm-soc, if that's not a problem.

Sure, that's fine. Seems like a rather clear case of a mach-shmobile
patch without any special dependencies.

Cheers,

/ magnus

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Magnus Damm]

2012/5/3 Rafael J. Wysocki <rjw@sisk.pl>:
> On Wednesday, May 02, 2012, Bastian Hecht wrote:
>> Supply the platform data for the FLSTEI error IRQ.
>>
>> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
>
> I'm going to take this patch into the renesas.git tree and push it
> upstream throuch arm-soc, if that's not a problem.

Sure, that's fine. Seems like a rather clear case of a mach-shmobile
patch without any special dependencies.

Cheers,

/ magnus

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > Supply the platform data for the FLSTEI error IRQ.
> > 
> > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> 
> I'm going to take this patch into the renesas.git tree and push it
> upstream throuch arm-soc, if that's not a problem.

It is already in the MTD tree with the rest of the 10 patches...
-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > Supply the platform data for the FLSTEI error IRQ.
> > 
> > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> 
> I'm going to take this patch into the renesas.git tree and push it
> upstream throuch arm-soc, if that's not a problem.

It is already in the MTD tree with the rest of the 10 patches...
-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > Supply the platform data for the FLSTEI error IRQ.
> > > 
> > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > 
> > I'm going to take this patch into the renesas.git tree and push it
> > upstream throuch arm-soc, if that's not a problem.
> 
> It is already in the MTD tree with the rest of the 10 patches...

Ops, sorry, this is not true. Then Bastian should separate it out and
not send it to the MTD tree.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > Supply the platform data for the FLSTEI error IRQ.
> > > 
> > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > 
> > I'm going to take this patch into the renesas.git tree and push it
> > upstream throuch arm-soc, if that's not a problem.
> 
> It is already in the MTD tree with the rest of the 10 patches...

Ops, sorry, this is not true. Then Bastian should separate it out and
not send it to the MTD tree.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Bastian Hecht]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

Am 03.05.2012 08:01 schrieb "Artem Bityutskiy" <dedekind1@gmail.com>:
>
> On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> > On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > > Supply the platform data for the FLSTEI error IRQ.
> > > >
> > > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > >
> > > I'm going to take this patch into the renesas.git tree and push it
> > > upstream throuch arm-soc, if that's not a problem.
> >
> > It is already in the MTD tree with the rest of the 10 patches...
>
> Ops, sorry, this is not true. Then Bastian should separate it out and
> not send it to the MTD tree.

Alright I'll do. I need to fix the compiler warnings of the mtd patches
anyway. So the board code is independant and can be pushed already by
Rafael, right? How is it supposed to work in general? I think the sh-mobile
list wants to be informed when I change something in the flctl code. Would
the right approach be to send 2 patchsets - posted on both MLs so that
Artem can take the mtd portions and Rafael the mackerel ones?

Best regards,

Bastian Hecht

> --
> Best Regards,
> Artem Bityutskiy

[-- Attachment #2: Type: text/html, Size: 1549 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

On Thu, 2012-05-03 at 11:29 +0200, Bastian Hecht wrote:
> 
> Am 03.05.2012 08:01 schrieb "Artem Bityutskiy" <dedekind1@gmail.com>:
> >
> > On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> > > On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > > > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > > > Supply the platform data for the FLSTEI error IRQ.
> > > > >
> > > > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > > >
> > > > I'm going to take this patch into the renesas.git tree and push
> it
> > > > upstream throuch arm-soc, if that's not a problem.
> > >
> > > It is already in the MTD tree with the rest of the 10 patches...
> >
> > Ops, sorry, this is not true. Then Bastian should separate it out
> and
> > not send it to the MTD tree.
> 
> Alright I'll do. I need to fix the compiler warnings of the mtd
> patches anyway. So the board code is independant and can be pushed
> already by Rafael, right? How is it supposed to work in general? I
> think the sh-mobile list wants to be informed when I change something
> in the flctl code. Would the right approach be to send 2 patchsets -
> posted on both MLs so that Artem can take the mtd portions and Rafael
> the mackerel ones?

If the patches are completely independent, post them separately. I think
it is also OK to CC more than one mailing list.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

On Thu, 2012-05-03 at 11:29 +0200, Bastian Hecht wrote:
> 
> Am 03.05.2012 08:01 schrieb "Artem Bityutskiy" <dedekind1@gmail.com>:
> >
> > On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> > > On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > > > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > > > Supply the platform data for the FLSTEI error IRQ.
> > > > >
> > > > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > > >
> > > > I'm going to take this patch into the renesas.git tree and push
> it
> > > > upstream throuch arm-soc, if that's not a problem.
> > >
> > > It is already in the MTD tree with the rest of the 10 patches...
> >
> > Ops, sorry, this is not true. Then Bastian should separate it out
> and
> > not send it to the MTD tree.
> 
> Alright I'll do. I need to fix the compiler warnings of the mtd
> patches anyway. So the board code is independant and can be pushed
> already by Rafael, right? How is it supposed to work in general? I
> think the sh-mobile list wants to be informed when I change something
> in the flctl code. Would the right approach be to send 2 patchsets -
> posted on both MLs so that Artem can take the mtd portions and Rafael
> the mackerel ones?

If the patches are completely independent, post them separately. I think
it is also OK to CC more than one mailing list.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Rafael J. Wysocki]

On Thursday, May 03, 2012, Artem Bityutskiy wrote:
> On Thu, 2012-05-03 at 11:29 +0200, Bastian Hecht wrote:
> > 
> > Am 03.05.2012 08:01 schrieb "Artem Bityutskiy" <dedekind1@gmail.com>:
> > >
> > > On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> > > > On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > > > > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > > > > Supply the platform data for the FLSTEI error IRQ.
> > > > > >
> > > > > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > > > >
> > > > > I'm going to take this patch into the renesas.git tree and push
> > it
> > > > > upstream throuch arm-soc, if that's not a problem.
> > > >
> > > > It is already in the MTD tree with the rest of the 10 patches...
> > >
> > > Ops, sorry, this is not true. Then Bastian should separate it out
> > and
> > > not send it to the MTD tree.
> > 
> > Alright I'll do. I need to fix the compiler warnings of the mtd
> > patches anyway. So the board code is independant and can be pushed
> > already by Rafael, right? How is it supposed to work in general? I
> > think the sh-mobile list wants to be informed when I change something
> > in the flctl code. Would the right approach be to send 2 patchsets -
> > posted on both MLs so that Artem can take the mtd portions and Rafael
> > the mackerel ones?
> 
> If the patches are completely independent, post them separately.

Yes, please.

> I think it is also OK to CC more than one mailing list.

Yes, it is.

Thanks,
Rafael

Re: [PATCH v3 02/10] ARM: sh-mobile: mackerel: Add error IRQ resource

[Rafael J. Wysocki]

On Thursday, May 03, 2012, Artem Bityutskiy wrote:
> On Thu, 2012-05-03 at 11:29 +0200, Bastian Hecht wrote:
> > 
> > Am 03.05.2012 08:01 schrieb "Artem Bityutskiy" <dedekind1@gmail.com>:
> > >
> > > On Thu, 2012-05-03 at 09:00 +0300, Artem Bityutskiy wrote:
> > > > On Wed, 2012-05-02 at 22:49 +0200, Rafael J. Wysocki wrote:
> > > > > On Wednesday, May 02, 2012, Bastian Hecht wrote:
> > > > > > Supply the platform data for the FLSTEI error IRQ.
> > > > > >
> > > > > > Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> > > > >
> > > > > I'm going to take this patch into the renesas.git tree and push
> > it
> > > > > upstream throuch arm-soc, if that's not a problem.
> > > >
> > > > It is already in the MTD tree with the rest of the 10 patches...
> > >
> > > Ops, sorry, this is not true. Then Bastian should separate it out
> > and
> > > not send it to the MTD tree.
> > 
> > Alright I'll do. I need to fix the compiler warnings of the mtd
> > patches anyway. So the board code is independant and can be pushed
> > already by Rafael, right? How is it supposed to work in general? I
> > think the sh-mobile list wants to be informed when I change something
> > in the flctl code. Would the right approach be to send 2 patchsets -
> > posted on both MLs so that Artem can take the mtd portions and Rafael
> > the mackerel ones?
> 
> If the patches are completely independent, post them separately.

Yes, please.

> I think it is also OK to CC more than one mailing list.

Yes, it is.

Thanks,
Rafael

[PATCH v3 03/10] mtd: sh_flctl: Add support for error IRQ

[Bastian Hecht]

When the data transfer between the controller and the NAND chip fails,
we now get notified.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |   34 +++++++++++++++++++++++++++++++---
 include/linux/mtd/sh_flctl.h |    9 +++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index ed86a64..dd00853 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -68,8 +69,8 @@ static struct nand_bbt_descr flctl_4secc_largepage = {
 
 static void empty_fifo(struct sh_flctl *flctl)
 {
-	writel(0x000c0000, FLINTDMACR(flctl));	/* FIFO Clear */
-	writel(0x00000000, FLINTDMACR(flctl));	/* Clear Error flags */
+	writel(flctl->flintdmacr_base | AC1CLR | AC0CLR, FLINTDMACR(flctl));
+	writel(flctl->flintdmacr_base, FLINTDMACR(flctl));
 }
 
 static void start_translation(struct sh_flctl *flctl)
@@ -838,6 +839,16 @@ static int flctl_chip_init_tail(struct mtd_info *mtd)
 	return 0;
 }
 
+static irqreturn_t flctl_handle_flste(int irq, void *dev_id)
+{
+	struct sh_flctl *flctl = dev_id;
+
+	dev_err(&flctl->pdev->dev, "flste irq: %x\n", readl(FLINTDMACR(flctl)));
+	writel(flctl->flintdmacr_base, FLINTDMACR(flctl));
+
+	return IRQ_HANDLED;
+}
+
 static int __devinit flctl_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -846,6 +857,7 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 	struct nand_chip *nand;
 	struct sh_flctl_platform_data *pdata;
 	int ret = -ENXIO;
+	int irq;
 
 	pdata = pdev->dev.platform_data;
 	if (pdata = NULL) {
@@ -871,14 +883,27 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 		goto err_iomap;
 	}
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get flste irq data\n");
+		goto err_flste;
+	}
+
+	ret = request_irq(irq, flctl_handle_flste, IRQF_SHARED, "flste", flctl);
+	if (ret) {
+		dev_err(&pdev->dev, "request interrupt failed.\n");
+		goto err_flste;
+	}
+
 	platform_set_drvdata(pdev, flctl);
 	flctl_mtd = &flctl->mtd;
 	nand = &flctl->chip;
 	flctl_mtd->priv = nand;
 	flctl->pdev = pdev;
-	flctl->flcmncr_base = pdata->flcmncr_val;
 	flctl->hwecc = pdata->has_hwecc;
 	flctl->holden = pdata->use_holden;
+	flctl->flcmncr_base = pdata->flcmncr_val;
+	flctl->flintdmacr_base = flctl->hwecc ? (STERINTE | ECERB) : STERINTE;
 
 	nand->options = NAND_NO_AUTOINCR;
 
@@ -919,6 +944,8 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 
 err_chip:
 	pm_runtime_disable(&pdev->dev);
+	free_irq(platform_get_irq(pdev, 0), flctl);
+err_flste:
 	iounmap(flctl->reg);
 err_iomap:
 	kfree(flctl);
@@ -931,6 +958,7 @@ static int __devexit flctl_remove(struct platform_device *pdev)
 
 	nand_release(&flctl->mtd);
 	pm_runtime_disable(&pdev->dev);
+	free_irq(platform_get_irq(pdev, 0), flctl);
 	iounmap(flctl->reg);
 	kfree(flctl);
 
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index a38e1fa..2daa43e 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -107,6 +107,14 @@
 #define DOCMD2_E	(0x1 << 17)	/* 2nd cmd stage execute */
 #define DOCMD1_E	(0x1 << 16)	/* 1st cmd stage execute */
 
+/* FLINTDMACR control bits */
+#define ESTERINTE	(0x1 << 24)	/* ECC error interrupt enable */
+#define AC1CLR		(0x1 << 19)	/* ECC FIFO clear */
+#define AC0CLR		(0x1 << 18)	/* Data FIFO clear */
+#define ECERB		(0x1 << 9)	/* ECC error */
+#define STERB		(0x1 << 8)	/* Status error */
+#define STERINTE	(0x1 << 4)	/* Status error enable */
+
 /* FLTRCR control bits */
 #define TRSTRT		(0x1 << 0)	/* translation start */
 #define TREND		(0x1 << 1)	/* translation end */
@@ -145,6 +153,7 @@ struct sh_flctl {
 	uint32_t erase_ADRCNT;		/* bits of FLCMDCR in ERASE1 cmd */
 	uint32_t rw_ADRCNT;	/* bits of FLCMDCR in READ WRITE cmd */
 	uint32_t flcmncr_base;	/* base value of FLCMNCR */
+	uint32_t flintdmacr_base;	/* irq enable bits */
 
 	int	hwecc_cant_correct[4];
 
-- 
1.7.5.4

[PATCH v3 03/10] mtd: sh_flctl: Add support for error IRQ

[Bastian Hecht]

When the data transfer between the controller and the NAND chip fails,
we now get notified.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |   34 +++++++++++++++++++++++++++++++---
 include/linux/mtd/sh_flctl.h |    9 +++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index ed86a64..dd00853 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -68,8 +69,8 @@ static struct nand_bbt_descr flctl_4secc_largepage = {
 
 static void empty_fifo(struct sh_flctl *flctl)
 {
-	writel(0x000c0000, FLINTDMACR(flctl));	/* FIFO Clear */
-	writel(0x00000000, FLINTDMACR(flctl));	/* Clear Error flags */
+	writel(flctl->flintdmacr_base | AC1CLR | AC0CLR, FLINTDMACR(flctl));
+	writel(flctl->flintdmacr_base, FLINTDMACR(flctl));
 }
 
 static void start_translation(struct sh_flctl *flctl)
@@ -838,6 +839,16 @@ static int flctl_chip_init_tail(struct mtd_info *mtd)
 	return 0;
 }
 
+static irqreturn_t flctl_handle_flste(int irq, void *dev_id)
+{
+	struct sh_flctl *flctl = dev_id;
+
+	dev_err(&flctl->pdev->dev, "flste irq: %x\n", readl(FLINTDMACR(flctl)));
+	writel(flctl->flintdmacr_base, FLINTDMACR(flctl));
+
+	return IRQ_HANDLED;
+}
+
 static int __devinit flctl_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -846,6 +857,7 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 	struct nand_chip *nand;
 	struct sh_flctl_platform_data *pdata;
 	int ret = -ENXIO;
+	int irq;
 
 	pdata = pdev->dev.platform_data;
 	if (pdata == NULL) {
@@ -871,14 +883,27 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 		goto err_iomap;
 	}
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get flste irq data\n");
+		goto err_flste;
+	}
+
+	ret = request_irq(irq, flctl_handle_flste, IRQF_SHARED, "flste", flctl);
+	if (ret) {
+		dev_err(&pdev->dev, "request interrupt failed.\n");
+		goto err_flste;
+	}
+
 	platform_set_drvdata(pdev, flctl);
 	flctl_mtd = &flctl->mtd;
 	nand = &flctl->chip;
 	flctl_mtd->priv = nand;
 	flctl->pdev = pdev;
-	flctl->flcmncr_base = pdata->flcmncr_val;
 	flctl->hwecc = pdata->has_hwecc;
 	flctl->holden = pdata->use_holden;
+	flctl->flcmncr_base = pdata->flcmncr_val;
+	flctl->flintdmacr_base = flctl->hwecc ? (STERINTE | ECERB) : STERINTE;
 
 	nand->options = NAND_NO_AUTOINCR;
 
@@ -919,6 +944,8 @@ static int __devinit flctl_probe(struct platform_device *pdev)
 
 err_chip:
 	pm_runtime_disable(&pdev->dev);
+	free_irq(platform_get_irq(pdev, 0), flctl);
+err_flste:
 	iounmap(flctl->reg);
 err_iomap:
 	kfree(flctl);
@@ -931,6 +958,7 @@ static int __devexit flctl_remove(struct platform_device *pdev)
 
 	nand_release(&flctl->mtd);
 	pm_runtime_disable(&pdev->dev);
+	free_irq(platform_get_irq(pdev, 0), flctl);
 	iounmap(flctl->reg);
 	kfree(flctl);
 
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index a38e1fa..2daa43e 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -107,6 +107,14 @@
 #define DOCMD2_E	(0x1 << 17)	/* 2nd cmd stage execute */
 #define DOCMD1_E	(0x1 << 16)	/* 1st cmd stage execute */
 
+/* FLINTDMACR control bits */
+#define ESTERINTE	(0x1 << 24)	/* ECC error interrupt enable */
+#define AC1CLR		(0x1 << 19)	/* ECC FIFO clear */
+#define AC0CLR		(0x1 << 18)	/* Data FIFO clear */
+#define ECERB		(0x1 << 9)	/* ECC error */
+#define STERB		(0x1 << 8)	/* Status error */
+#define STERINTE	(0x1 << 4)	/* Status error enable */
+
 /* FLTRCR control bits */
 #define TRSTRT		(0x1 << 0)	/* translation start */
 #define TREND		(0x1 << 1)	/* translation end */
@@ -145,6 +153,7 @@ struct sh_flctl {
 	uint32_t erase_ADRCNT;		/* bits of FLCMDCR in ERASE1 cmd */
 	uint32_t rw_ADRCNT;	/* bits of FLCMDCR in READ WRITE cmd */
 	uint32_t flcmncr_base;	/* base value of FLCMNCR */
+	uint32_t flintdmacr_base;	/* irq enable bits */
 
 	int	hwecc_cant_correct[4];
 
-- 
1.7.5.4

[PATCH v3 04/10] mtd: sh_flctl: Use different OOB layout

[Bastian Hecht]

The flctl hardware has changed and a new OOB layout must be adapted for
2k page size NAND chips when using hardware ECC.
The related bit fields ECCPOS[0-2] are gone - the bits are marked as
reserved now in the datasheet. As there are no official users of the
hardware ECC so far, they are completely removed.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |   18 ++++++++++++------
 include/linux/mtd/sh_flctl.h |    4 ----
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index dd00853..dda34a2 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -44,11 +44,17 @@ static struct nand_ecclayout flctl_4secc_oob_16 = {
 };
 
 static struct nand_ecclayout flctl_4secc_oob_64 = {
-	.eccbytes = 10,
-	.eccpos = {48, 49, 50, 51, 52, 53, 54, 55, 56, 57},
+	.eccbytes = 4 * 10,
+	.eccpos = {
+		 6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
+		22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+		38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+		54, 55, 56, 57, 58, 59, 60, 61, 62, 63 },
 	.oobfree = {
-		{.offset = 60,
-		. length = 4} },
+		{.offset =  2, .length = 4},
+		{.offset = 16, .length = 6},
+		{.offset = 32, .length = 6},
+		{.offset = 48, .length = 6} },
 };
 
 static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
@@ -62,7 +68,7 @@ static struct nand_bbt_descr flctl_4secc_smallpage = {
 
 static struct nand_bbt_descr flctl_4secc_largepage = {
 	.options = NAND_BBT_SCAN2NDPAGE,
-	.offs = 58,
+	.offs = 0,
 	.len = 2,
 	.pattern = scan_ff_pattern,
 };
@@ -831,7 +837,7 @@ static int flctl_chip_init_tail(struct mtd_info *mtd)
 		chip->ecc.mode = NAND_ECC_HW;
 
 		/* 4 symbols ECC enabled */
-		flctl->flcmncr_base |= _4ECCEN | ECCPOS2 | ECCPOS_02;
+		flctl->flcmncr_base |= _4ECCEN;
 	} else {
 		chip->ecc.mode = NAND_ECC_SOFT;
 	}
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index 2daa43e..3feaae0 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -49,7 +49,6 @@
 #define	FLERRADR(f)		(f->reg + 0x98)
 
 /* FLCMNCR control bits */
-#define ECCPOS2		(0x1 << 25)
 #define _4ECCCNTEN	(0x1 << 24)
 #define _4ECCEN		(0x1 << 23)
 #define _4ECCCORRECT	(0x1 << 22)
@@ -59,9 +58,6 @@
 #define QTSEL_E		(0x1 << 17)
 #define ENDIAN		(0x1 << 16)	/* 1 = little endian */
 #define FCKSEL_E	(0x1 << 15)
-#define ECCPOS_00	(0x00 << 12)
-#define ECCPOS_01	(0x01 << 12)
-#define ECCPOS_02	(0x02 << 12)
 #define ACM_SACCES_MODE	(0x01 << 10)
 #define NANWF_E		(0x1 << 9)
 #define SE_D		(0x1 << 8)	/* Spare area disable */
-- 
1.7.5.4

[PATCH v3 04/10] mtd: sh_flctl: Use different OOB layout

[Bastian Hecht]

The flctl hardware has changed and a new OOB layout must be adapted for
2k page size NAND chips when using hardware ECC.
The related bit fields ECCPOS[0-2] are gone - the bits are marked as
reserved now in the datasheet. As there are no official users of the
hardware ECC so far, they are completely removed.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |   18 ++++++++++++------
 include/linux/mtd/sh_flctl.h |    4 ----
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index dd00853..dda34a2 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -44,11 +44,17 @@ static struct nand_ecclayout flctl_4secc_oob_16 = {
 };
 
 static struct nand_ecclayout flctl_4secc_oob_64 = {
-	.eccbytes = 10,
-	.eccpos = {48, 49, 50, 51, 52, 53, 54, 55, 56, 57},
+	.eccbytes = 4 * 10,
+	.eccpos = {
+		 6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
+		22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+		38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+		54, 55, 56, 57, 58, 59, 60, 61, 62, 63 },
 	.oobfree = {
-		{.offset = 60,
-		. length = 4} },
+		{.offset =  2, .length = 4},
+		{.offset = 16, .length = 6},
+		{.offset = 32, .length = 6},
+		{.offset = 48, .length = 6} },
 };
 
 static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
@@ -62,7 +68,7 @@ static struct nand_bbt_descr flctl_4secc_smallpage = {
 
 static struct nand_bbt_descr flctl_4secc_largepage = {
 	.options = NAND_BBT_SCAN2NDPAGE,
-	.offs = 58,
+	.offs = 0,
 	.len = 2,
 	.pattern = scan_ff_pattern,
 };
@@ -831,7 +837,7 @@ static int flctl_chip_init_tail(struct mtd_info *mtd)
 		chip->ecc.mode = NAND_ECC_HW;
 
 		/* 4 symbols ECC enabled */
-		flctl->flcmncr_base |= _4ECCEN | ECCPOS2 | ECCPOS_02;
+		flctl->flcmncr_base |= _4ECCEN;
 	} else {
 		chip->ecc.mode = NAND_ECC_SOFT;
 	}
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index 2daa43e..3feaae0 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -49,7 +49,6 @@
 #define	FLERRADR(f)		(f->reg + 0x98)
 
 /* FLCMNCR control bits */
-#define ECCPOS2		(0x1 << 25)
 #define _4ECCCNTEN	(0x1 << 24)
 #define _4ECCEN		(0x1 << 23)
 #define _4ECCCORRECT	(0x1 << 22)
@@ -59,9 +58,6 @@
 #define QTSEL_E		(0x1 << 17)
 #define ENDIAN		(0x1 << 16)	/* 1 = little endian */
 #define FCKSEL_E	(0x1 << 15)
-#define ECCPOS_00	(0x00 << 12)
-#define ECCPOS_01	(0x01 << 12)
-#define ECCPOS_02	(0x02 << 12)
 #define ACM_SACCES_MODE	(0x01 << 10)
 #define NANWF_E		(0x1 << 9)
 #define SE_D		(0x1 << 8)	/* Spare area disable */
-- 
1.7.5.4

[PATCH v3 05/10] mtd: sh_flctl: Fix hardware ECC behaviour

[Bastian Hecht]

The flctl uses 10 bytes ECC data for every 512 bytes sector. This patch
makes the controller write all 40 bytes instead of 10 bytes only.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   30 +++++++-----------------------
 1 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index dda34a2..c8e58bb 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -427,30 +427,20 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 static void execmd_read_oob(struct mtd_info *mtd, int page_addr)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
+	int page_sectors = flctl->page_size ? 4 : 1;
+	int i;
 
 	set_cmd_regs(mtd, NAND_CMD_READ0,
 		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
 
 	empty_fifo(flctl);
-	if (flctl->page_size) {
-		int i;
-		/* In case that the page size is 2k */
-		for (i = 0; i < 16 * 3; i++)
-			flctl->done_buff[i] = 0xFF;
-
-		set_addr(mtd, 3 * 528 + 512, page_addr);
-		writel(16, FLDTCNTR(flctl));
 
-		start_translation(flctl);
-		read_fiforeg(flctl, 16, 16 * 3);
-		wait_completion(flctl);
-	} else {
-		/* In case that the page size is 512b */
-		set_addr(mtd, 512, page_addr);
+	for (i = 0; i < page_sectors; i++) {
+		set_addr(mtd, (512 + 16) * i + 512 , page_addr);
 		writel(16, FLDTCNTR(flctl));
 
 		start_translation(flctl);
-		read_fiforeg(flctl, 16, 0);
+		read_fiforeg(flctl, 16, 16 * i);
 		wait_completion(flctl);
 	}
 }
@@ -495,18 +485,12 @@ static void execmd_write_oob(struct mtd_info *mtd)
 	int page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
-	if (flctl->page_size) {
-		sector = 3;
-		page_sectors = 4;
-	} else {
-		sector = 0;
-		page_sectors = 1;
-	}
+	page_sectors = flctl->page_size ? 4 : 1;
 
 	set_cmd_regs(mtd, NAND_CMD_PAGEPROG,
 			(NAND_CMD_PAGEPROG << 8) | NAND_CMD_SEQIN);
 
-	for (; sector < page_sectors; sector++) {
+	for (sector = 0; sector < page_sectors; sector++) {
 		empty_fifo(flctl);
 		set_addr(mtd, sector * 528 + 512, page_addr);
 		writel(16, FLDTCNTR(flctl));	/* set read size */
-- 
1.7.5.4

[PATCH v3 05/10] mtd: sh_flctl: Fix hardware ECC behaviour

[Bastian Hecht]

The flctl uses 10 bytes ECC data for every 512 bytes sector. This patch
makes the controller write all 40 bytes instead of 10 bytes only.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   30 +++++++-----------------------
 1 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index dda34a2..c8e58bb 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -427,30 +427,20 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 static void execmd_read_oob(struct mtd_info *mtd, int page_addr)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
+	int page_sectors = flctl->page_size ? 4 : 1;
+	int i;
 
 	set_cmd_regs(mtd, NAND_CMD_READ0,
 		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
 
 	empty_fifo(flctl);
-	if (flctl->page_size) {
-		int i;
-		/* In case that the page size is 2k */
-		for (i = 0; i < 16 * 3; i++)
-			flctl->done_buff[i] = 0xFF;
-
-		set_addr(mtd, 3 * 528 + 512, page_addr);
-		writel(16, FLDTCNTR(flctl));
 
-		start_translation(flctl);
-		read_fiforeg(flctl, 16, 16 * 3);
-		wait_completion(flctl);
-	} else {
-		/* In case that the page size is 512b */
-		set_addr(mtd, 512, page_addr);
+	for (i = 0; i < page_sectors; i++) {
+		set_addr(mtd, (512 + 16) * i + 512 , page_addr);
 		writel(16, FLDTCNTR(flctl));
 
 		start_translation(flctl);
-		read_fiforeg(flctl, 16, 0);
+		read_fiforeg(flctl, 16, 16 * i);
 		wait_completion(flctl);
 	}
 }
@@ -495,18 +485,12 @@ static void execmd_write_oob(struct mtd_info *mtd)
 	int page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
-	if (flctl->page_size) {
-		sector = 3;
-		page_sectors = 4;
-	} else {
-		sector = 0;
-		page_sectors = 1;
-	}
+	page_sectors = flctl->page_size ? 4 : 1;
 
 	set_cmd_regs(mtd, NAND_CMD_PAGEPROG,
 			(NAND_CMD_PAGEPROG << 8) | NAND_CMD_SEQIN);
 
-	for (; sector < page_sectors; sector++) {
+	for (sector = 0; sector < page_sectors; sector++) {
 		empty_fifo(flctl);
 		set_addr(mtd, sector * 528 + 512, page_addr);
 		writel(16, FLDTCNTR(flctl));	/* set read size */
-- 
1.7.5.4

[PATCH v3 06/10] mtd: sh_flctl: Simplify the hardware ecc page read/write

[Bastian Hecht]

As the equation mtd->writesize = eccsteps * eccsize holds, we can
simplify the code. The second loop of the 1st hunk is never entered,
so we delete it.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   25 ++-----------------------
 1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 4ea3e20..b0389c5 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -346,35 +346,14 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
 static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	uint8_t *p = buf;
-	struct sh_flctl *flctl = mtd_to_flctl(mtd);
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->read_buf(mtd, p, eccsize);
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
-		if (flctl->hwecc_cant_correct[i])
-			mtd->ecc_stats.failed++;
-		else
-			mtd->ecc_stats.corrected += 0; /* FIXME */
-	}
-
+	chip->read_buf(mtd, buf, mtd->writesize);
 	return 0;
 }
 
 static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				   const uint8_t *buf)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	const uint8_t *p = buf;
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->write_buf(mtd, p, eccsize);
+	chip->write_buf(mtd, buf, mtd->writesize);
 }
 
 static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
-- 
1.7.5.4

[PATCH v3 06/10] mtd: sh_flctl: Simplify the hardware ecc page read/write

[Bastian Hecht]

As the equation mtd->writesize == eccsteps * eccsize holds, we can
simplify the code. The second loop of the 1st hunk is never entered,
so we delete it.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   25 ++-----------------------
 1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 4ea3e20..b0389c5 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -346,35 +346,14 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
 static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	uint8_t *p = buf;
-	struct sh_flctl *flctl = mtd_to_flctl(mtd);
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->read_buf(mtd, p, eccsize);
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
-		if (flctl->hwecc_cant_correct[i])
-			mtd->ecc_stats.failed++;
-		else
-			mtd->ecc_stats.corrected += 0; /* FIXME */
-	}
-
+	chip->read_buf(mtd, buf, mtd->writesize);
 	return 0;
 }
 
 static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				   const uint8_t *buf)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	const uint8_t *p = buf;
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->write_buf(mtd, p, eccsize);
+	chip->write_buf(mtd, buf, mtd->writesize);
 }
 
 static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
-- 
1.7.5.4

[PATCH v3 07/10] mtd: sh_flctl: Group sector accesses into a single transfer

[Bastian Hecht]

When we use hardware ecc, the flctl is run in so-called "sector access
mode". We can bundle 4 sector accesses when using 2k page sizes to read
a whole page at once and speed up things.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   44 ++++++++++++++++++------------------------
 1 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index b027370..96c88b8 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -368,25 +368,21 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
 	int sector, page_sectors;
 
-	if (flctl->page_size)
-		page_sectors = 4;
-	else
-		page_sectors = 1;
+	page_sectors = flctl->page_size ? 4 : 1;
+
+	set_cmd_regs(mtd, NAND_CMD_READ0,
+		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
 
 	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE | _4ECCCORRECT,
 		 FLCMNCR(flctl));
+	writel(readl(FLCMDCR(flctl)) | page_sectors, FLCMDCR(flctl));
+	writel(page_addr << 2, FLADR(flctl));
 
-	set_cmd_regs(mtd, NAND_CMD_READ0,
-		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
+	empty_fifo(flctl);
+	start_translation(flctl);
 
 	for (sector = 0; sector < page_sectors; sector++) {
 		int ret;
-
-		empty_fifo(flctl);
-		writel(readl(FLCMDCR(flctl)) | 1, FLCMDCR(flctl));
-		writel(page_addr << 2 | sector, FLADR(flctl));
-
-		start_translation(flctl);
 		read_fiforeg(flctl, 512, 512 * sector);
 
 		ret = read_ecfiforeg(flctl,
@@ -397,8 +393,10 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 			flctl->hwecc_cant_correct[sector] = 1;
 
 		writel(0x0, FL4ECCCR(flctl));
-		wait_completion(flctl);
 	}
+
+	wait_completion(flctl);
+
 	writel(readl(FLCMNCR(flctl)) & ~(ACM_SACCES_MODE | _4ECCCORRECT),
 			FLCMNCR(flctl));
 }
@@ -430,31 +428,27 @@ static void execmd_write_page_sector(struct mtd_info *mtd)
 	int i, page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
-	if (flctl->page_size)
-		page_sectors = 4;
-	else
-		page_sectors = 1;
-
-	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE, FLCMNCR(flctl));
+	page_sectors = flctl->page_size ? 4 : 1;
 
 	set_cmd_regs(mtd, NAND_CMD_PAGEPROG,
 			(NAND_CMD_PAGEPROG << 8) | NAND_CMD_SEQIN);
 
-	for (sector = 0; sector < page_sectors; sector++) {
-		empty_fifo(flctl);
-		writel(readl(FLCMDCR(flctl)) | 1, FLCMDCR(flctl));
-		writel(page_addr << 2 | sector, FLADR(flctl));
+	empty_fifo(flctl);
+	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE, FLCMNCR(flctl));
+	writel(readl(FLCMDCR(flctl)) | page_sectors, FLCMDCR(flctl));
+	writel(page_addr << 2, FLADR(flctl));
+	start_translation(flctl);
 
-		start_translation(flctl);
+	for (sector = 0; sector < page_sectors; sector++) {
 		write_fiforeg(flctl, 512, 512 * sector);
 
 		for (i = 0; i < 4; i++) {
 			wait_wecfifo_ready(flctl); /* wait for write ready */
 			writel(0xFFFFFFFF, FLECFIFO(flctl));
 		}
-		wait_completion(flctl);
 	}
 
+	wait_completion(flctl);
 	writel(readl(FLCMNCR(flctl)) & ~ACM_SACCES_MODE, FLCMNCR(flctl));
 }
 
-- 
1.7.5.4

[PATCH v3 07/10] mtd: sh_flctl: Group sector accesses into a single transfer

[Bastian Hecht]

When we use hardware ecc, the flctl is run in so-called "sector access
mode". We can bundle 4 sector accesses when using 2k page sizes to read
a whole page at once and speed up things.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   44 ++++++++++++++++++------------------------
 1 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index b027370..96c88b8 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -368,25 +368,21 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
 	int sector, page_sectors;
 
-	if (flctl->page_size)
-		page_sectors = 4;
-	else
-		page_sectors = 1;
+	page_sectors = flctl->page_size ? 4 : 1;
+
+	set_cmd_regs(mtd, NAND_CMD_READ0,
+		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
 
 	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE | _4ECCCORRECT,
 		 FLCMNCR(flctl));
+	writel(readl(FLCMDCR(flctl)) | page_sectors, FLCMDCR(flctl));
+	writel(page_addr << 2, FLADR(flctl));
 
-	set_cmd_regs(mtd, NAND_CMD_READ0,
-		(NAND_CMD_READSTART << 8) | NAND_CMD_READ0);
+	empty_fifo(flctl);
+	start_translation(flctl);
 
 	for (sector = 0; sector < page_sectors; sector++) {
 		int ret;
-
-		empty_fifo(flctl);
-		writel(readl(FLCMDCR(flctl)) | 1, FLCMDCR(flctl));
-		writel(page_addr << 2 | sector, FLADR(flctl));
-
-		start_translation(flctl);
 		read_fiforeg(flctl, 512, 512 * sector);
 
 		ret = read_ecfiforeg(flctl,
@@ -397,8 +393,10 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 			flctl->hwecc_cant_correct[sector] = 1;
 
 		writel(0x0, FL4ECCCR(flctl));
-		wait_completion(flctl);
 	}
+
+	wait_completion(flctl);
+
 	writel(readl(FLCMNCR(flctl)) & ~(ACM_SACCES_MODE | _4ECCCORRECT),
 			FLCMNCR(flctl));
 }
@@ -430,31 +428,27 @@ static void execmd_write_page_sector(struct mtd_info *mtd)
 	int i, page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
-	if (flctl->page_size)
-		page_sectors = 4;
-	else
-		page_sectors = 1;
-
-	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE, FLCMNCR(flctl));
+	page_sectors = flctl->page_size ? 4 : 1;
 
 	set_cmd_regs(mtd, NAND_CMD_PAGEPROG,
 			(NAND_CMD_PAGEPROG << 8) | NAND_CMD_SEQIN);
 
-	for (sector = 0; sector < page_sectors; sector++) {
-		empty_fifo(flctl);
-		writel(readl(FLCMDCR(flctl)) | 1, FLCMDCR(flctl));
-		writel(page_addr << 2 | sector, FLADR(flctl));
+	empty_fifo(flctl);
+	writel(readl(FLCMNCR(flctl)) | ACM_SACCES_MODE, FLCMNCR(flctl));
+	writel(readl(FLCMDCR(flctl)) | page_sectors, FLCMDCR(flctl));
+	writel(page_addr << 2, FLADR(flctl));
+	start_translation(flctl);
 
-		start_translation(flctl);
+	for (sector = 0; sector < page_sectors; sector++) {
 		write_fiforeg(flctl, 512, 512 * sector);
 
 		for (i = 0; i < 4; i++) {
 			wait_wecfifo_ready(flctl); /* wait for write ready */
 			writel(0xFFFFFFFF, FLECFIFO(flctl));
 		}
-		wait_completion(flctl);
 	}
 
+	wait_completion(flctl);
 	writel(readl(FLCMNCR(flctl)) & ~ACM_SACCES_MODE, FLCMNCR(flctl));
 }
 
-- 
1.7.5.4

[PATCH v3 08/10] mtd: sh_flctl: Restructure the hardware ECC handling

[Bastian Hecht]

There are multiple reasons for a rewrite:
 - a race exists: when _4ECCEND is set, _4ECCFA may become true too
   meanwhile, which is lost and a non-correctable error is treated as
   correctable.
 - the ECC statistics don't get properly propagated to the base code.
 - empty pages would get marked as corrupted

The rewrite resolves the issues and I hope it gives a more explicit
code flow structure.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |  120 ++++++++++++++++++++++++++++--------------
 include/linux/mtd/sh_flctl.h |   10 +++-
 2 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 96c88b8..c0eddeb 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -165,27 +165,56 @@ static void wait_wfifo_ready(struct sh_flctl *flctl)
 	timeout_error(flctl, __func__);
 }
 
-static int wait_recfifo_ready(struct sh_flctl *flctl, int sector_number)
+static enum flctl_ecc_res_t wait_recfifo_ready
+		(struct sh_flctl *flctl, int sector_number)
 {
 	uint32_t timeout = LOOP_TIMEOUT_MAX;
-	int checked[4];
 	void __iomem *ecc_reg[4];
 	int i;
+	int state = FL_SUCCESS;
 	uint32_t data, size;
 
-	memset(checked, 0, sizeof(checked));
-
+	/*
+	 * First this loops checks in FLDTCNTR if we are ready to read out the
+	 * oob data. This is the case if either all went fine without errors or
+	 * if the bottom part of the loop corrected the errors or marked them as
+	 * uncorrectable and the controller is given time to push the data into
+	 * the FIFO.
+	 */
 	while (timeout--) {
+		/* check if all is ok and we can read out the OOB */
 		size = readl(FLDTCNTR(flctl)) >> 24;
-		if (size & 0xFF)
-			return 0;	/* success */
+		if ((size & 0xFF) = 4)
+			return state;
+
+		/* check if a correction code has been calculated */
+		if (!(readl(FL4ECCCR(flctl)) & _4ECCEND)) {
+			/*
+			 * either we wait for the fifo to be filled or a
+			 * correction pattern is being generated
+			 */
+			udelay(1);
+			continue;
+		}
 
-		if (readl(FL4ECCCR(flctl)) & _4ECCFA)
-			return 1;	/* can't correct */
+		/* check for an uncorrectable error */
+		if (readl(FL4ECCCR(flctl)) & _4ECCFA) {
+			/* check if we face a non-empty page */
+			for (i = 0; i < 512; i++) {
+				if (flctl->done_buff[i] != 0xff) {
+					state = FL_ERROR; /* can't correct */
+					break;
+				}
+			}
 
-		udelay(1);
-		if (!(readl(FL4ECCCR(flctl)) & _4ECCEND))
+			if (state = FL_SUCCESS)
+				dev_dbg(&flctl->pdev->dev,
+				"reading empty sector %d, ecc error ignored\n",
+				sector_number);
+
+			writel(0, FL4ECCCR(flctl));
 			continue;
+		}
 
 		/* start error correction */
 		ecc_reg[0] = FL4ECCRESULT0(flctl);
@@ -194,28 +223,26 @@ static int wait_recfifo_ready(struct sh_flctl *flctl, int sector_number)
 		ecc_reg[3] = FL4ECCRESULT3(flctl);
 
 		for (i = 0; i < 3; i++) {
+			uint8_t org;
+			int index;
+
 			data = readl(ecc_reg[i]);
-			if (data != INIT_FL4ECCRESULT_VAL && !checked[i]) {
-				uint8_t org;
-				int index;
-
-				if (flctl->page_size)
-					index = (512 * sector_number) +
-						(data >> 16);
-				else
-					index = data >> 16;
-
-				org = flctl->done_buff[index];
-				flctl->done_buff[index] = org ^ (data & 0xFF);
-				checked[i] = 1;
-			}
-		}
 
+			if (flctl->page_size)
+				index = (512 * sector_number) +
+					(data >> 16);
+			else
+				index = data >> 16;
+
+			org = flctl->done_buff[index];
+			flctl->done_buff[index] = org ^ (data & 0xFF);
+		}
+		state = FL_REPAIRABLE;
 		writel(0, FL4ECCCR(flctl));
 	}
 
 	timeout_error(flctl, __func__);
-	return 1;	/* timeout */
+	return FL_TIMEOUT;	/* timeout */
 }
 
 static void wait_wecfifo_ready(struct sh_flctl *flctl)
@@ -259,20 +286,24 @@ static void read_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
 	}
 }
 
-static int read_ecfiforeg(struct sh_flctl *flctl, uint8_t *buff, int sector)
+static enum flctl_ecc_res_t read_ecfiforeg
+		(struct sh_flctl *flctl, uint8_t *buff, int sector)
 {
 	int i;
+	enum flctl_ecc_res_t res;
 	unsigned long *ecc_buf = (unsigned long *)buff;
 	void *fifo_addr = (void *)FLECFIFO(flctl);
 
-	for (i = 0; i < 4; i++) {
-		if (wait_recfifo_ready(flctl , sector))
-			return 1;
-		ecc_buf[i] = readl(fifo_addr);
-		ecc_buf[i] = be32_to_cpu(ecc_buf[i]);
+	res = wait_recfifo_ready(flctl , sector);
+
+	if (res != FL_ERROR) {
+		for (i = 0; i < 4; i++) {
+			ecc_buf[i] = readl(fifo_addr);
+			ecc_buf[i] = be32_to_cpu(ecc_buf[i]);
+		}
 	}
 
-	return 0;
+	return res;
 }
 
 static void write_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
@@ -367,6 +398,7 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
 	int sector, page_sectors;
+	enum flctl_ecc_res_t ecc_result;
 
 	page_sectors = flctl->page_size ? 4 : 1;
 
@@ -382,17 +414,27 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 	start_translation(flctl);
 
 	for (sector = 0; sector < page_sectors; sector++) {
-		int ret;
 		read_fiforeg(flctl, 512, 512 * sector);
 
-		ret = read_ecfiforeg(flctl,
+		ecc_result = read_ecfiforeg(flctl,
 			&flctl->done_buff[mtd->writesize + 16 * sector],
 			sector);
 
-		if (ret)
-			flctl->hwecc_cant_correct[sector] = 1;
-
-		writel(0x0, FL4ECCCR(flctl));
+		switch (ecc_result) {
+		case FL_REPAIRABLE:
+			dev_info(&flctl->pdev->dev,
+				"applied ecc on page 0x%x", page_addr);
+			flctl->mtd.ecc_stats.corrected++;
+			break;
+		case FL_ERROR:
+			dev_warn(&flctl->pdev->dev,
+				"page 0x%x contains corrupted data\n",
+				page_addr);
+			flctl->mtd.ecc_stats.failed++;
+			break;
+		default:
+			;
+		}
 	}
 
 	wait_completion(flctl);
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index 3feaae0..01e4b15 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -129,9 +129,15 @@
 #define	_4ECCEND	(0x1 << 1)	/* 4 symbols end */
 #define	_4ECCEXST	(0x1 << 0)	/* 4 symbols exist */
 
-#define INIT_FL4ECCRESULT_VAL	0x03FF03FF
 #define LOOP_TIMEOUT_MAX	0x00010000
 
+enum flctl_ecc_res_t {
+	FL_SUCCESS,
+	FL_REPAIRABLE,
+	FL_ERROR,
+	FL_TIMEOUT
+};
+
 struct sh_flctl {
 	struct mtd_info		mtd;
 	struct nand_chip	chip;
@@ -151,8 +157,6 @@ struct sh_flctl {
 	uint32_t flcmncr_base;	/* base value of FLCMNCR */
 	uint32_t flintdmacr_base;	/* irq enable bits */
 
-	int	hwecc_cant_correct[4];
-
 	unsigned page_size:1;	/* NAND page size (0 = 512, 1 = 2048) */
 	unsigned hwecc:1;	/* Hardware ECC (0 = disabled, 1 = enabled) */
 	unsigned holden:1;	/* Hardware has FLHOLDCR and HOLDEN is set */
-- 
1.7.5.4

[PATCH v3 08/10] mtd: sh_flctl: Restructure the hardware ECC handling

[Bastian Hecht]

There are multiple reasons for a rewrite:
 - a race exists: when _4ECCEND is set, _4ECCFA may become true too
   meanwhile, which is lost and a non-correctable error is treated as
   correctable.
 - the ECC statistics don't get properly propagated to the base code.
 - empty pages would get marked as corrupted

The rewrite resolves the issues and I hope it gives a more explicit
code flow structure.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c  |  120 ++++++++++++++++++++++++++++--------------
 include/linux/mtd/sh_flctl.h |   10 +++-
 2 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 96c88b8..c0eddeb 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -165,27 +165,56 @@ static void wait_wfifo_ready(struct sh_flctl *flctl)
 	timeout_error(flctl, __func__);
 }
 
-static int wait_recfifo_ready(struct sh_flctl *flctl, int sector_number)
+static enum flctl_ecc_res_t wait_recfifo_ready
+		(struct sh_flctl *flctl, int sector_number)
 {
 	uint32_t timeout = LOOP_TIMEOUT_MAX;
-	int checked[4];
 	void __iomem *ecc_reg[4];
 	int i;
+	int state = FL_SUCCESS;
 	uint32_t data, size;
 
-	memset(checked, 0, sizeof(checked));
-
+	/*
+	 * First this loops checks in FLDTCNTR if we are ready to read out the
+	 * oob data. This is the case if either all went fine without errors or
+	 * if the bottom part of the loop corrected the errors or marked them as
+	 * uncorrectable and the controller is given time to push the data into
+	 * the FIFO.
+	 */
 	while (timeout--) {
+		/* check if all is ok and we can read out the OOB */
 		size = readl(FLDTCNTR(flctl)) >> 24;
-		if (size & 0xFF)
-			return 0;	/* success */
+		if ((size & 0xFF) == 4)
+			return state;
+
+		/* check if a correction code has been calculated */
+		if (!(readl(FL4ECCCR(flctl)) & _4ECCEND)) {
+			/*
+			 * either we wait for the fifo to be filled or a
+			 * correction pattern is being generated
+			 */
+			udelay(1);
+			continue;
+		}
 
-		if (readl(FL4ECCCR(flctl)) & _4ECCFA)
-			return 1;	/* can't correct */
+		/* check for an uncorrectable error */
+		if (readl(FL4ECCCR(flctl)) & _4ECCFA) {
+			/* check if we face a non-empty page */
+			for (i = 0; i < 512; i++) {
+				if (flctl->done_buff[i] != 0xff) {
+					state = FL_ERROR; /* can't correct */
+					break;
+				}
+			}
 
-		udelay(1);
-		if (!(readl(FL4ECCCR(flctl)) & _4ECCEND))
+			if (state == FL_SUCCESS)
+				dev_dbg(&flctl->pdev->dev,
+				"reading empty sector %d, ecc error ignored\n",
+				sector_number);
+
+			writel(0, FL4ECCCR(flctl));
 			continue;
+		}
 
 		/* start error correction */
 		ecc_reg[0] = FL4ECCRESULT0(flctl);
@@ -194,28 +223,26 @@ static int wait_recfifo_ready(struct sh_flctl *flctl, int sector_number)
 		ecc_reg[3] = FL4ECCRESULT3(flctl);
 
 		for (i = 0; i < 3; i++) {
+			uint8_t org;
+			int index;
+
 			data = readl(ecc_reg[i]);
-			if (data != INIT_FL4ECCRESULT_VAL && !checked[i]) {
-				uint8_t org;
-				int index;
-
-				if (flctl->page_size)
-					index = (512 * sector_number) +
-						(data >> 16);
-				else
-					index = data >> 16;
-
-				org = flctl->done_buff[index];
-				flctl->done_buff[index] = org ^ (data & 0xFF);
-				checked[i] = 1;
-			}
-		}
 
+			if (flctl->page_size)
+				index = (512 * sector_number) +
+					(data >> 16);
+			else
+				index = data >> 16;
+
+			org = flctl->done_buff[index];
+			flctl->done_buff[index] = org ^ (data & 0xFF);
+		}
+		state = FL_REPAIRABLE;
 		writel(0, FL4ECCCR(flctl));
 	}
 
 	timeout_error(flctl, __func__);
-	return 1;	/* timeout */
+	return FL_TIMEOUT;	/* timeout */
 }
 
 static void wait_wecfifo_ready(struct sh_flctl *flctl)
@@ -259,20 +286,24 @@ static void read_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
 	}
 }
 
-static int read_ecfiforeg(struct sh_flctl *flctl, uint8_t *buff, int sector)
+static enum flctl_ecc_res_t read_ecfiforeg
+		(struct sh_flctl *flctl, uint8_t *buff, int sector)
 {
 	int i;
+	enum flctl_ecc_res_t res;
 	unsigned long *ecc_buf = (unsigned long *)buff;
 	void *fifo_addr = (void *)FLECFIFO(flctl);
 
-	for (i = 0; i < 4; i++) {
-		if (wait_recfifo_ready(flctl , sector))
-			return 1;
-		ecc_buf[i] = readl(fifo_addr);
-		ecc_buf[i] = be32_to_cpu(ecc_buf[i]);
+	res = wait_recfifo_ready(flctl , sector);
+
+	if (res != FL_ERROR) {
+		for (i = 0; i < 4; i++) {
+			ecc_buf[i] = readl(fifo_addr);
+			ecc_buf[i] = be32_to_cpu(ecc_buf[i]);
+		}
 	}
 
-	return 0;
+	return res;
 }
 
 static void write_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
@@ -367,6 +398,7 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
 	int sector, page_sectors;
+	enum flctl_ecc_res_t ecc_result;
 
 	page_sectors = flctl->page_size ? 4 : 1;
 
@@ -382,17 +414,27 @@ static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
 	start_translation(flctl);
 
 	for (sector = 0; sector < page_sectors; sector++) {
-		int ret;
 		read_fiforeg(flctl, 512, 512 * sector);
 
-		ret = read_ecfiforeg(flctl,
+		ecc_result = read_ecfiforeg(flctl,
 			&flctl->done_buff[mtd->writesize + 16 * sector],
 			sector);
 
-		if (ret)
-			flctl->hwecc_cant_correct[sector] = 1;
-
-		writel(0x0, FL4ECCCR(flctl));
+		switch (ecc_result) {
+		case FL_REPAIRABLE:
+			dev_info(&flctl->pdev->dev,
+				"applied ecc on page 0x%x", page_addr);
+			flctl->mtd.ecc_stats.corrected++;
+			break;
+		case FL_ERROR:
+			dev_warn(&flctl->pdev->dev,
+				"page 0x%x contains corrupted data\n",
+				page_addr);
+			flctl->mtd.ecc_stats.failed++;
+			break;
+		default:
+			;
+		}
 	}
 
 	wait_completion(flctl);
diff --git a/include/linux/mtd/sh_flctl.h b/include/linux/mtd/sh_flctl.h
index 3feaae0..01e4b15 100644
--- a/include/linux/mtd/sh_flctl.h
+++ b/include/linux/mtd/sh_flctl.h
@@ -129,9 +129,15 @@
 #define	_4ECCEND	(0x1 << 1)	/* 4 symbols end */
 #define	_4ECCEXST	(0x1 << 0)	/* 4 symbols exist */
 
-#define INIT_FL4ECCRESULT_VAL	0x03FF03FF
 #define LOOP_TIMEOUT_MAX	0x00010000
 
+enum flctl_ecc_res_t {
+	FL_SUCCESS,
+	FL_REPAIRABLE,
+	FL_ERROR,
+	FL_TIMEOUT
+};
+
 struct sh_flctl {
 	struct mtd_info		mtd;
 	struct nand_chip	chip;
@@ -151,8 +157,6 @@ struct sh_flctl {
 	uint32_t flcmncr_base;	/* base value of FLCMNCR */
 	uint32_t flintdmacr_base;	/* irq enable bits */
 
-	int	hwecc_cant_correct[4];
-
 	unsigned page_size:1;	/* NAND page size (0 = 512, 1 = 2048) */
 	unsigned hwecc:1;	/* Hardware ECC (0 = disabled, 1 = enabled) */
 	unsigned holden:1;	/* Hardware has FLHOLDCR and HOLDEN is set */
-- 
1.7.5.4

[PATCH v3 09/10] mtd: sh_flctl: Use user oob data in hardware ECC mode

[Bastian Hecht]

In hardware ecc mode, the flctl now writes and reads the oob data
provided by the user. Additionally the ECC is now returned in normal
page reads, not only when using the explicit NAND_CMD_READOOB command.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
Acked-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 95bf636..1b1dd7a 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -312,6 +312,19 @@ static void write_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
 	}
 }
 
+static void write_ec_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
+{
+	int i, len_4align;
+	unsigned long *data = (unsigned long *)&flctl->done_buff[offset];
+	void *fifo_addr = (void *)FLECFIFO(flctl);
+
+	len_4align = (rlen + 3) / 4;
+	for (i = 0; i < len_4align; i++) {
+		wait_wecfifo_ready(flctl);
+		writel(cpu_to_be32(data[i]), fifo_addr);
+	}
+}
+
 static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_val)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
@@ -378,6 +391,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int page)
 {
 	chip->read_buf(mtd, buf, mtd->writesize);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 	return 0;
 }
 
@@ -385,6 +399,7 @@ static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				   const uint8_t *buf)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
 }
 
 static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
@@ -470,7 +485,7 @@ static void execmd_read_oob(struct mtd_info *mtd, int page_addr)
 static void execmd_write_page_sector(struct mtd_info *mtd)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
-	int i, page_addr = flctl->seqin_page_addr;
+	int page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
 	page_sectors = flctl->page_size ? 4 : 1;
@@ -486,11 +501,7 @@ static void execmd_write_page_sector(struct mtd_info *mtd)
 
 	for (sector = 0; sector < page_sectors; sector++) {
 		write_fiforeg(flctl, 512, 512 * sector);
-
-		for (i = 0; i < 4; i++) {
-			wait_wecfifo_ready(flctl); /* wait for write ready */
-			writel(0xFFFFFFFF, FLECFIFO(flctl));
-		}
+		write_ec_fiforeg(flctl, 16, mtd->writesize + 16 * sector);
 	}
 
 	wait_completion(flctl);
-- 
1.7.5.4

[PATCH v3 09/10] mtd: sh_flctl: Use user oob data in hardware ECC mode

[Bastian Hecht]

In hardware ecc mode, the flctl now writes and reads the oob data
provided by the user. Additionally the ECC is now returned in normal
page reads, not only when using the explicit NAND_CMD_READOOB command.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
Acked-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/sh_flctl.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 95bf636..1b1dd7a 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -312,6 +312,19 @@ static void write_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
 	}
 }
 
+static void write_ec_fiforeg(struct sh_flctl *flctl, int rlen, int offset)
+{
+	int i, len_4align;
+	unsigned long *data = (unsigned long *)&flctl->done_buff[offset];
+	void *fifo_addr = (void *)FLECFIFO(flctl);
+
+	len_4align = (rlen + 3) / 4;
+	for (i = 0; i < len_4align; i++) {
+		wait_wecfifo_ready(flctl);
+		writel(cpu_to_be32(data[i]), fifo_addr);
+	}
+}
+
 static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_val)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
@@ -378,6 +391,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int page)
 {
 	chip->read_buf(mtd, buf, mtd->writesize);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 	return 0;
 }
 
@@ -385,6 +399,7 @@ static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				   const uint8_t *buf)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
 }
 
 static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
@@ -470,7 +485,7 @@ static void execmd_read_oob(struct mtd_info *mtd, int page_addr)
 static void execmd_write_page_sector(struct mtd_info *mtd)
 {
 	struct sh_flctl *flctl = mtd_to_flctl(mtd);
-	int i, page_addr = flctl->seqin_page_addr;
+	int page_addr = flctl->seqin_page_addr;
 	int sector, page_sectors;
 
 	page_sectors = flctl->page_size ? 4 : 1;
@@ -486,11 +501,7 @@ static void execmd_write_page_sector(struct mtd_info *mtd)
 
 	for (sector = 0; sector < page_sectors; sector++) {
 		write_fiforeg(flctl, 512, 512 * sector);
-
-		for (i = 0; i < 4; i++) {
-			wait_wecfifo_ready(flctl); /* wait for write ready */
-			writel(0xFFFFFFFF, FLECFIFO(flctl));
-		}
+		write_ec_fiforeg(flctl, 16, mtd->writesize + 16 * sector);
 	}
 
 	wait_completion(flctl);
-- 
1.7.5.4

[PATCH v3 10/10] ARM: sh-mobile: mackerel: Use hardware error correction

[Bastian Hecht]

Use the built-in hardware error code correction of the sh_flctl.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 arch/arm/mach-shmobile/board-mackerel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1de1fb7..f8289f4 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1003,6 +1003,7 @@ static struct sh_flctl_platform_data nand_flash_data = {
 	.flcmncr_val	= CLK_16B_12L_4H | TYPESEL_SET
 			| SHBUSSEL | SEL_16BIT | SNAND_E,
 	.use_holden	= 1,
+	.has_hwecc	= 1,
 };
 
 static struct platform_device nand_flash_device = {
-- 
1.7.5.4

[PATCH v3 10/10] ARM: sh-mobile: mackerel: Use hardware error correction

[Bastian Hecht]

Use the built-in hardware error code correction of the sh_flctl.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 arch/arm/mach-shmobile/board-mackerel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1de1fb7..f8289f4 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1003,6 +1003,7 @@ static struct sh_flctl_platform_data nand_flash_data = {
 	.flcmncr_val	= CLK_16B_12L_4H | TYPESEL_SET
 			| SHBUSSEL | SEL_16BIT | SNAND_E,
 	.use_holden	= 1,
+	.has_hwecc	= 1,
 };
 
 static struct platform_device nand_flash_device = {
-- 
1.7.5.4

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
> changelog v3:
> 
> Just a rebase on current l2-mtd.git.

Aiaiai says that your patch-set introduced several warnings, could you
please take a look:

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:

--- before_patching.log
+++ after_patching.log
@@ @@
-drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
-drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
-drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]

--------------------------------------------------------------------------------

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
> changelog v3:
> 
> Just a rebase on current l2-mtd.git.

Aiaiai says that your patch-set introduced several warnings, could you
please take a look:

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:

--- before_patching.log
+++ after_patching.log
@@ @@
-drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
-drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
-drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
+drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
+drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
+drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]

--------------------------------------------------------------------------------

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

Hi Artem,

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
>> changelog v3:
>>
>> Just a rebase on current l2-mtd.git.
>
> Aiaiai says that your patch-set introduced several warnings, could you
> please take a look:
>
> --------------------------------------------------------------------------------
>
> Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
>
> Bisecability test passed
>
> --------------------------------------------------------------------------------
>
> Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
>
> --- before_patching.log
> +++ after_patching.log
> @@ @@
> -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
> -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
>
> --------------------------------------------------------------------------------

Nice! Every day I learn about a new tool for kernel developing. I
cloned your aiaiai tree but am unsure how to run it as
CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
supply me with a command line how you tested it?

Thanks,

 Bastian Hecht


> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

Hi Artem,

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
>> changelog v3:
>>
>> Just a rebase on current l2-mtd.git.
>
> Aiaiai says that your patch-set introduced several warnings, could you
> please take a look:
>
> --------------------------------------------------------------------------------
>
> Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
>
> Bisecability test passed
>
> --------------------------------------------------------------------------------
>
> Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
>
> --- before_patching.log
> +++ after_patching.log
> @@ @@
> -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
> -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
> +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
>
> --------------------------------------------------------------------------------

Nice! Every day I learn about a new tool for kernel developing. I
cloned your aiaiai tree but am unsure how to run it as
CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
supply me with a command line how you tested it?

Thanks,

 Bastian Hecht


> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 3335 bytes --]

On Wed, 2012-05-02 at 13:51 +0200, Bastian Hecht wrote:
> Hi Artem,
> 
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> > On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
> >> changelog v3:
> >>
> >> Just a rebase on current l2-mtd.git.
> >
> > Aiaiai says that your patch-set introduced several warnings, could you
> > please take a look:
> >
> > --------------------------------------------------------------------------------
> >
> > Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
> >
> > Bisecability test passed
> >
> > --------------------------------------------------------------------------------
> >
> > Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
> >
> > --- before_patching.log
> > +++ after_patching.log
> > @@ @@
> > -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
> >
> > --------------------------------------------------------------------------------
> 
> Nice! Every day I learn about a new tool for kernel developing. I
> cloned your aiaiai tree but am unsure how to run it as
> CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
> supply me with a command line how you tested it?

For the trees I maintain I have this repository:

git://git.infradead.org/users/dedekind/maintaining.git

where I store various helper scripts. To test your patch-set I used
"l2-mtd-sh_flctl.sh" script, I run it like this:

../aiaiai/concat-mbox-files ~/tmp/bastian* | ./l2-mtd-general.sh

where ../aiaiai is a clone of aiaiai repository:

git://git.infradead.org/users/dedekind/aiaiai.git

and the concat-mbox-files is a helper script which concatenates several
mbox files. The ~/tmp/bastian* are your patches saved as mbox.

Those helper scripts require you to clone the l2-mtd.git tree and
checkout the "defconfigs" branch which contains the
'l2_makerel_defconfig' configuration.

Aiaiai also needs several packages to be installed - see README file in
the aiaiai repo. It also needs smatch and sparse - also documented in
README. Sorry that this stuff is not too user-friendly - I do not have
time to clean it up.

Feel free to ask for help.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 3335 bytes --]

On Wed, 2012-05-02 at 13:51 +0200, Bastian Hecht wrote:
> Hi Artem,
> 
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> > On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
> >> changelog v3:
> >>
> >> Just a rebase on current l2-mtd.git.
> >
> > Aiaiai says that your patch-set introduced several warnings, could you
> > please take a look:
> >
> > --------------------------------------------------------------------------------
> >
> > Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
> >
> > Bisecability test passed
> >
> > --------------------------------------------------------------------------------
> >
> > Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
> >
> > --- before_patching.log
> > +++ after_patching.log
> > @@ @@
> > -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
> > +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
> > +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
> >
> > --------------------------------------------------------------------------------
> 
> Nice! Every day I learn about a new tool for kernel developing. I
> cloned your aiaiai tree but am unsure how to run it as
> CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
> supply me with a command line how you tested it?

For the trees I maintain I have this repository:

git://git.infradead.org/users/dedekind/maintaining.git

where I store various helper scripts. To test your patch-set I used
"l2-mtd-sh_flctl.sh" script, I run it like this:

../aiaiai/concat-mbox-files ~/tmp/bastian* | ./l2-mtd-general.sh

where ../aiaiai is a clone of aiaiai repository:

git://git.infradead.org/users/dedekind/aiaiai.git

and the concat-mbox-files is a helper script which concatenates several
mbox files. The ~/tmp/bastian* are your patches saved as mbox.

Those helper scripts require you to clone the l2-mtd.git tree and
checkout the "defconfigs" branch which contains the
'l2_makerel_defconfig' configuration.

Aiaiai also needs several packages to be installed - see README file in
the aiaiai repo. It also needs smatch and sparse - also documented in
README. Sorry that this stuff is not too user-friendly - I do not have
time to clean it up.

Feel free to ask for help.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 13:51 +0200, Bastian Hecht wrote:
>> Hi Artem,
>>
>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
>> >> changelog v3:
>> >>
>> >> Just a rebase on current l2-mtd.git.
>> >
>> > Aiaiai says that your patch-set introduced several warnings, could you
>> > please take a look:
>> >
>> > --------------------------------------------------------------------------------
>> >
>> > Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
>> >
>> > Bisecability test passed
>> >
>> > --------------------------------------------------------------------------------
>> >
>> > Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
>> >
>> > --- before_patching.log
>> > +++ after_patching.log
>> > @@ @@
>> > -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
>> >
>> > --------------------------------------------------------------------------------
>>
>> Nice! Every day I learn about a new tool for kernel developing. I
>> cloned your aiaiai tree but am unsure how to run it as
>> CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
>> supply me with a command line how you tested it?
>
> For the trees I maintain I have this repository:
>
> git://git.infradead.org/users/dedekind/maintaining.git
>
> where I store various helper scripts. To test your patch-set I used
> "l2-mtd-sh_flctl.sh" script, I run it like this:
>
> ../aiaiai/concat-mbox-files ~/tmp/bastian* | ./l2-mtd-general.sh
>
> where ../aiaiai is a clone of aiaiai repository:
>
> git://git.infradead.org/users/dedekind/aiaiai.git
>
> and the concat-mbox-files is a helper script which concatenates several
> mbox files. The ~/tmp/bastian* are your patches saved as mbox.
>
> Those helper scripts require you to clone the l2-mtd.git tree and
> checkout the "defconfigs" branch which contains the
> 'l2_makerel_defconfig' configuration.
>
> Aiaiai also needs several packages to be installed - see README file in
> the aiaiai repo. It also needs smatch and sparse - also documented in
> README. Sorry that this stuff is not too user-friendly - I do not have
> time to clean it up.
>
> Feel free to ask for help.

Ok I'm slowly but surely fighting my way through it. Though I want to
add an advisor here:
Don't do this on your crappy laptop like I do. I guess I will move to
my 8Gb RAM desktop machine for this in the future ;)

There's only 1 thing I don't completely grasp: you pass a target path
to aiaiai/test-patchset and a working tree. I didn't manage to get it
take the l2_mackerel_defconfig from one and compile the patches on top
of the other. So I simply copied the defconfig to my kernel dir and
ran the command manually.

Thanks for the guidance. And I've learned to appreciate the service
offered by you to compile-test this before accepting.

Bastian Hecht

> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 13:51 +0200, Bastian Hecht wrote:
>> Hi Artem,
>>
>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Wed, 2012-05-02 at 11:41 +0200, Bastian Hecht wrote:
>> >> changelog v3:
>> >>
>> >> Just a rebase on current l2-mtd.git.
>> >
>> > Aiaiai says that your patch-set introduced several warnings, could you
>> > please take a look:
>> >
>> > --------------------------------------------------------------------------------
>> >
>> > Bisectability test results for configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-"
>> >
>> > Bisecability test passed
>> >
>> > --------------------------------------------------------------------------------
>> >
>> > Successfully built configuration "l2_mackerel_defconfig,arm,arm-unknown-linux-gnueabi-", results:
>> >
>> > --- before_patching.log
>> > +++ after_patching.log
>> > @@ @@
>> > -drivers/mtd/nand/sh_flctl.c:264:30: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > -drivers/mtd/nand/sh_flctl.c:264:30:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > -drivers/mtd/nand/sh_flctl.c:264:30:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:301:38:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:326:28: warning: cast removes address space of expression [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17: warning: incorrect type in argument 1 (different address spaces) [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17:    expected void const volatile [noderef] <asn:2>*<noident> [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:331:17:    got void *fifo_addr [sparse]
>> > +drivers/mtd/nand/sh_flctl.c:967 flctl_probe() warn: 'irq' was not released on error [smatch]
>> >
>> > --------------------------------------------------------------------------------
>>
>> Nice! Every day I learn about a new tool for kernel developing. I
>> cloned your aiaiai tree but am unsure how to run it as
>> CONFIG_MTD_NAND_SH_FLCTL is not set in the mackerel_defconfig. Can you
>> supply me with a command line how you tested it?
>
> For the trees I maintain I have this repository:
>
> git://git.infradead.org/users/dedekind/maintaining.git
>
> where I store various helper scripts. To test your patch-set I used
> "l2-mtd-sh_flctl.sh" script, I run it like this:
>
> ../aiaiai/concat-mbox-files ~/tmp/bastian* | ./l2-mtd-general.sh
>
> where ../aiaiai is a clone of aiaiai repository:
>
> git://git.infradead.org/users/dedekind/aiaiai.git
>
> and the concat-mbox-files is a helper script which concatenates several
> mbox files. The ~/tmp/bastian* are your patches saved as mbox.
>
> Those helper scripts require you to clone the l2-mtd.git tree and
> checkout the "defconfigs" branch which contains the
> 'l2_makerel_defconfig' configuration.
>
> Aiaiai also needs several packages to be installed - see README file in
> the aiaiai repo. It also needs smatch and sparse - also documented in
> README. Sorry that this stuff is not too user-friendly - I do not have
> time to clean it up.
>
> Feel free to ask for help.

Ok I'm slowly but surely fighting my way through it. Though I want to
add an advisor here:
Don't do this on your crappy laptop like I do. I guess I will move to
my 8Gb RAM desktop machine for this in the future ;)

There's only 1 thing I don't completely grasp: you pass a target path
to aiaiai/test-patchset and a working tree. I didn't manage to get it
take the l2_mackerel_defconfig from one and compile the patches on top
of the other. So I simply copied the defconfig to my kernel dir and
ran the command manually.

Thanks for the guidance. And I've learned to appreciate the service
offered by you to compile-test this before accepting.

Bastian Hecht

> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
> Ok I'm slowly but surely fighting my way through it. Though I want to
> add an advisor here:
> Don't do this on your crappy laptop like I do. I guess I will move to
> my 8Gb RAM desktop machine for this in the future ;)

Yeah, I run it on a powerful PC. I need to update the README file - but
if you send a patch - will be appreciated.

> There's only 1 thing I don't completely grasp: you pass a target path
> to aiaiai/test-patchset and a working tree. I didn't manage to get it
> take the l2_mackerel_defconfig from one and compile the patches on top
> of the other.

Yeah, this is in my TODO list to teach aiaiai to take defconfig files
which are not part of the tree. So you basically need to copy the
l2-mackerel_defconfig file to your tree and commit it, so that the
defconfig is part of the git tree you are testing against.

But I want to teach test-patchset to accept just stand-along defconfig
files.

The reason it is implemented the way it is implemented because it was
originally done for an internal project where we have a couple of
defconfigs which are part of the project tree.

>  So I simply copied the defconfig to my kernel dir and
> ran the command manually.

You should also commit it, of course. Because the way aiaiai works it
first clones your tree and then work with the clone. This is done to
make sure your tree is preserved intact.


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
> Ok I'm slowly but surely fighting my way through it. Though I want to
> add an advisor here:
> Don't do this on your crappy laptop like I do. I guess I will move to
> my 8Gb RAM desktop machine for this in the future ;)

Yeah, I run it on a powerful PC. I need to update the README file - but
if you send a patch - will be appreciated.

> There's only 1 thing I don't completely grasp: you pass a target path
> to aiaiai/test-patchset and a working tree. I didn't manage to get it
> take the l2_mackerel_defconfig from one and compile the patches on top
> of the other.

Yeah, this is in my TODO list to teach aiaiai to take defconfig files
which are not part of the tree. So you basically need to copy the
l2-mackerel_defconfig file to your tree and commit it, so that the
defconfig is part of the git tree you are testing against.

But I want to teach test-patchset to accept just stand-along defconfig
files.

The reason it is implemented the way it is implemented because it was
originally done for an internal project where we have a couple of
defconfigs which are part of the project tree.

>  So I simply copied the defconfig to my kernel dir and
> ran the command manually.

You should also commit it, of course. Because the way aiaiai works it
first clones your tree and then work with the clone. This is done to
make sure your tree is preserved intact.


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>> Ok I'm slowly but surely fighting my way through it. Though I want to
>> add an advisor here:
>> Don't do this on your crappy laptop like I do. I guess I will move to
>> my 8Gb RAM desktop machine for this in the future ;)
>
> Yeah, I run it on a powerful PC. I need to update the README file - but
> if you send a patch - will be appreciated.

I assumed people to be smarter than me and instantly realize that a
compile-test is resource intensive, so I'm unsure if this is
necessary.

>> There's only 1 thing I don't completely grasp: you pass a target path
>> to aiaiai/test-patchset and a working tree. I didn't manage to get it
>> take the l2_mackerel_defconfig from one and compile the patches on top
>> of the other.
>
> Yeah, this is in my TODO list to teach aiaiai to take defconfig files
> which are not part of the tree. So you basically need to copy the
> l2-mackerel_defconfig file to your tree and commit it, so that the
> defconfig is part of the git tree you are testing against.
>
> But I want to teach test-patchset to accept just stand-along defconfig
> files.
>
> The reason it is implemented the way it is implemented because it was
> originally done for an internal project where we have a couple of
> defconfigs which are part of the project tree.
>
>>  So I simply copied the defconfig to my kernel dir and
>> ran the command manually.
>
> You should also commit it, of course. Because the way aiaiai works it
> first clones your tree and then work with the clone. This is done to
> make sure your tree is preserved intact.
>

Ok, then I completely understood the concept. In fact I did commit the
defconfig after I used verbose output and saw the clones of the trees.
But I must admit that I'm still struggling to get the same warnings
from your post. But I'll manage I guess.

> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>> Ok I'm slowly but surely fighting my way through it. Though I want to
>> add an advisor here:
>> Don't do this on your crappy laptop like I do. I guess I will move to
>> my 8Gb RAM desktop machine for this in the future ;)
>
> Yeah, I run it on a powerful PC. I need to update the README file - but
> if you send a patch - will be appreciated.

I assumed people to be smarter than me and instantly realize that a
compile-test is resource intensive, so I'm unsure if this is
necessary.

>> There's only 1 thing I don't completely grasp: you pass a target path
>> to aiaiai/test-patchset and a working tree. I didn't manage to get it
>> take the l2_mackerel_defconfig from one and compile the patches on top
>> of the other.
>
> Yeah, this is in my TODO list to teach aiaiai to take defconfig files
> which are not part of the tree. So you basically need to copy the
> l2-mackerel_defconfig file to your tree and commit it, so that the
> defconfig is part of the git tree you are testing against.
>
> But I want to teach test-patchset to accept just stand-along defconfig
> files.
>
> The reason it is implemented the way it is implemented because it was
> originally done for an internal project where we have a couple of
> defconfigs which are part of the project tree.
>
>>  So I simply copied the defconfig to my kernel dir and
>> ran the command manually.
>
> You should also commit it, of course. Because the way aiaiai works it
> first clones your tree and then work with the clone. This is done to
> make sure your tree is preserved intact.
>

Ok, then I completely understood the concept. In fact I did commit the
defconfig after I used verbose output and saw the clones of the trees.
But I must admit that I'm still struggling to get the same warnings
from your post. But I'll manage I guess.

> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
> >> Ok I'm slowly but surely fighting my way through it. Though I want to
> >> add an advisor here:
> >> Don't do this on your crappy laptop like I do. I guess I will move to
> >> my 8Gb RAM desktop machine for this in the future ;)
> >
> > Yeah, I run it on a powerful PC. I need to update the README file - but
> > if you send a patch - will be appreciated.
> 
> I assumed people to be smarter than me and instantly realize that a
> compile-test is resource intensive, so I'm unsure if this is
> necessary.

You are not the first one :-)

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
> >> Ok I'm slowly but surely fighting my way through it. Though I want to
> >> add an advisor here:
> >> Don't do this on your crappy laptop like I do. I guess I will move to
> >> my 8Gb RAM desktop machine for this in the future ;)
> >
> > Yeah, I run it on a powerful PC. I need to update the README file - but
> > if you send a patch - will be appreciated.
> 
> I assumed people to be smarter than me and instantly realize that a
> compile-test is resource intensive, so I'm unsure if this is
> necessary.

You are not the first one :-)

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>> >> Ok I'm slowly but surely fighting my way through it. Though I want to
>> >> add an advisor here:
>> >> Don't do this on your crappy laptop like I do. I guess I will move to
>> >> my 8Gb RAM desktop machine for this in the future ;)
>> >
>> > Yeah, I run it on a powerful PC. I need to update the README file - but
>> > if you send a patch - will be appreciated.
>>
>> I assumed people to be smarter than me and instantly realize that a
>> compile-test is resource intensive, so I'm unsure if this is
>> necessary.
>
> You are not the first one :-)


Now I'm really stuck. The verbose log says the build target "all"
failed. When I used -p to keep the clone and used make ARCH=arm
CROSS-...=... l2_mackerel_defconfig; make ARCH=... CROSS=... all -
everything builds fine.

command used: ../aiaiai/concat-mbox-files /tmp/patch/* |
../aiaiai/test-patchset -j8 -p -v ../flctl_patch/ --cppcheck --sparse
l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-

The log looks like this:
HEAD is now at 07126b9 for test compiling
[2012-05-02 15:51:43] test-patchset: Build non-patched kernel
("/tmp/test-patchset.IxuP/before/src", configuration
"l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
mkdir: created directory
`/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
[2012-05-02 15:51:43] make-kernel: Making target
"l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:51:49] make-kernel: Making target "all" for
"/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
"/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting


> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
> On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>> >> Ok I'm slowly but surely fighting my way through it. Though I want to
>> >> add an advisor here:
>> >> Don't do this on your crappy laptop like I do. I guess I will move to
>> >> my 8Gb RAM desktop machine for this in the future ;)
>> >
>> > Yeah, I run it on a powerful PC. I need to update the README file - but
>> > if you send a patch - will be appreciated.
>>
>> I assumed people to be smarter than me and instantly realize that a
>> compile-test is resource intensive, so I'm unsure if this is
>> necessary.
>
> You are not the first one :-)


Now I'm really stuck. The verbose log says the build target "all"
failed. When I used -p to keep the clone and used make ARCH=arm
CROSS-...=... l2_mackerel_defconfig; make ARCH=... CROSS=... all -
everything builds fine.

command used: ../aiaiai/concat-mbox-files /tmp/patch/* |
../aiaiai/test-patchset -j8 -p -v ../flctl_patch/ --cppcheck --sparse
l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-

The log looks like this:
HEAD is now at 07126b9 for test compiling
[2012-05-02 15:51:43] test-patchset: Build non-patched kernel
("/tmp/test-patchset.IxuP/before/src", configuration
"l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
mkdir: created directory
`/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
[2012-05-02 15:51:43] make-kernel: Making target
"l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:51:49] make-kernel: Making target "all" for
"/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
"/tmp/test-patchset.IxuP/before/src
(O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
[2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting


> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Bastian Hecht <hechtb@googlemail.com>:
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
>>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>>> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>>> >> Ok I'm slowly but surely fighting my way through it. Though I want to
>>> >> add an advisor here:
>>> >> Don't do this on your crappy laptop like I do. I guess I will move to
>>> >> my 8Gb RAM desktop machine for this in the future ;)
>>> >
>>> > Yeah, I run it on a powerful PC. I need to update the README file - but
>>> > if you send a patch - will be appreciated.
>>>
>>> I assumed people to be smarter than me and instantly realize that a
>>> compile-test is resource intensive, so I'm unsure if this is
>>> necessary.
>>
>> You are not the first one :-)
>
>
> Now I'm really stuck. The verbose log says the build target "all"
> failed. When I used -p to keep the clone and used make ARCH=arm
> CROSS-...=... l2_mackerel_defconfig; make ARCH=... CROSS=... all -
> everything builds fine.
>
> command used: ../aiaiai/concat-mbox-files /tmp/patch/* |
> ../aiaiai/test-patchset -j8 -p -v ../flctl_patch/ --cppcheck --sparse
> l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-
>
> The log looks like this:
> HEAD is now at 07126b9 for test compiling
> [2012-05-02 15:51:43] test-patchset: Build non-patched kernel
> ("/tmp/test-patchset.IxuP/before/src", configuration
> "l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
> mkdir: created directory
> `/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
> [2012-05-02 15:51:43] make-kernel: Making target
> "l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:51:49] make-kernel: Making target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting


What I forgot to mention is:

The error log l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-.stderr.log
states:
...
...
make: *** [sub-make] Error 2
FAILURE

but in the whole log there is no error listed. It only appears at the
end without any references.

>
>> --
>> Best Regards,
>> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Bastian Hecht]

2012/5/2 Bastian Hecht <hechtb@googlemail.com>:
> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>> On Wed, 2012-05-02 at 16:00 +0200, Bastian Hecht wrote:
>>> 2012/5/2 Artem Bityutskiy <dedekind1@gmail.com>:
>>> > On Wed, 2012-05-02 at 15:41 +0200, Bastian Hecht wrote:
>>> >> Ok I'm slowly but surely fighting my way through it. Though I want to
>>> >> add an advisor here:
>>> >> Don't do this on your crappy laptop like I do. I guess I will move to
>>> >> my 8Gb RAM desktop machine for this in the future ;)
>>> >
>>> > Yeah, I run it on a powerful PC. I need to update the README file - but
>>> > if you send a patch - will be appreciated.
>>>
>>> I assumed people to be smarter than me and instantly realize that a
>>> compile-test is resource intensive, so I'm unsure if this is
>>> necessary.
>>
>> You are not the first one :-)
>
>
> Now I'm really stuck. The verbose log says the build target "all"
> failed. When I used -p to keep the clone and used make ARCH=arm
> CROSS-...=... l2_mackerel_defconfig; make ARCH=... CROSS=... all -
> everything builds fine.
>
> command used: ../aiaiai/concat-mbox-files /tmp/patch/* |
> ../aiaiai/test-patchset -j8 -p -v ../flctl_patch/ --cppcheck --sparse
> l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-
>
> The log looks like this:
> HEAD is now at 07126b9 for test compiling
> [2012-05-02 15:51:43] test-patchset: Build non-patched kernel
> ("/tmp/test-patchset.IxuP/before/src", configuration
> "l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
> mkdir: created directory
> `/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
> [2012-05-02 15:51:43] make-kernel: Making target
> "l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:51:49] make-kernel: Making target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting


What I forgot to mention is:

The error log l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-.stderr.log
states:
...
...
make: *** [sub-make] Error 2
FAILURE

but in the whole log there is no error listed. It only appears at the
end without any references.

>
>> --
>> Best Regards,
>> Artem Bityutskiy

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

On Wed, 2012-05-02 at 16:40 +0200, Bastian Hecht wrote:
> The log looks like this:
> HEAD is now at 07126b9 for test compiling
> [2012-05-02 15:51:43] test-patchset: Build non-patched kernel
> ("/tmp/test-patchset.IxuP/before/src", configuration
> "l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
> mkdir: created directory
> `/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
> [2012-05-02 15:51:43] make-kernel: Making target
> "l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:51:49] make-kernel: Making target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting

Do you have arm-none-linux-gnueabi-gcc in your PATH? Look at he
workdir/before/l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-.stderr
file - it should contain the build log - what it says?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 0/10] sh_flctl hardware ECC mode cleanup

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

On Wed, 2012-05-02 at 16:40 +0200, Bastian Hecht wrote:
> The log looks like this:
> HEAD is now at 07126b9 for test compiling
> [2012-05-02 15:51:43] test-patchset: Build non-patched kernel
> ("/tmp/test-patchset.IxuP/before/src", configuration
> "l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-")
> mkdir: created directory
> `/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-'
> [2012-05-02 15:51:43] make-kernel: Making target
> "l2_mackerel_defconfig" for "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:51:49] make-kernel: Making target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make target "all" for
> "/tmp/test-patchset.IxuP/before/src
> (O=/tmp/test-patchset.IxuP/before/obj.l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-)"
> [2012-05-02 15:52:16] make-kernel: Failed to make a target, exiting

Do you have arm-none-linux-gnueabi-gcc in your PATH? Look at he
workdir/before/l2_mackerel_defconfig,arm,arm-none-linux-gnueabi-.stderr
file - it should contain the build log - what it says?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]