mtd: gpmi: make blockmark swapping optional

With a flash-based BBT there is no reason to move the Factory Bad Block Marker from the data area buffer (to where it is mapped by the GPMI NAND controller) to the OOB buffer. Thus, make this feature configurable via DT. This is required for the Ka-Ro electronics platforms.
[...]

Hi Lothar,

Quoted 6 lines · On Wednesday 19 March 2014 14:23:26 y at karo-electronics.de wrote:

With a flash-based BBT there is no reason to move the Factory Bad Block Marker from the data area buffer (to where it is mapped by the GPMI NAND controller) to the OOB buffer. Thus, make this feature configurable via DT. This is required for the Ka-Ro electronics platforms.
[...]

How do you create the BBT the very first time with valuable data when the NAND already contains factory bad blocks?

configurable via DT. This is required for the Ka-Ro electronics platforms.

? 2014?03?19? 21:23, y at karo-electronics.de ??:

Quoted 3 lines

configurable via DT. This is required for the Ka-Ro electronics platforms.

If you disable the swapping, you will disable the NAND boot in actually.

Hi,

Quoted 6 lines · Huang Shijie

Quoted 3 lines · 于 2014年03月19日 21:23, [email protected] 写道:

configurable via DT. This is required for the Ka-Ro electronics platforms.

If you disable the swapping, you will disable the NAND boot in actually.

No. All our modules do boot from NAND and none of them does the swapping.

? 2014?03?20? 17:21, Lothar Wa?mann ??:

Quoted 9 lines

Hi,

Quoted 6 lines · Huang Shijie

? 2014?03?19? 21:23, y at karo-electronics.de ??:

Quoted 3 lines

configurable via DT. This is required for the Ka-Ro electronics platforms.

If you disable the swapping, you will disable the NAND boot in actually.

Quoted 4 lines

No. All our modules do boot from NAND and none of them does the swapping.

It seems the NAND boot runs well by luck.

The ROM will do the swapping when it reads the BBT table written by the kobs-ng (or something else?).

This can be disabled by parameter in the FCB (at least according to the i.MX6 reference Manual):

|DISBBM 172 4 If 0 ROM will swap BadBlockMarkerByte with
|             metadata[0] after reading a page using BCH40.
|             If the value set is 1 then ROM will not do
|             swapping

With a flash-based BBT there is no reason to move the Factory Bad Block Marker from the data area buffer (to where it is mapped by the GPMI NAND controller) to the OOB buffer. Thus, make this feature configurable via DT. This is required for the Ka-Ro electronics platforms.

Signed-off-by Lothar Wa?mann <[email protected]>

---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/mtd/gpmi-nand.txt +0 -0

index 458d596..f28949a 100644

Documentation/devicetree/bindings/mtd/gpmi-nand.txt +0 -0

 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +0 -3

index bb77f75..98562eb 100644
@@ -1632,9 +1632,6 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	struct bch_geometry *bch_geo = &this->bch_geometry;
 	int ret;
 
-	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
-	this->swap_block_mark = !GPMI_IS_MX23(this);
-
 	/* Set up the medium geometry */
 	ret = gpmi_set_geometry(this);
 	if (ret)

Quoted 12 lines · Lothar Wa?mann

With a flash-based BBT there is no reason to move the Factory Bad Block Marker from the data area buffer (to where it is mapped by the GPMI NAND controller) to the OOB buffer. Thus, make this feature configurable via DT. This is required for the Ka-Ro electronics platforms.

Signed-off-by Lothar Wa?mann <[email protected]>

---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/mtd/gpmi-nand.txt Optional properties: · line 30

Quoted 11 lines

Documentation/devicetree/bindings/mtd/gpmi-nand.txt +3 -0

index 458d596..f28949a 100644
@@ -25,6 +25,9 @@ Optional properties:
                        discoverable or this property is not enabled,
                        the software may chooses an implementation-defined
                        ECC scheme.
+  - fsl,no-blockmark-swap: Don't swap the bad block marker from the OOB
+                       area with the byte in the data area but rely on the
+                       BBT for identifying bad blocks.

Please add the following:
"NOTE: This property is not supported by the imx23 and imx28"

Quoted 17 lines

Documentation/devicetree/bindings/mtd/gpmi-nand.txt +0 -0

 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +0 -3

index bb77f75..98562eb 100644
@@ -1632,9 +1632,6 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	struct bch_geometry *bch_geo = &this->bch_geometry;
 	int ret;
 
-	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
-	this->swap_block_mark = !GPMI_IS_MX23(this);
-
 	/* Set up the medium geometry */
 	ret = gpmi_set_geometry(this);
 	if (ret)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c static int gpmi_nand_init(struct gpmi_nand_data *this) · line 1703

Quoted 6 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +3 -0

@@ -1701,6 +1698,13 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 	if (of_get_nand_on_flash_bbt(this->dev->of_node))
 		chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
 
+	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
+	if (!of_property_read_bool(this->dev->of_node, "fsl,no-blockmark-swap"))
+		this->swap_block_mark = !GPMI_IS_MX23(this);

the code should like this:
----------------------------------------------

	this->swap_block_mark = !GPMI_IS_MX23(this);

	if (!GPMI_IS_IMX23(this) && !GPMI_IS_MX28(this) &&

drivers/mtd/nand/gpmi-nand/gpmi-nand.c static int gpmi_nand_init(struct gpmi_nand_data *this) · line 1710

Quoted 7 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +4 -0

+
+	dev_dbg(this->dev, "Blockmark swapping %sabled\n",
+		this->swap_block_mark ? "en" : "dis");
+
 	/*
 	 * Allocate a temporary DMA buffer for reading ID in the
 	 * nand_scan_ident().

There are some bugs in the gpmi driver which is caused by the assumption that we always enable the swapping except the imx23.

So, there are some places should be changed with this patch:
[1] the subpage hook,
Please also change the gpmi_ecc_read_subpage() too.

[2] the OOB hook,
Please also change the gpmi_ecc_read_oob().

[3] the markbad hook,
Please also change the gpmi_block_markbad() .

I don't see why this should not be supported on i.MX28 (i.MX23 doesn't do byteswapping anyway, so this wouldn't change anything for i.MX23). The partitions used by Linux need not necessarily be accessible for the Boot ROM code (and vice versa).

? 2014?03?26? 16:51, Lothar Wa?mann ??:

Quoted 4 lines

I don't see why this should not be supported on i.MX28 (i.MX23 doesn't do byteswapping anyway, so this wouldn't change anything for i.MX23). The partitions used by Linux need not necessarily be accessible for the Boot ROM code (and vice versa).

But the first partition used to store the u-boot is accessible for the ROM.

Please see "Figure 12-13" in the 12.12.1.12:
"In order to preserve the BI (bad block information), flash updater
or gang programmer
applications need to swap Bad Block Information (BI) data to byte 0 of metadata area for
every page before programming NAND Flash. ROM when loading firmware, copies back
the value at metadata[0] to BI offset in page data. The following figure shows how the
factory bad block marker is preserved."

Hi,

Quoted 1 line · Huang Shijie

? 2014?03?26? 16:51, Lothar Wa?mann ??:

Quoted 6 lines

Quoted 4 lines

I don't see why this should not be supported on i.MX28 (i.MX23 doesn't do byteswapping anyway, so this wouldn't change anything for i.MX23). The partitions used by Linux need not necessarily be accessible for the Boot ROM code (and vice versa).

But the first partition used to store the u-boot is accessible for the ROM.

Whether this partition is available to Linux depends on the partition table passed in the DT.

Quoted 12 lines

Please see "Figure 12-13" in the 12.12.1.12:
"In order to preserve the BI (bad block information), flash updater
or gang programmer
applications need to swap Bad Block Information (BI) data to byte 0 of metadata area for
every page before programming NAND Flash. ROM when loading firmware, copies back
the value at metadata[0] to BI offset in page data. The following figure shows how the
factory bad block marker is preserved."

The inspection of the BB markers is only a fallback for the case that there is no DBBT. From the same chapter that you quoted above:

| ROM uses DBBT to skip any bad block that falls within firmware data
| on NAND Flash device.
| If the address of DBBT Search Area in FCB is 0, ROM will rely on
| factory marked bad block markers to find out if a block is good or bad.

Thus, even the boot ROM of i.MX28 can well live without blockmark swapping.

Quoted 13 lines

Quoted 11 lines

Please see "Figure 12-13" in the 12.12.1.12:
"In order to preserve the BI (bad block information), flash updater
or gang programmer
applications need to swap Bad Block Information (BI) data to byte 0 of metadata area for
every page before programming NAND Flash. ROM when loading firmware, copies back
the value at metadata[0] to BI offset in page data. The following figure shows how the
factory bad block marker is preserved."

The inspection of the BB markers is only a fallback for the case that there is no DBBT. From the same chapter that you quoted above:

Quoted 7 lines

| ROM uses DBBT to skip any bad block that falls within firmware data
| on NAND Flash device.
| If the address of DBBT Search Area in FCB is 0, ROM will rely on
| factory marked bad block markers to find out if a block is good or bad.

Thus, even the boot ROM of i.MX28 can well live without blockmark swapping.

Assume that there is a NAND block "A", and the A consist of 256 pages. the uboot is burned to the "A", can occupy 6 pages:

-----------------------------------------------------------------------------
| page 0 | page 1 | page 2 | page 3 | page 4 | page 5 | ... | ... | page 255 |
-----------------------------------------------------------------------------

\-------------------------------------- ------------------------------------/

                                         V  
                                        "A"					 

The DBBT is used to track if "A" is bad or not.
Assume we know that "A" is a good block, ROM then need to read out the uboot. When the ROM needs to read out the 6 pages one by one. And each time the ROM read the page, it should do the swapping for this page.

In this case, the ROM will do the swapping six times.

Please read the sector again, you will see the "every page" in it:
--------------------------------------------------------------------
"In order to preserve the BI (bad block information), flash updater
or gang programmer applications need to swap Bad Block Information (BI) data to byte 0 of metadata area for every page before programming NAND Flash. ROM when loading firmware, copies back
--------------------------------------------------------------------

I can assure you that the>100.000 i.MX28 based modules, that we sold up to now boot from NAND just fine without any block mark swapping in the U-Boot pages.

? 2014?03?27? 20:21, Lothar Wa?mann ??:

Quoted 4 lines

I can assure you that the>100.000 i.MX28 based modules, that we sold up to now boot from NAND just fine without any block mark swapping in the U-Boot pages.

Our driver should follow the Reference manual, not the product.

Still there is no need for the Linux NAND driver to be able to read or write partitions in a format that the Boot ROM can understand. Thus it

is perfectly legal to allow disregarding the BB marks and solely rely on a flash based BBT.

? 2014?03?28? 16:16, Lothar Wa?mann ??:

Quoted 2 lines

Still there is no need for the Linux NAND driver to be able to read or write partitions in a format that the Boot ROM can understand. Thus it

If you do not use the NAND boot, there really no need to do so.

Since you need the NAND boot, we should enable the swapping for imx28.

Quoted 3 lines

is perfectly legal to allow disregarding the BB marks and solely rely on a flash based BBT.

The BB mark is in the page 0 of a NAND block. But the swapping can occur in _each_ page of
a NAND block, _NOT_ only the page 0.

I think you are confusing at these two things.

- disables swapping of BB marks in the FCB the ROM won't swap bytes.

? 2014?03?28? 17:00, Sascha Hauer ??:

Quoted 1 line

- disables swapping of BB marks in the FCB the ROM won't swap bytes.

We can not disable the swapping in the FCB in the imx28. The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.

Hi,

Quoted 5 lines · Huang Shijie

Quoted 1 line · Sascha Hauer

- disables swapping of BB marks in the FCB the ROM won't swap bytes.

We can not disable the swapping in the FCB in the imx28. The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.

The Linux kernel does not have any business with the FCB, so it is completely irrelevant for Linux whether byte swapping can be disabled in the FCB or not.

? 2014?03?28? 17:31, Lothar Wa?mann ??:

Quoted 12 lines

Hi,

Quoted 5 lines · Huang Shijie

? 2014?03?28? 17:00, Sascha Hauer ??:

Quoted 1 line

- disables swapping of BB marks in the FCB the ROM won't swap bytes.

We can not disable the swapping in the FCB in the imx28. The DISBBM bit in the FCB does _NOT_ exit in the imx28's FCB.

The Linux kernel does not have any business with the FCB, so it is completely irrelevant for Linux whether byte swapping can be disabled in the FCB or not.

But the gpmi is relevant to the ROM.
the gpmi driver should keep consistency with the ROM.

I do not object you add the swapping optional the gpmi driver(since it's ok for imx50/imx6q/imx6sx),
i just object that you enable this feature for imx28.

? 2014?03?28? 16:16, Lothar Wa?mann ??:

Quoted 2 lines

Still there is no need for the Linux NAND driver to be able to read or write partitions in a format that the Boot ROM can understand. Thus it

If you do not use the NAND boot, there really no need to do so.

Since you need the NAND boot, we should enable the swapping for imx28.

Quoted 3 lines

is perfectly legal to allow disregarding the BB marks and solely rely on a flash based BBT.

The BB mark is in the page 0 of a NAND block. But the swapping can occur in _each_ page of
a NAND block, _NOT_ only the page 0.

I think you are confusing at these two things.

There is no need for the ROM code to access any other partition than the bootloader itself. Thus Linux can perfectly well be booted from

Quoted 2 lines · Lothar Waßmann

There is no need for the ROM code to access any other partition than the bootloader itself. Thus Linux can perfectly well be booted from

Assume the ROM only access the partition which contains the bootloader, and the ROM does the swapping for the pages(but we do not do the swapping to these pages),

what will happen?

? 2014?03?28? 17:33, Huang Shijie ??:

Quoted 7 lines

? 2014?03?28? 17:01, Lothar Wa?mann ??:

Quoted 2 lines

There is no need for the ROM code to access any other partition than the bootloader itself. Thus Linux can perfectly well be booted from

Assume the ROM only access the partition which contains the bootloader, and
the ROM does the swapping for the pages(but we do not do the swapping to these pages),

I mean the kernel do not do the swapping when we burn the uboot to the NAND.