[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Bo Shen]

Add possible to use software BCH ECC for atmel nand driver

Signed-off-by: Bo Shen <voice.shen@gmail.com>

---
 drivers/mtd/nand/atmel_nand.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 96aca00..52efbee 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1177,7 +1177,11 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)
 	mtd->priv = nand;
 	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
 
+#ifdef CONFIG_NAND_ECC_BCH
+	nand->ecc.mode = NAND_ECC_SOFT_BCH;
+#else
 	nand->ecc.mode = NAND_ECC_SOFT;
+#endif
 #ifdef CONFIG_SYS_NAND_DBW_16
 	nand->options = NAND_BUSWIDTH_16;
 #endif
-- 
1.7.10.4

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Andreas Bießmann]

Hi Bo,

On 08/28/2013 04:54 PM, Bo Shen wrote:
> Add possible to use software BCH ECC for atmel nand driver
> 
> Signed-off-by: Bo Shen <voice.shen@gmail.com>
> 
> ---
>  drivers/mtd/nand/atmel_nand.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 96aca00..52efbee 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -1177,7 +1177,11 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)
>  	mtd->priv = nand;
>  	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>  
> +#ifdef CONFIG_NAND_ECC_BCH
> +	nand->ecc.mode = NAND_ECC_SOFT_BCH;
> +#else
>  	nand->ecc.mode = NAND_ECC_SOFT;
> +#endif

I don't think this is enough for sw supported bch. Where do you feed the
libbch?

Best regards

Andreas Bie?mann

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Bo Shen]

Hi Andreas,

On 9/4/2013 6:23 PM, Andreas Bie?mann wrote:
> Hi Bo,
>
> On 08/28/2013 04:54 PM, Bo Shen wrote:
>> Add possible to use software BCH ECC for atmel nand driver
>>
>> Signed-off-by: Bo Shen <voice.shen@gmail.com>
>>
>> ---
>>   drivers/mtd/nand/atmel_nand.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 96aca00..52efbee 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -1177,7 +1177,11 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)
>>   	mtd->priv = nand;
>>   	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>>
>> +#ifdef CONFIG_NAND_ECC_BCH
>> +	nand->ecc.mode = NAND_ECC_SOFT_BCH;
>> +#else
>>   	nand->ecc.mode = NAND_ECC_SOFT;
>> +#endif
>
> I don't think this is enough for sw supported bch. Where do you feed the
> libbch?

Yes, we need libbch.

If we really want to enable software BCH support. It also need add 
following two options in board configuration file.
---8>---
#define CONFIG_NAND_ECC_BCH
#define CONFIG_BCH
---<8---

So, this patch give us option to enable software BCH.

> Best regards
>
> Andreas Bie?mann
>

Best Regards,
Bo Shen

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Andreas Bießmann]

Hi Bo,

On 09/04/2013 02:11 PM, Bo Shen wrote:
> Hi Andreas,
> 
> On 9/4/2013 6:23 PM, Andreas Bie?mann wrote:
>> Hi Bo,
>>
>> On 08/28/2013 04:54 PM, Bo Shen wrote:
>>> Add possible to use software BCH ECC for atmel nand driver
>>>
>>> Signed-off-by: Bo Shen <voice.shen@gmail.com>
>>>
>>> ---
>>>   drivers/mtd/nand/atmel_nand.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/atmel_nand.c
>>> b/drivers/mtd/nand/atmel_nand.c
>>> index 96aca00..52efbee 100644
>>> --- a/drivers/mtd/nand/atmel_nand.c
>>> +++ b/drivers/mtd/nand/atmel_nand.c
>>> @@ -1177,7 +1177,11 @@ int atmel_nand_chip_init(int devnum, ulong
>>> base_addr)
>>>       mtd->priv = nand;
>>>       nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>>>
>>> +#ifdef CONFIG_NAND_ECC_BCH
>>> +    nand->ecc.mode = NAND_ECC_SOFT_BCH;
>>> +#else
>>>       nand->ecc.mode = NAND_ECC_SOFT;
>>> +#endif
>>
>> I don't think this is enough for sw supported bch. Where do you feed the
>> libbch?
> 
> Yes, we need libbch.
> 
> If we really want to enable software BCH support. It also need add
> following two options in board configuration file.
> ---8>---
> #define CONFIG_NAND_ECC_BCH
> #define CONFIG_BCH
> ---<8---
> 
> So, this patch give us option to enable software BCH.

got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
mtd layer. I understand that this would be helpful for at91 SoC without
PMECC HW. But there is no user currently, so I hesitate to apply this.

Best regards

Andreas Bie?mann

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Bo Shen]

Hi Andreas,

On 9/4/2013 8:30 PM, Andreas Bie?mann wrote:
>> >Yes, we need libbch.
>> >
>> >If we really want to enable software BCH support. It also need add
>> >following two options in board configuration file.
>> >---8>---
>> >#define CONFIG_NAND_ECC_BCH
>> >#define CONFIG_BCH
>> >---<8---
>> >
>> >So, this patch give us option to enable software BCH.
> got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
> mtd layer. I understand that this would be helpful for at91 SoC without
> PMECC HW. But there is no user currently, so I hesitate to apply this.

Frankly, there is no EK boards from Atmel use software BCH now, however, 
a lot of customers use NAND with 224 bytes OOB, can not use software 
ECC, they need use software BCH.
So, I think it is better to apply this patch. If it will break the rule 
of u-boot, then I think we can wait real user in u-boot need this and 
then apply this patch.

> Best regards
>
> Andreas Bie?mann
>

Best Regards,
Bo Shen

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Andreas Bießmann]

Hi Bo,

On 09/04/2013 02:46 PM, Bo Shen wrote:
> On 9/4/2013 8:30 PM, Andreas Bie?mann wrote:
>>> >Yes, we need libbch.
>>> >
>>> >If we really want to enable software BCH support. It also need add
>>> >following two options in board configuration file.
>>> >---8>---
>>> >#define CONFIG_NAND_ECC_BCH
>>> >#define CONFIG_BCH
>>> >---<8---
>>> >
>>> >So, this patch give us option to enable software BCH.
>> got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
>> mtd layer. I understand that this would be helpful for at91 SoC without
>> PMECC HW. But there is no user currently, so I hesitate to apply this.
> 
> Frankly, there is no EK boards from Atmel use software BCH now, however,
> a lot of customers use NAND with 224 bytes OOB, can not use software
> ECC, they need use software BCH.

I understand this. But it will be a piece of dead code until a user of
it would be submitted.

> So, I think it is better to apply this patch. If it will break the rule
> of u-boot, then I think we can wait real user in u-boot need this and
> then apply this patch.

I'd like to hear Scott's comment on that.

Best regards

Andreas Bie?mann

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Scott Wood]

On Wed, 2013-09-04 at 17:15 +0200, Andreas Bie?mann wrote:
> Hi Bo,
> 
> On 09/04/2013 02:46 PM, Bo Shen wrote:
> > On 9/4/2013 8:30 PM, Andreas Bie?mann wrote:
> >>> >Yes, we need libbch.
> >>> >
> >>> >If we really want to enable software BCH support. It also need add
> >>> >following two options in board configuration file.
> >>> >---8>---
> >>> >#define CONFIG_NAND_ECC_BCH
> >>> >#define CONFIG_BCH
> >>> >---<8---
> >>> >
> >>> >So, this patch give us option to enable software BCH.
> >> got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
> >> mtd layer. I understand that this would be helpful for at91 SoC without
> >> PMECC HW. But there is no user currently, so I hesitate to apply this.
> > 
> > Frankly, there is no EK boards from Atmel use software BCH now, however,
> > a lot of customers use NAND with 224 bytes OOB, can not use software
> > ECC, they need use software BCH.
> 
> I understand this. But it will be a piece of dead code until a user of
> it would be submitted.
> 
> > So, I think it is better to apply this patch. If it will break the rule
> > of u-boot, then I think we can wait real user in u-boot need this and
> > then apply this patch.
> 
> I'd like to hear Scott's comment on that.

Is this for the benefit of out-of-tree boards, or for boards which will
be submitted but haven't yet?

In the latter case, it could be submitted at the same time.  In the
former case, of course we encourage the boards to be submitted, and we
don't generally add code solely for the benefit of out-of-tree boards.  

In any case, this is minor enough that I don't care all that much.  If
we ever get kconfig, then hopefully the "dead code" rules will relax to
code which could be enabled through some legal config, rather than code
which is enabled in some default config for a board.  Things like
allyesconfig and randconfig could help with build test coverage.

-Scott

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Andreas Bießmann]

Dear Scott Wood,

On 04.09.13 21:44, Scott Wood wrote:
> On Wed, 2013-09-04 at 17:15 +0200, Andreas Bie?mann wrote:
>> On 09/04/2013 02:46 PM, Bo Shen wrote:
>>> On 9/4/2013 8:30 PM, Andreas Bie?mann wrote:
>>>>>> Yes, we need libbch.
>>>>>>
>>>>>> If we really want to enable software BCH support. It also need add
>>>>>> following two options in board configuration file.
>>>>>> ---8>---
>>>>>> #define CONFIG_NAND_ECC_BCH
>>>>>> #define CONFIG_BCH
>>>>>> ---<8---
>>>>>>
>>>>>> So, this patch give us option to enable software BCH.
>>>> got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
>>>> mtd layer. I understand that this would be helpful for at91 SoC without
>>>> PMECC HW. But there is no user currently, so I hesitate to apply this.
>>>
>>> Frankly, there is no EK boards from Atmel use software BCH now, however,
>>> a lot of customers use NAND with 224 bytes OOB, can not use software
>>> ECC, they need use software BCH.
>>
>> I understand this. But it will be a piece of dead code until a user of
>> it would be submitted.
>>
>>> So, I think it is better to apply this patch. If it will break the rule
>>> of u-boot, then I think we can wait real user in u-boot need this and
>>> then apply this patch.
>>
>> I'd like to hear Scott's comment on that.
> 
> Is this for the benefit of out-of-tree boards, or for boards which will
> be submitted but haven't yet?
> 
> In the latter case, it could be submitted at the same time.  In the
> former case, of course we encourage the boards to be submitted, and we
> don't generally add code solely for the benefit of out-of-tree boards.  
> 
> In any case, this is minor enough that I don't care all that much.  If
> we ever get kconfig, then hopefully the "dead code" rules will relax to
> code which could be enabled through some legal config, rather than code
> which is enabled in some default config for a board.  Things like
> allyesconfig and randconfig could help with build test coverage.

I think this is a 'yes we take it'. Scott, would you pull it in or
should I do? Is it even that minor to pull it into 2013.10? It was
posted weeks after merge window closed.

Best regards

Andreas Bie?mann

[U-Boot] [PATCH] MTD: atmel_nand: support for software BCH ECC

[Scott Wood]

On Thu, 2013-09-05 at 08:28 +0200, Andreas Bie?mann wrote:
> Dear Scott Wood,
> 
> On 04.09.13 21:44, Scott Wood wrote:
> > On Wed, 2013-09-04 at 17:15 +0200, Andreas Bie?mann wrote:
> >> On 09/04/2013 02:46 PM, Bo Shen wrote:
> >>> On 9/4/2013 8:30 PM, Andreas Bie?mann wrote:
> >>>>>> Yes, we need libbch.
> >>>>>>
> >>>>>> If we really want to enable software BCH support. It also need add
> >>>>>> following two options in board configuration file.
> >>>>>> ---8>---
> >>>>>> #define CONFIG_NAND_ECC_BCH
> >>>>>> #define CONFIG_BCH
> >>>>>> ---<8---
> >>>>>>
> >>>>>> So, this patch give us option to enable software BCH.
> >>>> got it. So the NAND_ECC_BCH is the adoption for the SW BCH correction in
> >>>> mtd layer. I understand that this would be helpful for at91 SoC without
> >>>> PMECC HW. But there is no user currently, so I hesitate to apply this.
> >>>
> >>> Frankly, there is no EK boards from Atmel use software BCH now, however,
> >>> a lot of customers use NAND with 224 bytes OOB, can not use software
> >>> ECC, they need use software BCH.
> >>
> >> I understand this. But it will be a piece of dead code until a user of
> >> it would be submitted.
> >>
> >>> So, I think it is better to apply this patch. If it will break the rule
> >>> of u-boot, then I think we can wait real user in u-boot need this and
> >>> then apply this patch.
> >>
> >> I'd like to hear Scott's comment on that.
> > 
> > Is this for the benefit of out-of-tree boards, or for boards which will
> > be submitted but haven't yet?
> > 
> > In the latter case, it could be submitted at the same time.  In the
> > former case, of course we encourage the boards to be submitted, and we
> > don't generally add code solely for the benefit of out-of-tree boards.  
> > 
> > In any case, this is minor enough that I don't care all that much.  If
> > we ever get kconfig, then hopefully the "dead code" rules will relax to
> > code which could be enabled through some legal config, rather than code
> > which is enabled in some default config for a board.  Things like
> > allyesconfig and randconfig could help with build test coverage.
> 
> I think this is a 'yes we take it'. Scott, would you pull it in or
> should I do? Is it even that minor to pull it into 2013.10? It was
> posted weeks after merge window closed.

I can take it, but not for 2013.10.  It's not a bugfix.

-Scott