[PATCH v2] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[PATCH v2] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
Warning: The same solution needs to be applied to the boot loader and the
flash programmer.

This is a modified version of patch sent by Jürgen Lambrecht :
[PATCH] Add 'config IMX_NFC_V1_BISWAP' to swap the Bad block Indicator,
and use for imx27pdk nand support.

Signed-off-by: Gaëtan Carlier <gcembed@gmail.com>
---
 arch/arm/plat-mxc/include/mach/mxc_nand.h |    2 ++
 drivers/mtd/nand/mxc_nand.c               |   26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mxc_nand.h b/arch/arm/plat-mxc/include/mach/mxc_nand.h
index 6bb96ef..7c04437 100644
--- a/arch/arm/plat-mxc/include/mach/mxc_nand.h
+++ b/arch/arm/plat-mxc/include/mach/mxc_nand.h
@@ -26,6 +26,8 @@ struct mxc_nand_platform_data {
 	unsigned int width;	/* data bus width in bytes */
 	unsigned int hw_ecc:1;	/* 0 if suppress hardware ECC */
 	unsigned int flash_bbt:1; /* set to 1 to use a flash based bbt */
+	unsigned int biswap:1;	/* set to 1 to swap the Bad Block Indicator
+				   NFC v1 workaround */
 	struct mtd_partition *parts;	/* partition table */
 	int nr_parts;			/* size of parts */
 };
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 3f94e1f..7f5847a 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -682,6 +682,26 @@ static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 	return 0;
 }
 
+/*
+ * Swap the BI-byte on position 0x7D0 with a data byte at 0x835.
+ * To fix a bug in NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
+ * Warning: The same solution needs to be applied to the boot loader and the
+ * flash programmer.
+ */
+static void imx_bi_swap(struct mtd_info *mtd)
+{
+	struct nand_chip *nand_chip = mtd->priv;
+	struct mxc_nand_host *host = nand_chip->priv;
+	unsigned short temp1, temp2, new_temp1;
+
+	temp1 = *((volatile unsigned short*)(host->main_area0 + 0x7D0));
+	temp2 = *((volatile unsigned short*)(host->main_area0 + 0x834));
+	new_temp1 = (temp1 & 0xFF00) | (temp2 >> 8);
+	temp2 = (temp2 & 0x00FF) | (temp1 << 8);
+	*((volatile unsigned short*)(host->main_area0 + 0x7D0)) = new_temp1;
+	*((volatile unsigned short*)(host->main_area0 + 0x834)) = temp2;
+}
+
 static u_char mxc_nand_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *nand_chip = mtd->priv;
@@ -1093,6 +1113,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 
 		host->devtype_data->send_page(mtd, NFC_OUTPUT);
 
+		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
+			imx_bi_swap(mtd);
+
 		memcpy32_fromio(host->data_buf, host->main_area0,
 				mtd->writesize);
 		copy_spare(mtd, true);
@@ -1112,6 +1135,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 	case NAND_CMD_PAGEPROG:
 		memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
 		copy_spare(mtd, false);
+		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
+			imx_bi_swap(mtd);
+
 		host->devtype_data->send_page(mtd, NFC_INPUT);
 		host->devtype_data->send_cmd(host, command, true);
 		mxc_do_addr_cycle(mtd, column, page_addr);
-- 
1.7.7.4

Re: [PATCH v2] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Artem Bityutskiy]

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

Hi,

On Wed, 2012-08-15 at 15:04 +0200, Gaëtan Carlier wrote:
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 3f94e1f..7f5847a 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -682,6 +682,26 @@ static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
>  	return 0;
>  }
>  
> +/*
> + * Swap the BI-byte on position 0x7D0 with a data byte at 0x835.
> + * To fix a bug in NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> + * Warning: The same solution needs to be applied to the boot loader and the
> + * flash programmer.
> + */
> +static void imx_bi_swap(struct mtd_info *mtd)

Could you please name the function 'nfcv1_bi_swap_quirk()' or something
like this, to make it clear that this is a work-around.

> +{
> +	struct nand_chip *nand_chip = mtd->priv;
> +	struct mxc_nand_host *host = nand_chip->priv;
> +	unsigned short temp1, temp2, new_temp1;
> +
> +	temp1 = *((volatile unsigned short*)(host->main_area0 + 0x7D0));
> +	temp2 = *((volatile unsigned short*)(host->main_area0 + 0x834));
> +	new_temp1 = (temp1 & 0xFF00) | (temp2 >> 8);
> +	temp2 = (temp2 & 0x00FF) | (temp1 << 8);
> +	*((volatile unsigned short*)(host->main_area0 + 0x7D0)) = new_temp1;
> +	*((volatile unsigned short*)(host->main_area0 + 0x834)) = temp2;

Within the linux kernel all the I/O memory accesses should be done using
accessor functions, not directly.

-- 
Best Regards,
Artem Bityutskiy

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

[PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
Warning: The same solution needs to be applied to the boot loader and the
flash programmer.

This is a modified version of patch sent by Jürgen Lambrecht :
[PATCH] Add 'config IMX_NFC_V1_BISWAP' to swap the Bad block Indicator,
and use for imx27pdk nand support.

 v3: - use accessor function for I/O memory accesses as suggested by Artem
       Bityutskiy
     - rename function name as suggested by Artem Bityutskiy

 v2: - Use a flag in a mxc_nand_platform structure instead of adding option
       in Kconfig

 v1: - Original patch "Add 'config IMX_NFC_V1_BISWAP'" by Jürgen Lambrecht

Signed-off-by: Gaëtan Carlier <gcembed@gmail.com>
---
 arch/arm/plat-mxc/include/mach/mxc_nand.h |    2 ++
 drivers/mtd/nand/mxc_nand.c               |   26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mxc_nand.h b/arch/arm/plat-mxc/include/mach/mxc_nand.h
index 6bb96ef..7c04437 100644
--- a/arch/arm/plat-mxc/include/mach/mxc_nand.h
+++ b/arch/arm/plat-mxc/include/mach/mxc_nand.h
@@ -26,6 +26,8 @@ struct mxc_nand_platform_data {
 	unsigned int width;	/* data bus width in bytes */
 	unsigned int hw_ecc:1;	/* 0 if suppress hardware ECC */
 	unsigned int flash_bbt:1; /* set to 1 to use a flash based bbt */
+	unsigned int biswap:1;	/* set to 1 to swap the Bad Block Indicator
+				   NFC v1 workaround */
 	struct mtd_partition *parts;	/* partition table */
 	int nr_parts;			/* size of parts */
 };
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 3f94e1f..24fc9e1 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -682,6 +682,26 @@ static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 	return 0;
 }
 
+/*
+ * Swap the BI-byte on position 0x7D0 with a data byte at 0x835.
+ * To fix a bug in NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
+ * Warning: The same solution needs to be applied to the boot loader and the
+ * flash programmer.
+ */
+static void nfcv1_bi_swap_quirk(struct mtd_info *mtd)
+{
+	struct nand_chip *nand_chip = mtd->priv;
+	struct mxc_nand_host *host = nand_chip->priv;
+	unsigned short temp1, temp2, new_temp1;
+
+	temp1 = ioread16(host->main_area0 + 0x7D0);
+	temp2 = ioread16(host->main_area0 + 0x834);
+	new_temp1 = (temp1 & 0xFF00) | (temp2 >> 8);
+	temp2 = (temp2 & 0x00FF) | (temp1 << 8);
+	iowrite16(new_temp1, host->main_area0 + 0x7D0);
+	iowrite16(temp2, host->main_area0 + 0x834);
+}
+
 static u_char mxc_nand_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *nand_chip = mtd->priv;
@@ -1093,6 +1113,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 
 		host->devtype_data->send_page(mtd, NFC_OUTPUT);
 
+		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
+			nfcv1_bi_swap_quirk(mtd);
+
 		memcpy32_fromio(host->data_buf, host->main_area0,
 				mtd->writesize);
 		copy_spare(mtd, true);
@@ -1112,6 +1135,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 	case NAND_CMD_PAGEPROG:
 		memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
 		copy_spare(mtd, false);
+		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
+			nfcv1_bi_swap_quirk(mtd);
+
 		host->devtype_data->send_page(mtd, NFC_INPUT);
 		host->devtype_data->send_cmd(host, command, true);
 		mxc_do_addr_cycle(mtd, column, page_addr);
-- 
1.7.7.4

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

Hi,
Any comments for this patch ?
Regards,
Gaëtan Carlier.

On 08/24/2012 06:17 PM, Gaëtan Carlier wrote:
> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> Warning: The same solution needs to be applied to the boot loader and the
> flash programmer.
>
> This is a modified version of patch sent by Jürgen Lambrecht :
> [PATCH] Add 'config IMX_NFC_V1_BISWAP' to swap the Bad block Indicator,
> and use for imx27pdk nand support.
>
>   v3: - use accessor function for I/O memory accesses as suggested by Artem
>         Bityutskiy
>       - rename function name as suggested by Artem Bityutskiy
>
>   v2: - Use a flag in a mxc_nand_platform structure instead of adding option
>         in Kconfig
>
>   v1: - Original patch "Add 'config IMX_NFC_V1_BISWAP'" by Jürgen Lambrecht
>
> Signed-off-by: Gaëtan Carlier <gcembed@gmail.com>
> ---
>   arch/arm/plat-mxc/include/mach/mxc_nand.h |    2 ++
>   drivers/mtd/nand/mxc_nand.c               |   26 ++++++++++++++++++++++++++
>   2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-mxc/include/mach/mxc_nand.h b/arch/arm/plat-mxc/include/mach/mxc_nand.h
> index 6bb96ef..7c04437 100644
> --- a/arch/arm/plat-mxc/include/mach/mxc_nand.h
> +++ b/arch/arm/plat-mxc/include/mach/mxc_nand.h
> @@ -26,6 +26,8 @@ struct mxc_nand_platform_data {
>   	unsigned int width;	/* data bus width in bytes */
>   	unsigned int hw_ecc:1;	/* 0 if suppress hardware ECC */
>   	unsigned int flash_bbt:1; /* set to 1 to use a flash based bbt */
> +	unsigned int biswap:1;	/* set to 1 to swap the Bad Block Indicator
> +				   NFC v1 workaround */
>   	struct mtd_partition *parts;	/* partition table */
>   	int nr_parts;			/* size of parts */
>   };
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 3f94e1f..24fc9e1 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -682,6 +682,26 @@ static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
>   	return 0;
>   }
>
> +/*
> + * Swap the BI-byte on position 0x7D0 with a data byte at 0x835.
> + * To fix a bug in NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> + * Warning: The same solution needs to be applied to the boot loader and the
> + * flash programmer.
> + */
> +static void nfcv1_bi_swap_quirk(struct mtd_info *mtd)
> +{
> +	struct nand_chip *nand_chip = mtd->priv;
> +	struct mxc_nand_host *host = nand_chip->priv;
> +	unsigned short temp1, temp2, new_temp1;
> +
> +	temp1 = ioread16(host->main_area0 + 0x7D0);
> +	temp2 = ioread16(host->main_area0 + 0x834);
> +	new_temp1 = (temp1 & 0xFF00) | (temp2 >> 8);
> +	temp2 = (temp2 & 0x00FF) | (temp1 << 8);
> +	iowrite16(new_temp1, host->main_area0 + 0x7D0);
> +	iowrite16(temp2, host->main_area0 + 0x834);
> +}
> +
>   static u_char mxc_nand_read_byte(struct mtd_info *mtd)
>   {
>   	struct nand_chip *nand_chip = mtd->priv;
> @@ -1093,6 +1113,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
>
>   		host->devtype_data->send_page(mtd, NFC_OUTPUT);
>
> +		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
> +			nfcv1_bi_swap_quirk(mtd);
> +
>   		memcpy32_fromio(host->data_buf, host->main_area0,
>   				mtd->writesize);
>   		copy_spare(mtd, true);
> @@ -1112,6 +1135,9 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
>   	case NAND_CMD_PAGEPROG:
>   		memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
>   		copy_spare(mtd, false);
> +		if ((mtd->writesize > 512) && nfc_is_v1() && host->pdata.biswap)
> +			nfcv1_bi_swap_quirk(mtd);
> +
>   		host->devtype_data->send_page(mtd, NFC_INPUT);
>   		host->devtype_data->send_cmd(host, command, true);
>   		mxc_do_addr_cycle(mtd, column, page_addr);
>

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Artem Bityutskiy]

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

On Fri, 2012-08-24 at 18:17 +0200, Gaëtan Carlier wrote:
> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> Warning: The same solution needs to be applied to the boot loader and the
> flash programmer.
> 
> This is a modified version of patch sent by Jürgen Lambrecht :
> [PATCH] Add 'config IMX_NFC_V1_BISWAP' to swap the Bad block Indicator,
> and use for imx27pdk nand support.

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

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

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Juergen Beisert]

Hi Gaëtan,

Gaëtan Carlier wrote:
> Any comments for this patch ?

Sorry, a little bit late.

> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> Warning: The same solution needs to be applied to the boot loader and the
> flash programmer.

What sense does it make to swap the bytes at offset 0x7D0 and 0x835?

Background: the NFC in the i.MX27/i.MX31 (NFCv1) and i.MX25/i.MX35 (NFCv2) can 
only handle 512 byte pages with 16 bit OOB at once. To get them work with 2 k 
page NANDs they only do the 512 + 16 step four times. The result is, a 2 k 
page NAND is not handled as 2 k data and 64 bytes OOB, its handled instead as
a stream of 2112 bytes:

               |<-------- 2112 bytes --------->|
               512+16 | 512+16 | 512+16 | 512+16

Which means the NFC mixes data and OOB/checksums into its internal SRAM. But 
all upper routines still use this SRAM content with a 2048 (data) + 64 (OOB) 
bytes layout (to be more precise also the NFC hardware uses this layout). 
Result is, the factory bad block markers are lost, because the NFC stores the 
first 48 bytes from each OOB into the SRAM's data area (beginning with offset 
2000 (= 0x7D0) in the data area).

So, the main goal of this swap patch should be to keep the factory bad block 
markers _and_ to make use of them.

But the i.MX driver does not register its own bad block pattern description, 
so the default one is used with the BBM at position NAND_LARGE_BADBLOCK_POS 
(which means offset 0 in the OOB area).

So, I think, all upper layers still search for the BBM at offset 0 of the OOB 
area, which results to SRAM's offset 0x800 (for NFCv1) and not 0x835. This 
means this patch keeps the factory bad block markers, but they won't still be 
used as expected.

IMHO the bytes at SRAM's offset 0x7D0 and 0x800 for the NFCv1 must be swapped 
(and 0x7D0 and 0x1000 for the NFCv2) to keep the factory bad block markers 
_and_ to make use of them. Or am I wrong?

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

Hi Juergen,
On 09/06/2012 10:56 AM, Juergen Beisert wrote:
> Hi Gaëtan,
>
> Gaëtan Carlier wrote:
>> Any comments for this patch ?
>
> Sorry, a little bit late.
>
>> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
>> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
>> Warning: The same solution needs to be applied to the boot loader and the
>> flash programmer.
>
> What sense does it make to swap the bytes at offset 0x7D0 and 0x835?
>
> Background: the NFC in the i.MX27/i.MX31 (NFCv1) and i.MX25/i.MX35 (NFCv2) can
> only handle 512 byte pages with 16 bit OOB at once. To get them work with 2 k
> page NANDs they only do the 512 + 16 step four times. The result is, a 2 k
> page NAND is not handled as 2 k data and 64 bytes OOB, its handled instead as
> a stream of 2112 bytes:
>
>                 |<-------- 2112 bytes --------->|
>                 512+16 | 512+16 | 512+16 | 512+16
>
> Which means the NFC mixes data and OOB/checksums into its internal SRAM. But
> all upper routines still use this SRAM content with a 2048 (data) + 64 (OOB)
> bytes layout (to be more precise also the NFC hardware uses this layout).
> Result is, the factory bad block markers are lost, because the NFC stores the
> first 48 bytes from each OOB into the SRAM's data area (beginning with offset
> 2000 (= 0x7D0) in the data area).
>
> So, the main goal of this swap patch should be to keep the factory bad block
> markers _and_ to make use of them.
>
> But the i.MX driver does not register its own bad block pattern description,
> so the default one is used with the BBM at position NAND_LARGE_BADBLOCK_POS
> (which means offset 0 in the OOB area).
>
> So, I think, all upper layers still search for the BBM at offset 0 of the OOB
> area, which results to SRAM's offset 0x800 (for NFCv1) and not 0x835. This
> means this patch keeps the factory bad block markers, but they won't still be
> used as expected.
>
> IMHO the bytes at SRAM's offset 0x7D0 and 0x800 for the NFCv1 must be swapped
> (and 0x7D0 and 0x1000 for the NFCv2) to keep the factory bad block markers
> _and_ to make use of them. Or am I wrong?
>
> Regards,
> Juergen
>
The point is not the necessity of swapping bytes if factory bad block 
markers are not used.
In my case, the bytes must be swapped because all previous operations on 
flash swapped these bytes :
* Flashes redboot (bootloader) via JTAG using OpenOCD which swaps bytes;
* Writes kernel and rootfs via TFTP using redboot which swaps bytes;
So to be able to read rootfs partition, mxc-nand driver have to swap 
bytes otherwise, JFFS2 detects error on partition and is unable to mount it.

I have not chosen to swap these bytes. In a training, I received redboot 
and kernel 2.6.22 with set of patches to support i.MX27 and these 
patches included swap of bytes in nand driver. So I start my development 
with this workaround and it is now impossible to get back because boards 
are already in production.
Regards,
Gaëtan Carlier.

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Sascha Hauer]

On Mon, Sep 10, 2012 at 04:31:23PM +0200, Gaëtan Carlier wrote:
> Hi Juergen,
> On 09/06/2012 10:56 AM, Juergen Beisert wrote:
> >Hi Gaëtan,
> >
> >Gaëtan Carlier wrote:
> >>Any comments for this patch ?
> >
> >Sorry, a little bit late.
> >
> >>Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
> >>in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> >>Warning: The same solution needs to be applied to the boot loader and the
> >>flash programmer.
> >
> >What sense does it make to swap the bytes at offset 0x7D0 and 0x835?
> >
> >Background: the NFC in the i.MX27/i.MX31 (NFCv1) and i.MX25/i.MX35 (NFCv2) can
> >only handle 512 byte pages with 16 bit OOB at once. To get them work with 2 k
> >page NANDs they only do the 512 + 16 step four times. The result is, a 2 k
> >page NAND is not handled as 2 k data and 64 bytes OOB, its handled instead as
> >a stream of 2112 bytes:
> >
> >                |<-------- 2112 bytes --------->|
> >                512+16 | 512+16 | 512+16 | 512+16
> >
> >Which means the NFC mixes data and OOB/checksums into its internal SRAM. But
> >all upper routines still use this SRAM content with a 2048 (data) + 64 (OOB)
> >bytes layout (to be more precise also the NFC hardware uses this layout).
> >Result is, the factory bad block markers are lost, because the NFC stores the
> >first 48 bytes from each OOB into the SRAM's data area (beginning with offset
> >2000 (= 0x7D0) in the data area).
> >
> >So, the main goal of this swap patch should be to keep the factory bad block
> >markers _and_ to make use of them.
> >
> >But the i.MX driver does not register its own bad block pattern description,
> >so the default one is used with the BBM at position NAND_LARGE_BADBLOCK_POS
> >(which means offset 0 in the OOB area).
> >
> >So, I think, all upper layers still search for the BBM at offset 0 of the OOB
> >area, which results to SRAM's offset 0x800 (for NFCv1) and not 0x835. This
> >means this patch keeps the factory bad block markers, but they won't still be
> >used as expected.
> >
> >IMHO the bytes at SRAM's offset 0x7D0 and 0x800 for the NFCv1 must be swapped
> >(and 0x7D0 and 0x1000 for the NFCv2) to keep the factory bad block markers
> >_and_ to make use of them. Or am I wrong?
> >
> >Regards,
> >Juergen
> >
> The point is not the necessity of swapping bytes if factory bad
> block markers are not used.
> In my case, the bytes must be swapped because all previous
> operations on flash swapped these bytes :
> * Flashes redboot (bootloader) via JTAG using OpenOCD which swaps bytes;
> * Writes kernel and rootfs via TFTP using redboot which swaps bytes;
> So to be able to read rootfs partition, mxc-nand driver have to swap
> bytes otherwise, JFFS2 detects error on partition and is unable to
> mount it.

I am all in favour to be able to swap the factory bad block marker with
the position the mtd framework expects it.

That according to our understanding are bytes 0x7d0 in the main area
and byte 0x0 in the spare area.

> 
> I have not chosen to swap these bytes. In a training, I received
> redboot and kernel 2.6.22 with set of patches to support i.MX27 and
> these patches included swap of bytes in nand driver. So I start my
> development with this workaround and it is now impossible to get
> back because boards are already in production.

So you are swapping the factory bad block marker with some position
where the mtd layer does not expect the bad block marker. With this
you preserve the factor bad block marker, but do not use it to detect
bad blocks.

In this form the patch is simply not correct and should be dropped.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

On 09/11/2012 09:17 AM, Sascha Hauer wrote:
> On Mon, Sep 10, 2012 at 04:31:23PM +0200, Gaëtan Carlier wrote:
>> Hi Juergen,
>> On 09/06/2012 10:56 AM, Juergen Beisert wrote:
>>> Hi Gaëtan,
>>>
>>> Gaëtan Carlier wrote:
>>>> Any comments for this patch ?
>>>
>>> Sorry, a little bit late.
>>>
>>>> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
>>>> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
>>>> Warning: The same solution needs to be applied to the boot loader and the
>>>> flash programmer.
>>>
>>> What sense does it make to swap the bytes at offset 0x7D0 and 0x835?
>>>
>>> Background: the NFC in the i.MX27/i.MX31 (NFCv1) and i.MX25/i.MX35 (NFCv2) can
>>> only handle 512 byte pages with 16 bit OOB at once. To get them work with 2 k
>>> page NANDs they only do the 512 + 16 step four times. The result is, a 2 k
>>> page NAND is not handled as 2 k data and 64 bytes OOB, its handled instead as
>>> a stream of 2112 bytes:
>>>
>>>                 |<-------- 2112 bytes --------->|
>>>                 512+16 | 512+16 | 512+16 | 512+16
>>>
>>> Which means the NFC mixes data and OOB/checksums into its internal SRAM. But
>>> all upper routines still use this SRAM content with a 2048 (data) + 64 (OOB)
>>> bytes layout (to be more precise also the NFC hardware uses this layout).
>>> Result is, the factory bad block markers are lost, because the NFC stores the
>>> first 48 bytes from each OOB into the SRAM's data area (beginning with offset
>>> 2000 (= 0x7D0) in the data area).
>>>
>>> So, the main goal of this swap patch should be to keep the factory bad block
>>> markers _and_ to make use of them.
>>>
>>> But the i.MX driver does not register its own bad block pattern description,
>>> so the default one is used with the BBM at position NAND_LARGE_BADBLOCK_POS
>>> (which means offset 0 in the OOB area).
>>>
>>> So, I think, all upper layers still search for the BBM at offset 0 of the OOB
>>> area, which results to SRAM's offset 0x800 (for NFCv1) and not 0x835. This
>>> means this patch keeps the factory bad block markers, but they won't still be
>>> used as expected.
>>>
>>> IMHO the bytes at SRAM's offset 0x7D0 and 0x800 for the NFCv1 must be swapped
>>> (and 0x7D0 and 0x1000 for the NFCv2) to keep the factory bad block markers
>>> _and_ to make use of them. Or am I wrong?
>>>
>>> Regards,
>>> Juergen
>>>
>> The point is not the necessity of swapping bytes if factory bad
>> block markers are not used.
>> In my case, the bytes must be swapped because all previous
>> operations on flash swapped these bytes :
>> * Flashes redboot (bootloader) via JTAG using OpenOCD which swaps bytes;
>> * Writes kernel and rootfs via TFTP using redboot which swaps bytes;
>> So to be able to read rootfs partition, mxc-nand driver have to swap
>> bytes otherwise, JFFS2 detects error on partition and is unable to
>> mount it.
>
> I am all in favour to be able to swap the factory bad block marker with
> the position the mtd framework expects it.
>
> That according to our understanding are bytes 0x7d0 in the main area
> and byte 0x0 in the spare area.
>
>>
>> I have not chosen to swap these bytes. In a training, I received
>> redboot and kernel 2.6.22 with set of patches to support i.MX27 and
>> these patches included swap of bytes in nand driver. So I start my
>> development with this workaround and it is now impossible to get
>> back because boards are already in production.
>
> So you are swapping the factory bad block marker with some position
> where the mtd layer does not expect the bad block marker. With this
> you preserve the factor bad block marker, but do not use it to detect
> bad blocks.
The factor bad block marker are used by redboot. I don't know how kernel 
handles them.
>
> In this form the patch is simply not correct and should be dropped.
>
> Sascha
>
Regards,
Gaëtan Carlier.

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Sascha Hauer]

On Tue, Sep 11, 2012 at 10:03:01AM +0200, Gaëtan Carlier wrote:
> >>The point is not the necessity of swapping bytes if factory bad
> >>block markers are not used.
> >>In my case, the bytes must be swapped because all previous
> >>operations on flash swapped these bytes :
> >>* Flashes redboot (bootloader) via JTAG using OpenOCD which swaps bytes;
> >>* Writes kernel and rootfs via TFTP using redboot which swaps bytes;
> >>So to be able to read rootfs partition, mxc-nand driver have to swap
> >>bytes otherwise, JFFS2 detects error on partition and is unable to
> >>mount it.
> >
> >I am all in favour to be able to swap the factory bad block marker with
> >the position the mtd framework expects it.
> >
> >That according to our understanding are bytes 0x7d0 in the main area
> >and byte 0x0 in the spare area.
> >
> >>
> >>I have not chosen to swap these bytes. In a training, I received
> >>redboot and kernel 2.6.22 with set of patches to support i.MX27 and
> >>these patches included swap of bytes in nand driver. So I start my
> >>development with this workaround and it is now impossible to get
> >>back because boards are already in production.
> >
> >So you are swapping the factory bad block marker with some position
> >where the mtd layer does not expect the bad block marker. With this
> >you preserve the factor bad block marker, but do not use it to detect
> >bad blocks.
> The factor bad block marker are used by redboot. I don't know how
> kernel handles them.

This depends on chip->badblockpos. Normally this defaults to
NAND_LARGE_BADBLOCK_POS for 2k nands. This would be the first oob byte.

So you would have to swap the factory bad block marker with some other
oob byte, but you would also have to make sure that you set
chip->badblockpos to the location you swap with.

One BIG problem is that I've seen multiple variants of this patch, for
example one variant I've seen swaps with oob byte 5 or 6 (the standard
badblock marker for 512byte page nands). So you would not only have
to put a 'bool swap' into platformdata, but also the information with
which byte to swap.

This is all very annoying, because the swap location (or no swap at all)
is specific to a single board, *not* to a board type. This makes this
information unsuitable for platform_data (For example someone might
use redboot on some board which swaps the bad block marker, some other
persion might use another bootloader which does not swap on the very
same board type). The only sane solution would be to store this
information inside the flash chip itself.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Artem Bityutskiy]

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

On Tue, 2012-09-11 at 09:17 +0200, Sascha Hauer wrote:
> In this form the patch is simply not correct and should be dropped.
> 
OK, dropped it.
> 
-- 
Best Regards,
Artem Bityutskiy

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

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

Hi,
On 09/11/2012 09:17 AM, Sascha Hauer wrote:
> On Mon, Sep 10, 2012 at 04:31:23PM +0200, Gaëtan Carlier wrote:
>> Hi Juergen,
>> On 09/06/2012 10:56 AM, Juergen Beisert wrote:
>>> Hi Gaëtan,
>>>
>>> Gaëtan Carlier wrote:
>>>> Any comments for this patch ?
>>>
>>> Sorry, a little bit late.
>>>
>>>> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
>>>> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
>>>> Warning: The same solution needs to be applied to the boot loader and the
>>>> flash programmer.
>>>
>>> What sense does it make to swap the bytes at offset 0x7D0 and 0x835?
>>>
>>> Background: the NFC in the i.MX27/i.MX31 (NFCv1) and i.MX25/i.MX35 (NFCv2) can
>>> only handle 512 byte pages with 16 bit OOB at once. To get them work with 2 k
>>> page NANDs they only do the 512 + 16 step four times. The result is, a 2 k
>>> page NAND is not handled as 2 k data and 64 bytes OOB, its handled instead as
>>> a stream of 2112 bytes:
>>>
>>>                 |<-------- 2112 bytes --------->|
>>>                 512+16 | 512+16 | 512+16 | 512+16
>>>
>>> Which means the NFC mixes data and OOB/checksums into its internal SRAM. But
>>> all upper routines still use this SRAM content with a 2048 (data) + 64 (OOB)
>>> bytes layout (to be more precise also the NFC hardware uses this layout).
>>> Result is, the factory bad block markers are lost, because the NFC stores the
>>> first 48 bytes from each OOB into the SRAM's data area (beginning with offset
>>> 2000 (= 0x7D0) in the data area).
>>>
>>> So, the main goal of this swap patch should be to keep the factory bad block
>>> markers _and_ to make use of them.
>>>
>>> But the i.MX driver does not register its own bad block pattern description,
>>> so the default one is used with the BBM at position NAND_LARGE_BADBLOCK_POS
>>> (which means offset 0 in the OOB area).
>>>
>>> So, I think, all upper layers still search for the BBM at offset 0 of the OOB
>>> area, which results to SRAM's offset 0x800 (for NFCv1) and not 0x835. This
>>> means this patch keeps the factory bad block markers, but they won't still be
>>> used as expected.
>>>
>>> IMHO the bytes at SRAM's offset 0x7D0 and 0x800 for the NFCv1 must be swapped
>>> (and 0x7D0 and 0x1000 for the NFCv2) to keep the factory bad block markers
>>> _and_ to make use of them. Or am I wrong?
>>>
>>> Regards,
>>> Juergen
>>>
>> The point is not the necessity of swapping bytes if factory bad
>> block markers are not used.
>> In my case, the bytes must be swapped because all previous
>> operations on flash swapped these bytes :
>> * Flashes redboot (bootloader) via JTAG using OpenOCD which swaps bytes;
>> * Writes kernel and rootfs via TFTP using redboot which swaps bytes;
>> So to be able to read rootfs partition, mxc-nand driver have to swap
>> bytes otherwise, JFFS2 detects error on partition and is unable to
>> mount it.
>
> I am all in favour to be able to swap the factory bad block marker with
> the position the mtd framework expects it.
>
> That according to our understanding are bytes 0x7d0 in the main area
> and byte 0x0 in the spare area.
>
>>
>> I have not chosen to swap these bytes. In a training, I received
>> redboot and kernel 2.6.22 with set of patches to support i.MX27 and
>> these patches included swap of bytes in nand driver. So I start my
>> development with this workaround and it is now impossible to get
>> back because boards are already in production.
>
> So you are swapping the factory bad block marker with some position
> where the mtd layer does not expect the bad block marker. With this
> you preserve the factor bad block marker, but do not use it to detect
> bad blocks.
I don't see where the problem is. The fact that bad block marker are 
used or not, is not related to "my" patch. This patch just places data 
where they should be. If I take the Nand and connect it to a not-bugged 
NFC controller, all data will be read correctly with data is main area 
and OOB is spare area.
>
> In this form the patch is simply not correct and should be dropped.
>
Now I want to adapt and publish my platform file to latest kernel 
version and I am still stopped with this biswap patch that is not 
applied. As I said before, I must not be the only that have used this 
damned original patch from Freescale that swaps byte in their ATK 
programming soft, redboot and Kernel and it is impossible to fallback to 
current NAND handling (without swapping).

Is there any chance that if I send this patch against linux-next/master 
branch, it will be finally accepted. Only people that needs it will use 
it. There is so much option in menuconfig with so little explaination 
and mark as "Only say Yes if you known what you do".
> Sascha
>
Thank you.
Best regards,
Gaëtan Carlier.

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Sascha Hauer]

On Wed, Mar 26, 2014 at 02:53:33PM +0100, Gaëtan Carlier wrote:
> Hi,
> I don't see where the problem is. The fact that bad block marker are
> used or not, is not related to "my" patch. This patch just places
> data where they should be. If I take the Nand and connect it to a
> not-bugged NFC controller, all data will be read correctly with data
> is main area and OOB is spare area.
> >
> >In this form the patch is simply not correct and should be dropped.
> >
> Now I want to adapt and publish my platform file to latest kernel
> version and I am still stopped with this biswap patch that is not
> applied. As I said before, I must not be the only that have used
> this damned original patch from Freescale that swaps byte in their
> ATK programming soft, redboot and Kernel and it is impossible to
> fallback to current NAND handling (without swapping).
> 
> Is there any chance that if I send this patch against
> linux-next/master branch, it will be finally accepted. Only people
> that needs it will use it. There is so much option in menuconfig
> with so little explaination and mark as "Only say Yes if you known
> what you do".

I'm ok with supporting a devicetree option that decides whether to do
bad block marker swapping or not. It shouldn't be a Kconfig option.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Gaëtan Carlier]

On 03/28/2014 10:11 AM, Sascha Hauer wrote:
> On Wed, Mar 26, 2014 at 02:53:33PM +0100, Gaëtan Carlier wrote:
>> Hi,
>> I don't see where the problem is. The fact that bad block marker are
>> used or not, is not related to "my" patch. This patch just places
>> data where they should be. If I take the Nand and connect it to a
>> not-bugged NFC controller, all data will be read correctly with data
>> is main area and OOB is spare area.
>>>
>>> In this form the patch is simply not correct and should be dropped.
>>>
>> Now I want to adapt and publish my platform file to latest kernel
>> version and I am still stopped with this biswap patch that is not
>> applied. As I said before, I must not be the only that have used
>> this damned original patch from Freescale that swaps byte in their
>> ATK programming soft, redboot and Kernel and it is impossible to
>> fallback to current NAND handling (without swapping).
>>
>> Is there any chance that if I send this patch against
>> linux-next/master branch, it will be finally accepted. Only people
>> that needs it will use it. There is so much option in menuconfig
>> with so little explaination and mark as "Only say Yes if you known
>> what you do".
>
> I'm ok with supporting a devicetree option that decides whether to do
> bad block marker swapping or not. It shouldn't be a Kconfig option.
devicetree what a wonderful system... I stop fighting.
>
> Sascha
>
>
Best regards,
Gaëtan Carlier.

Re: [PATCH 1/1] mxc_nand : allow swapping the Bad block Indicator for NFC v1.

[Juergen Beisert]

Gaëtan Carlier wrote:
> Swap the BI-byte on position 0x7D0 with a data byte at 0x835.  To fix a bug
> in Freescale imx NFC v1 SoC's for 2K page NAND flashes: imx27 and imx31.
> Warning: The same solution needs to be applied to the boot loader and the
> flash programmer.
>
> This is a modified version of patch sent by Jürgen Lambrecht :
> [PATCH] Add 'config IMX_NFC_V1_BISWAP' to swap the Bad block Indicator,
> and use for imx27pdk nand support.
>
>  v3: - use accessor function for I/O memory accesses as suggested by Artem
>        Bityutskiy
>      - rename function name as suggested by Artem Bityutskiy
>
>  v2: - Use a flag in a mxc_nand_platform structure instead of adding option
>        in Kconfig
>
>  v1: - Original patch "Add 'config IMX_NFC_V1_BISWAP'" by Jürgen Lambrecht
>
> Signed-off-by: Gaëtan Carlier <gcembed@gmail.com>

One additional note: If a user intend to boot from NAND (aka "external boot") 
there is one drawback with this patch: the very first page of the NAND will 
be read in hardware to load the bootstrap code. But the hardware doesn't know 
anything about swapping bytes!
So, when writing the very first page of the NAND the swap *must* be omitted 
(which also means the factory bad block marker of the first block in the NAND 
has to be interpreted differently).

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |