Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-10 Thread Scott Wood
On Mon, 2015-08-10 at 09:17 +0800, Peng Fan wrote:
 Hi Scott,
 
 Do you have plan to pick the 3 patches?
 
 https://patchwork.ozlabs.org/patch/498050/
 https://patchwork.ozlabs.org/patch/498049/
 https://patchwork.ozlabs.org/patch/498048/

Yes.

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-09 Thread Peng Fan
Hi Scott,

Do you have plan to pick the 3 patches?

https://patchwork.ozlabs.org/patch/498050/
https://patchwork.ozlabs.org/patch/498049/
https://patchwork.ozlabs.org/patch/498048/

If not, then I prefer these 3 patches can go throught i.mx tree.

Thanks,
Peng.

On Sun, Aug 02, 2015 at 11:18:38AM +0800, Peng Fan wrote:
On Sat, Aug 01, 2015 at 01:54:48PM -0500, Scott Wood wrote:
On Sat, 2015-08-01 at 20:38 +0200, Marek Vasut wrote:
 On Saturday, August 01, 2015 at 08:32:07 PM, Scott Wood wrote:
  On Sat, 2015-08-01 at 17:18 +0200, Marek Vasut wrote:
   On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
 On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
  On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
   On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
If ecc chunk data size is 512 and oobsize is bigger than 512,
there
is a chance that block_mark_bit_offset conflicts with bch ecc
area.

The following graph is modified from kernel gpmi-nand.c driver
with
each data block 512 bytes. We can see that Block Mark 
conflicts
with
ecc area from bch view. We can enlarge the ecc chunk size to
avoid this problem to those oobsize which is larger than 512.
   
   Enlarge it by how much?  What does the layout look like in that
   case?
  
  Enlarge it to 1024 bytes.
 
 Then say so in the changelog.

You mean I need to add this in commit msg and send out a new patch
version?
Or you pick this one?
   
   This discussion is becoming ridiculous, can we please get this bugfix
   applied ?
   If you don't like some minor details in the commit message, can you
   please fix
   them while applying ?
  
  Yes, I can edit the changelog while applying, but that doesn't mean I'm 
  not
  going to complain about a difficult-to-understand changelog, and I still
  would like to understand what is actually going on here.  Don't assume I'm
  familiar with this hardware or its unusual page layout.  You can help by
  explaining things, or you can not help by throwing a fit...
 
 I can point you to MX28 datasheet [1] chapter 16.2.2 and onward if you want
 to educate yourself, it's all explained there, concisely and clearly.
 
 [1] http://free-electrons.com/~maxime/pub/datasheet/MCIMX28RM.pdf

Thanks.  That preempted a question I was just about to ask Peng, because it 
wasn't clear that the meta area was covered by ECC.

In mxs_nand.c driver, we use Combined Metadata  Block 0, unbalanced ECC 
coverage layout from chapter 16.2.2 of MX28 datasheet.

Peng.

-Scott


-- 

-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Peng Fan
On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
 On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
  On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
   If ecc chunk data size is 512 and oobsize is bigger than 512, there is
   a chance that block_mark_bit_offset conflicts with bch ecc area.
   
   The following graph is modified from kernel gpmi-nand.c driver with
   each data block 512 bytes. We can see that Block Mark conflicts with
   ecc area from bch view. We can enlarge the ecc chunk size to avoid
   this problem to those oobsize which is larger than 512.
  
  Enlarge it by how much?  What does the layout look like in that case?
 
 Enlarge it to 1024 bytes.

Then say so in the changelog.

You mean I need to add this in commit msg and send out a new patch version?
Or you pick this one?


 In normal case the Block Mark bit should locates at the data part, but
 not at the ECC area.

It seems like either would be bad...

No. To read/write with ecc enabled, the block mark will be swapped with byte in 
metadata area.
This is correct way for GPMI + BCH to perform nand reading/writing with ecc 
enabled.
See following:
645 /* Read DMA completed, now do the mark swapping. */
646 mxs_nand_swap_block_mark(mtd, nand_info-data_buf, 
nand_info-oob_buf);

When GPMI + BCH used for DMA read, meta data will be stored to oob_buf. Then
swap the data at Block Mark with data at oob_buf[0](actually metadata[0]).

Before DMA write, Block Mark will be swapped with metadata[0]. Then DMA write.
Then Block Mark should not conlicts with ECC area, but ok with DATA area. Anyway
Block Mark should be aways points to bit at a data block.


 You can take the following as an example.
 
|  P|
|-|
|   |
|  (Block Mark) |
|  P' |   | |   |
|---| D | | O'|
| |-| |-|
V V   V V   V
+---+---+-+---+-+---+
| M |   data|E|   data data   |E|   |
+---+---+-+---+-+---+
 ^  ^
 | O|
 ||
 
 
  
   
  |  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.
  
  D is in the diagram but not the key.  G, N, and C are in the key 
  but 
  not the diagram.
 C is ecc chunk data size, same the data area. Here it is 512 bytes. 
 N is chunk count. for exmaple page size is 2048 bytes, ecc chunk data size 
 is 512 bytes, then N is 2048/512.
 G is the Galois Field, its value is 13 or 14. The should be got from 
 reference mannual.
 D is in the digram , but not the key. I think it is enougth to show why 
 this patch is done.
 
 I put the digram here to show that Block Mark may conflicts with ECC area,
 and this is wrong, we need to find a way to avoid this. And the way is
 to enlarge the ecc chunk data size from 512 to 1024 bytes.
 
 I did not put the 

Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Marek Vasut
On Saturday, August 01, 2015 at 08:32:07 PM, Scott Wood wrote:
 On Sat, 2015-08-01 at 17:18 +0200, Marek Vasut wrote:
  On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
   On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
 On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
  On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
   If ecc chunk data size is 512 and oobsize is bigger than 512,
   there
   is a chance that block_mark_bit_offset conflicts with bch ecc
   area.
   
   The following graph is modified from kernel gpmi-nand.c driver
   with
   each data block 512 bytes. We can see that Block Mark conflicts
   with
   ecc area from bch view. We can enlarge the ecc chunk size to
   avoid this problem to those oobsize which is larger than 512.
  
  Enlarge it by how much?  What does the layout look like in that
  case?
 
 Enlarge it to 1024 bytes.

Then say so in the changelog.
   
   You mean I need to add this in commit msg and send out a new patch
   version?
   Or you pick this one?
  
  This discussion is becoming ridiculous, can we please get this bugfix
  applied ?
  If you don't like some minor details in the commit message, can you
  please fix
  them while applying ?
 
 Yes, I can edit the changelog while applying, but that doesn't mean I'm not
 going to complain about a difficult-to-understand changelog, and I still
 would like to understand what is actually going on here.  Don't assume I'm
 familiar with this hardware or its unusual page layout.  You can help by
 explaining things, or you can not help by throwing a fit...

I can point you to MX28 datasheet [1] chapter 16.2.2 and onward if you want
to educate yourself, it's all explained there, concisely and clearly.

[1] http://free-electrons.com/~maxime/pub/datasheet/MCIMX28RM.pdf

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Scott Wood
On Sat, 2015-08-01 at 17:18 +0200, Marek Vasut wrote:
 On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
  On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
   On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
 On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
  If ecc chunk data size is 512 and oobsize is bigger than 512, 
  there
  is a chance that block_mark_bit_offset conflicts with bch ecc 
  area.
  
  The following graph is modified from kernel gpmi-nand.c driver 
  with
  each data block 512 bytes. We can see that Block Mark conflicts 
  with
  ecc area from bch view. We can enlarge the ecc chunk size to avoid
  this problem to those oobsize which is larger than 512.
 
 Enlarge it by how much?  What does the layout look like in that 
 case?

Enlarge it to 1024 bytes.
   
   Then say so in the changelog.
  
  You mean I need to add this in commit msg and send out a new patch 
  version?
  Or you pick this one?
 
 This discussion is becoming ridiculous, can we please get this bugfix 
 applied ?
 If you don't like some minor details in the commit message, can you please 
 fix
 them while applying ?

Yes, I can edit the changelog while applying, but that doesn't mean I'm not 
going to complain about a difficult-to-understand changelog, and I still 
would like to understand what is actually going on here.  Don't assume I'm 
familiar with this hardware or its unusual page layout.  You can help by 
explaining things, or you can not help by throwing a fit...

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Peng Fan
On Sat, Aug 01, 2015 at 01:54:48PM -0500, Scott Wood wrote:
On Sat, 2015-08-01 at 20:38 +0200, Marek Vasut wrote:
 On Saturday, August 01, 2015 at 08:32:07 PM, Scott Wood wrote:
  On Sat, 2015-08-01 at 17:18 +0200, Marek Vasut wrote:
   On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
 On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
  On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
   On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
If ecc chunk data size is 512 and oobsize is bigger than 512,
there
is a chance that block_mark_bit_offset conflicts with bch ecc
area.

The following graph is modified from kernel gpmi-nand.c driver
with
each data block 512 bytes. We can see that Block Mark 
conflicts
with
ecc area from bch view. We can enlarge the ecc chunk size to
avoid this problem to those oobsize which is larger than 512.
   
   Enlarge it by how much?  What does the layout look like in that
   case?
  
  Enlarge it to 1024 bytes.
 
 Then say so in the changelog.

You mean I need to add this in commit msg and send out a new patch
version?
Or you pick this one?
   
   This discussion is becoming ridiculous, can we please get this bugfix
   applied ?
   If you don't like some minor details in the commit message, can you
   please fix
   them while applying ?
  
  Yes, I can edit the changelog while applying, but that doesn't mean I'm 
  not
  going to complain about a difficult-to-understand changelog, and I still
  would like to understand what is actually going on here.  Don't assume I'm
  familiar with this hardware or its unusual page layout.  You can help by
  explaining things, or you can not help by throwing a fit...
 
 I can point you to MX28 datasheet [1] chapter 16.2.2 and onward if you want
 to educate yourself, it's all explained there, concisely and clearly.
 
 [1] http://free-electrons.com/~maxime/pub/datasheet/MCIMX28RM.pdf

Thanks.  That preempted a question I was just about to ask Peng, because it 
wasn't clear that the meta area was covered by ECC.

In mxs_nand.c driver, we use Combined Metadata  Block 0, unbalanced ECC 
coverage layout from chapter 16.2.2 of MX28 datasheet.

Peng.

-Scott


-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Scott Wood
On Sat, 2015-08-01 at 20:38 +0200, Marek Vasut wrote:
 On Saturday, August 01, 2015 at 08:32:07 PM, Scott Wood wrote:
  On Sat, 2015-08-01 at 17:18 +0200, Marek Vasut wrote:
   On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
 On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
  On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
   On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
If ecc chunk data size is 512 and oobsize is bigger than 512,
there
is a chance that block_mark_bit_offset conflicts with bch ecc
area.

The following graph is modified from kernel gpmi-nand.c driver
with
each data block 512 bytes. We can see that Block Mark 
conflicts
with
ecc area from bch view. We can enlarge the ecc chunk size to
avoid this problem to those oobsize which is larger than 512.
   
   Enlarge it by how much?  What does the layout look like in that
   case?
  
  Enlarge it to 1024 bytes.
 
 Then say so in the changelog.

You mean I need to add this in commit msg and send out a new patch
version?
Or you pick this one?
   
   This discussion is becoming ridiculous, can we please get this bugfix
   applied ?
   If you don't like some minor details in the commit message, can you
   please fix
   them while applying ?
  
  Yes, I can edit the changelog while applying, but that doesn't mean I'm 
  not
  going to complain about a difficult-to-understand changelog, and I still
  would like to understand what is actually going on here.  Don't assume I'm
  familiar with this hardware or its unusual page layout.  You can help by
  explaining things, or you can not help by throwing a fit...
 
 I can point you to MX28 datasheet [1] chapter 16.2.2 and onward if you want
 to educate yourself, it's all explained there, concisely and clearly.
 
 [1] http://free-electrons.com/~maxime/pub/datasheet/MCIMX28RM.pdf

Thanks.  That preempted a question I was just about to ask Peng, because it 
wasn't clear that the meta area was covered by ECC.

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-08-01 Thread Marek Vasut
On Saturday, August 01, 2015 at 07:56:39 AM, Peng Fan wrote:
 On Fri, Jul 31, 2015 at 09:36:45PM -0500, Scott Wood wrote:
 On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
  On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
   On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
If ecc chunk data size is 512 and oobsize is bigger than 512, there
is a chance that block_mark_bit_offset conflicts with bch ecc area.

The following graph is modified from kernel gpmi-nand.c driver with
each data block 512 bytes. We can see that Block Mark conflicts with
ecc area from bch view. We can enlarge the ecc chunk size to avoid
this problem to those oobsize which is larger than 512.
   
   Enlarge it by how much?  What does the layout look like in that case?
  
  Enlarge it to 1024 bytes.
 
 Then say so in the changelog.
 
 You mean I need to add this in commit msg and send out a new patch version?
 Or you pick this one?

This discussion is becoming ridiculous, can we please get this bugfix applied ?
If you don't like some minor details in the commit message, can you please fix
them while applying ?

Thanks

[...]
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-07-31 Thread Peng Fan
On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
 If ecc chunk data size is 512 and oobsize is bigger than 512, there is
 a chance that block_mark_bit_offset conflicts with bch ecc area.
 
 The following graph is modified from kernel gpmi-nand.c driver with
 each data block 512 bytes. We can see that Block Mark conflicts with
 ecc area from bch view. We can enlarge the ecc chunk size to avoid
 this problem to those oobsize which is larger than 512.

Enlarge it by how much?  What does the layout look like in that case?

Enlarge it to 1024 bytes.
In normal case the Block Mark bit should locates at the data part, but
not at the ECC area.
You can take the following as an example.

   |  P|
   |-|
   |   |
   |  (Block Mark) |
   |  P' |   | |   |
   |---| D | | O'|
   | |-| |-|
   V V   V V   V
   +---+---+-+---+-+---+
   | M |   data|E|   data data   |E|   |
   +---+---+-+---+-+---+
^  ^
| O|
||



 
|  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.

D is in the diagram but not the key.  G, N, and C are in the key but 
not the diagram.
C is ecc chunk data size, same the data area. Here it is 512 bytes. 
N is chunk count. for exmaple page size is 2048 bytes, ecc chunk data size is 
512 bytes, then N is 2048/512.
G is the Galois Field, its value is 13 or 14. The should be got from reference 
mannual.
D is in the digram , but not the key. I think it is enougth to show why this 
patch is done.

I put the digram here to show that Block Mark may conflicts with ECC area,
and this is wrong, we need to find a way to avoid this. And the way is
to enlarge the ecc chunk data size from 512 to 1024 bytes.

I did not put the formula here. The keys is same to keys in kernel gpmi-nand.c.

  The formula for P is : 

  E * G * N
 P =  + P' + M
  8

The position of block mark moves forward in the ECC-based view of page, and the 
delta is:   

   E * G * (N - 1)
 D = ( + M)
  8 

I think there is no need to put the formula in commit msg. Also, to explain D 
is not relevant to this patch.


What is the metasize of per page?

10 bytes.

Peng.


-Scott


-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-07-31 Thread Tim Harvey
On Tue, Jul 21, 2015 at 1:15 AM, Peng Fan peng@freescale.com wrote:
 If ecc chunk data size is 512 and oobsize is bigger than 512, there is
 a chance that block_mark_bit_offset conflicts with bch ecc area.

 The following graph is modified from kernel gpmi-nand.c driver with
 each data block 512 bytes. We can see that Block Mark conflicts with
 ecc area from bch view. We can enlarge the ecc chunk size to avoid
 this problem to those oobsize which is larger than 512.

|  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.

 Signed-off-by: Peng Fan peng@freescale.com
 Reviewed-by: Marek Vasut ma...@denx.de
 ---

 Changes v2:
  none

 Changes v1:
 The previous patch is https://patchwork.ozlabs.org/patch/422757/
 This version contains a minor change to the name from gf_len to galois_field.
 Also add Marek's Reviewed-by.

  arch/arm/include/asm/imx-common/regs-bch.h |  2 ++
  drivers/mtd/nand/mxs_nand.c| 32 
 +-
  2 files changed, 25 insertions(+), 9 deletions(-)

 diff --git a/arch/arm/include/asm/imx-common/regs-bch.h 
 b/arch/arm/include/asm/imx-common/regs-bch.h
 index a33d341..5c47783 100644
 --- a/arch/arm/include/asm/imx-common/regs-bch.h
 +++ b/arch/arm/include/asm/imx-common/regs-bch.h
 @@ -148,6 +148,7 @@ struct mxs_bch_regs {
  #defineBCH_FLASHLAYOUT0_ECC0_ECC30 (0xf  12)
  #defineBCH_FLASHLAYOUT0_ECC0_ECC32 (0x10  12)
  #defineBCH_FLASHLAYOUT0_GF13_0_GF14_1  (1  10)
 +#defineBCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET   10
  #defineBCH_FLASHLAYOUT0_DATA0_SIZE_MASK0xfff
  #defineBCH_FLASHLAYOUT0_DATA0_SIZE_OFFSET  0

 @@ -178,6 +179,7 @@ struct mxs_bch_regs {
  #defineBCH_FLASHLAYOUT1_ECCN_ECC30 (0xf  12)
  #defineBCH_FLASHLAYOUT1_ECCN_ECC32 (0x10  12)
  #defineBCH_FLASHLAYOUT1_GF13_0_GF14_1  (1  10)
 +#defineBCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET   10
  #defineBCH_FLASHLAYOUT1_DATAN_SIZE_MASK0xfff
  #defineBCH_FLASHLAYOUT1_DATAN_SIZE_OFFSET  0

 diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
 index 9c144a2..33ce817 100644
 --- a/drivers/mtd/nand/mxs_nand.c
 +++ b/drivers/mtd/nand/mxs_nand.c
 @@ -68,6 +68,8 @@ struct mxs_nand_info {
  };

  struct nand_ecclayout fake_ecc_layout;
 +static int chunk_data_size = MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
 +static int galois_field = 13;

  /*
   * Cache management functions
 @@ -130,12 +132,12 @@ static void mxs_nand_return_dma_descs(struct 
 mxs_nand_info *info)

  static uint32_t mxs_nand_ecc_chunk_cnt(uint32_t page_data_size)
  {
 -   return page_data_size / MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
 +   return page_data_size / chunk_data_size;
  }

  static uint32_t mxs_nand_ecc_size_in_bits(uint32_t ecc_strength)
  {
 -   return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
 +   return ecc_strength * galois_field;
  }

  static uint32_t mxs_nand_aux_status_offset(void)
 @@ -157,8 +159,8 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t 
 page_data_size,
  *  (page oob size - meta data size) * (bits per byte)
  */
 ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
 -   / (MXS_NAND_BITS_PER_ECC_LEVEL *
 -   mxs_nand_ecc_chunk_cnt(page_data_size));
 +   / (galois_field *
 +  mxs_nand_ecc_chunk_cnt(page_data_size));

 return 

Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-07-31 Thread Scott Wood
On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
 If ecc chunk data size is 512 and oobsize is bigger than 512, there is
 a chance that block_mark_bit_offset conflicts with bch ecc area.
 
 The following graph is modified from kernel gpmi-nand.c driver with
 each data block 512 bytes. We can see that Block Mark conflicts with
 ecc area from bch view. We can enlarge the ecc chunk size to avoid
 this problem to those oobsize which is larger than 512.

Enlarge it by how much?  What does the layout look like in that case?

 
|  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.

D is in the diagram but not the key.  G, N, and C are in the key but 
not the diagram.

What is the metasize of per page?

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-07-31 Thread Scott Wood
On Sat, 2015-08-01 at 09:15 +0800, Peng Fan wrote:
 On Fri, Jul 31, 2015 at 12:07:50PM -0500, Scott Wood wrote:
  On Tue, 2015-07-21 at 16:15 +0800, Peng Fan wrote:
   If ecc chunk data size is 512 and oobsize is bigger than 512, there is
   a chance that block_mark_bit_offset conflicts with bch ecc area.
   
   The following graph is modified from kernel gpmi-nand.c driver with
   each data block 512 bytes. We can see that Block Mark conflicts with
   ecc area from bch view. We can enlarge the ecc chunk size to avoid
   this problem to those oobsize which is larger than 512.
  
  Enlarge it by how much?  What does the layout look like in that case?
 
 Enlarge it to 1024 bytes.

Then say so in the changelog.

 In normal case the Block Mark bit should locates at the data part, but
 not at the ECC area.

It seems like either would be bad...

 You can take the following as an example.
 
|  P|
|-|
|   |
|  (Block Mark) |
|  P' |   | |   |
|---| D | | O'|
| |-| |-|
V V   V V   V
+---+---+-+---+-+---+
| M |   data|E|   data data   |E|   |
+---+---+-+---+-+---+
 ^  ^
 | O|
 ||
 
 
  
   
  |  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.
  
  D is in the diagram but not the key.  G, N, and C are in the key 
  but 
  not the diagram.
 C is ecc chunk data size, same the data area. Here it is 512 bytes. 
 N is chunk count. for exmaple page size is 2048 bytes, ecc chunk data size 
 is 512 bytes, then N is 2048/512.
 G is the Galois Field, its value is 13 or 14. The should be got from 
 reference mannual.
 D is in the digram , but not the key. I think it is enougth to show why 
 this patch is done.
 
 I put the digram here to show that Block Mark may conflicts with ECC area,
 and this is wrong, we need to find a way to avoid this. And the way is
 to enlarge the ecc chunk data size from 512 to 1024 bytes.
 
 I did not put the formula here. The keys is same to keys in kernel gpmi-
 nand.c.
 
   The formula for P is : 
 
   E * G * N
  P =  + P' + M
   8
 
 The position of block mark moves forward in the ECC-based view of page, and 
 the delta is:   
 
E * G * (N - 1)
  D = ( + M)
   8 
 
 I think there is no need to put the formula in commit msg. Also, to explain 
 D is not relevant to this patch.

I'm not asking for the above to go into the commit message.  I'm asking for 
what does go in the commit message to be less cryptic.

  What is the metasize of per page?
 
 10 bytes.

But what does it mean?

-Scott

___
U-Boot mailing list
U-Boot@lists.denx.de

[U-Boot] [PATCH resend V2 1/3] mtd: nand: mxs support oobsize bigger than 512

2015-07-21 Thread Peng Fan
If ecc chunk data size is 512 and oobsize is bigger than 512, there is
a chance that block_mark_bit_offset conflicts with bch ecc area.

The following graph is modified from kernel gpmi-nand.c driver with
each data block 512 bytes. We can see that Block Mark conflicts with
ecc area from bch view. We can enlarge the ecc chunk size to avoid
this problem to those oobsize which is larger than 512.

   |  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.

Signed-off-by: Peng Fan peng@freescale.com
Reviewed-by: Marek Vasut ma...@denx.de
---

Changes v2:
 none

Changes v1:
The previous patch is https://patchwork.ozlabs.org/patch/422757/
This version contains a minor change to the name from gf_len to galois_field.
Also add Marek's Reviewed-by.

 arch/arm/include/asm/imx-common/regs-bch.h |  2 ++
 drivers/mtd/nand/mxs_nand.c| 32 +-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/imx-common/regs-bch.h 
b/arch/arm/include/asm/imx-common/regs-bch.h
index a33d341..5c47783 100644
--- a/arch/arm/include/asm/imx-common/regs-bch.h
+++ b/arch/arm/include/asm/imx-common/regs-bch.h
@@ -148,6 +148,7 @@ struct mxs_bch_regs {
 #defineBCH_FLASHLAYOUT0_ECC0_ECC30 (0xf  12)
 #defineBCH_FLASHLAYOUT0_ECC0_ECC32 (0x10  12)
 #defineBCH_FLASHLAYOUT0_GF13_0_GF14_1  (1  10)
+#defineBCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET   10
 #defineBCH_FLASHLAYOUT0_DATA0_SIZE_MASK0xfff
 #defineBCH_FLASHLAYOUT0_DATA0_SIZE_OFFSET  0
 
@@ -178,6 +179,7 @@ struct mxs_bch_regs {
 #defineBCH_FLASHLAYOUT1_ECCN_ECC30 (0xf  12)
 #defineBCH_FLASHLAYOUT1_ECCN_ECC32 (0x10  12)
 #defineBCH_FLASHLAYOUT1_GF13_0_GF14_1  (1  10)
+#defineBCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET   10
 #defineBCH_FLASHLAYOUT1_DATAN_SIZE_MASK0xfff
 #defineBCH_FLASHLAYOUT1_DATAN_SIZE_OFFSET  0
 
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 9c144a2..33ce817 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -68,6 +68,8 @@ struct mxs_nand_info {
 };
 
 struct nand_ecclayout fake_ecc_layout;
+static int chunk_data_size = MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
+static int galois_field = 13;
 
 /*
  * Cache management functions
@@ -130,12 +132,12 @@ static void mxs_nand_return_dma_descs(struct 
mxs_nand_info *info)
 
 static uint32_t mxs_nand_ecc_chunk_cnt(uint32_t page_data_size)
 {
-   return page_data_size / MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
+   return page_data_size / chunk_data_size;
 }
 
 static uint32_t mxs_nand_ecc_size_in_bits(uint32_t ecc_strength)
 {
-   return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
+   return ecc_strength * galois_field;
 }
 
 static uint32_t mxs_nand_aux_status_offset(void)
@@ -157,8 +159,8 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t 
page_data_size,
 *  (page oob size - meta data size) * (bits per byte)
 */
ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
-   / (MXS_NAND_BITS_PER_ECC_LEVEL *
-   mxs_nand_ecc_chunk_cnt(page_data_size));
+   / (galois_field *
+  mxs_nand_ecc_chunk_cnt(page_data_size));
 
return round_down(ecc_strength, 2);
 }
@@ -173,7 +175,7 @@ static inline uint32_t mxs_nand_get_mark_offset(uint32_t 
page_data_size,
uint32_t