From: Boris BREZILLON <hidden> Date: 2014-09-23 14:07:44
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Best Regards,
Boris
Changes since v2:
- fixed a bug in gpmi_move_bits
- add a raw_buffer field to be used when using raw access methods
(experienced memory corruptions when directly using page_buffer_virt
buffer)
- add raw OOB access functions
Boris BREZILLON (3):
mtd: nand: gpmi: add gpmi_move_bits function
mtd: nand: gpmi: add proper raw access support
mtd: nand: gpmi: add raw oob access functions
drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 88 ++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 144 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 6 ++
3 files changed, 238 insertions(+)
--
1.9.1
From: Boris BREZILLON <hidden> Date: 2014-09-23 14:07:47
Implement raw OOB access functions to retrieve OOB bytes when accessing the
NAND in raw mode.
Signed-off-by: Boris BREZILLON <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
From: Boris BREZILLON <hidden> Date: 2014-09-23 14:08:08
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
@@ -837,6 +838,9 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)if(!this->page_buffer_virt)gotoerror_alloc;+this->raw_buffer=kzalloc(mtd->writesize+mtd->oobsize,GFP_KERNEL);+if(!this->raw_buffer)+gotoerror_alloc;/* Slice up the page buffer. */this->payload_virt=this->page_buffer_virt;
@@ -1347,6 +1351,126 @@ gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)returnstatus&NAND_STATUS_FAIL?-EIO:0;}+staticintgpmi_ecc_read_page_raw(structmtd_info*mtd,+structnand_chip*chip,uint8_t*buf,+intoob_required,intpage)+{+structgpmi_nand_data*this=chip->priv;+structbch_geometry*nfc_geo=&this->bch_geometry;+inteccsize=nfc_geo->ecc_chunk_size;+inteccbits=nfc_geo->ecc_strength*nfc_geo->gf_len;+u8*tmp_buf=this->raw_buffer;+size_tsrc_bit_off;+size_toob_bit_off;+size_toob_byte_off;+uint8_t*oob=chip->oob_poi;+intstep;++chip->read_buf(mtd,tmp_buf,+mtd->writesize+mtd->oobsize);++if(this->swap_block_mark){+u8swap=tmp_buf[0];++tmp_buf[0]=tmp_buf[mtd->writesize];+tmp_buf[mtd->writesize]=swap;+}++if(oob_required)+memcpy(oob,tmp_buf,nfc_geo->metadata_size);++oob_bit_off=nfc_geo->metadata_size*8;+src_bit_off=oob_bit_off;++for(step=0;step<nfc_geo->ecc_chunk_count;step++){+if(buf)+gpmi_move_bits(buf,step*eccsize*8,+tmp_buf,src_bit_off,+eccsize*8);+src_bit_off+=eccsize*8;++if(oob_required)+gpmi_move_bits(oob,oob_bit_off,+tmp_buf,src_bit_off,+eccbits);++src_bit_off+=eccbits;+oob_bit_off+=eccbits;+}++if(oob_required&&oob_bit_off%8)+oob[oob_bit_off/8]&=GENMASK(oob_bit_off-1,0);++oob_byte_off=DIV_ROUND_UP(oob_bit_off,8);++if(oob_required&&oob_byte_off<mtd->oobsize)+memcpy(oob+oob_byte_off,+tmp_buf+mtd->writesize+oob_byte_off,+mtd->oobsize-oob_byte_off);++return0;+}++staticintgpmi_ecc_write_page_raw(structmtd_info*mtd,+structnand_chip*chip,+constuint8_t*buf,+intoob_required)+{+structgpmi_nand_data*this=chip->priv;+structbch_geometry*nfc_geo=&this->bch_geometry;+inteccsize=nfc_geo->ecc_chunk_size;+inteccbits=nfc_geo->ecc_strength*nfc_geo->gf_len;+u8*tmp_buf=this->raw_buffer;+uint8_t*oob=chip->oob_poi;+size_tdst_bit_off;+size_toob_bit_off;+size_toob_byte_off;+intstep;++if(!buf||!oob_required)+memset(tmp_buf,0xff,mtd->writesize+mtd->oobsize);++memcpy(tmp_buf,oob,nfc_geo->metadata_size);+oob_bit_off=nfc_geo->metadata_size*8;+dst_bit_off=oob_bit_off;++for(step=0;step<nfc_geo->ecc_chunk_count;step++){+if(buf)+gpmi_move_bits(tmp_buf,dst_bit_off,+buf,step*eccsize*8,eccsize*8);+dst_bit_off+=eccsize*8;++/* Pad last ECC block to align following data on a byte */+if(step==nfc_geo->ecc_chunk_count-1&&+(oob_bit_off+eccbits)%8)+eccbits+=8-((oob_bit_off+eccbits)%8);++if(oob_required)+gpmi_move_bits(tmp_buf,dst_bit_off,+oob,oob_bit_off,eccbits);++dst_bit_off+=eccbits;+oob_bit_off+=eccbits;+}++oob_byte_off=oob_bit_off/8;++if(oob_required&&oob_byte_off<mtd->oobsize)+memcpy(tmp_buf+mtd->writesize+oob_byte_off,+oob+oob_byte_off,mtd->oobsize-oob_byte_off);++if(this->swap_block_mark){+u8swap=tmp_buf[0];++tmp_buf[0]=tmp_buf[mtd->writesize];+tmp_buf[mtd->writesize]=swap;+}++chip->write_buf(mtd,tmp_buf,mtd->writesize+mtd->oobsize);++return0;+}+staticintgpmi_block_markbad(structmtd_info*mtd,loff_tofs){structnand_chip*chip=mtd->priv;
@@ -1664,6 +1788,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;
On Tue, Sep 23, 2014 at 04:07:35PM +0200, Boris BREZILLON wrote:
quoted hunk
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
On Tue, Sep 23, 2014 at 04:07:35PM +0200, Boris BREZILLON wrote:
quoted
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
@@ -837,6 +838,9 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)if(!this->page_buffer_virt)gotoerror_alloc;+this->raw_buffer=kzalloc(mtd->writesize+mtd->oobsize,GFP_KERNEL);
why add this buffer?
I don't why, but I experienced memory corruptions (triggering kernel
panics) when using the page_buffer_virt, even though I had resized it
according to the NAND writesize and oobsize (see my previous version).
Do you see anything that could generate an overflow ?
It seems to work when I allocate my own buffer...
did you meet some data overlapped?
quoted
+ if (!this->raw_buffer)
+ goto error_alloc;
/* Slice up the page buffer. */
this->payload_virt = this->page_buffer_virt;
@@ -1347,6 +1351,126 @@ 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 eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;+ u8 *tmp_buf = this->raw_buffer;+ size_t src_bit_off;+ size_t oob_bit_off;+ size_t oob_byte_off;+ uint8_t *oob = chip->oob_poi;+ int step;++ chip->read_buf(mtd, tmp_buf,+ mtd->writesize + mtd->oobsize);++ if (this->swap_block_mark) {+ u8 swap = tmp_buf[0];++ tmp_buf[0] = tmp_buf[mtd->writesize];+ tmp_buf[mtd->writesize] = swap;+ }++ if (oob_required)+ memcpy(oob, tmp_buf, nfc_geo->metadata_size);++ oob_bit_off = nfc_geo->metadata_size * 8;+ src_bit_off = oob_bit_off;++ for (step = 0; step < nfc_geo->ecc_chunk_count; step++) {+ if (buf)
could this @buf become NULL?
It's just a check to later support raw OOB accesses (see patch
3) ;-).
On Tue, Sep 23, 2014 at 04:07:35PM +0200, Boris BREZILLON wrote:
quoted hunk
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
@@ -837,6 +838,9 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)if(!this->page_buffer_virt)gotoerror_alloc;+this->raw_buffer=kzalloc(mtd->writesize+mtd->oobsize,GFP_KERNEL);+if(!this->raw_buffer)+gotoerror_alloc;/* Slice up the page buffer. */this->payload_virt=this->page_buffer_virt;
In actually, i suggest to call the ecc->read_page() in this
hook. And after the ecc->read_page(), copy the relative data to
the relative buffers. Is my suggestion right? please correct me.
On Tue, Sep 23, 2014 at 04:07:35PM +0200, Boris BREZILLON wrote:
quoted
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
@@ -837,6 +838,9 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)if(!this->page_buffer_virt)gotoerror_alloc;+this->raw_buffer=kzalloc(mtd->writesize+mtd->oobsize,GFP_KERNEL);+if(!this->raw_buffer)+gotoerror_alloc;/* Slice up the page buffer. */this->payload_virt=this->page_buffer_virt;
In actually, i suggest to call the ecc->read_page() in this
hook. And after the ecc->read_page(), copy the relative data to
the relative buffers. Is my suggestion right? please correct me.
Unfortunately it's not. The user expect that ECC correction is not
involved when accessing the NAND in raw mode, which is not the case in
your read_page implementation.
This is particularly useful when one want to see the real page status
(including bitflips).
Moreover, I like to see the generated ECC bytes/bits when using raw
access (but I'm not sure this is a requirement). This will help
deducing the BCH algorithm and/or XOR mask applied after producing
these bits if someone ever want to spend some time reverse engineering
it.
Sure, no problem.
Actually it's doing the exact same thing read_page_raw is doing expect
it's applying on a write access.
I copy the provided data (in-band and out-of-band is required) into a
temporary buffer to match the GPMI controller layout, then write it
without involving the BCH block (which means no ECC bits generation).
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Tue, Sep 23, 2014 at 04:07:35PM +0200, Boris BREZILLON wrote:
quoted
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 <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 126 +++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 +
2 files changed, 128 insertions(+)
@@ -837,6 +838,9 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)if(!this->page_buffer_virt)gotoerror_alloc;+this->raw_buffer=kzalloc(mtd->writesize+mtd->oobsize,GFP_KERNEL);+if(!this->raw_buffer)+gotoerror_alloc;/* Slice up the page buffer. */this->payload_virt=this->page_buffer_virt;
In actually, i suggest to call the ecc->read_page() in this
hook. And after the ecc->read_page(), copy the relative data to
the relative buffers. Is my suggestion right? please correct me.
Unfortunately it's not. The user expect that ECC correction is not
involved when accessing the NAND in raw mode, which is not the case in
your read_page implementation.
This is particularly useful when one want to see the real page status
(including bitflips).
Moreover, I like to see the generated ECC bytes/bits when using raw
access (but I'm not sure this is a requirement). This will help
deducing the BCH algorithm and/or XOR mask applied after producing
these bits if someone ever want to spend some time reverse engineering
it.
Sure, no problem.
Actually it's doing the exact same thing read_page_raw is doing expect
it's applying on a write access.
I copy the provided data (in-band and out-of-band is required) into a
^ if
temporary buffer to match the GPMI controller layout, then write it
without involving the BCH block (which means no ECC bits generation).
Best Regards,
Boris
From: Boris BREZILLON <hidden> Date: 2014-09-23 14:08:32
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
Signed-off-by: Boris BREZILLON <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 88 ++++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 4 ++
2 files changed, 92 insertions(+)
On Tue, Sep 23, 2014 at 04:07:34PM +0200, Boris BREZILLON wrote:
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
@@ -1353,3 +1353,91 @@ int gpmi_read_page(struct gpmi_nand_data *this,set_dma_type(this,DMA_FOR_READ_ECC_PAGE);returnstart_dma_with_bch_irq(this,desc);}++voidgpmi_move_bits(u8*dst,size_tdst_bit_off,+constu8*src,size_tsrc_bit_off,+size_tnbits)
we can simplify the code.
We could use the bytes to replace the @nbits.
The chunk data is always byte aligned.
Btw: please add more comments in this function.
thanks
Huang Shijie
On Tue, Sep 23, 2014 at 04:07:34PM +0200, Boris BREZILLON wrote:
quoted
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
@@ -1353,3 +1353,91 @@ int gpmi_read_page(struct gpmi_nand_data *this,set_dma_type(this,DMA_FOR_READ_ECC_PAGE);returnstart_dma_with_bch_irq(this,desc);}++voidgpmi_move_bits(u8*dst,size_tdst_bit_off,+constu8*src,size_tsrc_bit_off,+size_tnbits)
we can simplify the code.
Any suggestions ?
We could use the bytes to replace the @nbits.
The chunk data is always byte aligned.
This function is also used to store ECC bits in the OOB buffer and
these chunk of data are not byte aligned :-).
Btw: please add more comments in this function.
Yep, I should definitely comment it, and I'll do.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Tue, Sep 23, 2014 at 04:07:34PM +0200, Boris BREZILLON wrote:
quoted
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
@@ -1353,3 +1353,91 @@ int gpmi_read_page(struct gpmi_nand_data *this,set_dma_type(this,DMA_FOR_READ_ECC_PAGE);returnstart_dma_with_bch_irq(this,desc);}++voidgpmi_move_bits(u8*dst,size_tdst_bit_off,+constu8*src,size_tsrc_bit_off,+size_tnbits)
we can simplify the code.
Any suggestions ?
quoted
We could use the bytes to replace the @nbits.
The chunk data is always byte aligned.
This function is also used to store ECC bits in the OOB buffer and
these chunk of data are not byte aligned :-).
yes. you are right. I missed it.
could you also comment these two hooks in the patch set.
I hope Brian also can check it, and we can make it more clear about how
to implement them.
thanks
Huang Shijie
On Tue, Sep 23, 2014 at 04:07:34PM +0200, Boris BREZILLON wrote:
quoted
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
From: Boris BREZILLON <hidden> Date: 2014-09-23 15:26:05
Hi Huang,
I've added some code comments inline (and I'll squash them in my next
version).
On Tue, 23 Sep 2014 16:07:34 +0200
Boris BREZILLON [off-list ref] wrote:
quoted hunk
Add a new function to move bits (not bytes) from a memory region to
another one.
This function is similar to memmove except it acts at bit level.
This function is needed to implement GPMI raw access functions, given the
fact that ECC engine does not pad ECC bits to the next byte boundary.
Signed-off-by: Boris BREZILLON <redacted>
---
drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 88 ++++++++++++++++++++++++++++++++++
drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 4 ++
2 files changed, 92 insertions(+)
/*
* src buffer is not byte aligned, hence we have to copy
* each src byte to the src_byte variable (after
* applying the appropriate shift depending on the
* src bit offset).
* We still try to work on bytes until there's not
* enough available data in the src buffer.
*/
/*
* There were not enough bits to copy from src to dst to get a
* byte aligned dst buffer. In this case prepare the src_byte
* variable to match the dst organization (just shift src_byte
* by dst bit offset and retrieve least significant bits from
* dst).
*/
From: Boris Brezillon <hidden> Date: 2014-09-30 08:07:26
Hi,
On Tue, 23 Sep 2014 16:07:33 +0200
Boris BREZILLON [off-list ref] wrote:
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Brian, any chance you could take a look at this series and give your
opinion and/or suggest a new approach ?
Huang, did you have time to think about a better way to implement these
raw access functions ?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Tue, Sep 30, 2014 at 10:07:20AM +0200, Boris Brezillon wrote:
Hi,
On Tue, 23 Sep 2014 16:07:33 +0200
Boris BREZILLON [off-list ref] wrote:
quoted
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Brian, any chance you could take a look at this series and give your
opinion and/or suggest a new approach ?
Huang, did you have time to think about a better way to implement these
raw access functions ?
sorry for the later reply. I was busy recently.
You can send out the new version. Please do not forget add a patch to
add
more comments for these hooks.
I will test your patch(or your new patch set) on Oct 7 and Oct 8.
thanks
Huang Shijie
On Tue, Sep 23, 2014 at 04:07:33PM +0200, Boris BREZILLON wrote:
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Best Regards,
Boris
Changes since v2:
- fixed a bug in gpmi_move_bits
- add a raw_buffer field to be used when using raw access methods
(experienced memory corruptions when directly using page_buffer_virt
buffer)
- add raw OOB access functions
I tested this patch set today with the imx6dl-sabreauto board.
NAND: Micron MT29F64G08CBABAWP
8192MiB, MLC, page size: 8192, OOB size: 744
ECC: 40bit
The result:
[ 3672.779009] ==================================================
[ 3672.784974] mtd_nandbiterrs: MTD device: 0
[ 3672.789480] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3672.798169] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3672.804554] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
[ 3672.812497] mtd_nandbiterrs: incremental biterrors test
[ 3672.818688] mtd_nandbiterrs: write_page
[ 3672.825529] mtd_nandbiterrs: rewrite page
[ 3672.837290] mtd_nandbiterrs: read_page
[ 3672.848407] mtd_nandbiterrs: error: read failed at 0x0
[ 3672.853644] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3672.862932] mtd_nandbiterrs: finished successfully.
[ 3672.867837] ==================================================
[ 3745.282368] ==================================================
[ 3745.288227] mtd_nandbiterrs: MTD device: 0
[ 3745.292913] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3745.301897] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3745.308023] mtd_nandbiterrs: Using page=1, offset=8192, eraseblock=0
[ 3745.316778] mtd_nandbiterrs: incremental biterrors test
[ 3745.323017] mtd_nandbiterrs: write_page
[ 3745.328616] mtd_nandbiterrs: rewrite page
[ 3745.334191] mtd_nandbiterrs: read_page
[ 3745.346878] mtd_nandbiterrs: error: read failed at 0x2000
[ 3745.352352] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3745.361281] mtd_nandbiterrs: finished successfully.
[ 3745.366173] ==================================================
Is this okay?
thanks
Huang Shijie
From: Boris Brezillon <hidden> Date: 2014-10-08 15:10:42
On Wed, 8 Oct 2014 22:24:40 +0800
Huang Shijie [off-list ref] wrote:
On Tue, Sep 23, 2014 at 04:07:33PM +0200, Boris BREZILLON wrote:
quoted
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Best Regards,
Boris
Changes since v2:
- fixed a bug in gpmi_move_bits
- add a raw_buffer field to be used when using raw access methods
(experienced memory corruptions when directly using page_buffer_virt
buffer)
- add raw OOB access functions
I tested this patch set today with the imx6dl-sabreauto board.
NAND: Micron MT29F64G08CBABAWP
8192MiB, MLC, page size: 8192, OOB size: 744
ECC: 40bit
The result:
[ 3672.779009] ==================================================
[ 3672.784974] mtd_nandbiterrs: MTD device: 0
[ 3672.789480] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3672.798169] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3672.804554] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
[ 3672.812497] mtd_nandbiterrs: incremental biterrors test
[ 3672.818688] mtd_nandbiterrs: write_page
[ 3672.825529] mtd_nandbiterrs: rewrite page
[ 3672.837290] mtd_nandbiterrs: read_page
[ 3672.848407] mtd_nandbiterrs: error: read failed at 0x0
[ 3672.853644] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3672.862932] mtd_nandbiterrs: finished successfully.
[ 3672.867837] ==================================================
[ 3745.282368] ==================================================
[ 3745.288227] mtd_nandbiterrs: MTD device: 0
[ 3745.292913] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3745.301897] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3745.308023] mtd_nandbiterrs: Using page=1, offset=8192, eraseblock=0
[ 3745.316778] mtd_nandbiterrs: incremental biterrors test
[ 3745.323017] mtd_nandbiterrs: write_page
[ 3745.328616] mtd_nandbiterrs: rewrite page
[ 3745.334191] mtd_nandbiterrs: read_page
[ 3745.346878] mtd_nandbiterrs: error: read failed at 0x2000
[ 3745.352352] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3745.361281] mtd_nandbiterrs: finished successfully.
[ 3745.366173] ==================================================
Is this okay?
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
Can you also try to write zeros to a NAND page and then dump it in
raw mode (and provide me with the resulted /tmp/dump file or an
hexdump output of this file) ?
flash_erase /dev/mtdX 0 1
dd if=/dev/zero of=/tmp/zero bs=8k count=1
nandwrite /dev/mtdX /tmp/zero
nanddump -n -o -l 8192 -f /tmp/dump /dev/mtdX
Thanks,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Wed, Oct 08, 2014 at 05:10:34PM +0200, Boris Brezillon wrote:
On Wed, 8 Oct 2014 22:24:40 +0800
Huang Shijie [off-list ref] wrote:
quoted
On Tue, Sep 23, 2014 at 04:07:33PM +0200, Boris BREZILLON wrote:
quoted
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Best Regards,
Boris
Changes since v2:
- fixed a bug in gpmi_move_bits
- add a raw_buffer field to be used when using raw access methods
(experienced memory corruptions when directly using page_buffer_virt
buffer)
- add raw OOB access functions
I tested this patch set today with the imx6dl-sabreauto board.
NAND: Micron MT29F64G08CBABAWP
8192MiB, MLC, page size: 8192, OOB size: 744
ECC: 40bit
The result:
[ 3672.779009] ==================================================
[ 3672.784974] mtd_nandbiterrs: MTD device: 0
[ 3672.789480] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3672.798169] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3672.804554] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
[ 3672.812497] mtd_nandbiterrs: incremental biterrors test
[ 3672.818688] mtd_nandbiterrs: write_page
[ 3672.825529] mtd_nandbiterrs: rewrite page
[ 3672.837290] mtd_nandbiterrs: read_page
[ 3672.848407] mtd_nandbiterrs: error: read failed at 0x0
[ 3672.853644] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3672.862932] mtd_nandbiterrs: finished successfully.
[ 3672.867837] ==================================================
[ 3745.282368] ==================================================
[ 3745.288227] mtd_nandbiterrs: MTD device: 0
[ 3745.292913] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3745.301897] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3745.308023] mtd_nandbiterrs: Using page=1, offset=8192, eraseblock=0
[ 3745.316778] mtd_nandbiterrs: incremental biterrors test
[ 3745.323017] mtd_nandbiterrs: write_page
[ 3745.328616] mtd_nandbiterrs: rewrite page
[ 3745.334191] mtd_nandbiterrs: read_page
[ 3745.346878] mtd_nandbiterrs: error: read failed at 0x2000
[ 3745.352352] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3745.361281] mtd_nandbiterrs: finished successfully.
[ 3745.366173] ==================================================
Is this okay?
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
From: Boris Brezillon <hidden> Date: 2014-10-10 14:53:11
On Fri, 10 Oct 2014 22:42:51 +0800
Huang Shijie [off-list ref] wrote:
On Wed, Oct 08, 2014 at 05:10:34PM +0200, Boris Brezillon wrote:
quoted
On Wed, 8 Oct 2014 22:24:40 +0800
Huang Shijie [off-list ref] wrote:
quoted
On Tue, Sep 23, 2014 at 04:07:33PM +0200, Boris BREZILLON wrote:
quoted
Hello Huang, Brian,
This is just a new proposal to support raw accesses in a more standard way
in the GPMI driver.
This series has been tested on an imx28 board.
Any suggestions are welcome.
Best Regards,
Boris
Changes since v2:
- fixed a bug in gpmi_move_bits
- add a raw_buffer field to be used when using raw access methods
(experienced memory corruptions when directly using page_buffer_virt
buffer)
- add raw OOB access functions
I tested this patch set today with the imx6dl-sabreauto board.
NAND: Micron MT29F64G08CBABAWP
8192MiB, MLC, page size: 8192, OOB size: 744
ECC: 40bit
The result:
[ 3672.779009] ==================================================
[ 3672.784974] mtd_nandbiterrs: MTD device: 0
[ 3672.789480] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3672.798169] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3672.804554] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
[ 3672.812497] mtd_nandbiterrs: incremental biterrors test
[ 3672.818688] mtd_nandbiterrs: write_page
[ 3672.825529] mtd_nandbiterrs: rewrite page
[ 3672.837290] mtd_nandbiterrs: read_page
[ 3672.848407] mtd_nandbiterrs: error: read failed at 0x0
[ 3672.853644] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3672.862932] mtd_nandbiterrs: finished successfully.
[ 3672.867837] ==================================================
[ 3745.282368] ==================================================
[ 3745.288227] mtd_nandbiterrs: MTD device: 0
[ 3745.292913] mtd_nandbiterrs: MTD device size 16777216, eraseblock=2097152, page=8192, oob=744
[ 3745.301897] mtd_nandbiterrs: Device uses 1 subpages of 8192 bytes
[ 3745.308023] mtd_nandbiterrs: Using page=1, offset=8192, eraseblock=0
[ 3745.316778] mtd_nandbiterrs: incremental biterrors test
[ 3745.323017] mtd_nandbiterrs: write_page
[ 3745.328616] mtd_nandbiterrs: rewrite page
[ 3745.334191] mtd_nandbiterrs: read_page
[ 3745.346878] mtd_nandbiterrs: error: read failed at 0x2000
[ 3745.352352] mtd_nandbiterrs: After 0 biterrors per subpage, read reported error -74
[ 3745.361281] mtd_nandbiterrs: finished successfully.
[ 3745.366173] ==================================================
Is this okay?
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
That's great news!
Thanks for testing it.
I'll send a new version soon.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
Hi Huang,
just out of interest, have you tried this on the MLC NAND without the patch?
I'm aware that MLC says you shouldn't write multiple times, but that is
with a view towards specified data endurance. I would only expect a few
additional bit errors during the test.
Did you try the overwrite test?
I'm curious how MLC NAND does when subjected to multiple writes.
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.
______________________________________________________________________
On Tue, Oct 14, 2014 at 04:50:27PM +1100, Iwo Mergler wrote:
quoted
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
Hi Huang,
just out of interest, have you tried this on the MLC NAND without the patch?
yes. I tried. As i posted, it will failed.
I'm aware that MLC says you shouldn't write multiple times, but that is
with a view towards specified data endurance. I would only expect a few
additional bit errors during the test.
Did you try the overwrite test?
not yet. I can test it tomorrow.
I'm curious how MLC NAND does when subjected to multiple writes.
We should not do the multiple writes to the MLC nand.
We can do so with the SLC nand.
thank
Huang Shijie
On Tue, Oct 14, 2014 at 04:50:27PM +1100, Iwo Mergler wrote:
quoted
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this patch
to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not supported
by MLC flashes).
Hi Huang,
just out of interest, have you tried this on the MLC NAND without the patch?
I'm aware that MLC says you shouldn't write multiple times, but that is
with a view towards specified data endurance. I would only expect a few
additional bit errors during the test.
Did you try the overwrite test?
The following is the test result for overwrite with this MLC patch:
root at imx6qdlsolo:~# insmod mtd_nandbiterrs.ko dev=1 mode=1
[ 762.534714]
[ 762.536259] ==================================================
[ 762.542115] mtd_nandbiterrs: MTD device: 1
[ 762.546326] mtd_nandbiterrs: MTD device size 16777216,
eraseblock=2097152, page=8192, oob=744
[ 762.554937] mtd_nandbiterrs: Device uses 1 subpages of 8192
bytes
[ 762.561059] mtd_nandbiterrs: Using page=0, offset=0,
eraseblock=0
[ 762.571333] mtd_nandbiterrs: overwrite biterrors test
[ 762.576715] mtd_nandbiterrs: write_page
[ 762.590448] mtd_nandbiterrs: error: read failed at 0x0
[ 762.595650] mtd_nandbiterrs: Read reported error -74
[ 762.600625] mtd_nandbiterrs: Bit error histogram (0
operations total):
[ 762.608586] mtd_nandbiterrs: finished successfully.
[ 762.613501]
==================================================
insmod: ERROR: could not insert module mtd_nandbiterrs.ko:
Input/output error
thanks
Huang Shijie
On Sun, 19 Oct 2014 13:20:38 +1100 Huang Shijie [off-list ref] wrote:
On Tue, Oct 14, 2014 at 04:50:27PM +1100, Iwo Mergler wrote:
quoted
quoted
No, it doesn't seem to be correct.
But it's an MLC flash, so you'll most probably need to apply this
patch to nandbiterrs testsuite:
http://code.bulix.org/f69wuu-87021
This patch is flashing the block between each bitflip insertion to
avoid multiple write without erasure (which, AFAIK, is not
supported by MLC flashes).
Hi Huang,
just out of interest, have you tried this on the MLC NAND without
the patch?
I'm aware that MLC says you shouldn't write multiple times, but
that is with a view towards specified data endurance. I would only
expect a few additional bit errors during the test.
Did you try the overwrite test?
The following is the test result for overwrite with this MLC patch:
root at imx6qdlsolo:~# insmod mtd_nandbiterrs.ko dev=1 mode=1
[ 762.534714]
[ 762.536259] ==================================================
[ 762.542115] mtd_nandbiterrs: MTD device: 1
[ 762.546326] mtd_nandbiterrs: MTD device size 16777216,
eraseblock=2097152, page=8192, oob=744
[ 762.554937] mtd_nandbiterrs: Device uses 1 subpages of 8192
bytes
[ 762.561059] mtd_nandbiterrs: Using page=0, offset=0,
eraseblock=0
[ 762.571333] mtd_nandbiterrs: overwrite biterrors test
[ 762.576715] mtd_nandbiterrs: write_page
[ 762.590448] mtd_nandbiterrs: error: read failed at 0x0
[ 762.595650] mtd_nandbiterrs: Read reported error -74
[ 762.600625] mtd_nandbiterrs: Bit error histogram (0
operations total):
[ 762.608586] mtd_nandbiterrs: finished successfully.
[ 762.613501]
==================================================
insmod: ERROR: could not insert module mtd_nandbiterrs.ko:
Input/output error
Thanks. If I read this correctly, this means that writing the same data twice,
already generates more bit errors than the ECC can fix.
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.
______________________________________________________________________