* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-10 8:55 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-10 8:55 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie
Cc: linux-kernel, linux-arm-kernel, Mike Voytovich, Roy Lee, Boris BREZILLON
Several MTD users (either in user or kernel space) expect a valid raw
access support to NAND chip devices.
This is particularly true for testing tools which are often touching the
data stored in a NAND chip in raw mode to artificially generate errors.
The GPMI drivers do not implemenent raw access functions, and thus rely on
default HW_ECC scheme implementation.
The default implementation consider the data and OOB area as properly
separated in their respective NAND section, which is not true for the GPMI
controller.
In this driver/controller some OOB data are stored at the beginning of the
NAND data area (these data are called metadata in the driver), then ECC
bytes are interleaved with data chunk (which is similar to the
HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
OOB data.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
Hello,
This patch is providing raw access support to the GPMI driver which is
particularly useful to run some tests on the NAND (the one coming in
mind is the mtd_nandbiterrs testsuite).
I know this rework might break several user space tools which are relying
on the default raw access implementation (I already experienced an issue
with the kobs-ng tool provided by freescale), but many other tools will
now work as expected.
Huang, Brian, let me know what you think of this approach ?
Best Regards,
Boris
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 119 ++++++++++++++++++++++++++++++++-
1 file changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..b26e032 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -831,7 +831,13 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)
* power of two and is much larger than four, which guarantees the
* auxiliary buffer will appear on a 32-bit boundary.
*/
- this->page_buffer_size = geo->payload_size + geo->auxiliary_size;
+ if (geo->payload_size + geo->auxiliary_size >
+ mtd->writesize + mtd->oobsize)
+ this->page_buffer_size =
+ geo->payload_size + geo->auxiliary_size;
+ else
+ this->page_buffer_size = mtd->writesize + mtd->oobsize;
+
this->page_buffer_virt = dma_alloc_coherent(dev, this->page_buffer_size,
&this->page_buffer_phys, GFP_DMA);
if (!this->page_buffer_virt)
@@ -1347,6 +1353,115 @@ gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
return status & NAND_STATUS_FAIL ? -EIO : 0;
}
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip, uint8_t *buf,
+ int oob_required, int page)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+ uint8_t *orig_buf = buf;
+
+ chip->read_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+
+ column += nfc_geo->metadata_size;
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->read_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ chip->read_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ column += eccbytes;
+ }
+
+ if (column < mtd->writesize + mtd->oobsize)
+ chip->read_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ block_mark_swapping(this, orig_buf, chip->oob_poi);
+
+ return 0;
+}
+
+static int gpmi_ecc_write_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ const uint8_t *buf,
+ int oob_required)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+
+ if (this->swap_block_mark) {
+ /*
+ * If control arrives here, we're doing block mark swapping.
+ * Since we can't modify the caller's buffers, we must copy them
+ * into our own.
+ */
+ memcpy(this->page_buffer_virt, buf, mtd->writesize);
+ if (oob_required)
+ memcpy(this->page_buffer_virt + mtd->writesize,
+ chip->oob_poi, mtd->oobsize);
+ else
+ memset(this->page_buffer_virt + mtd->writesize,
+ 0xff, mtd->oobsize);
+
+ /* Handle block mark swapping. */
+ block_mark_swapping(this, this->page_buffer_virt,
+ this->page_buffer_virt + mtd->writesize);
+
+ oob = this->page_buffer_virt + mtd->writesize;
+ buf = this->page_buffer_virt;
+ }
+
+ if (oob_required) {
+ chip->write_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+ } else {
+ /*
+ * Write the data byte in the OOB area if BB marker swapping
+ * is requested.
+ */
+ if (this->swap_block_mark)
+ chip->write_buf(mtd, oob, 1);
+
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column, -1);
+ }
+ column += nfc_geo->metadata_size;
+
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->write_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ if (oob_required) {
+ chip->write_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ } else {
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column + eccbytes, -1);
+ }
+ column += eccbytes;
+ }
+
+ if (oob_required && column < mtd->writesize + mtd->oobsize)
+ chip->write_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ return 0;
+}
+
static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
@@ -1664,6 +1779,8 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
ecc->write_page = gpmi_ecc_write_page;
ecc->read_oob = gpmi_ecc_read_oob;
ecc->write_oob = gpmi_ecc_write_oob;
+ ecc->read_page_raw = gpmi_ecc_read_page_raw;
+ ecc->write_page_raw = gpmi_ecc_write_page_raw;
ecc->mode = NAND_ECC_HW;
ecc->size = bch_geo->ecc_chunk_size;
ecc->strength = bch_geo->ecc_strength;
--
1.9.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-10 8:55 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-10 8:55 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie
Cc: Mike Voytovich, Boris BREZILLON, linux-kernel, linux-arm-kernel, Roy Lee
Several MTD users (either in user or kernel space) expect a valid raw
access support to NAND chip devices.
This is particularly true for testing tools which are often touching the
data stored in a NAND chip in raw mode to artificially generate errors.
The GPMI drivers do not implemenent raw access functions, and thus rely on
default HW_ECC scheme implementation.
The default implementation consider the data and OOB area as properly
separated in their respective NAND section, which is not true for the GPMI
controller.
In this driver/controller some OOB data are stored at the beginning of the
NAND data area (these data are called metadata in the driver), then ECC
bytes are interleaved with data chunk (which is similar to the
HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
OOB data.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
Hello,
This patch is providing raw access support to the GPMI driver which is
particularly useful to run some tests on the NAND (the one coming in
mind is the mtd_nandbiterrs testsuite).
I know this rework might break several user space tools which are relying
on the default raw access implementation (I already experienced an issue
with the kobs-ng tool provided by freescale), but many other tools will
now work as expected.
Huang, Brian, let me know what you think of this approach ?
Best Regards,
Boris
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 119 ++++++++++++++++++++++++++++++++-
1 file changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..b26e032 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -831,7 +831,13 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)
* power of two and is much larger than four, which guarantees the
* auxiliary buffer will appear on a 32-bit boundary.
*/
- this->page_buffer_size = geo->payload_size + geo->auxiliary_size;
+ if (geo->payload_size + geo->auxiliary_size >
+ mtd->writesize + mtd->oobsize)
+ this->page_buffer_size =
+ geo->payload_size + geo->auxiliary_size;
+ else
+ this->page_buffer_size = mtd->writesize + mtd->oobsize;
+
this->page_buffer_virt = dma_alloc_coherent(dev, this->page_buffer_size,
&this->page_buffer_phys, GFP_DMA);
if (!this->page_buffer_virt)
@@ -1347,6 +1353,115 @@ gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
return status & NAND_STATUS_FAIL ? -EIO : 0;
}
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip, uint8_t *buf,
+ int oob_required, int page)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+ uint8_t *orig_buf = buf;
+
+ chip->read_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+
+ column += nfc_geo->metadata_size;
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->read_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ chip->read_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ column += eccbytes;
+ }
+
+ if (column < mtd->writesize + mtd->oobsize)
+ chip->read_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ block_mark_swapping(this, orig_buf, chip->oob_poi);
+
+ return 0;
+}
+
+static int gpmi_ecc_write_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ const uint8_t *buf,
+ int oob_required)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+
+ if (this->swap_block_mark) {
+ /*
+ * If control arrives here, we're doing block mark swapping.
+ * Since we can't modify the caller's buffers, we must copy them
+ * into our own.
+ */
+ memcpy(this->page_buffer_virt, buf, mtd->writesize);
+ if (oob_required)
+ memcpy(this->page_buffer_virt + mtd->writesize,
+ chip->oob_poi, mtd->oobsize);
+ else
+ memset(this->page_buffer_virt + mtd->writesize,
+ 0xff, mtd->oobsize);
+
+ /* Handle block mark swapping. */
+ block_mark_swapping(this, this->page_buffer_virt,
+ this->page_buffer_virt + mtd->writesize);
+
+ oob = this->page_buffer_virt + mtd->writesize;
+ buf = this->page_buffer_virt;
+ }
+
+ if (oob_required) {
+ chip->write_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+ } else {
+ /*
+ * Write the data byte in the OOB area if BB marker swapping
+ * is requested.
+ */
+ if (this->swap_block_mark)
+ chip->write_buf(mtd, oob, 1);
+
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column, -1);
+ }
+ column += nfc_geo->metadata_size;
+
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->write_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ if (oob_required) {
+ chip->write_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ } else {
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column + eccbytes, -1);
+ }
+ column += eccbytes;
+ }
+
+ if (oob_required && column < mtd->writesize + mtd->oobsize)
+ chip->write_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ return 0;
+}
+
static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
@@ -1664,6 +1779,8 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
ecc->write_page = gpmi_ecc_write_page;
ecc->read_oob = gpmi_ecc_read_oob;
ecc->write_oob = gpmi_ecc_write_oob;
+ ecc->read_page_raw = gpmi_ecc_read_page_raw;
+ ecc->write_page_raw = gpmi_ecc_write_page_raw;
ecc->mode = NAND_ECC_HW;
ecc->size = bch_geo->ecc_chunk_size;
ecc->strength = bch_geo->ecc_strength;
--
1.9.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-10 8:55 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-10 8:55 UTC (permalink / raw)
To: linux-arm-kernel
Several MTD users (either in user or kernel space) expect a valid raw
access support to NAND chip devices.
This is particularly true for testing tools which are often touching the
data stored in a NAND chip in raw mode to artificially generate errors.
The GPMI drivers do not implemenent raw access functions, and thus rely on
default HW_ECC scheme implementation.
The default implementation consider the data and OOB area as properly
separated in their respective NAND section, which is not true for the GPMI
controller.
In this driver/controller some OOB data are stored at the beginning of the
NAND data area (these data are called metadata in the driver), then ECC
bytes are interleaved with data chunk (which is similar to the
HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
OOB data.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
Hello,
This patch is providing raw access support to the GPMI driver which is
particularly useful to run some tests on the NAND (the one coming in
mind is the mtd_nandbiterrs testsuite).
I know this rework might break several user space tools which are relying
on the default raw access implementation (I already experienced an issue
with the kobs-ng tool provided by freescale), but many other tools will
now work as expected.
Huang, Brian, let me know what you think of this approach ?
Best Regards,
Boris
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 119 ++++++++++++++++++++++++++++++++-
1 file changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 959cb9b..b26e032 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -831,7 +831,13 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)
* power of two and is much larger than four, which guarantees the
* auxiliary buffer will appear on a 32-bit boundary.
*/
- this->page_buffer_size = geo->payload_size + geo->auxiliary_size;
+ if (geo->payload_size + geo->auxiliary_size >
+ mtd->writesize + mtd->oobsize)
+ this->page_buffer_size =
+ geo->payload_size + geo->auxiliary_size;
+ else
+ this->page_buffer_size = mtd->writesize + mtd->oobsize;
+
this->page_buffer_virt = dma_alloc_coherent(dev, this->page_buffer_size,
&this->page_buffer_phys, GFP_DMA);
if (!this->page_buffer_virt)
@@ -1347,6 +1353,115 @@ gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
return status & NAND_STATUS_FAIL ? -EIO : 0;
}
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip, uint8_t *buf,
+ int oob_required, int page)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+ uint8_t *orig_buf = buf;
+
+ chip->read_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+
+ column += nfc_geo->metadata_size;
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->read_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ chip->read_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ column += eccbytes;
+ }
+
+ if (column < mtd->writesize + mtd->oobsize)
+ chip->read_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ block_mark_swapping(this, orig_buf, chip->oob_poi);
+
+ return 0;
+}
+
+static int gpmi_ecc_write_page_raw(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ const uint8_t *buf,
+ int oob_required)
+{
+ struct gpmi_nand_data *this = chip->priv;
+ struct bch_geometry *nfc_geo = &this->bch_geometry;
+ int eccsize = nfc_geo->ecc_chunk_size;
+ int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
+ 8);
+ uint8_t *oob = chip->oob_poi;
+ int step;
+ int column = 0;
+
+ if (this->swap_block_mark) {
+ /*
+ * If control arrives here, we're doing block mark swapping.
+ * Since we can't modify the caller's buffers, we must copy them
+ * into our own.
+ */
+ memcpy(this->page_buffer_virt, buf, mtd->writesize);
+ if (oob_required)
+ memcpy(this->page_buffer_virt + mtd->writesize,
+ chip->oob_poi, mtd->oobsize);
+ else
+ memset(this->page_buffer_virt + mtd->writesize,
+ 0xff, mtd->oobsize);
+
+ /* Handle block mark swapping. */
+ block_mark_swapping(this, this->page_buffer_virt,
+ this->page_buffer_virt + mtd->writesize);
+
+ oob = this->page_buffer_virt + mtd->writesize;
+ buf = this->page_buffer_virt;
+ }
+
+ if (oob_required) {
+ chip->write_buf(mtd, oob, nfc_geo->metadata_size);
+ oob += nfc_geo->metadata_size;
+ } else {
+ /*
+ * Write the data byte in the OOB area if BB marker swapping
+ * is requested.
+ */
+ if (this->swap_block_mark)
+ chip->write_buf(mtd, oob, 1);
+
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column, -1);
+ }
+ column += nfc_geo->metadata_size;
+
+ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
+ chip->write_buf(mtd, buf, eccsize);
+ buf += eccsize;
+ column += eccsize;
+ if (oob_required) {
+ chip->write_buf(mtd, oob, eccbytes);
+ oob += eccbytes;
+ } else {
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN,
+ column + eccbytes, -1);
+ }
+ column += eccbytes;
+ }
+
+ if (oob_required && column < mtd->writesize + mtd->oobsize)
+ chip->write_buf(mtd, oob,
+ mtd->writesize + mtd->oobsize - column);
+
+ return 0;
+}
+
static int gpmi_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd->priv;
@@ -1664,6 +1779,8 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
ecc->write_page = gpmi_ecc_write_page;
ecc->read_oob = gpmi_ecc_read_oob;
ecc->write_oob = gpmi_ecc_write_oob;
+ ecc->read_page_raw = gpmi_ecc_read_page_raw;
+ ecc->write_page_raw = gpmi_ecc_write_page_raw;
ecc->mode = NAND_ECC_HW;
ecc->size = bch_geo->ecc_chunk_size;
ecc->strength = bch_geo->ecc_strength;
--
1.9.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-10 8:55 ` Boris BREZILLON
(?)
@ 2014-09-11 12:09 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 12:09 UTC (permalink / raw)
To: Boris BREZILLON
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> Several MTD users (either in user or kernel space) expect a valid raw
> access support to NAND chip devices.
> This is particularly true for testing tools which are often touching the
> data stored in a NAND chip in raw mode to artificially generate errors.
>
> The GPMI drivers do not implemenent raw access functions, and thus rely on
> default HW_ECC scheme implementation.
> The default implementation consider the data and OOB area as properly
> separated in their respective NAND section, which is not true for the GPMI
> controller.
> In this driver/controller some OOB data are stored at the beginning of the
> NAND data area (these data are called metadata in the driver), then ECC
> bytes are interleaved with data chunk (which is similar to the
> HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> OOB data.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> Hello,
>
> This patch is providing raw access support to the GPMI driver which is
> particularly useful to run some tests on the NAND (the one coming in
> mind is the mtd_nandbiterrs testsuite).
>
> I know this rework might break several user space tools which are relying
> on the default raw access implementation (I already experienced an issue
> with the kobs-ng tool provided by freescale), but many other tools will
> now work as expected.
If the kobs-ng can not works, there is no meaning that other tools
works. So I do not think we need to implement these hooks.
sorry, I will not Ack this patch.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 12:09 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 12:09 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> Several MTD users (either in user or kernel space) expect a valid raw
> access support to NAND chip devices.
> This is particularly true for testing tools which are often touching the
> data stored in a NAND chip in raw mode to artificially generate errors.
>
> The GPMI drivers do not implemenent raw access functions, and thus rely on
> default HW_ECC scheme implementation.
> The default implementation consider the data and OOB area as properly
> separated in their respective NAND section, which is not true for the GPMI
> controller.
> In this driver/controller some OOB data are stored at the beginning of the
> NAND data area (these data are called metadata in the driver), then ECC
> bytes are interleaved with data chunk (which is similar to the
> HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> OOB data.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> Hello,
>
> This patch is providing raw access support to the GPMI driver which is
> particularly useful to run some tests on the NAND (the one coming in
> mind is the mtd_nandbiterrs testsuite).
>
> I know this rework might break several user space tools which are relying
> on the default raw access implementation (I already experienced an issue
> with the kobs-ng tool provided by freescale), but many other tools will
> now work as expected.
If the kobs-ng can not works, there is no meaning that other tools
works. So I do not think we need to implement these hooks.
sorry, I will not Ack this patch.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 12:09 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 12:09 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> Several MTD users (either in user or kernel space) expect a valid raw
> access support to NAND chip devices.
> This is particularly true for testing tools which are often touching the
> data stored in a NAND chip in raw mode to artificially generate errors.
>
> The GPMI drivers do not implemenent raw access functions, and thus rely on
> default HW_ECC scheme implementation.
> The default implementation consider the data and OOB area as properly
> separated in their respective NAND section, which is not true for the GPMI
> controller.
> In this driver/controller some OOB data are stored at the beginning of the
> NAND data area (these data are called metadata in the driver), then ECC
> bytes are interleaved with data chunk (which is similar to the
> HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> OOB data.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> Hello,
>
> This patch is providing raw access support to the GPMI driver which is
> particularly useful to run some tests on the NAND (the one coming in
> mind is the mtd_nandbiterrs testsuite).
>
> I know this rework might break several user space tools which are relying
> on the default raw access implementation (I already experienced an issue
> with the kobs-ng tool provided by freescale), but many other tools will
> now work as expected.
If the kobs-ng can not works, there is no meaning that other tools
works. So I do not think we need to implement these hooks.
sorry, I will not Ack this patch.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 12:09 ` Huang Shijie
(?)
@ 2014-09-11 12:36 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 12:36 UTC (permalink / raw)
To: Huang Shijie
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
Hi Huang,
On Thu, 11 Sep 2014 20:09:30 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > Several MTD users (either in user or kernel space) expect a valid raw
> > access support to NAND chip devices.
> > This is particularly true for testing tools which are often touching the
> > data stored in a NAND chip in raw mode to artificially generate errors.
> >
> > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > default HW_ECC scheme implementation.
> > The default implementation consider the data and OOB area as properly
> > separated in their respective NAND section, which is not true for the GPMI
> > controller.
> > In this driver/controller some OOB data are stored at the beginning of the
> > NAND data area (these data are called metadata in the driver), then ECC
> > bytes are interleaved with data chunk (which is similar to the
> > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > OOB data.
> >
> > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > ---
> > Hello,
> >
> > This patch is providing raw access support to the GPMI driver which is
> > particularly useful to run some tests on the NAND (the one coming in
> > mind is the mtd_nandbiterrs testsuite).
> >
> > I know this rework might break several user space tools which are relying
> > on the default raw access implementation (I already experienced an issue
> > with the kobs-ng tool provided by freescale), but many other tools will
> > now work as expected.
> If the kobs-ng can not works, there is no meaning that other tools
> works. So I do not think we need to implement these hooks.
Well, I don't know about freescale specific tools, but at least I have
an example with mtd_nandbiterrs module.
This module is assuming it can write only the data part of a NAND page
without modifying the OOB area (see [1]), which in GPMI controller case
is impossible because raw write function store the data as if there
were no specific scheme, while there is one:
(metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
Moreover, IMHO, nanddump and nandwrite tools (which can use raw
access mode when passing the appropriate option) should always return
the same kind of data no matter what NAND controller is in use on the
system => (DATA + OOB_DATA), and this is definitely not the case with
the GPMI driver.
See how raw access on HW_ECC_SYNDROME scheme is implemented in
nand_base.c [2]. It hides to the mtd user the fact that DATA and ECC
bytes are interleaved, and give him the expected (DATA + OOB_DATA)
representation.
Best Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/mtd/tests/nandbiterrs.c#L112
[2]http://lxr.free-electrons.com/source/drivers/mtd/nand/nand_base.c#L1364
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 12:36 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 12:36 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
Hi Huang,
On Thu, 11 Sep 2014 20:09:30 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > Several MTD users (either in user or kernel space) expect a valid raw
> > access support to NAND chip devices.
> > This is particularly true for testing tools which are often touching the
> > data stored in a NAND chip in raw mode to artificially generate errors.
> >
> > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > default HW_ECC scheme implementation.
> > The default implementation consider the data and OOB area as properly
> > separated in their respective NAND section, which is not true for the GPMI
> > controller.
> > In this driver/controller some OOB data are stored at the beginning of the
> > NAND data area (these data are called metadata in the driver), then ECC
> > bytes are interleaved with data chunk (which is similar to the
> > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > OOB data.
> >
> > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > ---
> > Hello,
> >
> > This patch is providing raw access support to the GPMI driver which is
> > particularly useful to run some tests on the NAND (the one coming in
> > mind is the mtd_nandbiterrs testsuite).
> >
> > I know this rework might break several user space tools which are relying
> > on the default raw access implementation (I already experienced an issue
> > with the kobs-ng tool provided by freescale), but many other tools will
> > now work as expected.
> If the kobs-ng can not works, there is no meaning that other tools
> works. So I do not think we need to implement these hooks.
Well, I don't know about freescale specific tools, but at least I have
an example with mtd_nandbiterrs module.
This module is assuming it can write only the data part of a NAND page
without modifying the OOB area (see [1]), which in GPMI controller case
is impossible because raw write function store the data as if there
were no specific scheme, while there is one:
(metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
Moreover, IMHO, nanddump and nandwrite tools (which can use raw
access mode when passing the appropriate option) should always return
the same kind of data no matter what NAND controller is in use on the
system => (DATA + OOB_DATA), and this is definitely not the case with
the GPMI driver.
See how raw access on HW_ECC_SYNDROME scheme is implemented in
nand_base.c [2]. It hides to the mtd user the fact that DATA and ECC
bytes are interleaved, and give him the expected (DATA + OOB_DATA)
representation.
Best Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/mtd/tests/nandbiterrs.c#L112
[2]http://lxr.free-electrons.com/source/drivers/mtd/nand/nand_base.c#L1364
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 12:36 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 12:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi Huang,
On Thu, 11 Sep 2014 20:09:30 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > Several MTD users (either in user or kernel space) expect a valid raw
> > access support to NAND chip devices.
> > This is particularly true for testing tools which are often touching the
> > data stored in a NAND chip in raw mode to artificially generate errors.
> >
> > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > default HW_ECC scheme implementation.
> > The default implementation consider the data and OOB area as properly
> > separated in their respective NAND section, which is not true for the GPMI
> > controller.
> > In this driver/controller some OOB data are stored at the beginning of the
> > NAND data area (these data are called metadata in the driver), then ECC
> > bytes are interleaved with data chunk (which is similar to the
> > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > OOB data.
> >
> > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > ---
> > Hello,
> >
> > This patch is providing raw access support to the GPMI driver which is
> > particularly useful to run some tests on the NAND (the one coming in
> > mind is the mtd_nandbiterrs testsuite).
> >
> > I know this rework might break several user space tools which are relying
> > on the default raw access implementation (I already experienced an issue
> > with the kobs-ng tool provided by freescale), but many other tools will
> > now work as expected.
> If the kobs-ng can not works, there is no meaning that other tools
> works. So I do not think we need to implement these hooks.
Well, I don't know about freescale specific tools, but at least I have
an example with mtd_nandbiterrs module.
This module is assuming it can write only the data part of a NAND page
without modifying the OOB area (see [1]), which in GPMI controller case
is impossible because raw write function store the data as if there
were no specific scheme, while there is one:
(metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
Moreover, IMHO, nanddump and nandwrite tools (which can use raw
access mode when passing the appropriate option) should always return
the same kind of data no matter what NAND controller is in use on the
system => (DATA + OOB_DATA), and this is definitely not the case with
the GPMI driver.
See how raw access on HW_ECC_SYNDROME scheme is implemented in
nand_base.c [2]. It hides to the mtd user the fact that DATA and ECC
bytes are interleaved, and give him the expected (DATA + OOB_DATA)
representation.
Best Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/mtd/tests/nandbiterrs.c#L112
[2]http://lxr.free-electrons.com/source/drivers/mtd/nand/nand_base.c#L1364
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 12:36 ` Boris BREZILLON
(?)
@ 2014-09-11 14:25 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:25 UTC (permalink / raw)
To: Boris BREZILLON
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
Hi Boris,
On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 20:09:30 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > Several MTD users (either in user or kernel space) expect a valid raw
> > > access support to NAND chip devices.
> > > This is particularly true for testing tools which are often touching the
> > > data stored in a NAND chip in raw mode to artificially generate errors.
> > >
> > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > default HW_ECC scheme implementation.
> > > The default implementation consider the data and OOB area as properly
> > > separated in their respective NAND section, which is not true for the GPMI
> > > controller.
> > > In this driver/controller some OOB data are stored at the beginning of the
> > > NAND data area (these data are called metadata in the driver), then ECC
> > > bytes are interleaved with data chunk (which is similar to the
> > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > OOB data.
> > >
> > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > ---
> > > Hello,
> > >
> > > This patch is providing raw access support to the GPMI driver which is
> > > particularly useful to run some tests on the NAND (the one coming in
> > > mind is the mtd_nandbiterrs testsuite).
> > >
> > > I know this rework might break several user space tools which are relying
> > > on the default raw access implementation (I already experienced an issue
> > > with the kobs-ng tool provided by freescale), but many other tools will
> > > now work as expected.
> > If the kobs-ng can not works, there is no meaning that other tools
> > works. So I do not think we need to implement these hooks.
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
The gpmi uses the hardware ECC for the bitflips.
I really do not know why the mtd_nandbiterrs is needed.
IMHO, the mtd_nandbiterrs is useless for the gpmi.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller case
> is impossible because raw write function store the data as if there
> were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
> Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> access mode when passing the appropriate option) should always return
> the same kind of data no matter what NAND controller is in use on the
> system => (DATA + OOB_DATA), and this is definitely not the case with
> the GPMI driver.
>
> See how raw access on HW_ECC_SYNDROME scheme is implemented in
The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Even you really want to support the nanddump, i do not agree to add the
write hook, it may crash the system.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:25 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:25 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
Hi Boris,
On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 20:09:30 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > Several MTD users (either in user or kernel space) expect a valid raw
> > > access support to NAND chip devices.
> > > This is particularly true for testing tools which are often touching the
> > > data stored in a NAND chip in raw mode to artificially generate errors.
> > >
> > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > default HW_ECC scheme implementation.
> > > The default implementation consider the data and OOB area as properly
> > > separated in their respective NAND section, which is not true for the GPMI
> > > controller.
> > > In this driver/controller some OOB data are stored at the beginning of the
> > > NAND data area (these data are called metadata in the driver), then ECC
> > > bytes are interleaved with data chunk (which is similar to the
> > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > OOB data.
> > >
> > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > ---
> > > Hello,
> > >
> > > This patch is providing raw access support to the GPMI driver which is
> > > particularly useful to run some tests on the NAND (the one coming in
> > > mind is the mtd_nandbiterrs testsuite).
> > >
> > > I know this rework might break several user space tools which are relying
> > > on the default raw access implementation (I already experienced an issue
> > > with the kobs-ng tool provided by freescale), but many other tools will
> > > now work as expected.
> > If the kobs-ng can not works, there is no meaning that other tools
> > works. So I do not think we need to implement these hooks.
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
The gpmi uses the hardware ECC for the bitflips.
I really do not know why the mtd_nandbiterrs is needed.
IMHO, the mtd_nandbiterrs is useless for the gpmi.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller case
> is impossible because raw write function store the data as if there
> were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
> Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> access mode when passing the appropriate option) should always return
> the same kind of data no matter what NAND controller is in use on the
> system => (DATA + OOB_DATA), and this is definitely not the case with
> the GPMI driver.
>
> See how raw access on HW_ECC_SYNDROME scheme is implemented in
The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Even you really want to support the nanddump, i do not agree to add the
write hook, it may crash the system.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:25 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Boris,
On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 20:09:30 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > Several MTD users (either in user or kernel space) expect a valid raw
> > > access support to NAND chip devices.
> > > This is particularly true for testing tools which are often touching the
> > > data stored in a NAND chip in raw mode to artificially generate errors.
> > >
> > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > default HW_ECC scheme implementation.
> > > The default implementation consider the data and OOB area as properly
> > > separated in their respective NAND section, which is not true for the GPMI
> > > controller.
> > > In this driver/controller some OOB data are stored at the beginning of the
> > > NAND data area (these data are called metadata in the driver), then ECC
> > > bytes are interleaved with data chunk (which is similar to the
> > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > OOB data.
> > >
> > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > ---
> > > Hello,
> > >
> > > This patch is providing raw access support to the GPMI driver which is
> > > particularly useful to run some tests on the NAND (the one coming in
> > > mind is the mtd_nandbiterrs testsuite).
> > >
> > > I know this rework might break several user space tools which are relying
> > > on the default raw access implementation (I already experienced an issue
> > > with the kobs-ng tool provided by freescale), but many other tools will
> > > now work as expected.
> > If the kobs-ng can not works, there is no meaning that other tools
> > works. So I do not think we need to implement these hooks.
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
The gpmi uses the hardware ECC for the bitflips.
I really do not know why the mtd_nandbiterrs is needed.
IMHO, the mtd_nandbiterrs is useless for the gpmi.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller case
> is impossible because raw write function store the data as if there
> were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
> Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> access mode when passing the appropriate option) should always return
> the same kind of data no matter what NAND controller is in use on the
> system => (DATA + OOB_DATA), and this is definitely not the case with
> the GPMI driver.
>
> See how raw access on HW_ECC_SYNDROME scheme is implemented in
The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Even you really want to support the nanddump, i do not agree to add the
write hook, it may crash the system.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-10 8:55 ` Boris BREZILLON
(?)
@ 2014-09-11 14:29 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:29 UTC (permalink / raw)
To: Boris BREZILLON
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> + struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page)
> +{
> + struct gpmi_nand_data *this = chip->priv;
> + struct bch_geometry *nfc_geo = &this->bch_geometry;
> + int eccsize = nfc_geo->ecc_chunk_size;
> + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> + 8);
In actually, the ECC can be _NOT_ bytes aligned.
you should not round up to byte.
it's hard to implement this hook.
thanks
Huang Shijie
> + uint8_t *oob = chip->oob_poi;
> + int step;
> + int column = 0;
> + uint8_t *orig_buf = buf;
> +
> + chip->read_buf(mtd, oob, nfc_geo->metadata_size);
> + oob += nfc_geo->metadata_size;
> +
> + column += nfc_geo->metadata_size;
> + for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
> + chip->read_buf(mtd, buf, eccsize);
> + buf += eccsize;
> + column += eccsize;
> + chip->read_buf(mtd, oob, eccbytes);
> + oob += eccbytes;
> + column += eccbytes;
> + }
> +
> + if (column < mtd->writesize + mtd->oobsize)
> + chip->read_buf(mtd, oob,
> + mtd->writesize + mtd->oobsize - column);
> +
> + block_mark_swapping(this, orig_buf, chip->oob_poi);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:29 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:29 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> + struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page)
> +{
> + struct gpmi_nand_data *this = chip->priv;
> + struct bch_geometry *nfc_geo = &this->bch_geometry;
> + int eccsize = nfc_geo->ecc_chunk_size;
> + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> + 8);
In actually, the ECC can be _NOT_ bytes aligned.
you should not round up to byte.
it's hard to implement this hook.
thanks
Huang Shijie
> + uint8_t *oob = chip->oob_poi;
> + int step;
> + int column = 0;
> + uint8_t *orig_buf = buf;
> +
> + chip->read_buf(mtd, oob, nfc_geo->metadata_size);
> + oob += nfc_geo->metadata_size;
> +
> + column += nfc_geo->metadata_size;
> + for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
> + chip->read_buf(mtd, buf, eccsize);
> + buf += eccsize;
> + column += eccsize;
> + chip->read_buf(mtd, oob, eccbytes);
> + oob += eccbytes;
> + column += eccbytes;
> + }
> +
> + if (column < mtd->writesize + mtd->oobsize)
> + chip->read_buf(mtd, oob,
> + mtd->writesize + mtd->oobsize - column);
> +
> + block_mark_swapping(this, orig_buf, chip->oob_poi);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:29 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-11 14:29 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
+static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> + struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page)
> +{
> + struct gpmi_nand_data *this = chip->priv;
> + struct bch_geometry *nfc_geo = &this->bch_geometry;
> + int eccsize = nfc_geo->ecc_chunk_size;
> + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> + 8);
In actually, the ECC can be _NOT_ bytes aligned.
you should not round up to byte.
it's hard to implement this hook.
thanks
Huang Shijie
> + uint8_t *oob = chip->oob_poi;
> + int step;
> + int column = 0;
> + uint8_t *orig_buf = buf;
> +
> + chip->read_buf(mtd, oob, nfc_geo->metadata_size);
> + oob += nfc_geo->metadata_size;
> +
> + column += nfc_geo->metadata_size;
> + for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {
> + chip->read_buf(mtd, buf, eccsize);
> + buf += eccsize;
> + column += eccsize;
> + chip->read_buf(mtd, oob, eccbytes);
> + oob += eccbytes;
> + column += eccbytes;
> + }
> +
> + if (column < mtd->writesize + mtd->oobsize)
> + chip->read_buf(mtd, oob,
> + mtd->writesize + mtd->oobsize - column);
> +
> + block_mark_swapping(this, orig_buf, chip->oob_poi);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 14:25 ` Huang Shijie
(?)
@ 2014-09-11 14:38 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:38 UTC (permalink / raw)
To: Huang Shijie
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
Hi Huang,
On Thu, 11 Sep 2014 22:25:13 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> Hi Boris,
>
> On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 20:09:30 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > access support to NAND chip devices.
> > > > This is particularly true for testing tools which are often touching the
> > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > >
> > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > default HW_ECC scheme implementation.
> > > > The default implementation consider the data and OOB area as properly
> > > > separated in their respective NAND section, which is not true for the GPMI
> > > > controller.
> > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > bytes are interleaved with data chunk (which is similar to the
> > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > OOB data.
> > > >
> > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > ---
> > > > Hello,
> > > >
> > > > This patch is providing raw access support to the GPMI driver which is
> > > > particularly useful to run some tests on the NAND (the one coming in
> > > > mind is the mtd_nandbiterrs testsuite).
> > > >
> > > > I know this rework might break several user space tools which are relying
> > > > on the default raw access implementation (I already experienced an issue
> > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > now work as expected.
> > > If the kobs-ng can not works, there is no meaning that other tools
> > > works. So I do not think we need to implement these hooks.
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
>
> The gpmi uses the hardware ECC for the bitflips.
> I really do not know why the mtd_nandbiterrs is needed.
> IMHO, the mtd_nandbiterrs is useless for the gpmi.
Because some folks would like to test their NAND controller/chip on
their system.
Just because you don't need it, doesn't mean others won't, and actually
the reason I worked on these raw function is becaused I needed to
validate the ECC capabilities of the GPMI ECC controller.
>
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller case
> > is impossible because raw write function store the data as if there
> > were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
> > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > access mode when passing the appropriate option) should always return
> > the same kind of data no matter what NAND controller is in use on the
> > system => (DATA + OOB_DATA), and this is definitely not the case with
> > the GPMI driver.
> >
> > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
to show you that NAND controller specific layout should be hidden to
the MTD user.
> Even you really want to support the nanddump, i do not agree to add the
> write hook, it may crash the system.
We can't have an asymetric behaviour here, either we move both read and
write raw functions or none. Moving only one of them would make the MTD
user work even more complicated.
I really don't get your point here. What's really bothering you (BTW, I
fixed kobs-ng to handle this new behaviour) ?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:38 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:38 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
Hi Huang,
On Thu, 11 Sep 2014 22:25:13 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> Hi Boris,
>
> On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 20:09:30 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > access support to NAND chip devices.
> > > > This is particularly true for testing tools which are often touching the
> > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > >
> > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > default HW_ECC scheme implementation.
> > > > The default implementation consider the data and OOB area as properly
> > > > separated in their respective NAND section, which is not true for the GPMI
> > > > controller.
> > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > bytes are interleaved with data chunk (which is similar to the
> > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > OOB data.
> > > >
> > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > ---
> > > > Hello,
> > > >
> > > > This patch is providing raw access support to the GPMI driver which is
> > > > particularly useful to run some tests on the NAND (the one coming in
> > > > mind is the mtd_nandbiterrs testsuite).
> > > >
> > > > I know this rework might break several user space tools which are relying
> > > > on the default raw access implementation (I already experienced an issue
> > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > now work as expected.
> > > If the kobs-ng can not works, there is no meaning that other tools
> > > works. So I do not think we need to implement these hooks.
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
>
> The gpmi uses the hardware ECC for the bitflips.
> I really do not know why the mtd_nandbiterrs is needed.
> IMHO, the mtd_nandbiterrs is useless for the gpmi.
Because some folks would like to test their NAND controller/chip on
their system.
Just because you don't need it, doesn't mean others won't, and actually
the reason I worked on these raw function is becaused I needed to
validate the ECC capabilities of the GPMI ECC controller.
>
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller case
> > is impossible because raw write function store the data as if there
> > were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
> > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > access mode when passing the appropriate option) should always return
> > the same kind of data no matter what NAND controller is in use on the
> > system => (DATA + OOB_DATA), and this is definitely not the case with
> > the GPMI driver.
> >
> > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
to show you that NAND controller specific layout should be hidden to
the MTD user.
> Even you really want to support the nanddump, i do not agree to add the
> write hook, it may crash the system.
We can't have an asymetric behaviour here, either we move both read and
write raw functions or none. Moving only one of them would make the MTD
user work even more complicated.
I really don't get your point here. What's really bothering you (BTW, I
fixed kobs-ng to handle this new behaviour) ?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:38 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi Huang,
On Thu, 11 Sep 2014 22:25:13 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> Hi Boris,
>
> On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 20:09:30 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > access support to NAND chip devices.
> > > > This is particularly true for testing tools which are often touching the
> > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > >
> > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > default HW_ECC scheme implementation.
> > > > The default implementation consider the data and OOB area as properly
> > > > separated in their respective NAND section, which is not true for the GPMI
> > > > controller.
> > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > bytes are interleaved with data chunk (which is similar to the
> > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > OOB data.
> > > >
> > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > ---
> > > > Hello,
> > > >
> > > > This patch is providing raw access support to the GPMI driver which is
> > > > particularly useful to run some tests on the NAND (the one coming in
> > > > mind is the mtd_nandbiterrs testsuite).
> > > >
> > > > I know this rework might break several user space tools which are relying
> > > > on the default raw access implementation (I already experienced an issue
> > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > now work as expected.
> > > If the kobs-ng can not works, there is no meaning that other tools
> > > works. So I do not think we need to implement these hooks.
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
>
> The gpmi uses the hardware ECC for the bitflips.
> I really do not know why the mtd_nandbiterrs is needed.
> IMHO, the mtd_nandbiterrs is useless for the gpmi.
Because some folks would like to test their NAND controller/chip on
their system.
Just because you don't need it, doesn't mean others won't, and actually
the reason I worked on these raw function is becaused I needed to
validate the ECC capabilities of the GPMI ECC controller.
>
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller case
> > is impossible because raw write function store the data as if there
> > were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
> > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > access mode when passing the appropriate option) should always return
> > the same kind of data no matter what NAND controller is in use on the
> > system => (DATA + OOB_DATA), and this is definitely not the case with
> > the GPMI driver.
> >
> > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
to show you that NAND controller specific layout should be hidden to
the MTD user.
> Even you really want to support the nanddump, i do not agree to add the
> write hook, it may crash the system.
We can't have an asymetric behaviour here, either we move both read and
write raw functions or none. Moving only one of them would make the MTD
user work even more complicated.
I really don't get your point here. What's really bothering you (BTW, I
fixed kobs-ng to handle this new behaviour) ?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 14:29 ` Huang Shijie
(?)
@ 2014-09-11 14:45 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:45 UTC (permalink / raw)
To: Huang Shijie
Cc: David Woodhouse, Brian Norris, linux-mtd, Huang Shijie,
Mike Voytovich, linux-kernel, linux-arm-kernel, Roy Lee
On Thu, 11 Sep 2014 22:29:32 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > + struct nand_chip *chip, uint8_t *buf,
> > + int oob_required, int page)
> > +{
> > + struct gpmi_nand_data *this = chip->priv;
> > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > + int eccsize = nfc_geo->ecc_chunk_size;
> > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > + 8);
>
> In actually, the ECC can be _NOT_ bytes aligned.
> you should not round up to byte.
You mean, on the NAND storage ? That would be weird, but I'll check.
When accessing in raw mode I'm not using the ECC engine (or at least
that's what I'm trying to achieve when using read_buf and not
gpmi_ecc_read_page).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:45 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:45 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
On Thu, 11 Sep 2014 22:29:32 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > + struct nand_chip *chip, uint8_t *buf,
> > + int oob_required, int page)
> > +{
> > + struct gpmi_nand_data *this = chip->priv;
> > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > + int eccsize = nfc_geo->ecc_chunk_size;
> > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > + 8);
>
> In actually, the ECC can be _NOT_ bytes aligned.
> you should not round up to byte.
You mean, on the NAND storage ? That would be weird, but I'll check.
When accessing in raw mode I'm not using the ECC engine (or at least
that's what I'm trying to achieve when using read_buf and not
gpmi_ecc_read_page).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-11 14:45 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-11 14:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 11 Sep 2014 22:29:32 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > + struct nand_chip *chip, uint8_t *buf,
> > + int oob_required, int page)
> > +{
> > + struct gpmi_nand_data *this = chip->priv;
> > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > + int eccsize = nfc_geo->ecc_chunk_size;
> > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > + 8);
>
> In actually, the ECC can be _NOT_ bytes aligned.
> you should not round up to byte.
You mean, on the NAND storage ? That would be weird, but I'll check.
When accessing in raw mode I'm not using the ECC engine (or at least
that's what I'm trying to achieve when using read_buf and not
gpmi_ecc_read_page).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 14:45 ` Boris BREZILLON
(?)
@ 2014-09-12 0:40 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:40 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
On Thu, Sep 11, 2014 at 04:45:36PM +0200, Boris BREZILLON wrote:
> On Thu, 11 Sep 2014 22:29:32 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > > + struct nand_chip *chip, uint8_t *buf,
> > > + int oob_required, int page)
> > > +{
> > > + struct gpmi_nand_data *this = chip->priv;
> > > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > > + int eccsize = nfc_geo->ecc_chunk_size;
> > > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > > + 8);
> >
> > In actually, the ECC can be _NOT_ bytes aligned.
> > you should not round up to byte.
>
> You mean, on the NAND storage ? That would be weird, but I'll check.
yes. it is weird.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 0:40 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:40 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, Huang Shijie, David Woodhouse, linux-arm-kernel
On Thu, Sep 11, 2014 at 04:45:36PM +0200, Boris BREZILLON wrote:
> On Thu, 11 Sep 2014 22:29:32 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > > + struct nand_chip *chip, uint8_t *buf,
> > > + int oob_required, int page)
> > > +{
> > > + struct gpmi_nand_data *this = chip->priv;
> > > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > > + int eccsize = nfc_geo->ecc_chunk_size;
> > > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > > + 8);
> >
> > In actually, the ECC can be _NOT_ bytes aligned.
> > you should not round up to byte.
>
> You mean, on the NAND storage ? That would be weird, but I'll check.
yes. it is weird.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 0:40 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:40 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 11, 2014 at 04:45:36PM +0200, Boris BREZILLON wrote:
> On Thu, 11 Sep 2014 22:29:32 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > +static int gpmi_ecc_read_page_raw(struct mtd_info *mtd,
> > > + struct nand_chip *chip, uint8_t *buf,
> > > + int oob_required, int page)
> > > +{
> > > + struct gpmi_nand_data *this = chip->priv;
> > > + struct bch_geometry *nfc_geo = &this->bch_geometry;
> > > + int eccsize = nfc_geo->ecc_chunk_size;
> > > + int eccbytes = DIV_ROUND_UP(nfc_geo->ecc_strength * nfc_geo->gf_len,
> > > + 8);
> >
> > In actually, the ECC can be _NOT_ bytes aligned.
> > you should not round up to byte.
>
> You mean, on the NAND storage ? That would be weird, but I'll check.
yes. it is weird.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 14:38 ` Boris BREZILLON
(?)
@ 2014-09-12 0:45 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:45 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 22:25:13 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > Hi Boris,
> >
> > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > Hi Huang,
> > >
> > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > Huang Shijie <shijie8@gmail.com> wrote:
> > >
> > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > access support to NAND chip devices.
> > > > > This is particularly true for testing tools which are often touching the
> > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > >
> > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > default HW_ECC scheme implementation.
> > > > > The default implementation consider the data and OOB area as properly
> > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > controller.
> > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > OOB data.
> > > > >
> > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > ---
> > > > > Hello,
> > > > >
> > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > mind is the mtd_nandbiterrs testsuite).
> > > > >
> > > > > I know this rework might break several user space tools which are relying
> > > > > on the default raw access implementation (I already experienced an issue
> > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > now work as expected.
> > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > works. So I do not think we need to implement these hooks.
> > >
> > > Well, I don't know about freescale specific tools, but at least I have
> > > an example with mtd_nandbiterrs module.
> >
> > The gpmi uses the hardware ECC for the bitflips.
> > I really do not know why the mtd_nandbiterrs is needed.
> > IMHO, the mtd_nandbiterrs is useless for the gpmi.
>
> Because some folks would like to test their NAND controller/chip on
> their system.
>
> Just because you don't need it, doesn't mean others won't, and actually
> the reason I worked on these raw function is becaused I needed to
> validate the ECC capabilities of the GPMI ECC controller.
The BCH's algorithm is confidential to Freescale.
How can you validate the ECC capabilities?
So You can not emulate the BCH to create the ECC data, even you can fake
some bitflips in the data chunk.
>
> >
> > > This module is assuming it can write only the data part of a NAND page
> > > without modifying the OOB area (see [1]), which in GPMI controller case
> > > is impossible because raw write function store the data as if there
> > > were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> > > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > > access mode when passing the appropriate option) should always return
> > > the same kind of data no matter what NAND controller is in use on the
> > > system => (DATA + OOB_DATA), and this is definitely not the case with
> > > the GPMI driver.
> > >
> > > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> > The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
>
> Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
> to show you that NAND controller specific layout should be hidden to
> the MTD user.
>
> > Even you really want to support the nanddump, i do not agree to add the
> > write hook, it may crash the system.
>
> We can't have an asymetric behaviour here, either we move both read and
> write raw functions or none. Moving only one of them would make the MTD
> user work even more complicated.
>
> I really don't get your point here. What's really bothering you (BTW, I
> fixed kobs-ng to handle this new behaviour) ?
see the comment above.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 0:45 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:45 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, Huang Shijie, David Woodhouse, linux-arm-kernel
On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 22:25:13 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > Hi Boris,
> >
> > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > Hi Huang,
> > >
> > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > Huang Shijie <shijie8@gmail.com> wrote:
> > >
> > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > access support to NAND chip devices.
> > > > > This is particularly true for testing tools which are often touching the
> > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > >
> > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > default HW_ECC scheme implementation.
> > > > > The default implementation consider the data and OOB area as properly
> > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > controller.
> > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > OOB data.
> > > > >
> > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > ---
> > > > > Hello,
> > > > >
> > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > mind is the mtd_nandbiterrs testsuite).
> > > > >
> > > > > I know this rework might break several user space tools which are relying
> > > > > on the default raw access implementation (I already experienced an issue
> > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > now work as expected.
> > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > works. So I do not think we need to implement these hooks.
> > >
> > > Well, I don't know about freescale specific tools, but at least I have
> > > an example with mtd_nandbiterrs module.
> >
> > The gpmi uses the hardware ECC for the bitflips.
> > I really do not know why the mtd_nandbiterrs is needed.
> > IMHO, the mtd_nandbiterrs is useless for the gpmi.
>
> Because some folks would like to test their NAND controller/chip on
> their system.
>
> Just because you don't need it, doesn't mean others won't, and actually
> the reason I worked on these raw function is becaused I needed to
> validate the ECC capabilities of the GPMI ECC controller.
The BCH's algorithm is confidential to Freescale.
How can you validate the ECC capabilities?
So You can not emulate the BCH to create the ECC data, even you can fake
some bitflips in the data chunk.
>
> >
> > > This module is assuming it can write only the data part of a NAND page
> > > without modifying the OOB area (see [1]), which in GPMI controller case
> > > is impossible because raw write function store the data as if there
> > > were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> > > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > > access mode when passing the appropriate option) should always return
> > > the same kind of data no matter what NAND controller is in use on the
> > > system => (DATA + OOB_DATA), and this is definitely not the case with
> > > the GPMI driver.
> > >
> > > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> > The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
>
> Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
> to show you that NAND controller specific layout should be hidden to
> the MTD user.
>
> > Even you really want to support the nanddump, i do not agree to add the
> > write hook, it may crash the system.
>
> We can't have an asymetric behaviour here, either we move both read and
> write raw functions or none. Moving only one of them would make the MTD
> user work even more complicated.
>
> I really don't get your point here. What's really bothering you (BTW, I
> fixed kobs-ng to handle this new behaviour) ?
see the comment above.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 0:45 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-12 0:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> Hi Huang,
>
> On Thu, 11 Sep 2014 22:25:13 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > Hi Boris,
> >
> > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > Hi Huang,
> > >
> > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > Huang Shijie <shijie8@gmail.com> wrote:
> > >
> > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > access support to NAND chip devices.
> > > > > This is particularly true for testing tools which are often touching the
> > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > >
> > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > default HW_ECC scheme implementation.
> > > > > The default implementation consider the data and OOB area as properly
> > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > controller.
> > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > OOB data.
> > > > >
> > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > ---
> > > > > Hello,
> > > > >
> > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > mind is the mtd_nandbiterrs testsuite).
> > > > >
> > > > > I know this rework might break several user space tools which are relying
> > > > > on the default raw access implementation (I already experienced an issue
> > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > now work as expected.
> > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > works. So I do not think we need to implement these hooks.
> > >
> > > Well, I don't know about freescale specific tools, but at least I have
> > > an example with mtd_nandbiterrs module.
> >
> > The gpmi uses the hardware ECC for the bitflips.
> > I really do not know why the mtd_nandbiterrs is needed.
> > IMHO, the mtd_nandbiterrs is useless for the gpmi.
>
> Because some folks would like to test their NAND controller/chip on
> their system.
>
> Just because you don't need it, doesn't mean others won't, and actually
> the reason I worked on these raw function is becaused I needed to
> validate the ECC capabilities of the GPMI ECC controller.
The BCH's algorithm is confidential to Freescale.
How can you validate the ECC capabilities?
So You can not emulate the BCH to create the ECC data, even you can fake
some bitflips in the data chunk.
>
> >
> > > This module is assuming it can write only the data part of a NAND page
> > > without modifying the OOB area (see [1]), which in GPMI controller case
> > > is impossible because raw write function store the data as if there
> > > were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> > > Moreover, IMHO, nanddump and nandwrite tools (which can use raw
> > > access mode when passing the appropriate option) should always return
> > > the same kind of data no matter what NAND controller is in use on the
> > > system => (DATA + OOB_DATA), and this is definitely not the case with
> > > the GPMI driver.
> > >
> > > See how raw access on HW_ECC_SYNDROME scheme is implemented in
> > The gpmi uses the NAND_ECC_HW, not the NAND_ECC_HW_SYNDROME.
>
> Yes I know. I pointed out the NAND_ECC_HW_SYNDROME scheme as an example
> to show you that NAND controller specific layout should be hidden to
> the MTD user.
>
> > Even you really want to support the nanddump, i do not agree to add the
> > write hook, it may crash the system.
>
> We can't have an asymetric behaviour here, either we move both read and
> write raw functions or none. Moving only one of them would make the MTD
> user work even more complicated.
>
> I really don't get your point here. What's really bothering you (BTW, I
> fixed kobs-ng to handle this new behaviour) ?
see the comment above.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-12 0:45 ` Huang Shijie
(?)
@ 2014-09-12 12:30 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-12 12:30 UTC (permalink / raw)
To: Huang Shijie
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
On Fri, 12 Sep 2014 08:45:50 +0800
Huang Shijie <shijie.huang@intel.com> wrote:
> On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 22:25:13 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > Hi Boris,
> > >
> > > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > > Hi Huang,
> > > >
> > > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > > Huang Shijie <shijie8@gmail.com> wrote:
> > > >
> > > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > > access support to NAND chip devices.
> > > > > > This is particularly true for testing tools which are often touching the
> > > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > > >
> > > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > > default HW_ECC scheme implementation.
> > > > > > The default implementation consider the data and OOB area as properly
> > > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > > controller.
> > > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > > OOB data.
> > > > > >
> > > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > > ---
> > > > > > Hello,
> > > > > >
> > > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > > mind is the mtd_nandbiterrs testsuite).
> > > > > >
> > > > > > I know this rework might break several user space tools which are relying
> > > > > > on the default raw access implementation (I already experienced an issue
> > > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > > now work as expected.
> > > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > > works. So I do not think we need to implement these hooks.
> > > >
> > > > Well, I don't know about freescale specific tools, but at least I have
> > > > an example with mtd_nandbiterrs module.
> > >
> > > The gpmi uses the hardware ECC for the bitflips.
> > > I really do not know why the mtd_nandbiterrs is needed.
> > > IMHO, the mtd_nandbiterrs is useless for the gpmi.
> >
> > Because some folks would like to test their NAND controller/chip on
> > their system.
> >
> > Just because you don't need it, doesn't mean others won't, and actually
> > the reason I worked on these raw function is becaused I needed to
> > validate the ECC capabilities of the GPMI ECC controller.
> The BCH's algorithm is confidential to Freescale.
> How can you validate the ECC capabilities?
My bad, capabilities might not be the appropriate word in this context.
I meant ECC algorithm strength, and that's exactly the purpose of
nandbiterrs testsuite: insert bitflips in in-band data until the MTD
layer complains about uncorrectable errors.
This test validates what's returned by ecc_strength file in sysfs
(which in turn is specified by the NAND controller when initializing
the NAND chip).
Doing this should not imply knowing the ECC algorithm in use in the
NAND controller or the layout used to store data on NAND.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 12:30 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-12 12:30 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, Huang Shijie, David Woodhouse, linux-arm-kernel
On Fri, 12 Sep 2014 08:45:50 +0800
Huang Shijie <shijie.huang@intel.com> wrote:
> On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 22:25:13 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > Hi Boris,
> > >
> > > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > > Hi Huang,
> > > >
> > > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > > Huang Shijie <shijie8@gmail.com> wrote:
> > > >
> > > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > > access support to NAND chip devices.
> > > > > > This is particularly true for testing tools which are often touching the
> > > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > > >
> > > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > > default HW_ECC scheme implementation.
> > > > > > The default implementation consider the data and OOB area as properly
> > > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > > controller.
> > > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > > OOB data.
> > > > > >
> > > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > > ---
> > > > > > Hello,
> > > > > >
> > > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > > mind is the mtd_nandbiterrs testsuite).
> > > > > >
> > > > > > I know this rework might break several user space tools which are relying
> > > > > > on the default raw access implementation (I already experienced an issue
> > > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > > now work as expected.
> > > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > > works. So I do not think we need to implement these hooks.
> > > >
> > > > Well, I don't know about freescale specific tools, but at least I have
> > > > an example with mtd_nandbiterrs module.
> > >
> > > The gpmi uses the hardware ECC for the bitflips.
> > > I really do not know why the mtd_nandbiterrs is needed.
> > > IMHO, the mtd_nandbiterrs is useless for the gpmi.
> >
> > Because some folks would like to test their NAND controller/chip on
> > their system.
> >
> > Just because you don't need it, doesn't mean others won't, and actually
> > the reason I worked on these raw function is becaused I needed to
> > validate the ECC capabilities of the GPMI ECC controller.
> The BCH's algorithm is confidential to Freescale.
> How can you validate the ECC capabilities?
My bad, capabilities might not be the appropriate word in this context.
I meant ECC algorithm strength, and that's exactly the purpose of
nandbiterrs testsuite: insert bitflips in in-band data until the MTD
layer complains about uncorrectable errors.
This test validates what's returned by ecc_strength file in sysfs
(which in turn is specified by the NAND controller when initializing
the NAND chip).
Doing this should not imply knowing the ECC algorithm in use in the
NAND controller or the layout used to store data on NAND.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-12 12:30 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-12 12:30 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 12 Sep 2014 08:45:50 +0800
Huang Shijie <shijie.huang@intel.com> wrote:
> On Thu, Sep 11, 2014 at 04:38:47PM +0200, Boris BREZILLON wrote:
> > Hi Huang,
> >
> > On Thu, 11 Sep 2014 22:25:13 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > Hi Boris,
> > >
> > > On Thu, Sep 11, 2014 at 02:36:16PM +0200, Boris BREZILLON wrote:
> > > > Hi Huang,
> > > >
> > > > On Thu, 11 Sep 2014 20:09:30 +0800
> > > > Huang Shijie <shijie8@gmail.com> wrote:
> > > >
> > > > > On Wed, Sep 10, 2014 at 10:55:39AM +0200, Boris BREZILLON wrote:
> > > > > > Several MTD users (either in user or kernel space) expect a valid raw
> > > > > > access support to NAND chip devices.
> > > > > > This is particularly true for testing tools which are often touching the
> > > > > > data stored in a NAND chip in raw mode to artificially generate errors.
> > > > > >
> > > > > > The GPMI drivers do not implemenent raw access functions, and thus rely on
> > > > > > default HW_ECC scheme implementation.
> > > > > > The default implementation consider the data and OOB area as properly
> > > > > > separated in their respective NAND section, which is not true for the GPMI
> > > > > > controller.
> > > > > > In this driver/controller some OOB data are stored at the beginning of the
> > > > > > NAND data area (these data are called metadata in the driver), then ECC
> > > > > > bytes are interleaved with data chunk (which is similar to the
> > > > > > HW_ECC_SYNDROME scheme), and eventually the remaining bytes are used as
> > > > > > OOB data.
> > > > > >
> > > > > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > > > > > ---
> > > > > > Hello,
> > > > > >
> > > > > > This patch is providing raw access support to the GPMI driver which is
> > > > > > particularly useful to run some tests on the NAND (the one coming in
> > > > > > mind is the mtd_nandbiterrs testsuite).
> > > > > >
> > > > > > I know this rework might break several user space tools which are relying
> > > > > > on the default raw access implementation (I already experienced an issue
> > > > > > with the kobs-ng tool provided by freescale), but many other tools will
> > > > > > now work as expected.
> > > > > If the kobs-ng can not works, there is no meaning that other tools
> > > > > works. So I do not think we need to implement these hooks.
> > > >
> > > > Well, I don't know about freescale specific tools, but at least I have
> > > > an example with mtd_nandbiterrs module.
> > >
> > > The gpmi uses the hardware ECC for the bitflips.
> > > I really do not know why the mtd_nandbiterrs is needed.
> > > IMHO, the mtd_nandbiterrs is useless for the gpmi.
> >
> > Because some folks would like to test their NAND controller/chip on
> > their system.
> >
> > Just because you don't need it, doesn't mean others won't, and actually
> > the reason I worked on these raw function is becaused I needed to
> > validate the ECC capabilities of the GPMI ECC controller.
> The BCH's algorithm is confidential to Freescale.
> How can you validate the ECC capabilities?
My bad, capabilities might not be the appropriate word in this context.
I meant ECC algorithm strength, and that's exactly the purpose of
nandbiterrs testsuite: insert bitflips in in-band data until the MTD
layer complains about uncorrectable errors.
This test validates what's returned by ecc_strength file in sysfs
(which in turn is specified by the NAND controller when initializing
the NAND chip).
Doing this should not imply knowing the ECC algorithm in use in the
NAND controller or the layout used to store data on NAND.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-12 12:30 ` Boris BREZILLON
(?)
@ 2014-09-13 15:36 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-13 15:36 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> On Fri, 12 Sep 2014 08:45:50 +0800
>
> My bad, capabilities might not be the appropriate word in this context.
> I meant ECC algorithm strength, and that's exactly the purpose of
> nandbiterrs testsuite: insert bitflips in in-band data until the MTD
> layer complains about uncorrectable errors.
okay.
> This test validates what's returned by ecc_strength file in sysfs
> (which in turn is specified by the NAND controller when initializing
> the NAND chip).
>
> Doing this should not imply knowing the ECC algorithm in use in the
> NAND controller or the layout used to store data on NAND.
the difficulty is that the ECC parity area can be not byte aligned.
As I ever said, it is hard to implement the two hooks.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-13 15:36 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-13 15:36 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Huang Shijie, Brian Norris, David Woodhouse, linux-arm-kernel
On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> On Fri, 12 Sep 2014 08:45:50 +0800
>
> My bad, capabilities might not be the appropriate word in this context.
> I meant ECC algorithm strength, and that's exactly the purpose of
> nandbiterrs testsuite: insert bitflips in in-band data until the MTD
> layer complains about uncorrectable errors.
okay.
> This test validates what's returned by ecc_strength file in sysfs
> (which in turn is specified by the NAND controller when initializing
> the NAND chip).
>
> Doing this should not imply knowing the ECC algorithm in use in the
> NAND controller or the layout used to store data on NAND.
the difficulty is that the ECC parity area can be not byte aligned.
As I ever said, it is hard to implement the two hooks.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-13 15:36 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-13 15:36 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> On Fri, 12 Sep 2014 08:45:50 +0800
>
> My bad, capabilities might not be the appropriate word in this context.
> I meant ECC algorithm strength, and that's exactly the purpose of
> nandbiterrs testsuite: insert bitflips in in-band data until the MTD
> layer complains about uncorrectable errors.
okay.
> This test validates what's returned by ecc_strength file in sysfs
> (which in turn is specified by the NAND controller when initializing
> the NAND chip).
>
> Doing this should not imply knowing the ECC algorithm in use in the
> NAND controller or the layout used to store data on NAND.
the difficulty is that the ECC parity area can be not byte aligned.
As I ever said, it is hard to implement the two hooks.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-13 15:36 ` Huang Shijie
(?)
@ 2014-09-13 17:38 ` Brian Norris
-1 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2014-09-13 17:38 UTC (permalink / raw)
To: Huang Shijie
Cc: Boris BREZILLON, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > This test validates what's returned by ecc_strength file in sysfs
> > (which in turn is specified by the NAND controller when initializing
> > the NAND chip).
> >
> > Doing this should not imply knowing the ECC algorithm in use in the
> > NAND controller or the layout used to store data on NAND.
> the difficulty is that the ECC parity area can be not byte aligned.
Is there a problem with just rounding up to the nearest byte alignment
and ignoring the few bits that are wasted?
> As I ever said, it is hard to implement the two hooks.
"Hard" doesn't mean we shouldn't. I really would like to encourage more
NAND drivers to be programmed against the expected MTD behavior -- that
(if possible with the given hardware) they can pass the MTD tests
(drivers/mtd/tests/*).
Brian
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-13 17:38 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2014-09-13 17:38 UTC (permalink / raw)
To: Huang Shijie
Cc: Boris BREZILLON, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Huang Shijie, David Woodhouse,
linux-arm-kernel
On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > This test validates what's returned by ecc_strength file in sysfs
> > (which in turn is specified by the NAND controller when initializing
> > the NAND chip).
> >
> > Doing this should not imply knowing the ECC algorithm in use in the
> > NAND controller or the layout used to store data on NAND.
> the difficulty is that the ECC parity area can be not byte aligned.
Is there a problem with just rounding up to the nearest byte alignment
and ignoring the few bits that are wasted?
> As I ever said, it is hard to implement the two hooks.
"Hard" doesn't mean we shouldn't. I really would like to encourage more
NAND drivers to be programmed against the expected MTD behavior -- that
(if possible with the given hardware) they can pass the MTD tests
(drivers/mtd/tests/*).
Brian
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-13 17:38 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2014-09-13 17:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > This test validates what's returned by ecc_strength file in sysfs
> > (which in turn is specified by the NAND controller when initializing
> > the NAND chip).
> >
> > Doing this should not imply knowing the ECC algorithm in use in the
> > NAND controller or the layout used to store data on NAND.
> the difficulty is that the ECC parity area can be not byte aligned.
Is there a problem with just rounding up to the nearest byte alignment
and ignoring the few bits that are wasted?
> As I ever said, it is hard to implement the two hooks.
"Hard" doesn't mean we shouldn't. I really would like to encourage more
NAND drivers to be programmed against the expected MTD behavior -- that
(if possible with the given hardware) they can pass the MTD tests
(drivers/mtd/tests/*).
Brian
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-13 17:38 ` Brian Norris
(?)
@ 2014-09-14 14:07 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-14 14:07 UTC (permalink / raw)
To: Brian Norris
Cc: Huang Shijie, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
Hi Brian, Huang,
On Sat, 13 Sep 2014 10:38:41 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
>
> > As I ever said, it is hard to implement the two hooks.
>
> "Hard" doesn't mean we shouldn't. I really would like to encourage more
> NAND drivers to be programmed against the expected MTD behavior -- that
> (if possible with the given hardware) they can pass the MTD tests
> (drivers/mtd/tests/*).
Here is a draft for a gpmi_move_bits function we could use to move bits
(not bytes :-) from one memory region to another:
void gpmi_move_bits(u8 *dst, size_t dst_bit_off,
const u8 *src, size_t src_bit_off,
size_t nbits)
{
size_t i;
size_t nbytes;
u32 src_byte = 0;
src += src_bit_off / 8;
src_bit_off %= 8;
dst += dst_bit_off / 8;
dst_bit_off %= 8;
if (src_bit_off) {
src_byte = src[0] >> src_bit_off;
nbits -= 8 - src_bit_off;
src++;
}
nbytes = nbits / 8;
if (src_bit_off <= dst_bit_off) {
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += (8 - dst_bit_off);
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
} else if (nbytes) {
src_byte |= src[0] << (8 - src_bit_off);
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += dst_bit_off;
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
nbytes--;
src++;
if (src_bit_off > 7) {
src_bit_off -= 8;
dst[0] = src_byte;
dst++;
src_byte >>= 8;
}
}
if (!src_bit_off && !dst_bit_off) {
if (nbytes)
memcpy(dst, src, nbytes);
} else {
for (i = 0; i < nbytes; i++) {
src_byte |= src[i] << (8 - src_bit_off);
dst[i] = src_byte;
src_byte >>= 8;
}
}
dst += nbytes;
src += nbytes;
nbits %= 8;
if (!nbits && !src_bit_off)
return;
if (nbits)
src_byte |= (*src & GENMASK(nbits - 1, 0)) <<
((8 - src_bit_off) % 8);
nbits += (8 - src_bit_off) % 8;
if (dst_bit_off)
src_byte = (src_byte << dst_bit_off) |
(*dst & GENMASK(dst_bit_off - 1, 0));
nbits += dst_bit_off;
if (nbits % 8)
src_byte |= (dst[nbits / 8] & GENMASK(7, nbits % 8)) <<
(nbits / 8);
nbytes = DIV_ROUND_UP(nbits, 8);
for (i = 0; i < nbytes; i++) {
dst[i] = src_byte;
src_byte >>= 8;
}
}
I haven't tested it, and I think there is room for optimization.
My point is that performance is not a key aspect of raw functions
(those are often used by testing and debugging tools), hence we could
rely on this move_bits function to address the ECC bit alignment
problem.
Let me know what's your opinion on this approach.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-14 14:07 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-14 14:07 UTC (permalink / raw)
To: Brian Norris
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Huang Shijie, Huang Shijie, David Woodhouse, linux-arm-kernel
Hi Brian, Huang,
On Sat, 13 Sep 2014 10:38:41 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
>
> > As I ever said, it is hard to implement the two hooks.
>
> "Hard" doesn't mean we shouldn't. I really would like to encourage more
> NAND drivers to be programmed against the expected MTD behavior -- that
> (if possible with the given hardware) they can pass the MTD tests
> (drivers/mtd/tests/*).
Here is a draft for a gpmi_move_bits function we could use to move bits
(not bytes :-) from one memory region to another:
void gpmi_move_bits(u8 *dst, size_t dst_bit_off,
const u8 *src, size_t src_bit_off,
size_t nbits)
{
size_t i;
size_t nbytes;
u32 src_byte = 0;
src += src_bit_off / 8;
src_bit_off %= 8;
dst += dst_bit_off / 8;
dst_bit_off %= 8;
if (src_bit_off) {
src_byte = src[0] >> src_bit_off;
nbits -= 8 - src_bit_off;
src++;
}
nbytes = nbits / 8;
if (src_bit_off <= dst_bit_off) {
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += (8 - dst_bit_off);
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
} else if (nbytes) {
src_byte |= src[0] << (8 - src_bit_off);
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += dst_bit_off;
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
nbytes--;
src++;
if (src_bit_off > 7) {
src_bit_off -= 8;
dst[0] = src_byte;
dst++;
src_byte >>= 8;
}
}
if (!src_bit_off && !dst_bit_off) {
if (nbytes)
memcpy(dst, src, nbytes);
} else {
for (i = 0; i < nbytes; i++) {
src_byte |= src[i] << (8 - src_bit_off);
dst[i] = src_byte;
src_byte >>= 8;
}
}
dst += nbytes;
src += nbytes;
nbits %= 8;
if (!nbits && !src_bit_off)
return;
if (nbits)
src_byte |= (*src & GENMASK(nbits - 1, 0)) <<
((8 - src_bit_off) % 8);
nbits += (8 - src_bit_off) % 8;
if (dst_bit_off)
src_byte = (src_byte << dst_bit_off) |
(*dst & GENMASK(dst_bit_off - 1, 0));
nbits += dst_bit_off;
if (nbits % 8)
src_byte |= (dst[nbits / 8] & GENMASK(7, nbits % 8)) <<
(nbits / 8);
nbytes = DIV_ROUND_UP(nbits, 8);
for (i = 0; i < nbytes; i++) {
dst[i] = src_byte;
src_byte >>= 8;
}
}
I haven't tested it, and I think there is room for optimization.
My point is that performance is not a key aspect of raw functions
(those are often used by testing and debugging tools), hence we could
rely on this move_bits function to address the ECC bit alignment
problem.
Let me know what's your opinion on this approach.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-14 14:07 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-14 14:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Brian, Huang,
On Sat, 13 Sep 2014 10:38:41 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
>
> > As I ever said, it is hard to implement the two hooks.
>
> "Hard" doesn't mean we shouldn't. I really would like to encourage more
> NAND drivers to be programmed against the expected MTD behavior -- that
> (if possible with the given hardware) they can pass the MTD tests
> (drivers/mtd/tests/*).
Here is a draft for a gpmi_move_bits function we could use to move bits
(not bytes :-) from one memory region to another:
void gpmi_move_bits(u8 *dst, size_t dst_bit_off,
const u8 *src, size_t src_bit_off,
size_t nbits)
{
size_t i;
size_t nbytes;
u32 src_byte = 0;
src += src_bit_off / 8;
src_bit_off %= 8;
dst += dst_bit_off / 8;
dst_bit_off %= 8;
if (src_bit_off) {
src_byte = src[0] >> src_bit_off;
nbits -= 8 - src_bit_off;
src++;
}
nbytes = nbits / 8;
if (src_bit_off <= dst_bit_off) {
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += (8 - dst_bit_off);
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
} else if (nbytes) {
src_byte |= src[0] << (8 - src_bit_off);
dst[0] &= GENMASK(dst_bit_off - 1, 0);
dst[0] |= src_byte << dst_bit_off;
src_bit_off += dst_bit_off;
src_byte >>= (8 - dst_bit_off);
dst_bit_off = 0;
dst++;
nbytes--;
src++;
if (src_bit_off > 7) {
src_bit_off -= 8;
dst[0] = src_byte;
dst++;
src_byte >>= 8;
}
}
if (!src_bit_off && !dst_bit_off) {
if (nbytes)
memcpy(dst, src, nbytes);
} else {
for (i = 0; i < nbytes; i++) {
src_byte |= src[i] << (8 - src_bit_off);
dst[i] = src_byte;
src_byte >>= 8;
}
}
dst += nbytes;
src += nbytes;
nbits %= 8;
if (!nbits && !src_bit_off)
return;
if (nbits)
src_byte |= (*src & GENMASK(nbits - 1, 0)) <<
((8 - src_bit_off) % 8);
nbits += (8 - src_bit_off) % 8;
if (dst_bit_off)
src_byte = (src_byte << dst_bit_off) |
(*dst & GENMASK(dst_bit_off - 1, 0));
nbits += dst_bit_off;
if (nbits % 8)
src_byte |= (dst[nbits / 8] & GENMASK(7, nbits % 8)) <<
(nbits / 8);
nbytes = DIV_ROUND_UP(nbits, 8);
for (i = 0; i < nbytes; i++) {
dst[i] = src_byte;
src_byte >>= 8;
}
}
I haven't tested it, and I think there is room for optimization.
My point is that performance is not a key aspect of raw functions
(those are often used by testing and debugging tools), hence we could
rely on this move_bits function to address the ECC bit alignment
problem.
Let me know what's your opinion on this approach.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-13 17:38 ` Brian Norris
(?)
@ 2014-09-15 14:43 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-15 14:43 UTC (permalink / raw)
To: Brian Norris
Cc: Boris BREZILLON, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
I feel a little confused with the two hooks.
does the ecc->write_page_raw need to write the ECC parity data?
The page's layout after an ecc->write_page will look like this:
(Block Mark)
| |
| D |
|<---->|
V V
+---+----------+-+----------+-+----------+-+----------+-+
| M | data |E| data |E| data |E| data |E|
+---+----------+-+----------+-+----------+-+----------+-+
What will the page's layout look like after the ecc->write_page_raw?
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-15 14:43 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-15 14:43 UTC (permalink / raw)
To: Brian Norris
Cc: Boris BREZILLON, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Huang Shijie, David Woodhouse,
linux-arm-kernel
On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
I feel a little confused with the two hooks.
does the ecc->write_page_raw need to write the ECC parity data?
The page's layout after an ecc->write_page will look like this:
(Block Mark)
| |
| D |
|<---->|
V V
+---+----------+-+----------+-+----------+-+----------+-+
| M | data |E| data |E| data |E| data |E|
+---+----------+-+----------+-+----------+-+----------+-+
What will the page's layout look like after the ecc->write_page_raw?
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-15 14:43 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-15 14:43 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > This test validates what's returned by ecc_strength file in sysfs
> > > (which in turn is specified by the NAND controller when initializing
> > > the NAND chip).
> > >
> > > Doing this should not imply knowing the ECC algorithm in use in the
> > > NAND controller or the layout used to store data on NAND.
> > the difficulty is that the ECC parity area can be not byte aligned.
>
> Is there a problem with just rounding up to the nearest byte alignment
> and ignoring the few bits that are wasted?
I feel a little confused with the two hooks.
does the ecc->write_page_raw need to write the ECC parity data?
The page's layout after an ecc->write_page will look like this:
(Block Mark)
| |
| D |
|<---->|
V V
+---+----------+-+----------+-+----------+-+----------+-+
| M | data |E| data |E| data |E| data |E|
+---+----------+-+----------+-+----------+-+----------+-+
What will the page's layout look like after the ecc->write_page_raw?
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-15 14:43 ` Huang Shijie
(?)
@ 2014-09-15 20:12 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-15 20:12 UTC (permalink / raw)
To: Huang Shijie
Cc: Brian Norris, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
On Mon, 15 Sep 2014 22:43:02 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > This test validates what's returned by ecc_strength file in sysfs
> > > > (which in turn is specified by the NAND controller when initializing
> > > > the NAND chip).
> > > >
> > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > NAND controller or the layout used to store data on NAND.
> > > the difficulty is that the ECC parity area can be not byte aligned.
> >
> > Is there a problem with just rounding up to the nearest byte alignment
> > and ignoring the few bits that are wasted?
>
> I feel a little confused with the two hooks.
>
> does the ecc->write_page_raw need to write the ECC parity data?
Depending on the oob_required argument, it might be allowed to
overwrite the ECC bytes even if this implies breaking page reliability
(which is exactly what's expected).
When using raw write with with oob write option the writer should take
care of regenerating ECC bytes (which you said was impossible in GPMI
case) or copying them from a previous raw read.
Here is a real example of what one could test with raw write + oob:
1) read a page in raw mode
2) flip some bits in the generated ECC bytes (or what you references as
parity data) (this case can actually happen in real life)
3) write the modified page in raw mode
4) read back the same page in normal and check that ECC correction still
works as expected
>
> The page's layout after an ecc->write_page will look like this:
> (Block Mark)
> | |
> | D |
> |<---->|
> V V
> +---+----------+-+----------+-+----------+-+----------+-+
> | M | data |E| data |E| data |E| data |E|
> +---+----------+-+----------+-+----------+-+----------+-+
>
> What will the page's layout look like after the ecc->write_page_raw?
The same, but the user won't see this specific layout when reading a
page in raw mode, and won't have to bother about disposing data a
described above when writing in raw mode.
Here is the layout as seen by an MTD user, even though the real one is
the one you previously described.
(Block Mark)
|
|
|
V
+------------------------------------------+-----+-------+---+
| data | M | E | R |
+------------------------------------------+-----+-------+---+
<------OOB------->
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-15 20:12 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-15 20:12 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Huang Shijie, Brian Norris, David Woodhouse, linux-arm-kernel
On Mon, 15 Sep 2014 22:43:02 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > This test validates what's returned by ecc_strength file in sysfs
> > > > (which in turn is specified by the NAND controller when initializing
> > > > the NAND chip).
> > > >
> > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > NAND controller or the layout used to store data on NAND.
> > > the difficulty is that the ECC parity area can be not byte aligned.
> >
> > Is there a problem with just rounding up to the nearest byte alignment
> > and ignoring the few bits that are wasted?
>
> I feel a little confused with the two hooks.
>
> does the ecc->write_page_raw need to write the ECC parity data?
Depending on the oob_required argument, it might be allowed to
overwrite the ECC bytes even if this implies breaking page reliability
(which is exactly what's expected).
When using raw write with with oob write option the writer should take
care of regenerating ECC bytes (which you said was impossible in GPMI
case) or copying them from a previous raw read.
Here is a real example of what one could test with raw write + oob:
1) read a page in raw mode
2) flip some bits in the generated ECC bytes (or what you references as
parity data) (this case can actually happen in real life)
3) write the modified page in raw mode
4) read back the same page in normal and check that ECC correction still
works as expected
>
> The page's layout after an ecc->write_page will look like this:
> (Block Mark)
> | |
> | D |
> |<---->|
> V V
> +---+----------+-+----------+-+----------+-+----------+-+
> | M | data |E| data |E| data |E| data |E|
> +---+----------+-+----------+-+----------+-+----------+-+
>
> What will the page's layout look like after the ecc->write_page_raw?
The same, but the user won't see this specific layout when reading a
page in raw mode, and won't have to bother about disposing data a
described above when writing in raw mode.
Here is the layout as seen by an MTD user, even though the real one is
the one you previously described.
(Block Mark)
|
|
|
V
+------------------------------------------+-----+-------+---+
| data | M | E | R |
+------------------------------------------+-----+-------+---+
<------OOB------->
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-15 20:12 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-15 20:12 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 15 Sep 2014 22:43:02 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > This test validates what's returned by ecc_strength file in sysfs
> > > > (which in turn is specified by the NAND controller when initializing
> > > > the NAND chip).
> > > >
> > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > NAND controller or the layout used to store data on NAND.
> > > the difficulty is that the ECC parity area can be not byte aligned.
> >
> > Is there a problem with just rounding up to the nearest byte alignment
> > and ignoring the few bits that are wasted?
>
> I feel a little confused with the two hooks.
>
> does the ecc->write_page_raw need to write the ECC parity data?
Depending on the oob_required argument, it might be allowed to
overwrite the ECC bytes even if this implies breaking page reliability
(which is exactly what's expected).
When using raw write with with oob write option the writer should take
care of regenerating ECC bytes (which you said was impossible in GPMI
case) or copying them from a previous raw read.
Here is a real example of what one could test with raw write + oob:
1) read a page in raw mode
2) flip some bits in the generated ECC bytes (or what you references as
parity data) (this case can actually happen in real life)
3) write the modified page in raw mode
4) read back the same page in normal and check that ECC correction still
works as expected
>
> The page's layout after an ecc->write_page will look like this:
> (Block Mark)
> | |
> | D |
> |<---->|
> V V
> +---+----------+-+----------+-+----------+-+----------+-+
> | M | data |E| data |E| data |E| data |E|
> +---+----------+-+----------+-+----------+-+----------+-+
>
> What will the page's layout look like after the ecc->write_page_raw?
The same, but the user won't see this specific layout when reading a
page in raw mode, and won't have to bother about disposing data a
described above when writing in raw mode.
Here is the layout as seen by an MTD user, even though the real one is
the one you previously described.
(Block Mark)
|
|
|
V
+------------------------------------------+-----+-------+---+
| data | M | E | R |
+------------------------------------------+-----+-------+---+
<------OOB------->
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-15 20:12 ` Boris BREZILLON
(?)
@ 2014-09-17 15:26 ` Huang Shijie
-1 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-17 15:26 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Brian Norris, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> On Mon, 15 Sep 2014 22:43:02 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > (which in turn is specified by the NAND controller when initializing
> > > > > the NAND chip).
> > > > >
> > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > NAND controller or the layout used to store data on NAND.
> > > > the difficulty is that the ECC parity area can be not byte aligned.
> > >
> > > Is there a problem with just rounding up to the nearest byte alignment
> > > and ignoring the few bits that are wasted?
> >
> > I feel a little confused with the two hooks.
> >
> > does the ecc->write_page_raw need to write the ECC parity data?
>
> Depending on the oob_required argument, it might be allowed to
> overwrite the ECC bytes even if this implies breaking page reliability
> (which is exactly what's expected).
>
> When using raw write with with oob write option the writer should take
> care of regenerating ECC bytes (which you said was impossible in GPMI
> case) or copying them from a previous raw read.
Thanks for the explanation.
If we do not write the OOB, should we write the ECC bytes?
The hooks should comment clearly about how to implement them :(
>
> Here is a real example of what one could test with raw write + oob:
> 1) read a page in raw mode
> 2) flip some bits in the generated ECC bytes (or what you references as
> parity data) (this case can actually happen in real life)
> 3) write the modified page in raw mode
> 4) read back the same page in normal and check that ECC correction still
> works as expected
the nandbiterr test mode does the test as above.
But i think the multi-writes to the same page should occur only for the
SLC nand.
I will read your new patch set carefully in this weekend.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-17 15:26 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-17 15:26 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Huang Shijie, Brian Norris, David Woodhouse, linux-arm-kernel
On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> On Mon, 15 Sep 2014 22:43:02 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > (which in turn is specified by the NAND controller when initializing
> > > > > the NAND chip).
> > > > >
> > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > NAND controller or the layout used to store data on NAND.
> > > > the difficulty is that the ECC parity area can be not byte aligned.
> > >
> > > Is there a problem with just rounding up to the nearest byte alignment
> > > and ignoring the few bits that are wasted?
> >
> > I feel a little confused with the two hooks.
> >
> > does the ecc->write_page_raw need to write the ECC parity data?
>
> Depending on the oob_required argument, it might be allowed to
> overwrite the ECC bytes even if this implies breaking page reliability
> (which is exactly what's expected).
>
> When using raw write with with oob write option the writer should take
> care of regenerating ECC bytes (which you said was impossible in GPMI
> case) or copying them from a previous raw read.
Thanks for the explanation.
If we do not write the OOB, should we write the ECC bytes?
The hooks should comment clearly about how to implement them :(
>
> Here is a real example of what one could test with raw write + oob:
> 1) read a page in raw mode
> 2) flip some bits in the generated ECC bytes (or what you references as
> parity data) (this case can actually happen in real life)
> 3) write the modified page in raw mode
> 4) read back the same page in normal and check that ECC correction still
> works as expected
the nandbiterr test mode does the test as above.
But i think the multi-writes to the same page should occur only for the
SLC nand.
I will read your new patch set carefully in this weekend.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-17 15:26 ` Huang Shijie
0 siblings, 0 replies; 60+ messages in thread
From: Huang Shijie @ 2014-09-17 15:26 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> On Mon, 15 Sep 2014 22:43:02 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
>
> > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > (which in turn is specified by the NAND controller when initializing
> > > > > the NAND chip).
> > > > >
> > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > NAND controller or the layout used to store data on NAND.
> > > > the difficulty is that the ECC parity area can be not byte aligned.
> > >
> > > Is there a problem with just rounding up to the nearest byte alignment
> > > and ignoring the few bits that are wasted?
> >
> > I feel a little confused with the two hooks.
> >
> > does the ecc->write_page_raw need to write the ECC parity data?
>
> Depending on the oob_required argument, it might be allowed to
> overwrite the ECC bytes even if this implies breaking page reliability
> (which is exactly what's expected).
>
> When using raw write with with oob write option the writer should take
> care of regenerating ECC bytes (which you said was impossible in GPMI
> case) or copying them from a previous raw read.
Thanks for the explanation.
If we do not write the OOB, should we write the ECC bytes?
The hooks should comment clearly about how to implement them :(
>
> Here is a real example of what one could test with raw write + oob:
> 1) read a page in raw mode
> 2) flip some bits in the generated ECC bytes (or what you references as
> parity data) (this case can actually happen in real life)
> 3) write the modified page in raw mode
> 4) read back the same page in normal and check that ECC correction still
> works as expected
the nandbiterr test mode does the test as above.
But i think the multi-writes to the same page should occur only for the
SLC nand.
I will read your new patch set carefully in this weekend.
thanks
Huang Shijie
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-17 15:26 ` Huang Shijie
(?)
@ 2014-09-17 18:16 ` Boris BREZILLON
-1 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-17 18:16 UTC (permalink / raw)
To: Huang Shijie
Cc: Brian Norris, Huang Shijie, Mike Voytovich, linux-kernel,
Huang Shijie, linux-mtd, Roy Lee, David Woodhouse,
linux-arm-kernel
Hi Huang,
On Wed, 17 Sep 2014 23:26:11 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> > On Mon, 15 Sep 2014 22:43:02 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > > (which in turn is specified by the NAND controller when initializing
> > > > > > the NAND chip).
> > > > > >
> > > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > > NAND controller or the layout used to store data on NAND.
> > > > > the difficulty is that the ECC parity area can be not byte aligned.
> > > >
> > > > Is there a problem with just rounding up to the nearest byte alignment
> > > > and ignoring the few bits that are wasted?
> > >
> > > I feel a little confused with the two hooks.
> > >
> > > does the ecc->write_page_raw need to write the ECC parity data?
> >
> > Depending on the oob_required argument, it might be allowed to
> > overwrite the ECC bytes even if this implies breaking page reliability
> > (which is exactly what's expected).
> >
> > When using raw write with with oob write option the writer should take
> > care of regenerating ECC bytes (which you said was impossible in GPMI
> > case) or copying them from a previous raw read.
> Thanks for the explanation.
>
> If we do not write the OOB, should we write the ECC bytes?
No we shouldn't, and if you take a look at my new proposal you'll see
that I always write ECC bytes even when oob_required is not required,
but this is not a problem because in this case the nand_base code has
already set the oob area to 0xff and, if I'm correct, writing 0xff is
just like leaving the area unwritten.
Of course leaving the ECC bytes to 0xff after a page erase will
certainly trigger ECC errors when reading the page back in normal mode,
but this is the MTD user responsibility to know what he's doing.
> The hooks should comment clearly about how to implement them :(
Absolutely, and if we all agree on the expected behavior, I'll be happy
to document it ;-).
>
> >
> > Here is a real example of what one could test with raw write + oob:
> > 1) read a page in raw mode
> > 2) flip some bits in the generated ECC bytes (or what you references as
> > parity data) (this case can actually happen in real life)
> > 3) write the modified page in raw mode
> > 4) read back the same page in normal and check that ECC correction still
> > works as expected
> the nandbiterr test mode does the test as above.
> But i think the multi-writes to the same page should occur only for the
> SLC nand.
I already have a patch for that :-):
http://code.bulix.org/f69wuu-87021
This patch is erasing the NAND block between each bitflip insertion so
that it can work on MLC nandflashes too.
Anyway, IMHO this nandbiterrs testsuite should be implemented in a
user-space tool (which could be part of mtd-utils) because we already
have all the primitives we need to do it from there (ioctls to access
NAND chips in raw mode).
>
> I will read your new patch set carefully in this weekend.
Thanks.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-17 18:16 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-17 18:16 UTC (permalink / raw)
To: Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Huang Shijie, Brian Norris, David Woodhouse, linux-arm-kernel
Hi Huang,
On Wed, 17 Sep 2014 23:26:11 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> > On Mon, 15 Sep 2014 22:43:02 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > > (which in turn is specified by the NAND controller when initializing
> > > > > > the NAND chip).
> > > > > >
> > > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > > NAND controller or the layout used to store data on NAND.
> > > > > the difficulty is that the ECC parity area can be not byte aligned.
> > > >
> > > > Is there a problem with just rounding up to the nearest byte alignment
> > > > and ignoring the few bits that are wasted?
> > >
> > > I feel a little confused with the two hooks.
> > >
> > > does the ecc->write_page_raw need to write the ECC parity data?
> >
> > Depending on the oob_required argument, it might be allowed to
> > overwrite the ECC bytes even if this implies breaking page reliability
> > (which is exactly what's expected).
> >
> > When using raw write with with oob write option the writer should take
> > care of regenerating ECC bytes (which you said was impossible in GPMI
> > case) or copying them from a previous raw read.
> Thanks for the explanation.
>
> If we do not write the OOB, should we write the ECC bytes?
No we shouldn't, and if you take a look at my new proposal you'll see
that I always write ECC bytes even when oob_required is not required,
but this is not a problem because in this case the nand_base code has
already set the oob area to 0xff and, if I'm correct, writing 0xff is
just like leaving the area unwritten.
Of course leaving the ECC bytes to 0xff after a page erase will
certainly trigger ECC errors when reading the page back in normal mode,
but this is the MTD user responsibility to know what he's doing.
> The hooks should comment clearly about how to implement them :(
Absolutely, and if we all agree on the expected behavior, I'll be happy
to document it ;-).
>
> >
> > Here is a real example of what one could test with raw write + oob:
> > 1) read a page in raw mode
> > 2) flip some bits in the generated ECC bytes (or what you references as
> > parity data) (this case can actually happen in real life)
> > 3) write the modified page in raw mode
> > 4) read back the same page in normal and check that ECC correction still
> > works as expected
> the nandbiterr test mode does the test as above.
> But i think the multi-writes to the same page should occur only for the
> SLC nand.
I already have a patch for that :-):
http://code.bulix.org/f69wuu-87021
This patch is erasing the NAND block between each bitflip insertion so
that it can work on MLC nandflashes too.
Anyway, IMHO this nandbiterrs testsuite should be implemented in a
user-space tool (which could be part of mtd-utils) because we already
have all the primitives we need to do it from there (ioctls to access
NAND chips in raw mode).
>
> I will read your new patch set carefully in this weekend.
Thanks.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-17 18:16 ` Boris BREZILLON
0 siblings, 0 replies; 60+ messages in thread
From: Boris BREZILLON @ 2014-09-17 18:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Huang,
On Wed, 17 Sep 2014 23:26:11 +0800
Huang Shijie <shijie8@gmail.com> wrote:
> On Mon, Sep 15, 2014 at 10:12:10PM +0200, Boris BREZILLON wrote:
> > On Mon, 15 Sep 2014 22:43:02 +0800
> > Huang Shijie <shijie8@gmail.com> wrote:
> >
> > > On Sat, Sep 13, 2014 at 10:38:41AM -0700, Brian Norris wrote:
> > > > On Sat, Sep 13, 2014 at 11:36:24PM +0800, Huang Shijie wrote:
> > > > > On Fri, Sep 12, 2014 at 02:30:50PM +0200, Boris BREZILLON wrote:
> > > > > > This test validates what's returned by ecc_strength file in sysfs
> > > > > > (which in turn is specified by the NAND controller when initializing
> > > > > > the NAND chip).
> > > > > >
> > > > > > Doing this should not imply knowing the ECC algorithm in use in the
> > > > > > NAND controller or the layout used to store data on NAND.
> > > > > the difficulty is that the ECC parity area can be not byte aligned.
> > > >
> > > > Is there a problem with just rounding up to the nearest byte alignment
> > > > and ignoring the few bits that are wasted?
> > >
> > > I feel a little confused with the two hooks.
> > >
> > > does the ecc->write_page_raw need to write the ECC parity data?
> >
> > Depending on the oob_required argument, it might be allowed to
> > overwrite the ECC bytes even if this implies breaking page reliability
> > (which is exactly what's expected).
> >
> > When using raw write with with oob write option the writer should take
> > care of regenerating ECC bytes (which you said was impossible in GPMI
> > case) or copying them from a previous raw read.
> Thanks for the explanation.
>
> If we do not write the OOB, should we write the ECC bytes?
No we shouldn't, and if you take a look at my new proposal you'll see
that I always write ECC bytes even when oob_required is not required,
but this is not a problem because in this case the nand_base code has
already set the oob area to 0xff and, if I'm correct, writing 0xff is
just like leaving the area unwritten.
Of course leaving the ECC bytes to 0xff after a page erase will
certainly trigger ECC errors when reading the page back in normal mode,
but this is the MTD user responsibility to know what he's doing.
> The hooks should comment clearly about how to implement them :(
Absolutely, and if we all agree on the expected behavior, I'll be happy
to document it ;-).
>
> >
> > Here is a real example of what one could test with raw write + oob:
> > 1) read a page in raw mode
> > 2) flip some bits in the generated ECC bytes (or what you references as
> > parity data) (this case can actually happen in real life)
> > 3) write the modified page in raw mode
> > 4) read back the same page in normal and check that ECC correction still
> > works as expected
> the nandbiterr test mode does the test as above.
> But i think the multi-writes to the same page should occur only for the
> SLC nand.
I already have a patch for that :-):
http://code.bulix.org/f69wuu-87021
This patch is erasing the NAND block between each bitflip insertion so
that it can work on MLC nandflashes too.
Anyway, IMHO this nandbiterrs testsuite should be implemented in a
user-space tool (which could be part of mtd-utils) because we already
have all the primitives we need to do it from there (ioctls to access
NAND chips in raw mode).
>
> I will read your new patch set carefully in this weekend.
Thanks.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* RE: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-11 12:36 ` Boris BREZILLON
(?)
@ 2014-09-29 1:22 ` Iwo Mergler
-1 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-09-29 1:22 UTC (permalink / raw)
To: Boris BREZILLON, Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
On Thu, 11 Sep 2014 22:36:16 +1000
Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller
> case is impossible because raw write function store the data as if
> there were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
Hi Boris,
just as an aside, only the incremental bit errors test in nandbiterrs
positively requires raw data write.
The overwrite test (re-write the same page data repeatedly without
erase), only uses raw access because I was lazy. A normal ECC write
would do just as well.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
* RE: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-29 1:22 ` Iwo Mergler
0 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-09-29 1:22 UTC (permalink / raw)
To: Boris BREZILLON, Huang Shijie
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, David Woodhouse, linux-arm-kernel
On Thu, 11 Sep 2014 22:36:16 +1000
Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller
> case is impossible because raw write function store the data as if
> there were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
Hi Boris,
just as an aside, only the incremental bit errors test in nandbiterrs
positively requires raw data write.
The overwrite test (re-write the same page data repeatedly without
erase), only uses raw access because I was lazy. A normal ECC write
would do just as well.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-29 1:22 ` Iwo Mergler
0 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-09-29 1:22 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 11 Sep 2014 22:36:16 +1000
Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
>
> Well, I don't know about freescale specific tools, but at least I have
> an example with mtd_nandbiterrs module.
> This module is assuming it can write only the data part of a NAND page
> without modifying the OOB area (see [1]), which in GPMI controller
> case is impossible because raw write function store the data as if
> there were no specific scheme, while there is one:
> (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
>
Hi Boris,
just as an aside, only the incremental bit errors test in nandbiterrs
positively requires raw data write.
The overwrite test (re-write the same page data repeatedly without
erase), only uses raw access because I was lazy. A normal ECC write
would do just as well.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-29 1:22 ` Iwo Mergler
(?)
@ 2014-09-30 8:04 ` Boris Brezillon
-1 siblings, 0 replies; 60+ messages in thread
From: Boris Brezillon @ 2014-09-30 8:04 UTC (permalink / raw)
To: Iwo Mergler
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
Hi Iwo,
On Mon, 29 Sep 2014 11:22:11 +1000
Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
> On Thu, 11 Sep 2014 22:36:16 +1000
> Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller
> > case is impossible because raw write function store the data as if
> > there were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
>
> Hi Boris,
>
>
> just as an aside, only the incremental bit errors test in nandbiterrs
> positively requires raw data write.
>
> The overwrite test (re-write the same page data repeatedly without
> erase), only uses raw access because I was lazy. A normal ECC write
> would do just as well.
Okay.
Anyway, the test I'm really interested in is the incremental bit errors
test :-).
BTW, any reason you chose to implement this test/testsuite as a
module ?
>From my understanding (and tell me if I'm wrong) we could do the same
from user-space.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-30 8:04 ` Boris Brezillon
0 siblings, 0 replies; 60+ messages in thread
From: Boris Brezillon @ 2014-09-30 8:04 UTC (permalink / raw)
To: Iwo Mergler
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, Huang Shijie, David Woodhouse, linux-arm-kernel
Hi Iwo,
On Mon, 29 Sep 2014 11:22:11 +1000
Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
> On Thu, 11 Sep 2014 22:36:16 +1000
> Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller
> > case is impossible because raw write function store the data as if
> > there were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
>
> Hi Boris,
>
>
> just as an aside, only the incremental bit errors test in nandbiterrs
> positively requires raw data write.
>
> The overwrite test (re-write the same page data repeatedly without
> erase), only uses raw access because I was lazy. A normal ECC write
> would do just as well.
Okay.
Anyway, the test I'm really interested in is the incremental bit errors
test :-).
BTW, any reason you chose to implement this test/testsuite as a
module ?
From my understanding (and tell me if I'm wrong) we could do the same
from user-space.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-09-30 8:04 ` Boris Brezillon
0 siblings, 0 replies; 60+ messages in thread
From: Boris Brezillon @ 2014-09-30 8:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi Iwo,
On Mon, 29 Sep 2014 11:22:11 +1000
Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
> On Thu, 11 Sep 2014 22:36:16 +1000
> Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> >
> > Well, I don't know about freescale specific tools, but at least I have
> > an example with mtd_nandbiterrs module.
> > This module is assuming it can write only the data part of a NAND page
> > without modifying the OOB area (see [1]), which in GPMI controller
> > case is impossible because raw write function store the data as if
> > there were no specific scheme, while there is one:
> > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> >
>
> Hi Boris,
>
>
> just as an aside, only the incremental bit errors test in nandbiterrs
> positively requires raw data write.
>
> The overwrite test (re-write the same page data repeatedly without
> erase), only uses raw access because I was lazy. A normal ECC write
> would do just as well.
Okay.
Anyway, the test I'm really interested in is the incremental bit errors
test :-).
BTW, any reason you chose to implement this test/testsuite as a
module ?
>From my understanding (and tell me if I'm wrong) we could do the same
from user-space.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* RE: [PATCH] mtd: nand: gpmi: add proper raw access support
2014-09-30 8:04 ` Boris Brezillon
(?)
@ 2014-10-02 6:52 ` Iwo Mergler
-1 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-10-02 6:52 UTC (permalink / raw)
To: Boris Brezillon
Cc: Huang Shijie, Mike Voytovich, linux-kernel, Huang Shijie,
linux-mtd, Roy Lee, Brian Norris, David Woodhouse,
linux-arm-kernel
On Tue, 30 Sep 2014 18:04:15 +1000
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> Hi Iwo,
>
> On Mon, 29 Sep 2014 11:22:11 +1000
> Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
>
> > On Thu, 11 Sep 2014 22:36:16 +1000
> > Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> > >
> > > Well, I don't know about freescale specific tools, but at least I
> > > have an example with mtd_nandbiterrs module.
> > > This module is assuming it can write only the data part of a NAND
> > > page without modifying the OOB area (see [1]), which in GPMI
> > > controller case is impossible because raw write function store
> > > the data as if there were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> >
> > Hi Boris,
> >
> >
> > just as an aside, only the incremental bit errors test in
> > nandbiterrs positively requires raw data write.
> >
> > The overwrite test (re-write the same page data repeatedly without
> > erase), only uses raw access because I was lazy. A normal ECC write
> > would do just as well.
>
> Okay.
> Anyway, the test I'm really interested in is the incremental bit
> errors test :-).
>
> BTW, any reason you chose to implement this test/testsuite as a
> module ?
> From my understanding (and tell me if I'm wrong) we could do the same
> from user-space.
>
> Best Regards,
>
> Boris
>
Hi Boris,
can't say I have good excuses, but here goes. :-)
I was writing a NAND driver, so my brain was in kernel mode. It made it
slightly easier to trace the operations from end-to-end.
It's also possible that I didn't have an userspace at the time and had the
test compiled in, my memory is a little fuzzy...
Otherwise, yes, you could probably do it from userspace. I'm not sure
if there is a way to determine the number of corrected bit errors for
a specific page read. ECCGETSTATS seems to be global.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
* RE: [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-10-02 6:52 ` Iwo Mergler
0 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-10-02 6:52 UTC (permalink / raw)
To: Boris Brezillon
Cc: Mike Voytovich, linux-kernel, Huang Shijie, linux-mtd, Roy Lee,
Brian Norris, Huang Shijie, David Woodhouse, linux-arm-kernel
On Tue, 30 Sep 2014 18:04:15 +1000
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> Hi Iwo,
>
> On Mon, 29 Sep 2014 11:22:11 +1000
> Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
>
> > On Thu, 11 Sep 2014 22:36:16 +1000
> > Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> > >
> > > Well, I don't know about freescale specific tools, but at least I
> > > have an example with mtd_nandbiterrs module.
> > > This module is assuming it can write only the data part of a NAND
> > > page without modifying the OOB area (see [1]), which in GPMI
> > > controller case is impossible because raw write function store
> > > the data as if there were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> >
> > Hi Boris,
> >
> >
> > just as an aside, only the incremental bit errors test in
> > nandbiterrs positively requires raw data write.
> >
> > The overwrite test (re-write the same page data repeatedly without
> > erase), only uses raw access because I was lazy. A normal ECC write
> > would do just as well.
>
> Okay.
> Anyway, the test I'm really interested in is the incremental bit
> errors test :-).
>
> BTW, any reason you chose to implement this test/testsuite as a
> module ?
> From my understanding (and tell me if I'm wrong) we could do the same
> from user-space.
>
> Best Regards,
>
> Boris
>
Hi Boris,
can't say I have good excuses, but here goes. :-)
I was writing a NAND driver, so my brain was in kernel mode. It made it
slightly easier to trace the operations from end-to-end.
It's also possible that I didn't have an userspace at the time and had the
test compiled in, my memory is a little fuzzy...
Otherwise, yes, you could probably do it from userspace. I'm not sure
if there is a way to determine the number of corrected bit errors for
a specific page read. ECCGETSTATS seems to be global.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] mtd: nand: gpmi: add proper raw access support
@ 2014-10-02 6:52 ` Iwo Mergler
0 siblings, 0 replies; 60+ messages in thread
From: Iwo Mergler @ 2014-10-02 6:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 30 Sep 2014 18:04:15 +1000
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> Hi Iwo,
>
> On Mon, 29 Sep 2014 11:22:11 +1000
> Iwo Mergler <Iwo.Mergler@netcommwireless.com> wrote:
>
> > On Thu, 11 Sep 2014 22:36:16 +1000
> > Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> > >
> > > Well, I don't know about freescale specific tools, but at least I
> > > have an example with mtd_nandbiterrs module.
> > > This module is assuming it can write only the data part of a NAND
> > > page without modifying the OOB area (see [1]), which in GPMI
> > > controller case is impossible because raw write function store
> > > the data as if there were no specific scheme, while there is one:
> > > (metadata + n x (data_chunk + ECC bytes) + remaining_bytes).
> > >
> >
> > Hi Boris,
> >
> >
> > just as an aside, only the incremental bit errors test in
> > nandbiterrs positively requires raw data write.
> >
> > The overwrite test (re-write the same page data repeatedly without
> > erase), only uses raw access because I was lazy. A normal ECC write
> > would do just as well.
>
> Okay.
> Anyway, the test I'm really interested in is the incremental bit
> errors test :-).
>
> BTW, any reason you chose to implement this test/testsuite as a
> module ?
> From my understanding (and tell me if I'm wrong) we could do the same
> from user-space.
>
> Best Regards,
>
> Boris
>
Hi Boris,
can't say I have good excuses, but here goes. :-)
I was writing a NAND driver, so my brain was in kernel mode. It made it
slightly easier to trace the operations from end-to-end.
It's also possible that I didn't have an userspace at the time and had the
test compiled in, my memory is a little fuzzy...
Otherwise, yes, you could probably do it from userspace. I'm not sure
if there is a way to determine the number of corrected bit errors for
a specific page read. ECCGETSTATS seems to be global.
Best regards,
Iwo
______________________________________________________________________
This communication contains information which may be confidential or privileged. The information is intended solely for the use of the individual or entity named above. If you are not the intended recipient, be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this communication in error, please notify me by telephone immediately.
______________________________________________________________________
^ permalink raw reply [flat|nested] 60+ messages in thread
end of thread, other threads:[~2014-10-02 6:59 UTC | newest]
Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10 8:55 [PATCH] mtd: nand: gpmi: add proper raw access support Boris BREZILLON
2014-09-10 8:55 ` Boris BREZILLON
2014-09-10 8:55 ` Boris BREZILLON
2014-09-11 12:09 ` Huang Shijie
2014-09-11 12:09 ` Huang Shijie
2014-09-11 12:09 ` Huang Shijie
2014-09-11 12:36 ` Boris BREZILLON
2014-09-11 12:36 ` Boris BREZILLON
2014-09-11 12:36 ` Boris BREZILLON
2014-09-11 14:25 ` Huang Shijie
2014-09-11 14:25 ` Huang Shijie
2014-09-11 14:25 ` Huang Shijie
2014-09-11 14:38 ` Boris BREZILLON
2014-09-11 14:38 ` Boris BREZILLON
2014-09-11 14:38 ` Boris BREZILLON
2014-09-12 0:45 ` Huang Shijie
2014-09-12 0:45 ` Huang Shijie
2014-09-12 0:45 ` Huang Shijie
2014-09-12 12:30 ` Boris BREZILLON
2014-09-12 12:30 ` Boris BREZILLON
2014-09-12 12:30 ` Boris BREZILLON
2014-09-13 15:36 ` Huang Shijie
2014-09-13 15:36 ` Huang Shijie
2014-09-13 15:36 ` Huang Shijie
2014-09-13 17:38 ` Brian Norris
2014-09-13 17:38 ` Brian Norris
2014-09-13 17:38 ` Brian Norris
2014-09-14 14:07 ` Boris BREZILLON
2014-09-14 14:07 ` Boris BREZILLON
2014-09-14 14:07 ` Boris BREZILLON
2014-09-15 14:43 ` Huang Shijie
2014-09-15 14:43 ` Huang Shijie
2014-09-15 14:43 ` Huang Shijie
2014-09-15 20:12 ` Boris BREZILLON
2014-09-15 20:12 ` Boris BREZILLON
2014-09-15 20:12 ` Boris BREZILLON
2014-09-17 15:26 ` Huang Shijie
2014-09-17 15:26 ` Huang Shijie
2014-09-17 15:26 ` Huang Shijie
2014-09-17 18:16 ` Boris BREZILLON
2014-09-17 18:16 ` Boris BREZILLON
2014-09-17 18:16 ` Boris BREZILLON
2014-09-29 1:22 ` Iwo Mergler
2014-09-29 1:22 ` Iwo Mergler
2014-09-29 1:22 ` Iwo Mergler
2014-09-30 8:04 ` Boris Brezillon
2014-09-30 8:04 ` Boris Brezillon
2014-09-30 8:04 ` Boris Brezillon
2014-10-02 6:52 ` Iwo Mergler
2014-10-02 6:52 ` Iwo Mergler
2014-10-02 6:52 ` Iwo Mergler
2014-09-11 14:29 ` Huang Shijie
2014-09-11 14:29 ` Huang Shijie
2014-09-11 14:29 ` Huang Shijie
2014-09-11 14:45 ` Boris BREZILLON
2014-09-11 14:45 ` Boris BREZILLON
2014-09-11 14:45 ` Boris BREZILLON
2014-09-12 0:40 ` Huang Shijie
2014-09-12 0:40 ` Huang Shijie
2014-09-12 0:40 ` Huang Shijie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.