[U-Boot] [PATCH 1/4 v2] mtd: nand: fsmc: Add BCH4 SW ECC support for SPEAr600

[Stefan Roese <sr@denx.de>]

Hi Scott,

On 11.09.2015 00:31, Scott Wood wrote:
> On Wed, 2015-09-02 at 14:29 +0200, Stefan Roese wrote:
>> This patch adds support for 4-bit ECC BCH4 for the SPEAr600 SoC. This can
>> be used by boards equipped with a NAND chip that requires 4-bit ECC
>> strength.
>> The SPEAr600 HW ECC only supports 1-bit ECC strength.
>>
>> To enable SW BCH4, you need to specify this in your config header:
>>
>> #define CONFIG_NAND_ECC_BCH
>> #define CONFIG_BCH
>>
>> And use the command "nandecc bch4" to select this ECC scheme upon runtime.
>>
>> Tested on SPEAr600 x600 board.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Scott Wood <scottwood@freescale.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> v2:
>> - Removed err = 0 initialization as suggested by Viresh
>> - Completed the commit text
>> - Added Viresh's Acked-by
>>
>>   drivers/mtd/nand/fsmc_nand.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
>> index 567eff0..0976a67 100644
>> --- a/drivers/mtd/nand/fsmc_nand.c
>> +++ b/drivers/mtd/nand/fsmc_nand.c
>> @@ -13,6 +13,7 @@
>>   #include <asm/io.h>
>>   #include <linux/bitops.h>
>>   #include <linux/err.h>
>> +#include <linux/mtd/nand_bch.h>
>>   #include <linux/mtd/nand_ecc.h>
>>   #include <linux/mtd/fsmc_nand.h>
>>   #include <asm/arch/hardware.h>
>> @@ -390,6 +391,45 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd,
>> struct nand_chip *chip,
>>        return 0;
>>   }
>>
>> +#ifndef CONFIG_SPL_BUILD
>> +/*
>> + * fsmc_nand_switch_ecc - switch the ECC operation between different
>> engines
>> + *
>> + * @eccstrength              - the number of bits that could be corrected
>> + *                     (1 - HW, 4 - SW BCH4)
>> + */
>> +int __maybe_unused fsmc_nand_switch_ecc(uint32_t eccstrength)
>
> What calls this function?  You didn't CC me on the rest of the patchset...
> I'm guessing it's a copy-and-paste of what arch/arm/cpu/armv7/omap3/board.c
> does?

Yes.

>> +{
>> +     struct nand_chip *nand;
>> +     struct mtd_info *mtd;
>> +     int err;
>> +
>> +     mtd = &nand_info[nand_curr_device];
>> +     nand = mtd->priv;
>> +
>> +     /* Setup the ecc configurations again */
>> +     if (eccstrength == 1) {
>> +             nand->ecc.mode = NAND_ECC_HW;
>> +             nand->ecc.bytes = 3;
>> +             nand->ecc.strength = 1;
>> +             nand->ecc.layout = &fsmc_ecc1_layout;
>> +             nand->ecc.correct = nand_correct_data;
>> +     } else {
>> +             nand->ecc.mode = NAND_ECC_SOFT_BCH;
>> +             nand->ecc.calculate = nand_bch_calculate_ecc;
>> +             nand->ecc.correct = nand_bch_correct_data;
>> +             nand->ecc.bytes = 7;
>> +             nand->ecc.strength = 4;
>> +             nand->ecc.layout = NULL;
>> +     }
>
> nand_scan_tail() should already set .caclulate, .correct, and .bytes for
> NAND_ECC_SOFT_BCH.

Yes, thanks for pointing this out.

> When switching from BCH to HW, how does .calculate get set back to
> fsmc_read_hwecc?

Probably not at all. I must have missed testing this.

> What stops this from being called with FSMC_VER8 which appears to have BCH8
> hw ecc?

This function will not be called by any platform that supports FSMC_VER8 
- only from SPEArxxx. Which unfortunately only supports 1 bit HW ECC. I 
could add a comment to make this clear.

Thanks,
Stefan