* [PATCHv4 3/5] mtd: gpmi: make blockmark swapping optional
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: linux-arm-kernel
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.
In the original code 'this->swap_block_mark' was synonymous with
'!GPMI_IS_MX23()', so use the latter at the relevant places.
Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
---
.../devicetree/bindings/mtd/gpmi-nand.txt | 10 ++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 51 ++++++++++++--------
2 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index 458d596..a011fdf 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -25,6 +25,16 @@ 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
+ flash based BBT for identifying bad blocks.
+ NOTE: this is only valid in conjunction with
+ 'nand-on-flash-bbt'.
+ WARNING: on i.MX28 blockmark swapping cannot be
+ disabled for the BootROM in the FCB. Thus,
+ partitions written from Linux with this feature
+ turned on may not be accessible by the BootROM
+ code.
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 16a533a..959cb9b 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1081,6 +1081,7 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
int first, last, marker_pos;
int ecc_parity_size;
int col = 0;
+ int old_swap_block_mark = this->swap_block_mark;
/* The size of ECC parity */
ecc_parity_size = geo->gf_len * geo->ecc_strength / 8;
@@ -1089,17 +1090,21 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
first = offs / size;
last = (offs + len - 1) / size;
- /*
- * Find the chunk which contains the Block Marker. If this chunk is
- * in the range of [first, last], we have to read out the whole page.
- * Why? since we had swapped the data at the position of Block Marker
- * to the metadata which is bound with the chunk 0.
- */
- marker_pos = geo->block_mark_byte_offset / size;
- if (last >= marker_pos && first <= marker_pos) {
- dev_dbg(this->dev, "page:%d, first:%d, last:%d, marker at:%d\n",
+ if (this->swap_block_mark) {
+ /*
+ * Find the chunk which contains the Block Marker.
+ * If this chunk is in the range of [first, last],
+ * we have to read out the whole page.
+ * Why? since we had swapped the data at the position of Block
+ * Marker to the metadata which is bound with the chunk 0.
+ */
+ marker_pos = geo->block_mark_byte_offset / size;
+ if (last >= marker_pos && first <= marker_pos) {
+ dev_dbg(this->dev,
+ "page:%d, first:%d, last:%d, marker at:%d\n",
page, first, last, marker_pos);
- return gpmi_ecc_read_page(mtd, chip, buf, 0, page);
+ return gpmi_ecc_read_page(mtd, chip, buf, 0, page);
+ }
}
meta = geo->metadata_size;
@@ -1145,7 +1150,7 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
writel(r1_old, bch_regs + HW_BCH_FLASH0LAYOUT0);
writel(r2_old, bch_regs + HW_BCH_FLASH0LAYOUT1);
this->bch_geometry = old_geo;
- this->swap_block_mark = true;
+ this->swap_block_mark = old_swap_block_mark;
return max_bitflips;
}
@@ -1309,10 +1314,10 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
/*
* Now, we want to make sure the block mark is correct. In the
- * Swapping/Raw case, we already have it. Otherwise, we need to
- * explicitly read it.
+ * non-transcribing case (!GPMI_IS_MX23()), we already have it.
+ * Otherwise, we need to explicitly read it.
*/
- if (!this->swap_block_mark) {
+ if (GPMI_IS_MX23(this)) {
/* Read the block mark into the first byte of the OOB buffer. */
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
chip->oob_poi[0] = chip->read_byte(mtd);
@@ -1353,7 +1358,7 @@ static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
chipnr = (int)(ofs >> chip->chip_shift);
chip->select_chip(mtd, chipnr);
- column = this->swap_block_mark ? mtd->writesize : 0;
+ column = !GPMI_IS_MX23(this) ? mtd->writesize : 0;
/* Write the block mark. */
block_mark = this->data_buffer_dma;
@@ -1649,9 +1654,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)
@@ -1715,9 +1717,20 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
chip->badblock_pattern = &gpmi_bbt_descr;
chip->block_markbad = gpmi_block_markbad;
chip->options |= NAND_NO_SUBPAGE_WRITE;
- if (of_get_nand_on_flash_bbt(this->dev->of_node))
+
+ /* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
+ this->swap_block_mark = !GPMI_IS_MX23(this);
+
+ if (of_get_nand_on_flash_bbt(this->dev->of_node)) {
chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
+ if (of_property_read_bool(this->dev->of_node,
+ "fsl,no-blockmark-swap"))
+ this->swap_block_mark = false;
+ }
+ 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().
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 3/5] mtd: gpmi: make blockmark swapping optional
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: Arnd Bergmann, Artem Bityutskiy, Brian Norris, David Woodhouse,
Fabio Estevam, Huang Shijie, Ian Campbell, Kumar Gala,
Lothar Waßmann, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Russell King, Sascha Hauer, Shawn Guo, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-mtd, Shawn Guo,
Boris BREZILLON, Ezequiel Garcia, Grant Likely, Jingoo Han,
Michael Grzeschik, Michael Opdenacker, Wei Yongjun
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.
In the original code 'this->swap_block_mark' was synonymous with
'!GPMI_IS_MX23()', so use the latter at the relevant places.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
.../devicetree/bindings/mtd/gpmi-nand.txt | 10 ++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 51 ++++++++++++--------
2 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index 458d596..a011fdf 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -25,6 +25,16 @@ 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
+ flash based BBT for identifying bad blocks.
+ NOTE: this is only valid in conjunction with
+ 'nand-on-flash-bbt'.
+ WARNING: on i.MX28 blockmark swapping cannot be
+ disabled for the BootROM in the FCB. Thus,
+ partitions written from Linux with this feature
+ turned on may not be accessible by the BootROM
+ code.
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 16a533a..959cb9b 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1081,6 +1081,7 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
int first, last, marker_pos;
int ecc_parity_size;
int col = 0;
+ int old_swap_block_mark = this->swap_block_mark;
/* The size of ECC parity */
ecc_parity_size = geo->gf_len * geo->ecc_strength / 8;
@@ -1089,17 +1090,21 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
first = offs / size;
last = (offs + len - 1) / size;
- /*
- * Find the chunk which contains the Block Marker. If this chunk is
- * in the range of [first, last], we have to read out the whole page.
- * Why? since we had swapped the data at the position of Block Marker
- * to the metadata which is bound with the chunk 0.
- */
- marker_pos = geo->block_mark_byte_offset / size;
- if (last >= marker_pos && first <= marker_pos) {
- dev_dbg(this->dev, "page:%d, first:%d, last:%d, marker at:%d\n",
+ if (this->swap_block_mark) {
+ /*
+ * Find the chunk which contains the Block Marker.
+ * If this chunk is in the range of [first, last],
+ * we have to read out the whole page.
+ * Why? since we had swapped the data at the position of Block
+ * Marker to the metadata which is bound with the chunk 0.
+ */
+ marker_pos = geo->block_mark_byte_offset / size;
+ if (last >= marker_pos && first <= marker_pos) {
+ dev_dbg(this->dev,
+ "page:%d, first:%d, last:%d, marker at:%d\n",
page, first, last, marker_pos);
- return gpmi_ecc_read_page(mtd, chip, buf, 0, page);
+ return gpmi_ecc_read_page(mtd, chip, buf, 0, page);
+ }
}
meta = geo->metadata_size;
@@ -1145,7 +1150,7 @@ static int gpmi_ecc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
writel(r1_old, bch_regs + HW_BCH_FLASH0LAYOUT0);
writel(r2_old, bch_regs + HW_BCH_FLASH0LAYOUT1);
this->bch_geometry = old_geo;
- this->swap_block_mark = true;
+ this->swap_block_mark = old_swap_block_mark;
return max_bitflips;
}
@@ -1309,10 +1314,10 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
/*
* Now, we want to make sure the block mark is correct. In the
- * Swapping/Raw case, we already have it. Otherwise, we need to
- * explicitly read it.
+ * non-transcribing case (!GPMI_IS_MX23()), we already have it.
+ * Otherwise, we need to explicitly read it.
*/
- if (!this->swap_block_mark) {
+ if (GPMI_IS_MX23(this)) {
/* Read the block mark into the first byte of the OOB buffer. */
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
chip->oob_poi[0] = chip->read_byte(mtd);
@@ -1353,7 +1358,7 @@ static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
chipnr = (int)(ofs >> chip->chip_shift);
chip->select_chip(mtd, chipnr);
- column = this->swap_block_mark ? mtd->writesize : 0;
+ column = !GPMI_IS_MX23(this) ? mtd->writesize : 0;
/* Write the block mark. */
block_mark = this->data_buffer_dma;
@@ -1649,9 +1654,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)
@@ -1715,9 +1717,20 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
chip->badblock_pattern = &gpmi_bbt_descr;
chip->block_markbad = gpmi_block_markbad;
chip->options |= NAND_NO_SUBPAGE_WRITE;
- if (of_get_nand_on_flash_bbt(this->dev->of_node))
+
+ /* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
+ this->swap_block_mark = !GPMI_IS_MX23(this);
+
+ if (of_get_nand_on_flash_bbt(this->dev->of_node)) {
chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
+ if (of_property_read_bool(this->dev->of_node,
+ "fsl,no-blockmark-swap"))
+ this->swap_block_mark = false;
+ }
+ 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().
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
2014-06-12 13:20 ` Lothar Waßmann
(?)
@ 2014-06-12 13:20 ` Lothar Waßmann
-1 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: Arnd Bergmann, Artem Bityutskiy, Brian Norris, David Woodhouse,
Fabio Estevam, Huang Shijie, Ian Campbell, Kumar Gala,
Lothar Waßmann, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Russell King, Sascha Hauer, Shawn Guo, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-mtd, Shawn Guo,
Boris BREZILLON
add a boolean property 'nand-no-oob-bbm' and helper function to be
able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
and use it for i.MX and MXS nand drivers.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
Documentation/devicetree/bindings/mtd/nand.txt | 1 +
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
drivers/mtd/nand/mxc_nand.c | 2 ++
drivers/of/of_mtd.c | 12 ++++++++++++
include/linux/of_mtd.h | 6 ++++++
5 files changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..e46bfbe 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -5,6 +5,7 @@
"soft_bch".
- nand-bus-width : 8 or 16 bus width if not present 8
- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
+- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
- nand-ecc-strength: integer representing the number of bits to correct
per ECC step.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..37537b4 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1724,6 +1724,9 @@ 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;
+ if (of_get_nand_no_oob_bbm(this->dev->of_node))
+ chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
+
if (of_property_read_bool(this->dev->of_node,
"fsl,no-blockmark-swap"))
this->swap_block_mark = false;
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index dba262b..bb54a2a 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
this->bbt_md = &bbt_mirror_descr;
/* update flash based bbt */
this->bbt_options |= NAND_BBT_USE_FLASH;
+ if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
+ this->bbt_options |= NAND_BBT_NO_OOB_BBM;
}
init_completion(&host->op_completion);
diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..d947acc 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
return of_property_read_bool(np, "nand-on-flash-bbt");
}
EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
+
+/**
+ * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present, false otherwise
+ */
+bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return of_property_read_bool(np, "nand-no-oob-bbm");
+}
+EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa..6ece1a9 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
int of_get_nand_ecc_strength(struct device_node *np);
int of_get_nand_bus_width(struct device_node *np);
bool of_get_nand_on_flash_bbt(struct device_node *np);
+bool of_get_nand_no_oob_bbm(struct device_node *np);
#else /* CONFIG_OF_MTD */
@@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
return false;
}
+static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return false;
+}
+
#endif /* CONFIG_OF_MTD */
#endif /* __LINUX_OF_MTD_H */
--
1.7.10.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: linux-arm-kernel
add a boolean property 'nand-no-oob-bbm' and helper function to be
able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
and use it for i.MX and MXS nand drivers.
Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
---
Documentation/devicetree/bindings/mtd/nand.txt | 1 +
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
drivers/mtd/nand/mxc_nand.c | 2 ++
drivers/of/of_mtd.c | 12 ++++++++++++
include/linux/of_mtd.h | 6 ++++++
5 files changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..e46bfbe 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -5,6 +5,7 @@
"soft_bch".
- nand-bus-width : 8 or 16 bus width if not present 8
- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
+- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
- nand-ecc-strength: integer representing the number of bits to correct
per ECC step.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..37537b4 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1724,6 +1724,9 @@ 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;
+ if (of_get_nand_no_oob_bbm(this->dev->of_node))
+ chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
+
if (of_property_read_bool(this->dev->of_node,
"fsl,no-blockmark-swap"))
this->swap_block_mark = false;
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index dba262b..bb54a2a 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
this->bbt_md = &bbt_mirror_descr;
/* update flash based bbt */
this->bbt_options |= NAND_BBT_USE_FLASH;
+ if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
+ this->bbt_options |= NAND_BBT_NO_OOB_BBM;
}
init_completion(&host->op_completion);
diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..d947acc 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
return of_property_read_bool(np, "nand-on-flash-bbt");
}
EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
+
+/**
+ * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present, false otherwise
+ */
+bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return of_property_read_bool(np, "nand-no-oob-bbm");
+}
+EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa..6ece1a9 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
int of_get_nand_ecc_strength(struct device_node *np);
int of_get_nand_bus_width(struct device_node *np);
bool of_get_nand_on_flash_bbt(struct device_node *np);
+bool of_get_nand_no_oob_bbm(struct device_node *np);
#else /* CONFIG_OF_MTD */
@@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
return false;
}
+static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return false;
+}
+
#endif /* CONFIG_OF_MTD */
#endif /* __LINUX_OF_MTD_H */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: Arnd Bergmann, Artem Bityutskiy, Brian Norris, David Woodhouse,
Fabio Estevam, Huang Shijie, Ian Campbell, Kumar Gala,
Lothar Waßmann, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Russell King, Sascha Hauer, Shawn Guo, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-mtd, Shawn Guo,
Boris BREZILLON, Ezequiel Garcia, Grant Likely, Jingoo Han,
Michael Grzeschik, Michael Opdenacker, Wei Yongjun
add a boolean property 'nand-no-oob-bbm' and helper function to be
able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
and use it for i.MX and MXS nand drivers.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
Documentation/devicetree/bindings/mtd/nand.txt | 1 +
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
drivers/mtd/nand/mxc_nand.c | 2 ++
drivers/of/of_mtd.c | 12 ++++++++++++
include/linux/of_mtd.h | 6 ++++++
5 files changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..e46bfbe 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -5,6 +5,7 @@
"soft_bch".
- nand-bus-width : 8 or 16 bus width if not present 8
- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
+- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
- nand-ecc-strength: integer representing the number of bits to correct
per ECC step.
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..37537b4 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1724,6 +1724,9 @@ 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;
+ if (of_get_nand_no_oob_bbm(this->dev->of_node))
+ chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
+
if (of_property_read_bool(this->dev->of_node,
"fsl,no-blockmark-swap"))
this->swap_block_mark = false;
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index dba262b..bb54a2a 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
this->bbt_md = &bbt_mirror_descr;
/* update flash based bbt */
this->bbt_options |= NAND_BBT_USE_FLASH;
+ if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
+ this->bbt_options |= NAND_BBT_NO_OOB_BBM;
}
init_completion(&host->op_completion);
diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..d947acc 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
return of_property_read_bool(np, "nand-on-flash-bbt");
}
EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
+
+/**
+ * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present, false otherwise
+ */
+bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return of_property_read_bool(np, "nand-no-oob-bbm");
+}
+EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa..6ece1a9 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
int of_get_nand_ecc_strength(struct device_node *np);
int of_get_nand_bus_width(struct device_node *np);
bool of_get_nand_on_flash_bbt(struct device_node *np);
+bool of_get_nand_no_oob_bbm(struct device_node *np);
#else /* CONFIG_OF_MTD */
@@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
return false;
}
+static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
+{
+ return false;
+}
+
#endif /* CONFIG_OF_MTD */
#endif /* __LINUX_OF_MTD_H */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
2014-06-12 13:20 ` Lothar Waßmann
(?)
@ 2014-06-12 13:20 ` Lothar Waßmann
-1 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: Arnd Bergmann, Artem Bityutskiy, Brian Norris, David Woodhouse,
Fabio Estevam, Huang Shijie, Ian Campbell, Kumar Gala,
Lothar Waßmann, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Russell King, Sascha Hauer, Shawn Guo, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-mtd, Shawn Guo,
Boris BREZILLON
Without blockmark swapping, there is no use in creating a BBT from
scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
case.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 37537b4..fc710d7 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
.pattern = scan_ff_pattern
};
+static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
+static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
+
+static struct nand_bbt_descr bbt_main_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = bbt_pattern,
+};
+
+static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = mirror_pattern,
+};
+
/*
* We may change the layout if we can get the ECC info from the datasheet,
* else we will use all the (page + OOB).
@@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
if (of_property_read_bool(this->dev->of_node,
- "fsl,no-blockmark-swap"))
+ "fsl,no-blockmark-swap")) {
this->swap_block_mark = false;
+ chip->bbt_td = &bbt_main_no_oob_descr;
+ chip->bbt_md = &bbt_mirror_no_oob_descr;
+ }
}
dev_dbg(this->dev, "Blockmark swapping %sabled\n",
this->swap_block_mark ? "en" : "dis");
--
1.7.10.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: linux-arm-kernel
Without blockmark swapping, there is no use in creating a BBT from
scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
case.
Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 37537b4..fc710d7 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
.pattern = scan_ff_pattern
};
+static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
+static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
+
+static struct nand_bbt_descr bbt_main_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = bbt_pattern,
+};
+
+static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = mirror_pattern,
+};
+
/*
* We may change the layout if we can get the ECC info from the datasheet,
* else we will use all the (page + OOB).
@@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
if (of_property_read_bool(this->dev->of_node,
- "fsl,no-blockmark-swap"))
+ "fsl,no-blockmark-swap")) {
this->swap_block_mark = false;
+ chip->bbt_td = &bbt_main_no_oob_descr;
+ chip->bbt_md = &bbt_mirror_no_oob_descr;
+ }
}
dev_dbg(this->dev, "Blockmark swapping %sabled\n",
this->swap_block_mark ? "en" : "dis");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-06-12 13:20 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-06-12 13:20 UTC (permalink / raw)
To: Arnd Bergmann, Artem Bityutskiy, Brian Norris, David Woodhouse,
Fabio Estevam, Huang Shijie, Ian Campbell, Kumar Gala,
Lothar Waßmann, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Russell King, Sascha Hauer, Shawn Guo, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-mtd, Shawn Guo,
Boris BREZILLON, Ezequiel Garcia, Grant Likely, Jingoo Han,
Michael Grzeschik, Michael Opdenacker, Wei Yongjun
Without blockmark swapping, there is no use in creating a BBT from
scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
case.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 37537b4..fc710d7 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
.pattern = scan_ff_pattern
};
+static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
+static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
+
+static struct nand_bbt_descr bbt_main_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = bbt_pattern,
+};
+
+static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
+ NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
+ NAND_BBT_NO_OOB,
+ .len = 4,
+ .veroffs = 4,
+ .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
+ .pattern = mirror_pattern,
+};
+
/*
* We may change the layout if we can get the ECC info from the datasheet,
* else we will use all the (page + OOB).
@@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
if (of_property_read_bool(this->dev->of_node,
- "fsl,no-blockmark-swap"))
+ "fsl,no-blockmark-swap")) {
this->swap_block_mark = false;
+ chip->bbt_td = &bbt_main_no_oob_descr;
+ chip->bbt_md = &bbt_mirror_no_oob_descr;
+ }
}
dev_dbg(this->dev, "Blockmark swapping %sabled\n",
this->swap_block_mark ? "en" : "dis");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
2014-06-12 13:20 ` Lothar Waßmann
(?)
@ 2014-07-28 5:29 ` Brian Norris
-1 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-28 5:29 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang
Hi Lothar,
On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Waßmann wrote:
> Without blockmark swapping, there is no use in creating a BBT from
> scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> case.
I'm curious: what is your plan if there is no BBT available on your
device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
you have no bad blocks, and it will never write a bad block table to
flash. This also means no subsequent discoverable bad blocks can be
recorded across power cycles, I believe.
Maybe you don't want to specify your own nand_bbt_descr's at all, but
you just need to set:
chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
(Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
some are targeted for the nand_chip::bbt_options field, and others
belong in struct nand_bbt_descr::options.)
But if for some reason we need to keep this patch, a comment below:
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 37537b4..fc710d7 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> .pattern = scan_ff_pattern
> };
>
> +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> +
> +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Please indent the above two lines a bit, preferably matching the
indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
continuation of the '.options' initialization.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = bbt_pattern,
> +};
> +
> +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Same here.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = mirror_pattern,
> +};
> +
> /*
> * We may change the layout if we can get the ECC info from the datasheet,
> * else we will use all the (page + OOB).
> @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
>
> if (of_property_read_bool(this->dev->of_node,
> - "fsl,no-blockmark-swap"))
> + "fsl,no-blockmark-swap")) {
> this->swap_block_mark = false;
> + chip->bbt_td = &bbt_main_no_oob_descr;
> + chip->bbt_md = &bbt_mirror_no_oob_descr;
My initial recommendation for this patch and the previous patch means
that you could just drop both patches and replace them with the
following:
/* Comment here to explain why... */
chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
NAND_BBT_NO_OOB |
NAND_BBT_NO_OOB_BBM;
> + }
> }
> dev_dbg(this->dev, "Blockmark swapping %sabled\n",
> this->swap_block_mark ? "en" : "dis");
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-07-28 5:29 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-28 5:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lothar,
On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Wa?mann wrote:
> Without blockmark swapping, there is no use in creating a BBT from
> scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> case.
I'm curious: what is your plan if there is no BBT available on your
device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
you have no bad blocks, and it will never write a bad block table to
flash. This also means no subsequent discoverable bad blocks can be
recorded across power cycles, I believe.
Maybe you don't want to specify your own nand_bbt_descr's at all, but
you just need to set:
chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
(Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
some are targeted for the nand_chip::bbt_options field, and others
belong in struct nand_bbt_descr::options.)
But if for some reason we need to keep this patch, a comment below:
> Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> ---
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 37537b4..fc710d7 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> .pattern = scan_ff_pattern
> };
>
> +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> +
> +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Please indent the above two lines a bit, preferably matching the
indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
continuation of the '.options' initialization.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = bbt_pattern,
> +};
> +
> +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Same here.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = mirror_pattern,
> +};
> +
> /*
> * We may change the layout if we can get the ECC info from the datasheet,
> * else we will use all the (page + OOB).
> @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
>
> if (of_property_read_bool(this->dev->of_node,
> - "fsl,no-blockmark-swap"))
> + "fsl,no-blockmark-swap")) {
> this->swap_block_mark = false;
> + chip->bbt_td = &bbt_main_no_oob_descr;
> + chip->bbt_md = &bbt_mirror_no_oob_descr;
My initial recommendation for this patch and the previous patch means
that you could just drop both patches and replace them with the
following:
/* Comment here to explain why... */
chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
NAND_BBT_NO_OOB |
NAND_BBT_NO_OOB_BBM;
> + }
> }
> dev_dbg(this->dev, "Blockmark swapping %sabled\n",
> this->swap_block_mark ? "en" : "dis");
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-07-28 5:29 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-28 5:29 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang Shijie, Rob Landley,
Kumar Gala, Shawn Guo, David Woodhouse
Hi Lothar,
On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Waßmann wrote:
> Without blockmark swapping, there is no use in creating a BBT from
> scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> case.
I'm curious: what is your plan if there is no BBT available on your
device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
you have no bad blocks, and it will never write a bad block table to
flash. This also means no subsequent discoverable bad blocks can be
recorded across power cycles, I believe.
Maybe you don't want to specify your own nand_bbt_descr's at all, but
you just need to set:
chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
(Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
some are targeted for the nand_chip::bbt_options field, and others
belong in struct nand_bbt_descr::options.)
But if for some reason we need to keep this patch, a comment below:
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 37537b4..fc710d7 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> .pattern = scan_ff_pattern
> };
>
> +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> +
> +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Please indent the above two lines a bit, preferably matching the
indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
continuation of the '.options' initialization.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = bbt_pattern,
> +};
> +
> +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> + NAND_BBT_NO_OOB,
Same here.
> + .len = 4,
> + .veroffs = 4,
> + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> + .pattern = mirror_pattern,
> +};
> +
> /*
> * We may change the layout if we can get the ECC info from the datasheet,
> * else we will use all the (page + OOB).
> @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
>
> if (of_property_read_bool(this->dev->of_node,
> - "fsl,no-blockmark-swap"))
> + "fsl,no-blockmark-swap")) {
> this->swap_block_mark = false;
> + chip->bbt_td = &bbt_main_no_oob_descr;
> + chip->bbt_md = &bbt_mirror_no_oob_descr;
My initial recommendation for this patch and the previous patch means
that you could just drop both patches and replace them with the
following:
/* Comment here to explain why... */
chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
NAND_BBT_NO_OOB |
NAND_BBT_NO_OOB_BBM;
> + }
> }
> dev_dbg(this->dev, "Blockmark swapping %sabled\n",
> this->swap_block_mark ? "en" : "dis");
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
2014-07-28 5:29 ` Brian Norris
(?)
@ 2014-07-29 6:31 ` Lothar Waßmann
-1 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-29 6:31 UTC (permalink / raw)
To: Brian Norris
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang
Hi,
Brian Norris wrote:
> Hi Lothar,
>
> On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Waßmann wrote:
> > Without blockmark swapping, there is no use in creating a BBT from
> > scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> > case.
>
> I'm curious: what is your plan if there is no BBT available on your
> device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
> you have no bad blocks, and it will never write a bad block table to
> flash. This also means no subsequent discoverable bad blocks can be
> recorded across power cycles, I believe.
>
That won't happen (unless it's not possible to create a BBT because all
the possible blocks for the BBT are bad), because the bootloader will
have created one before Linux is started.
> Maybe you don't want to specify your own nand_bbt_descr's at all, but
> you just need to set:
>
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
>
> (Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
> some are targeted for the nand_chip::bbt_options field, and others
> belong in struct nand_bbt_descr::options.)
>
> But if for some reason we need to keep this patch, a comment below:
>
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > index 37537b4..fc710d7 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> > .pattern = scan_ff_pattern
> > };
> >
> > +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> > +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> > +
> > +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Please indent the above two lines a bit, preferably matching the
> indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
> continuation of the '.options' initialization.
>
OK.
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = bbt_pattern,
> > +};
> > +
> > +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Same here.
>
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = mirror_pattern,
> > +};
> > +
> > /*
> > * We may change the layout if we can get the ECC info from the datasheet,
> > * else we will use all the (page + OOB).
> > @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> > chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> >
> > if (of_property_read_bool(this->dev->of_node,
> > - "fsl,no-blockmark-swap"))
> > + "fsl,no-blockmark-swap")) {
> > this->swap_block_mark = false;
> > + chip->bbt_td = &bbt_main_no_oob_descr;
> > + chip->bbt_md = &bbt_mirror_no_oob_descr;
>
> My initial recommendation for this patch and the previous patch means
> that you could just drop both patches and replace them with the
> following:
>
> /* Comment here to explain why... */
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
> NAND_BBT_NO_OOB |
> NAND_BBT_NO_OOB_BBM;
OK.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-07-29 6:31 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-29 6:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Brian Norris wrote:
> Hi Lothar,
>
> On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Wa?mann wrote:
> > Without blockmark swapping, there is no use in creating a BBT from
> > scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> > case.
>
> I'm curious: what is your plan if there is no BBT available on your
> device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
> you have no bad blocks, and it will never write a bad block table to
> flash. This also means no subsequent discoverable bad blocks can be
> recorded across power cycles, I believe.
>
That won't happen (unless it's not possible to create a BBT because all
the possible blocks for the BBT are bad), because the bootloader will
have created one before Linux is started.
> Maybe you don't want to specify your own nand_bbt_descr's at all, but
> you just need to set:
>
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
>
> (Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
> some are targeted for the nand_chip::bbt_options field, and others
> belong in struct nand_bbt_descr::options.)
>
> But if for some reason we need to keep this patch, a comment below:
>
> > Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> > ---
> > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > index 37537b4..fc710d7 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> > .pattern = scan_ff_pattern
> > };
> >
> > +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> > +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> > +
> > +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Please indent the above two lines a bit, preferably matching the
> indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
> continuation of the '.options' initialization.
>
OK.
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = bbt_pattern,
> > +};
> > +
> > +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Same here.
>
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = mirror_pattern,
> > +};
> > +
> > /*
> > * We may change the layout if we can get the ECC info from the datasheet,
> > * else we will use all the (page + OOB).
> > @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> > chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> >
> > if (of_property_read_bool(this->dev->of_node,
> > - "fsl,no-blockmark-swap"))
> > + "fsl,no-blockmark-swap")) {
> > this->swap_block_mark = false;
> > + chip->bbt_td = &bbt_main_no_oob_descr;
> > + chip->bbt_md = &bbt_mirror_no_oob_descr;
>
> My initial recommendation for this patch and the previous patch means
> that you could just drop both patches and replace them with the
> following:
>
> /* Comment here to explain why... */
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
> NAND_BBT_NO_OOB |
> NAND_BBT_NO_OOB_BBM;
OK.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCHv4 5/5] mtd: gpmi: prevent creating a new BBT when blockmark swapping is disabled
@ 2014-07-29 6:31 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-29 6:31 UTC (permalink / raw)
To: Brian Norris
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang Shijie, Rob Landley,
Kumar Gala, Shawn Guo, David Woodhouse
Hi,
Brian Norris wrote:
> Hi Lothar,
>
> On Thu, Jun 12, 2014 at 03:20:45PM +0200, Lothar Waßmann wrote:
> > Without blockmark swapping, there is no use in creating a BBT from
> > scratch, so use a BBT descriptor with NAND_BBT_CREATE unset in this
> > case.
>
> I'm curious: what is your plan if there is no BBT available on your
> device, or if it ever gets corrupted? IIUC, nand_bbt will just assume
> you have no bad blocks, and it will never write a bad block table to
> flash. This also means no subsequent discoverable bad blocks can be
> recorded across power cycles, I believe.
>
That won't happen (unless it's not possible to create a BBT because all
the possible blocks for the BBT are bad), because the bootloader will
have created one before Linux is started.
> Maybe you don't want to specify your own nand_bbt_descr's at all, but
> you just need to set:
>
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY | NAND_BBT_NO_OOB;
>
> (Note: there's a little bit of fuzziness about NAND_BBT_* flags, where
> some are targeted for the nand_chip::bbt_options field, and others
> belong in struct nand_bbt_descr::options.)
>
> But if for some reason we need to keep this patch, a comment below:
>
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > index 37537b4..fc710d7 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -43,6 +43,29 @@ static struct nand_bbt_descr gpmi_bbt_descr = {
> > .pattern = scan_ff_pattern
> > };
> >
> > +static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
> > +static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
> > +
> > +static struct nand_bbt_descr bbt_main_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Please indent the above two lines a bit, preferably matching the
> indentation of NAND_BBT_LASTBLOCK. It should be clear that this is a
> continuation of the '.options' initialization.
>
OK.
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = bbt_pattern,
> > +};
> > +
> > +static struct nand_bbt_descr bbt_mirror_no_oob_descr = {
> > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_WRITE |
> > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP |
> > + NAND_BBT_NO_OOB,
>
> Same here.
>
> > + .len = 4,
> > + .veroffs = 4,
> > + .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > + .pattern = mirror_pattern,
> > +};
> > +
> > /*
> > * We may change the layout if we can get the ECC info from the datasheet,
> > * else we will use all the (page + OOB).
> > @@ -1728,8 +1751,11 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
> > chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> >
> > if (of_property_read_bool(this->dev->of_node,
> > - "fsl,no-blockmark-swap"))
> > + "fsl,no-blockmark-swap")) {
> > this->swap_block_mark = false;
> > + chip->bbt_td = &bbt_main_no_oob_descr;
> > + chip->bbt_md = &bbt_mirror_no_oob_descr;
>
> My initial recommendation for this patch and the previous patch means
> that you could just drop both patches and replace them with the
> following:
>
> /* Comment here to explain why... */
> chip->bbt_options |= NAND_BBT_CREATE_EMPTY |
> NAND_BBT_NO_OOB |
> NAND_BBT_NO_OOB_BBM;
OK.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
2014-06-12 13:20 ` Lothar Waßmann
(?)
@ 2014-07-24 2:06 ` Brian Norris
-1 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 2:06 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang
(BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
are interested in this series; e.g., Russel and the ARM list seem
unrelated)
Hi Lothar,
Sorry for the delay on this. I get busy enough that I can't/don't reply
to everything quickly...
On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> add a boolean property 'nand-no-oob-bbm' and helper function to be
> able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> and use it for i.MX and MXS nand drivers.
If I'm understanding your previous conversations with Huang correctly,
you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
fsl,no-blockmark-swap option. Correct? If so, then you might not need
a separate 'nand-no-oob-bbm' binding; your driver should imply from
'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
Also, as I noted in [1], I don't really like exposing a ton of
individual boolean DT properties like this. (At least this property is
orthogonal to the bad block table; I was a little off-base in [1].)
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2014-July/054764.html
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 1 +
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
> drivers/mtd/nand/mxc_nand.c | 2 ++
> drivers/of/of_mtd.c | 12 ++++++++++++
> include/linux/of_mtd.h | 6 ++++++
> 5 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..e46bfbe 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -5,6 +5,7 @@
> "soft_bch".
> - nand-bus-width : 8 or 16 bus width if not present 8
> - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
> +- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
>
> - nand-ecc-strength: integer representing the number of bits to correct
> per ECC step.
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 959cb9b..37537b4 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1724,6 +1724,9 @@ 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;
>
> + if (of_get_nand_no_oob_bbm(this->dev->of_node))
> + chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> +
> if (of_property_read_bool(this->dev->of_node,
> "fsl,no-blockmark-swap"))
> this->swap_block_mark = false;
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index dba262b..bb54a2a 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
> this->bbt_md = &bbt_mirror_descr;
> /* update flash based bbt */
> this->bbt_options |= NAND_BBT_USE_FLASH;
> + if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
> + this->bbt_options |= NAND_BBT_NO_OOB_BBM;
> }
>
> init_completion(&host->op_completion);
> diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..d947acc 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
> return of_property_read_bool(np, "nand-on-flash-bbt");
> }
> EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
> +
> +/**
> + * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
> + * @np: Pointer to the given device_node
> + *
> + * return true if present, false otherwise
> + */
> +bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return of_property_read_bool(np, "nand-no-oob-bbm");
> +}
> +EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
> diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
> index e266caa..6ece1a9 100644
> --- a/include/linux/of_mtd.h
> +++ b/include/linux/of_mtd.h
> @@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
> int of_get_nand_ecc_strength(struct device_node *np);
> int of_get_nand_bus_width(struct device_node *np);
> bool of_get_nand_on_flash_bbt(struct device_node *np);
> +bool of_get_nand_no_oob_bbm(struct device_node *np);
>
> #else /* CONFIG_OF_MTD */
>
> @@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
> return false;
> }
>
> +static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return false;
> +}
> +
> #endif /* CONFIG_OF_MTD */
>
> #endif /* __LINUX_OF_MTD_H */
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 2:06 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 2:06 UTC (permalink / raw)
To: linux-arm-kernel
(BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
are interested in this series; e.g., Russel and the ARM list seem
unrelated)
Hi Lothar,
Sorry for the delay on this. I get busy enough that I can't/don't reply
to everything quickly...
On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Wa?mann wrote:
> add a boolean property 'nand-no-oob-bbm' and helper function to be
> able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> and use it for i.MX and MXS nand drivers.
If I'm understanding your previous conversations with Huang correctly,
you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
fsl,no-blockmark-swap option. Correct? If so, then you might not need
a separate 'nand-no-oob-bbm' binding; your driver should imply from
'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
Also, as I noted in [1], I don't really like exposing a ton of
individual boolean DT properties like this. (At least this property is
orthogonal to the bad block table; I was a little off-base in [1].)
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2014-July/054764.html
> Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 1 +
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
> drivers/mtd/nand/mxc_nand.c | 2 ++
> drivers/of/of_mtd.c | 12 ++++++++++++
> include/linux/of_mtd.h | 6 ++++++
> 5 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..e46bfbe 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -5,6 +5,7 @@
> "soft_bch".
> - nand-bus-width : 8 or 16 bus width if not present 8
> - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
> +- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
>
> - nand-ecc-strength: integer representing the number of bits to correct
> per ECC step.
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 959cb9b..37537b4 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1724,6 +1724,9 @@ 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;
>
> + if (of_get_nand_no_oob_bbm(this->dev->of_node))
> + chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> +
> if (of_property_read_bool(this->dev->of_node,
> "fsl,no-blockmark-swap"))
> this->swap_block_mark = false;
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index dba262b..bb54a2a 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
> this->bbt_md = &bbt_mirror_descr;
> /* update flash based bbt */
> this->bbt_options |= NAND_BBT_USE_FLASH;
> + if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
> + this->bbt_options |= NAND_BBT_NO_OOB_BBM;
> }
>
> init_completion(&host->op_completion);
> diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..d947acc 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
> return of_property_read_bool(np, "nand-on-flash-bbt");
> }
> EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
> +
> +/**
> + * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
> + * @np: Pointer to the given device_node
> + *
> + * return true if present, false otherwise
> + */
> +bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return of_property_read_bool(np, "nand-no-oob-bbm");
> +}
> +EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
> diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
> index e266caa..6ece1a9 100644
> --- a/include/linux/of_mtd.h
> +++ b/include/linux/of_mtd.h
> @@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
> int of_get_nand_ecc_strength(struct device_node *np);
> int of_get_nand_bus_width(struct device_node *np);
> bool of_get_nand_on_flash_bbt(struct device_node *np);
> +bool of_get_nand_no_oob_bbm(struct device_node *np);
>
> #else /* CONFIG_OF_MTD */
>
> @@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
> return false;
> }
>
> +static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return false;
> +}
> +
> #endif /* CONFIG_OF_MTD */
>
> #endif /* __LINUX_OF_MTD_H */
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 2:06 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 2:06 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang Shijie, Rob Landley,
Kumar Gala, Shawn Guo, David Woodhouse
(BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
are interested in this series; e.g., Russel and the ARM list seem
unrelated)
Hi Lothar,
Sorry for the delay on this. I get busy enough that I can't/don't reply
to everything quickly...
On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> add a boolean property 'nand-no-oob-bbm' and helper function to be
> able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> and use it for i.MX and MXS nand drivers.
If I'm understanding your previous conversations with Huang correctly,
you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
fsl,no-blockmark-swap option. Correct? If so, then you might not need
a separate 'nand-no-oob-bbm' binding; your driver should imply from
'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
Also, as I noted in [1], I don't really like exposing a ton of
individual boolean DT properties like this. (At least this property is
orthogonal to the bad block table; I was a little off-base in [1].)
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2014-July/054764.html
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 1 +
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 3 +++
> drivers/mtd/nand/mxc_nand.c | 2 ++
> drivers/of/of_mtd.c | 12 ++++++++++++
> include/linux/of_mtd.h | 6 ++++++
> 5 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..e46bfbe 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -5,6 +5,7 @@
> "soft_bch".
> - nand-bus-width : 8 or 16 bus width if not present 8
> - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
> +- nand-no-oob-bbm: boolean to disable writing bad block markers to flash
>
> - nand-ecc-strength: integer representing the number of bits to correct
> per ECC step.
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 959cb9b..37537b4 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1724,6 +1724,9 @@ 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;
>
> + if (of_get_nand_no_oob_bbm(this->dev->of_node))
> + chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
> +
> if (of_property_read_bool(this->dev->of_node,
> "fsl,no-blockmark-swap"))
> this->swap_block_mark = false;
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index dba262b..bb54a2a 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -1496,6 +1496,8 @@ static int mxcnd_probe(struct platform_device *pdev)
> this->bbt_md = &bbt_mirror_descr;
> /* update flash based bbt */
> this->bbt_options |= NAND_BBT_USE_FLASH;
> + if (of_get_nand_no_oob_bbm(pdev->dev.of_node))
> + this->bbt_options |= NAND_BBT_NO_OOB_BBM;
> }
>
> init_completion(&host->op_completion);
> diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..d947acc 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -117,3 +117,15 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
> return of_property_read_bool(np, "nand-on-flash-bbt");
> }
> EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
> +
> +/**
> + * of_get_nand_no_oob_bbm - Get nand no oob bbm for given device_node
> + * @np: Pointer to the given device_node
> + *
> + * return true if present, false otherwise
> + */
> +bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return of_property_read_bool(np, "nand-no-oob-bbm");
> +}
> +EXPORT_SYMBOL_GPL(of_get_nand_no_oob_bbm);
> diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
> index e266caa..6ece1a9 100644
> --- a/include/linux/of_mtd.h
> +++ b/include/linux/of_mtd.h
> @@ -17,6 +17,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
> int of_get_nand_ecc_strength(struct device_node *np);
> int of_get_nand_bus_width(struct device_node *np);
> bool of_get_nand_on_flash_bbt(struct device_node *np);
> +bool of_get_nand_no_oob_bbm(struct device_node *np);
>
> #else /* CONFIG_OF_MTD */
>
> @@ -45,6 +46,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)
> return false;
> }
>
> +static inline bool of_get_nand_no_oob_bbm(struct device_node *np)
> +{
> + return false;
> +}
> +
> #endif /* CONFIG_OF_MTD */
>
> #endif /* __LINUX_OF_MTD_H */
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
2014-07-24 2:06 ` Brian Norris
(?)
@ 2014-07-24 6:49 ` Lothar Waßmann
-1 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-24 6:49 UTC (permalink / raw)
To: Brian Norris
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang
Hi,
Brian Norris wrote:
> (BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
> are interested in this series; e.g., Russel and the ARM list seem
> unrelated)
>
> Hi Lothar,
>
> Sorry for the delay on this. I get busy enough that I can't/don't reply
> to everything quickly...
>
> On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > and use it for i.MX and MXS nand drivers.
>
> If I'm understanding your previous conversations with Huang correctly,
> you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> fsl,no-blockmark-swap option. Correct? If so, then you might not need
> a separate 'nand-no-oob-bbm' binding; your driver should imply from
> 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
>
no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
independent from no-blockmark-swap.
IMO writing a bad block marker to flash (which is prevented by
the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
the BB mark in the first place. The manufacturer guarantees that blocks
which are initially bad will have at least one zero bit in the position
of the BB mark. That's all to it.
There is no guarantee, that you will even be able to write any
deterministic data to a block that has turned bad due to wearout or
other flash defects. It is rather bogus to rely on data written to a
known bad block to reflect the state of the block.
> Also, as I noted in [1], I don't really like exposing a ton of
> individual boolean DT properties like this. (At least this property is
> orthogonal to the bad block table; I was a little off-base in [1].)
>
How else should this information be conveyed to the flash drivers?
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 6:49 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-24 6:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Brian Norris wrote:
> (BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
> are interested in this series; e.g., Russel and the ARM list seem
> unrelated)
>
> Hi Lothar,
>
> Sorry for the delay on this. I get busy enough that I can't/don't reply
> to everything quickly...
>
> On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Wa?mann wrote:
> > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > and use it for i.MX and MXS nand drivers.
>
> If I'm understanding your previous conversations with Huang correctly,
> you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> fsl,no-blockmark-swap option. Correct? If so, then you might not need
> a separate 'nand-no-oob-bbm' binding; your driver should imply from
> 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
>
no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
independent from no-blockmark-swap.
IMO writing a bad block marker to flash (which is prevented by
the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
the BB mark in the first place. The manufacturer guarantees that blocks
which are initially bad will have at least one zero bit in the position
of the BB mark. That's all to it.
There is no guarantee, that you will even be able to write any
deterministic data to a block that has turned bad due to wearout or
other flash defects. It is rather bogus to rely on data written to a
known bad block to reflect the state of the block.
> Also, as I noted in [1], I don't really like exposing a ton of
> individual boolean DT properties like this. (At least this property is
> orthogonal to the bad block table; I was a little off-base in [1].)
>
How else should this information be conveyed to the flash drivers?
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 6:49 ` Lothar Waßmann
0 siblings, 0 replies; 39+ messages in thread
From: Lothar Waßmann @ 2014-07-24 6:49 UTC (permalink / raw)
To: Brian Norris
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang Shijie, Rob Landley,
Kumar Gala, Shawn Guo, David Woodhouse
Hi,
Brian Norris wrote:
> (BTW, that's a mighty CC list you have! I'm not sure all CC'd parties
> are interested in this series; e.g., Russel and the ARM list seem
> unrelated)
>
> Hi Lothar,
>
> Sorry for the delay on this. I get busy enough that I can't/don't reply
> to everything quickly...
>
> On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > and use it for i.MX and MXS nand drivers.
>
> If I'm understanding your previous conversations with Huang correctly,
> you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> fsl,no-blockmark-swap option. Correct? If so, then you might not need
> a separate 'nand-no-oob-bbm' binding; your driver should imply from
> 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
>
no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
independent from no-blockmark-swap.
IMO writing a bad block marker to flash (which is prevented by
the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
the BB mark in the first place. The manufacturer guarantees that blocks
which are initially bad will have at least one zero bit in the position
of the BB mark. That's all to it.
There is no guarantee, that you will even be able to write any
deterministic data to a block that has turned bad due to wearout or
other flash defects. It is rather bogus to rely on data written to a
known bad block to reflect the state of the block.
> Also, as I noted in [1], I don't really like exposing a ton of
> individual boolean DT properties like this. (At least this property is
> orthogonal to the bad block table; I was a little off-base in [1].)
>
How else should this information be conveyed to the flash drivers?
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
2014-07-24 6:49 ` Lothar Waßmann
(?)
@ 2014-07-24 7:47 ` Brian Norris
-1 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 7:47 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang
On Thu, Jul 24, 2014 at 08:49:15AM +0200, Lothar Waßmann wrote:
> Brian Norris wrote:
> > On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> > > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > > and use it for i.MX and MXS nand drivers.
> >
> > If I'm understanding your previous conversations with Huang correctly,
> > you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> > fsl,no-blockmark-swap option. Correct? If so, then you might not need
> > a separate 'nand-no-oob-bbm' binding; your driver should imply from
> > 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
> >
> no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
> independent from no-blockmark-swap.
Why would you want NO_OOB_BBM without no-blockmark-swap? If the block is
bad, why do you care what's written to it? (For that matter, why is it
ever important to use NO_OOB_BBM? At worst, the extra BB marks are
useless / written to the wrong place.)
> IMO writing a bad block marker to flash (which is prevented by
> the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
> the BB mark in the first place. The manufacturer guarantees that blocks
> which are initially bad will have at least one zero bit in the position
> of the BB mark. That's all to it.
Yes, it is a misinterpretation, and it's really irrelevant in many cases
whether or not the BB mark is written to each block's OOB. But it does
still provide some resilience in case the on-flash table ever gets
completely corrupted -- nand_bbt will rescan the flash for BB marks on
the next boot (and this will be totally broken--with or without
NO_OOB_BBM--for hardware like yours). Or to put it another way, it
supports some legacy scenarios without (AFAICT) any real negative
effects.
> There is no guarantee, that you will even be able to write any
> deterministic data to a block that has turned bad due to wearout or
> other flash defects.
Certainly. But that's not an argument against attempting.
> It is rather bogus to rely on data written to a
> known bad block to reflect the state of the block.
We don't "rely" on this. If the BBT (and its mirrors) never completely
fails, these marks are never used.
> > Also, as I noted in [1], I don't really like exposing a ton of
> > individual boolean DT properties like this. (At least this property is
> > orthogonal to the bad block table; I was a little off-base in [1].)
> >
> How else should this information be conveyed to the flash drivers?
I'm not convinced the NO_OOB_BBM DT property is actually necessary at
all.
I was more concerned about bad block *table* properties, where I see
that at least some users (e.g. ST Micro's BCH NAND driver) expect a
different BBT format than the default, and we might begin to see extra
boolean flags for random bits of differentiation. This is apparently
still just a theoretical concern, though.
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 7:47 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 7:47 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 24, 2014 at 08:49:15AM +0200, Lothar Wa?mann wrote:
> Brian Norris wrote:
> > On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Wa?mann wrote:
> > > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > > and use it for i.MX and MXS nand drivers.
> >
> > If I'm understanding your previous conversations with Huang correctly,
> > you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> > fsl,no-blockmark-swap option. Correct? If so, then you might not need
> > a separate 'nand-no-oob-bbm' binding; your driver should imply from
> > 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
> >
> no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
> independent from no-blockmark-swap.
Why would you want NO_OOB_BBM without no-blockmark-swap? If the block is
bad, why do you care what's written to it? (For that matter, why is it
ever important to use NO_OOB_BBM? At worst, the extra BB marks are
useless / written to the wrong place.)
> IMO writing a bad block marker to flash (which is prevented by
> the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
> the BB mark in the first place. The manufacturer guarantees that blocks
> which are initially bad will have at least one zero bit in the position
> of the BB mark. That's all to it.
Yes, it is a misinterpretation, and it's really irrelevant in many cases
whether or not the BB mark is written to each block's OOB. But it does
still provide some resilience in case the on-flash table ever gets
completely corrupted -- nand_bbt will rescan the flash for BB marks on
the next boot (and this will be totally broken--with or without
NO_OOB_BBM--for hardware like yours). Or to put it another way, it
supports some legacy scenarios without (AFAICT) any real negative
effects.
> There is no guarantee, that you will even be able to write any
> deterministic data to a block that has turned bad due to wearout or
> other flash defects.
Certainly. But that's not an argument against attempting.
> It is rather bogus to rely on data written to a
> known bad block to reflect the state of the block.
We don't "rely" on this. If the BBT (and its mirrors) never completely
fails, these marks are never used.
> > Also, as I noted in [1], I don't really like exposing a ton of
> > individual boolean DT properties like this. (At least this property is
> > orthogonal to the bad block table; I was a little off-base in [1].)
> >
> How else should this information be conveyed to the flash drivers?
I'm not convinced the NO_OOB_BBM DT property is actually necessary at
all.
I was more concerned about bad block *table* properties, where I see
that at least some users (e.g. ST Micro's BCH NAND driver) expect a
different BBT format than the default, and we might begin to see extra
boolean flags for random bits of differentiation. This is apparently
still just a theoretical concern, though.
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCHv4 4/5] of/mtd/nand: add generic binding and helper for NAND_BBT_NO_OOB_BBM
@ 2014-07-24 7:47 ` Brian Norris
0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2014-07-24 7:47 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Mark Rutland, Boris BREZILLON, linux-doc, Artem Bityutskiy,
Wei Yongjun, linux-mtd, Michael Grzeschik, Russell King,
Arnd Bergmann, Jingoo Han, Shawn Guo, Ezequiel Garcia,
Michael Opdenacker, Grant Likely, devicetree, Sascha Hauer,
Pawel Moll, Ian Campbell, Rob Herring, linux-arm-kernel,
Fabio Estevam, linux-kernel, Huang Shijie, Rob Landley,
Kumar Gala, Shawn Guo, David Woodhouse
On Thu, Jul 24, 2014 at 08:49:15AM +0200, Lothar Waßmann wrote:
> Brian Norris wrote:
> > On Thu, Jun 12, 2014 at 03:20:44PM +0200, Lothar Waßmann wrote:
> > > add a boolean property 'nand-no-oob-bbm' and helper function to be
> > > able to set the NAND_BBT_NO_OOB_BBM flag in DT capable NAND drivers
> > > and use it for i.MX and MXS nand drivers.
> >
> > If I'm understanding your previous conversations with Huang correctly,
> > you *must* use NAND_BBT_NO_OOB_BBM if you're going to use the
> > fsl,no-blockmark-swap option. Correct? If so, then you might not need
> > a separate 'nand-no-oob-bbm' binding; your driver should imply from
> > 'fsl,no-blockmark-swap' that it must also enable NAND_BBT_NO_OOB_BBM.
> >
> no-blockmark-swap implies NO_OOB_BBM but NO_OOB_BBM may also be used
> independent from no-blockmark-swap.
Why would you want NO_OOB_BBM without no-blockmark-swap? If the block is
bad, why do you care what's written to it? (For that matter, why is it
ever important to use NO_OOB_BBM? At worst, the extra BB marks are
useless / written to the wrong place.)
> IMO writing a bad block marker to flash (which is prevented by
> the NAND_BBT_NO_OOB_BBM flag) is a misinterpretation of the purpose of
> the BB mark in the first place. The manufacturer guarantees that blocks
> which are initially bad will have at least one zero bit in the position
> of the BB mark. That's all to it.
Yes, it is a misinterpretation, and it's really irrelevant in many cases
whether or not the BB mark is written to each block's OOB. But it does
still provide some resilience in case the on-flash table ever gets
completely corrupted -- nand_bbt will rescan the flash for BB marks on
the next boot (and this will be totally broken--with or without
NO_OOB_BBM--for hardware like yours). Or to put it another way, it
supports some legacy scenarios without (AFAICT) any real negative
effects.
> There is no guarantee, that you will even be able to write any
> deterministic data to a block that has turned bad due to wearout or
> other flash defects.
Certainly. But that's not an argument against attempting.
> It is rather bogus to rely on data written to a
> known bad block to reflect the state of the block.
We don't "rely" on this. If the BBT (and its mirrors) never completely
fails, these marks are never used.
> > Also, as I noted in [1], I don't really like exposing a ton of
> > individual boolean DT properties like this. (At least this property is
> > orthogonal to the bad block table; I was a little off-base in [1].)
> >
> How else should this information be conveyed to the flash drivers?
I'm not convinced the NO_OOB_BBM DT property is actually necessary at
all.
I was more concerned about bad block *table* properties, where I see
that at least some users (e.g. ST Micro's BCH NAND driver) expect a
different BBT format than the default, and we might begin to see extra
boolean flags for random bits of differentiation. This is apparently
still just a theoretical concern, though.
Brian
^ permalink raw reply [flat|nested] 39+ messages in thread