Patchwork [v4,6/6] mtd: update the ABI document about the ecc step size

login
register
mail settings
Submitter Brian Norris
Date Aug. 17, 2013, 6:14 p.m.
Message ID <20130817181447.GC5034@norris.computersforpeace.net>
Download mbox | patch
Permalink /patch/267995/
State New
Headers show

Comments

Brian Norris - Aug. 17, 2013, 6:14 p.m.
On Fri, Aug 16, 2013 at 11:26:47PM -0400, Huang Shijie wrote:
> On Fri, Aug 16, 2013 at 04:45:59PM +0300, Artem Bityutskiy wrote:
> > On Fri, 2013-08-16 at 10:10 +0800, Huang Shijie wrote:
> > > +
> > > +What:          /sys/class/mtd/mtdX/ecc_step_size
> > > +Date:          May 2013
> > > +KernelVersion: 3.10
> > > +Contact:       linux-mtd@lists.infradead.org
> > > +Description:
> > > +               The size of each ECC step which is used for ECC.
> > > +               Note that some devices will have multiple ecc steps within each
> > > +               writesize region.
> > 
> > Actually this phrase is a bit confusing because it may be interpreted as
> > that one write-size may have ECC steps of multiple sizes. Would you
> > re-phrase, may be?
> What's about the following:
>  -----------------------------------------------------------------
>    The size of each ECC step which is used for ECC.
>    Note that some devices will have multiple ecc steps within each
>    writesize region, and the ecc steps share the same size.
>  -----------------------------------------------------------------

I took pieces of your message and rewrote it myself. Diff pasted below
(I edited ecc_strength to be less redundant and added a few details that
were worth mentioning). Let me know if you want to revise it, but I'll
push it to l2-mtd.git.

Brian

---
 Documentation/ABI/testing/sysfs-class-mtd | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
Huang Shijie - Aug. 18, 2013, 2:29 p.m.
On Sat, Aug 17, 2013 at 11:14:47AM -0700, Brian Norris wrote:
> On Fri, Aug 16, 2013 at 11:26:47PM -0400, Huang Shijie wrote:
> 
> I took pieces of your message and rewrote it myself. Diff pasted below
> (I edited ecc_strength to be less redundant and added a few details that
> were worth mentioning). Let me know if you want to revise it, but I'll

thanks a lot!

I really appreciate it. I always feel embarrassed when i descibe
something in English :(


> push it to l2-mtd.git.
> 
> Brian
> 
> ---
>  Documentation/ABI/testing/sysfs-class-mtd | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
> index 3105644..a795582 100644
> --- a/Documentation/ABI/testing/sysfs-class-mtd
> +++ b/Documentation/ABI/testing/sysfs-class-mtd
> @@ -128,9 +128,8 @@ KernelVersion:	3.4
>  Contact:	linux-mtd@lists.infradead.org
>  Description:
>  		Maximum number of bit errors that the device is capable of
> -		correcting within each region covering an ecc step.  This will
> -		always be a non-negative integer.  Note that some devices will
> -		have multiple ecc steps within each writesize region.
> +		correcting within each region covering an ECC step (see
> +		ecc_step_size).  This will always be a non-negative integer.
>  
>  		In the case of devices lacking any ECC capability, it is 0.
>  
> @@ -173,3 +172,16 @@ Description:
>  		This is generally applicable only to NAND flash devices with ECC
>  		capability.  It is ignored on devices lacking ECC capability;
>  		i.e., devices for which ecc_strength is zero.
> +
> +What:		/sys/class/mtd/mtdX/ecc_step_size
> +Date:		May 2013
> +KernelVersion:	3.10
> +Contact:	linux-mtd@lists.infradead.org
> +Description:
> +		The size of a single region covered by ECC, known as the ECC
> +		step.  Devices may have several equally sized ECC steps within
> +		each writesize region.  The step size counts only the data area,
> +		not the spare area.

Maybe this sentence is not accurate enough.

As far as i know, when the gpmi does the hardware ECC, the last ECC step
will use parts of the spare area. Just like:

----------------------------------------------------------------------------------
	 *    |                          P                            |
	 *    |<----------------------------------------------------->|
	 *    |                                                       |
	 *    |                                        (Block Mark)   |
	 *    |                      P'                      |      | |     |
	 *    |<-------------------------------------------->|  D   | |  O' |
	 *    |                                              |<---->| |<--->|
	 *    V                                              V      V V     V
	 *    +---+----------+-+----------+-+----------+-+----------+-+-----+
	 *    | M |   data   |E|   data   |E|   data   |E|   data   |E|     |
	 *    +---+----------+-+----------+-+----------+-+----------+-+-----+
	 *                                                   ^              ^
	 *                                                   |      O       |
	 *                                                   |<------------>|
	 *
	 *	P : the page size for BCH module.
	 *	E : The ECC strength.
	 *	G : the length of Galois Field.
	 *	N : The chunk count of per page.
	 *	M : the metasize of per page.
	 *	C : the ecc chunk size, aka the "data" above.
	 *	P': the nand chip's page size.
	 *	O : the nand chip's oob size.
	 *	O': the free oob.
----------------------------------------------------------------------------------
 In this diagram, the "O" stands for the spare area, and the last ECC
 step will use part of the spare area, the "Block Mark" is the boundary
 for the page and OOB.

thanks
Huang Shijie
Brian Norris - Aug. 20, 2013, 1:02 a.m.
On Sun, Aug 18, 2013 at 7:29 AM, Huang Shijie <shijie8@gmail.com> wrote:
> On Sat, Aug 17, 2013 at 11:14:47AM -0700, Brian Norris wrote:
>> On Fri, Aug 16, 2013 at 11:26:47PM -0400, Huang Shijie wrote:
>> I took pieces of your message and rewrote it myself. Diff pasted below
>> (I edited ecc_strength to be less redundant and added a few details that
>> were worth mentioning). Let me know if you want to revise it, but I'll
>
> thanks a lot!
>
> I really appreciate it. I always feel embarrassed when i descibe
> something in English :(

No need to be embarrassed. By this last iteration, it was actually
quite correct; there were just a few issues of style and an extra bit
of redundancy, now that ecc_strength *and* ecc_step_size were
documented.

...
>> @@ -173,3 +172,16 @@ Description:
>>               This is generally applicable only to NAND flash devices with ECC
>>               capability.  It is ignored on devices lacking ECC capability;
>>               i.e., devices for which ecc_strength is zero.
>> +
>> +What:                /sys/class/mtd/mtdX/ecc_step_size
>> +Date:                May 2013
>> +KernelVersion:       3.10
>> +Contact:     linux-mtd@lists.infradead.org
>> +Description:
>> +             The size of a single region covered by ECC, known as the ECC
>> +             step.  Devices may have several equally sized ECC steps within
>> +             each writesize region.  The step size counts only the data area,
>> +             not the spare area.
>
> Maybe this sentence is not accurate enough.
>
> As far as i know, when the gpmi does the hardware ECC, the last ECC step
> will use parts of the spare area. Just like:
>
> ----------------------------------------------------------------------------------
>          *    |                          P                            |
>          *    |<----------------------------------------------------->|
>          *    |                                                       |
>          *    |                                        (Block Mark)   |
>          *    |                      P'                      |      | |     |
>          *    |<-------------------------------------------->|  D   | |  O' |
>          *    |                                              |<---->| |<--->|
>          *    V                                              V      V V     V
>          *    +---+----------+-+----------+-+----------+-+----------+-+-----+
>          *    | M |   data   |E|   data   |E|   data   |E|   data   |E|     |
>          *    +---+----------+-+----------+-+----------+-+----------+-+-----+
>          *                                                   ^              ^
>          *                                                   |      O       |
>          *                                                   |<------------>|
>          *
>          *      P : the page size for BCH module.
>          *      E : The ECC strength.
>          *      G : the length of Galois Field.
>          *      N : The chunk count of per page.
>          *      M : the metasize of per page.
>          *      C : the ecc chunk size, aka the "data" above.
>          *      P': the nand chip's page size.
>          *      O : the nand chip's oob size.
>          *      O': the free oob.
> ----------------------------------------------------------------------------------
>  In this diagram, the "O" stands for the spare area, and the last ECC
>  step will use part of the spare area, the "Block Mark" is the boundary
>  for the page and OOB.

Apparently I totally ignored the possibility that ECC layouts place
their metadata in the data area (the 'E' regions above) not just in
the spare area ('O'). Out of curiosity, is this very common?

The point I really was trying to get to was that the "step" is only
measuring the division of the data area, not of any additional
metadata or spare area. So in your case, I still expect that
ecc_step_size * number-of-steps = writesize
So the step size is still counting "only the data" (not "data + E"),
but its corresponding data is not necessarily placed entirely in the
data area.

I think your objection is correct, and I just rebased and dropped the
inaccurate sentence.

Thanks,
Brian
Huang Shijie - Aug. 20, 2013, 2:11 a.m.
于 2013年08月20日 09:02, Brian Norris 写道:
> Apparently I totally ignored the possibility that ECC layouts place
> their metadata in the data area (the 'E' regions above) not just in
> the spare area ('O'). Out of curiosity, is this very common?
>
I think it is not common. AFAIK, only the gpmi uses this layout.

thanks for the rebase.

Huang Shijie

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
index 3105644..a795582 100644
--- a/Documentation/ABI/testing/sysfs-class-mtd
+++ b/Documentation/ABI/testing/sysfs-class-mtd
@@ -128,9 +128,8 @@  KernelVersion:	3.4
 Contact:	linux-mtd@lists.infradead.org
 Description:
 		Maximum number of bit errors that the device is capable of
-		correcting within each region covering an ecc step.  This will
-		always be a non-negative integer.  Note that some devices will
-		have multiple ecc steps within each writesize region.
+		correcting within each region covering an ECC step (see
+		ecc_step_size).  This will always be a non-negative integer.
 
 		In the case of devices lacking any ECC capability, it is 0.
 
@@ -173,3 +172,16 @@  Description:
 		This is generally applicable only to NAND flash devices with ECC
 		capability.  It is ignored on devices lacking ECC capability;
 		i.e., devices for which ecc_strength is zero.
+
+What:		/sys/class/mtd/mtdX/ecc_step_size
+Date:		May 2013
+KernelVersion:	3.10
+Contact:	linux-mtd@lists.infradead.org
+Description:
+		The size of a single region covered by ECC, known as the ECC
+		step.  Devices may have several equally sized ECC steps within
+		each writesize region.  The step size counts only the data area,
+		not the spare area.
+
+		It will always be a non-negative integer.  In the case of
+		devices lacking any ECC capability, it is 0.