| Message ID | 1377701666-9632-1-git-send-email-voice.shen@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Scott Wood |
| Headers | show |
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
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
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
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
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
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
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
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
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
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(+)