All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
@ 2014-12-19  4:39 Peng Fan
  2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Peng Fan @ 2014-12-19  4:39 UTC (permalink / raw)
  To: u-boot

Calculate ecc strength according oobsize, but not hardcoded
which is not aligned with kernel driver

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Signed-off-by: Ye.Li <b37916@freescale.com>
---
 drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 7a064ab..a45fcf9 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
 static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
 						uint32_t page_oob_size)
 {
-	if (page_data_size == 2048) {
-		if (page_oob_size == 64)
-			return 8;
+	int ecc_strength;
 
-		if (page_oob_size == 112)
-			return 14;
-	}
-
-	if (page_data_size == 4096) {
-		if (page_oob_size == 128)
-			return 8;
-
-		if (page_oob_size == 218)
-			return 16;
+	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
+			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
 
-		if (page_oob_size == 224)
-			return 16;
-	}
-
-	return 0;
+	return round_down(ecc_strength, 2);
 }
 
 static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512
  2014-12-19  4:39 [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
@ 2014-12-19  4:39 ` Peng Fan
  2015-01-20  6:35   ` Peng Fan
  2015-01-20 11:03   ` Marek Vasut
  2015-01-20  6:35 ` [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Peng Fan @ 2014-12-19  4:39 UTC (permalink / raw)
  To: u-boot

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.Fan@freescale.com>
---
 arch/arm/include/asm/imx-common/regs-bch.h |  2 ++
 drivers/mtd/nand/mxs_nand.c                | 33 ++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 8 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 {
 #define	BCH_FLASHLAYOUT0_ECC0_ECC30			(0xf << 12)
 #define	BCH_FLASHLAYOUT0_ECC0_ECC32			(0x10 << 12)
 #define	BCH_FLASHLAYOUT0_GF13_0_GF14_1			(1 << 10)
+#define	BCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET		10
 #define	BCH_FLASHLAYOUT0_DATA0_SIZE_MASK		0xfff
 #define	BCH_FLASHLAYOUT0_DATA0_SIZE_OFFSET		0
 
@@ -178,6 +179,7 @@ struct mxs_bch_regs {
 #define	BCH_FLASHLAYOUT1_ECCN_ECC30			(0xf << 12)
 #define	BCH_FLASHLAYOUT1_ECCN_ECC32			(0x10 << 12)
 #define	BCH_FLASHLAYOUT1_GF13_0_GF14_1			(1 << 10)
+#define	BCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET		10
 #define	BCH_FLASHLAYOUT1_DATAN_SIZE_MASK		0xfff
 #define	BCH_FLASHLAYOUT1_DATAN_SIZE_OFFSET		0
 
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index a45fcf9..0db9eb3 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -29,6 +29,7 @@
 
 #define	MXS_NAND_DMA_DESCRIPTOR_COUNT		4
 
+#define	MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE	1024
 #define	MXS_NAND_CHUNK_DATA_CHUNK_SIZE		512
 #if defined(CONFIG_MX6)
 #define	MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT	2
@@ -68,6 +69,8 @@ struct mxs_nand_info {
 };
 
 struct nand_ecclayout fake_ecc_layout;
+static int chunk_data_size = MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
+static int gf_len = 13;
 
 /*
  * Cache management functions
@@ -130,12 +133,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 * 13;
+	return ecc_strength * gf_len;
 }
 
 static uint32_t mxs_nand_aux_status_offset(void)
@@ -149,7 +152,7 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
 	int ecc_strength;
 
 	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
-			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
+			/ (gf_len * mxs_nand_ecc_chunk_cnt(page_data_size));
 
 	return round_down(ecc_strength, 2);
 }
@@ -164,7 +167,7 @@ static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
 	uint32_t block_mark_chunk_bit_offset;
 	uint32_t block_mark_bit_offset;
 
-	chunk_data_size_in_bits = MXS_NAND_CHUNK_DATA_CHUNK_SIZE * 8;
+	chunk_data_size_in_bits = chunk_data_size * 8;
 	chunk_ecc_size_in_bits  = mxs_nand_ecc_size_in_bits(ecc_strength);
 
 	chunk_total_size_in_bits =
@@ -963,6 +966,20 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
 	struct mxs_bch_regs *bch_regs = (struct mxs_bch_regs *)MXS_BCH_BASE;
 	uint32_t tmp;
 
+	if (mtd->oobsize > MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE) {
+		printf("We do not support the NAND device whose oob size is larger then %d bytes!\n", MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE);
+		return -EINVAL;
+	}
+
+	/*
+	 * If oobsize bigger than 512, galois filed should be 14, ecc chunk
+	 * data size should be 1024 but not 512.
+	 */
+	if (mtd->oobsize > MXS_NAND_CHUNK_DATA_CHUNK_SIZE) {
+		gf_len = 14;
+		chunk_data_size = MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE;
+	}
+
 	/* Configure BCH and set NFC geometry */
 	mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
 
@@ -972,16 +989,16 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
 	tmp |= MXS_NAND_METADATA_SIZE << BCH_FLASHLAYOUT0_META_SIZE_OFFSET;
 	tmp |= (mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1)
 		<< BCH_FLASHLAYOUT0_ECC0_OFFSET;
-	tmp |= MXS_NAND_CHUNK_DATA_CHUNK_SIZE
-		>> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
+	tmp |= chunk_data_size >> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
+	tmp |= (14 == gf_len ? 1 : 0) << BCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET;
 	writel(tmp, &bch_regs->hw_bch_flash0layout0);
 
 	tmp = (mtd->writesize + mtd->oobsize)
 		<< BCH_FLASHLAYOUT1_PAGE_SIZE_OFFSET;
 	tmp |= (mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1)
 		<< BCH_FLASHLAYOUT1_ECCN_OFFSET;
-	tmp |= MXS_NAND_CHUNK_DATA_CHUNK_SIZE
-		>> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
+	tmp |= chunk_data_size >> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
+	tmp |= (14 == gf_len ? 1 : 0) << BCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET;
 	writel(tmp, &bch_regs->hw_bch_flash0layout1);
 
 	/* Set *all* chip selects to use layout 0 */
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
  2014-12-19  4:39 [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
  2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
@ 2015-01-20  6:35 ` Peng Fan
  2015-01-20 11:04   ` Marek Vasut
  2015-01-20 11:02 ` Marek Vasut
  2015-01-27 23:14 ` Jörg Krause
  3 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2015-01-20  6:35 UTC (permalink / raw)
  To: u-boot

Hi Marek,

Since you are familiar with this driver, would you please help review 
this patch?

On 12/19/2014 12:39 PM, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>
> ---
>   drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
>   1 file changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 7a064ab..a45fcf9 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
>   static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>   						uint32_t page_oob_size)
>   {
> -	if (page_data_size == 2048) {
> -		if (page_oob_size == 64)
> -			return 8;
> +	int ecc_strength;
>   
> -		if (page_oob_size == 112)
> -			return 14;
> -	}
> -
> -	if (page_data_size == 4096) {
> -		if (page_oob_size == 128)
> -			return 8;
> -
> -		if (page_oob_size == 218)
> -			return 16;
> +	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> +			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
>   
> -		if (page_oob_size == 224)
> -			return 16;
> -	}
> -
> -	return 0;
> +	return round_down(ecc_strength, 2);
>   }
>   
>   static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
Thanks,
Peng.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512
  2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
@ 2015-01-20  6:35   ` Peng Fan
  2015-01-20 11:03   ` Marek Vasut
  1 sibling, 0 replies; 11+ messages in thread
From: Peng Fan @ 2015-01-20  6:35 UTC (permalink / raw)
  To: u-boot

Hi Marek,

And this one.

On 12/19/2014 12:39 PM, 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.
>
>     |                          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.Fan@freescale.com>
> ---
>   arch/arm/include/asm/imx-common/regs-bch.h |  2 ++
>   drivers/mtd/nand/mxs_nand.c                | 33 ++++++++++++++++++++++--------
>   2 files changed, 27 insertions(+), 8 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 {
>   #define	BCH_FLASHLAYOUT0_ECC0_ECC30			(0xf << 12)
>   #define	BCH_FLASHLAYOUT0_ECC0_ECC32			(0x10 << 12)
>   #define	BCH_FLASHLAYOUT0_GF13_0_GF14_1			(1 << 10)
> +#define	BCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET		10
>   #define	BCH_FLASHLAYOUT0_DATA0_SIZE_MASK		0xfff
>   #define	BCH_FLASHLAYOUT0_DATA0_SIZE_OFFSET		0
>   
> @@ -178,6 +179,7 @@ struct mxs_bch_regs {
>   #define	BCH_FLASHLAYOUT1_ECCN_ECC30			(0xf << 12)
>   #define	BCH_FLASHLAYOUT1_ECCN_ECC32			(0x10 << 12)
>   #define	BCH_FLASHLAYOUT1_GF13_0_GF14_1			(1 << 10)
> +#define	BCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET		10
>   #define	BCH_FLASHLAYOUT1_DATAN_SIZE_MASK		0xfff
>   #define	BCH_FLASHLAYOUT1_DATAN_SIZE_OFFSET		0
>   
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index a45fcf9..0db9eb3 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -29,6 +29,7 @@
>   
>   #define	MXS_NAND_DMA_DESCRIPTOR_COUNT		4
>   
> +#define	MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE	1024
>   #define	MXS_NAND_CHUNK_DATA_CHUNK_SIZE		512
>   #if defined(CONFIG_MX6)
>   #define	MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT	2
> @@ -68,6 +69,8 @@ struct mxs_nand_info {
>   };
>   
>   struct nand_ecclayout fake_ecc_layout;
> +static int chunk_data_size = MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
> +static int gf_len = 13;
>   
>   /*
>    * Cache management functions
> @@ -130,12 +133,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 * 13;
> +	return ecc_strength * gf_len;
>   }
>   
>   static uint32_t mxs_nand_aux_status_offset(void)
> @@ -149,7 +152,7 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>   	int ecc_strength;
>   
>   	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> -			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
> +			/ (gf_len * mxs_nand_ecc_chunk_cnt(page_data_size));
>   
>   	return round_down(ecc_strength, 2);
>   }
> @@ -164,7 +167,7 @@ static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
>   	uint32_t block_mark_chunk_bit_offset;
>   	uint32_t block_mark_bit_offset;
>   
> -	chunk_data_size_in_bits = MXS_NAND_CHUNK_DATA_CHUNK_SIZE * 8;
> +	chunk_data_size_in_bits = chunk_data_size * 8;
>   	chunk_ecc_size_in_bits  = mxs_nand_ecc_size_in_bits(ecc_strength);
>   
>   	chunk_total_size_in_bits =
> @@ -963,6 +966,20 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
>   	struct mxs_bch_regs *bch_regs = (struct mxs_bch_regs *)MXS_BCH_BASE;
>   	uint32_t tmp;
>   
> +	if (mtd->oobsize > MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE) {
> +		printf("We do not support the NAND device whose oob size is larger then %d bytes!\n", MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * If oobsize bigger than 512, galois filed should be 14, ecc chunk
> +	 * data size should be 1024 but not 512.
> +	 */
> +	if (mtd->oobsize > MXS_NAND_CHUNK_DATA_CHUNK_SIZE) {
> +		gf_len = 14;
> +		chunk_data_size = MXS_NAND_MAX_CHUNK_DATA_CHUNK_SIZE;
> +	}
> +
>   	/* Configure BCH and set NFC geometry */
>   	mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
>   
> @@ -972,16 +989,16 @@ static int mxs_nand_scan_bbt(struct mtd_info *mtd)
>   	tmp |= MXS_NAND_METADATA_SIZE << BCH_FLASHLAYOUT0_META_SIZE_OFFSET;
>   	tmp |= (mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1)
>   		<< BCH_FLASHLAYOUT0_ECC0_OFFSET;
> -	tmp |= MXS_NAND_CHUNK_DATA_CHUNK_SIZE
> -		>> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
> +	tmp |= chunk_data_size >> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
> +	tmp |= (14 == gf_len ? 1 : 0) << BCH_FLASHLAYOUT0_GF13_0_GF14_1_OFFSET;
>   	writel(tmp, &bch_regs->hw_bch_flash0layout0);
>   
>   	tmp = (mtd->writesize + mtd->oobsize)
>   		<< BCH_FLASHLAYOUT1_PAGE_SIZE_OFFSET;
>   	tmp |= (mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1)
>   		<< BCH_FLASHLAYOUT1_ECCN_OFFSET;
> -	tmp |= MXS_NAND_CHUNK_DATA_CHUNK_SIZE
> -		>> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
> +	tmp |= chunk_data_size >> MXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT;
> +	tmp |= (14 == gf_len ? 1 : 0) << BCH_FLASHLAYOUT1_GF13_0_GF14_1_OFFSET;
>   	writel(tmp, &bch_regs->hw_bch_flash0layout1);
>   
>   	/* Set *all* chip selects to use layout 0 */
Thanks,
Peng.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
  2014-12-19  4:39 [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
  2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
  2015-01-20  6:35 ` [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
@ 2015-01-20 11:02 ` Marek Vasut
  2015-01-27 23:14 ` Jörg Krause
  3 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2015-01-20 11:02 UTC (permalink / raw)
  To: u-boot

On Friday, December 19, 2014 at 05:39:12 AM, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512
  2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
  2015-01-20  6:35   ` Peng Fan
@ 2015-01-20 11:03   ` Marek Vasut
  2015-01-20 13:12     ` Peng Fan
  1 sibling, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2015-01-20 11:03 UTC (permalink / raw)
  To: u-boot

On Friday, December 19, 2014 at 05:39:13 AM, 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.

What exactly is the impact of this patch for current installations of U-Boot?
Does the NAND need to be rewritten with new content? Is the format introduced
by this patch compatible with Linux?

Thanks!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
  2015-01-20  6:35 ` [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
@ 2015-01-20 11:04   ` Marek Vasut
  2015-01-20 13:18     ` Peng Fan
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2015-01-20 11:04 UTC (permalink / raw)
  To: u-boot

On Tuesday, January 20, 2015 at 07:35:26 AM, Peng Fan wrote:
> Hi Marek,
> 
> Since you are familiar with this driver, would you please help review
> this patch?

Hi!

I commented on both. Next time, please CC me and Stefano, since the patches
might slip just like this one did.

Thank you for this work !

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512
  2015-01-20 11:03   ` Marek Vasut
@ 2015-01-20 13:12     ` Peng Fan
  2015-01-22  7:09       ` Marek Vasut
  0 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2015-01-20 13:12 UTC (permalink / raw)
  To: u-boot

Hi, Marek

On 1/20/2015 7:03 PM, Marek Vasut wrote:
> On Friday, December 19, 2014 at 05:39:13 AM, 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.
> What exactly is the impact of this patch for current installations of U-Boot?
> Does the NAND need to be rewritten with new content? Is the format introduced
> by this patch compatible with Linux?
The patch does not affect current installations of U-boot.  The NAND 
will not be rewritten with new content for chips whose oobsize is 
smaller than 512. To chips whose oobsize is bigger than 512, new 
format(saying gf_len is 14 and ecc chunk data size is 1024) introduced 
in this patch will be used.

This patch is to support nand chips whose oobsize bigger than 512, since 
the current mxs nand driver only supports oobisze smaller than 512. Data 
maybe corrupted if oobsize is bigger than 512 while ecc chunk data size 
is still 512, this is what this patch is try to fix. Yeah the format is 
compatible with gpmi-nand.c in linux.
> Thanks!
>
> Best regards,
> Marek Vasut
Thanks,
Peng.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
  2015-01-20 11:04   ` Marek Vasut
@ 2015-01-20 13:18     ` Peng Fan
  0 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2015-01-20 13:18 UTC (permalink / raw)
  To: u-boot

Hi, Marek

On 1/20/2015 7:04 PM, Marek Vasut wrote:
> On Tuesday, January 20, 2015 at 07:35:26 AM, Peng Fan wrote:
>> Hi Marek,
>>
>> Since you are familiar with this driver, would you please help review
>> this patch?
> Hi!
>
> I commented on both. Next time, please CC me and Stefano, since the patches
> might slip just like this one did.
>
> Thank you for this work !
Sorry for this and thanks for this tip.
>
> Best regards,
> Marek Vasut
Best regards,
Peng.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512
  2015-01-20 13:12     ` Peng Fan
@ 2015-01-22  7:09       ` Marek Vasut
  0 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2015-01-22  7:09 UTC (permalink / raw)
  To: u-boot

On Tuesday, January 20, 2015 at 02:12:41 PM, Peng Fan wrote:
> Hi, Marek
> 
> On 1/20/2015 7:03 PM, Marek Vasut wrote:
> > On Friday, December 19, 2014 at 05:39:13 AM, 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.
> > 
> > What exactly is the impact of this patch for current installations of
> > U-Boot? Does the NAND need to be rewritten with new content? Is the
> > format introduced by this patch compatible with Linux?
> 
> The patch does not affect current installations of U-boot.  The NAND
> will not be rewritten with new content for chips whose oobsize is
> smaller than 512. To chips whose oobsize is bigger than 512, new
> format(saying gf_len is 14 and ecc chunk data size is 1024) introduced
> in this patch will be used.
> 
> This patch is to support nand chips whose oobsize bigger than 512, since
> the current mxs nand driver only supports oobisze smaller than 512. Data
> maybe corrupted if oobsize is bigger than 512 while ecc chunk data size
> is still 512, this is what this patch is try to fix. Yeah the format is
> compatible with gpmi-nand.c in linux.

Thanks for clarifying.

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
  2014-12-19  4:39 [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
                   ` (2 preceding siblings ...)
  2015-01-20 11:02 ` Marek Vasut
@ 2015-01-27 23:14 ` Jörg Krause
  3 siblings, 0 replies; 11+ messages in thread
From: Jörg Krause @ 2015-01-27 23:14 UTC (permalink / raw)
  To: u-boot

On Fr, 2014-12-19 at 12:39 +0800, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>
> ---
>  drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 7a064ab..a45fcf9 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
>  static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>  						uint32_t page_oob_size)
>  {
> -	if (page_data_size == 2048) {
> -		if (page_oob_size == 64)
> -			return 8;
> +	int ecc_strength;
>  
> -		if (page_oob_size == 112)
> -			return 14;
> -	}
> -
> -	if (page_data_size == 4096) {
> -		if (page_oob_size == 128)
> -			return 8;
> -
> -		if (page_oob_size == 218)
> -			return 16;
> +	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> +			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
>  
> -		if (page_oob_size == 224)
> -			return 16;
> -	}
> -
> -	return 0;
> +	return round_down(ecc_strength, 2);
>  }
>  
>  static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,

Many thanks for the patch! But this patch affects mxsboot which is no
not aligned with the U-Boot mxs nand driver.

I was able to fix mxsboot, but I had difficulties with round_down, which
is a macro definition in linux/kernel.h. I've copied the macro
definition to mxsboot. I will submit the patch in a seperate mail.

I would like to see a comment or a macro for the magic number 13, which
is the value for the Galois Field, just for clarification

With fixing mxsboot, I was able to test the patch on a custom
i.MX28-based board assembled with a 1Gbit NAND flash (page size = 2048
bytes, oob size = 128 bytes).

U-Boot correctly reads the NAND info
=> nand info
        Device 0: nand0, sector size 128 KiB
          Page size      2048 b
          OOB size        128 b
          Erase size   131072 b

Before the patch linux failed to read from the UBI device with an ECC
error:
        UBI error: ubi_io_read: error -74 (ECC error)

This patch resolves the error. Linux can read the UBI device now. This
is kernel message:
        nand: device found, Manufacturer ID: 0x98, Chip ID: 0xf1
        [    1.327810] nand: Toshiba NAND 128MiB 3,3V 8-bit
        [    1.332482] nand: 128MiB, SLC, page size: 2048, OOB size: 128
        BCH Geometry :
        [    1.594658] GF length              : 13
        [    1.594658] ECC Strength           : 18
        [    1.594658] Page Size in Bytes     : 2176
        [    1.594658] Metadata Size in Bytes : 10
        [    1.594658] ECC Chunk Size in Bytes: 512
        [    1.594658] ECC Chunk Count        : 4
        [    1.594658] Payload Size in Bytes  : 2048
        [    1.594658] Auxiliary Size in Bytes: 16
        [    1.594658] Auxiliary Status Offset: 12
        [    1.594658] Block Mark Byte Offset : 1950
        [    1.594658] Block Mark Bit Offset  : 2

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-01-27 23:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-19  4:39 [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
2014-12-19  4:39 ` [U-Boot] [PATCH 2/2] mtd:mxs:nand support oobsize bigger than 512 Peng Fan
2015-01-20  6:35   ` Peng Fan
2015-01-20 11:03   ` Marek Vasut
2015-01-20 13:12     ` Peng Fan
2015-01-22  7:09       ` Marek Vasut
2015-01-20  6:35 ` [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically Peng Fan
2015-01-20 11:04   ` Marek Vasut
2015-01-20 13:18     ` Peng Fan
2015-01-20 11:02 ` Marek Vasut
2015-01-27 23:14 ` Jörg Krause

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.