Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field

[Boris Brezillon]

On Sun, 17 Apr 2016 18:50:05 +0200
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:

> On Sun, 17 Apr 2016 18:27:40 +0200
> RafaÅ MiÅecki <zajec5@xxxxxxxxx> wrote:
> 
> > On 16 April 2016 at 09:58, Boris Brezillon
> > <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
> > > On Fri, 15 Apr 2016 21:54:10 +0200
> > > RafaÅ MiÅecki <zajec5@xxxxxxxxx> wrote:
> > >
> > >> Now we have all drivers properly setting this new field we can start
> > >> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
> > >>
> > >> Signed-off-by: RafaÅ MiÅecki <zajec5@xxxxxxxxx>
> > >> ---
> > >>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
> > >>  1 file changed, 53 insertions(+), 45 deletions(-)
> > >>
> > >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > >> index e1f3cf8..ffd1b32 100644
> > >> --- a/drivers/mtd/nand/nand_base.c
> > >> +++ b/drivers/mtd/nand/nand_base.c
> > >> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> > >>       /*
> > >>        * If no default placement scheme is given, select an appropriate one.
> > >>        */
> > >> -     if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
> > >> +     if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
> > >
> > > Should be:
> > >
> > >         if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
> > >             ecc->algo != NAND_ECC_BCH) {
> > >
> > > Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
> > > account.
> > 
> > Pretty much yes, but without
> > [PATCH 11/12] of: mtd: drop support for NAND_ECC_SOFT_BCH as "soft_bch" mapping
> > we may be getting NAND_ECC_SOFT as well as NAND_ECC_SOFT_BCH. And
> > 11/12 can't be applied before 10/12. We'll need to support both values
> > for this very short moment (after 10/12 but before 11/12).
> 
> 
> Why can't you move patch 11 before patch 10. The DT parsing code has
> already been patched to set both ecc->algo to NAND_ECC_BCH and
> ecc->mode to NAND_ECC_SOFT, so, to it seems perfectly safe to reorder
> those changes to me (and change the test as suggested).
> 
> Am I missing something?
> 

Forget what I just said. How about merging those 2 patches into a
single one?

BTW, I did not complain so far, but you're trying to split every
single change into it's own patch, and I don't think it's required for
such simple change as assigning ecc->algo to NAND_ECC_BCH wherever
ecc->algo is set to NAND_ECC_SOFT_BCH (even if the patch touches
different drivers).
And the problem we're facing here is partly caused by this attempt to
split every change in its own patch.

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com