Patchwork mtd: gpmi: make blockmark swapping optional

login
register
mail settings
Submitter y@karo-electronics.de
Date March 19, 2014, 1:23 p.m.
Message ID <1395235406-13449-1-git-send-email-y>
Download mbox | patch
Permalink /patch/331730/
State New
Headers show

Comments

y@karo-electronics.de - March 19, 2014, 1:23 p.m.
From: Lothar Waßmann <LW@KARO-electronics.de>

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 <LW@KARO-electronics.de>
---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt |    3 +++
 arch/arm/boot/dts/imx28-tx28.dts                    |    1 +
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c              |   10 +++++++---
 3 files changed, 11 insertions(+), 3 deletions(-)
Juergen Beisert - March 20, 2014, 8:33 a.m.
Hi Lothar,

On Wednesday 19 March 2014 14:23:26 y@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?

jbe
Huang Shijie - March 20, 2014, 9:15 a.m.
于 2014年03月19日 21:23, y@karo-electronics.de 写道:
> 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.

do you need the nand boot?

could you please tell me more detail background?

thanks
Huang Shijie
Lothar Waßmann - March 20, 2014, 9:20 a.m.
Hi,

Juergen Beisert wrote:
> Hi Lothar,
> 
> On Wednesday 19 March 2014 14:23:26 y@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?
> 
With the flashtool that does the initial flash programming.


Lothar Waßmann
Lothar Waßmann - March 20, 2014, 9:21 a.m.
Hi,

Huang Shijie wrote:
> 于 2014年03月19日 21:23, y@karo-electronics.de 写道:
> > 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.


Lothar Waßmann
Huang Shijie - March 20, 2014, 9:47 a.m.
于 2014年03月20日 17:21, Lothar Waßmann 写道:
> Hi,
>
> Huang Shijie wrote:
>> 于 2014年03月19日 21:23, y@karo-electronics.de 写道:
>>> 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.
>
>
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?).

If the BBT table is small, not bigger then a page, the NAND boot may 
runs well when you disable the swapping;
But it the BBT table is bigger, the ROM will get a corrupted BBT table.



thanks
Huang Shijie
Lothar Waßmann - March 20, 2014, 10:09 a.m.
Hi,

Huang Shijie wrote:
> 于 2014年03月20日 17:21, Lothar Waßmann 写道:
> > Hi,
> >
> > Huang Shijie wrote:
> >> 于 2014年03月19日 21:23, y@karo-electronics.de 写道:
> >>> 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.
> >
> >
> 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


Lothar Waßmann
Huang Shijie - March 20, 2014, 10:56 a.m.
于 2014年03月20日 18:09, Lothar Waßmann 写道:
> This can be disabled by parameter in the FCB (at least according to
> the i.MX6 reference Manual):
from imx28's reference manual, there is not such bit in the FCB to 
disable the
swapping. But you are disabling the swapping in the imx28's DTS file, 
isn't it?

I think it is dangerous.



> |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
>
>
I will consult with our ROM colleague about this DISBBM bit.

I am not sure if the ROM still do the swapping for the uboot when we set 
DISBBM bit.

thanks
Huang Shijie
Huang Shijie - March 21, 2014, 5:36 a.m.
于 2014年03月19日 21:23, y@karo-electronics.de 写道:
> +	/* 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);


Our ROM guy had confirmed that the Rom will disable the swapping if we 
set the DISBBM bit.
But please do not change any logic for imx23/imx28.

I really do not know what's the benefit we can get from this patch.
Please send the new version about this patch if you want this feature.


thanks
Huang Shijie
Lothar Waßmann - March 28, 2014, 10:35 a.m.
patch added:
- add an additional option to turn of BB mark writing

Patch

diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index 458d596..f28949a 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -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.
 
 The device tree may optionally contain sub-nodes describing partitions of the
 address space. See partition.txt for more detail.
diff --git a/arch/arm/boot/dts/imx28-tx28.dts b/arch/arm/boot/dts/imx28-tx28.dts
index e14bd86..7d2bea8 100644
--- a/arch/arm/boot/dts/imx28-tx28.dts
+++ b/arch/arm/boot/dts/imx28-tx28.dts
@@ -247,6 +247,7 @@ 
 &gpmi {
 	pinctrl-0 = <&gpmi_pins_a &gpmi_status_cfg>;
 	nand-on-flash-bbt;
+	fsl,no-blockmark-swap;
 	status = "okay";
 };
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index bb77f75..98562eb 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -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)
@@ -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);
+
+	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().