| Message ID | 1441196952-13885-1-git-send-email-sr@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
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? > +{ > + 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. When switching from BCH to HW, how does .calculate get set back to fsmc_read_hwecc? What stops this from being called with FSMC_VER8 which appears to have BCH8 hw ecc? -Scott
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
On Wed, Sep 02, 2015 at 02:29:12PM +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> Applied to u-boot/master, thanks!
On Sat, 2015-09-12 at 08:50 -0400, Tom Rini wrote: > On Wed, Sep 02, 2015 at 02:29:12PM +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> > > Applied to u-boot/master, thanks! > There's a v3, and some minor comments even on that one... -Scott
On Sat, Sep 12, 2015 at 09:36:54AM -0500, Scott Wood wrote: > On Sat, 2015-09-12 at 08:50 -0400, Tom Rini wrote: > > On Wed, Sep 02, 2015 at 02:29:12PM +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> > > > > Applied to u-boot/master, thanks! > > > > There's a v3, and some minor comments even on that one... Mutter, sorry. Would you rather a revert of the series or just incremental on top? And aside, this is one reason I prefer resends of the whole series for anything non-trivial. 6 or 9 patches in patchwork is a lot clear that "oh, still in progress" as I clean things up rather than 1.
On Sat, 2015-09-12 at 10:46 -0400, Tom Rini wrote: > On Sat, Sep 12, 2015 at 09:36:54AM -0500, Scott Wood wrote: > > On Sat, 2015-09-12 at 08:50 -0400, Tom Rini wrote: > > > On Wed, Sep 02, 2015 at 02:29:12PM +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> > > > > > > Applied to u-boot/master, thanks! > > > > > > > There's a v3, and some minor comments even on that one... > > Mutter, sorry. Would you rather a revert of the series or just > incremental on top? Either is fine -- I just wanted to make sure it didn't get forgotten. -Scott
On Sat, Sep 12, 2015 at 09:46:51AM -0500, Scott Wood wrote: > On Sat, 2015-09-12 at 10:46 -0400, Tom Rini wrote: > > On Sat, Sep 12, 2015 at 09:36:54AM -0500, Scott Wood wrote: > > > On Sat, 2015-09-12 at 08:50 -0400, Tom Rini wrote: > > > > On Wed, Sep 02, 2015 at 02:29:12PM +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> > > > > > > > > Applied to u-boot/master, thanks! > > > > > > > > > > There's a v3, and some minor comments even on that one... > > > > Mutter, sorry. Would you rather a revert of the series or just > > incremental on top? > > Either is fine -- I just wanted to make sure it didn't get forgotten. OK, Stefan, please do an incremental on top of master now to cover what's been noted in v3, thanks!
On 12.09.2015 17:20, Tom Rini wrote: > On Sat, Sep 12, 2015 at 09:46:51AM -0500, Scott Wood wrote: >> On Sat, 2015-09-12 at 10:46 -0400, Tom Rini wrote: >>> On Sat, Sep 12, 2015 at 09:36:54AM -0500, Scott Wood wrote: >>>> On Sat, 2015-09-12 at 08:50 -0400, Tom Rini wrote: >>>>> On Wed, Sep 02, 2015 at 02:29:12PM +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> >>>>> >>>>> Applied to u-boot/master, thanks! >>>>> >>>> >>>> There's a v3, and some minor comments even on that one... >>> >>> Mutter, sorry. Would you rather a revert of the series or just >>> incremental on top? >> >> Either is fine -- I just wanted to make sure it didn't get forgotten. > > OK, Stefan, please do an incremental on top of master now to cover > what's been noted in v3, thanks! Sure. Thanks, Stefan
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) +{ + 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; + } + + /* Update NAND handling after ECC mode switch */ + err = nand_scan_tail(mtd); + + return err; +} +#endif /* CONFIG_SPL_BUILD */ + int fsmc_nand_init(struct nand_chip *nand) { static int chip_nr;