On Fri, 3 May 2019 14:40:30 +0200 Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > Hi Boris, > > Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Sun, 31 Mar > 2019 13:55:13 +0200: > > > On Mon, 4 Mar 2019 23:28:12 +0100 > > Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > > > In new code, the use of typedef is discouraged. Before moving this > > > section out of the raw NAND base, let's switch the nand_ecc_modes_t > > > type into a regular nand_ecc_mode enumeration. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> > > > --- > > > drivers/mtd/nand/raw/nand_base.c | 4 ++-- > > > include/linux/mtd/rawnand.h | 6 +++--- > > > include/linux/platform_data/mtd-davinci.h | 2 +- > > > include/linux/platform_data/mtd-nand-s3c2410.h | 2 +- > > > 4 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > > > index e14f02a01efd..05174c6a3099 100644 > > > --- a/drivers/mtd/nand/raw/nand_base.c > > > +++ b/drivers/mtd/nand/raw/nand_base.c > > > @@ -4881,8 +4881,8 @@ static int of_get_nand_ecc_mode(struct device_node *np) > > > > > > /* > > > * For backward compatibility we support few obsoleted values that don't > > > - * have their mappings into nand_ecc_modes_t anymore (they were merged > > > - * with other enums). > > > + * have their mappings into the nand_ecc_mode enum anymore (they were > > > + * merged with other enums). > > > */ > > > if (!strcasecmp(pm, "soft_bch")) > > > return NAND_ECC_SOFT; > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > > index 14748183508b..c5bf6bb49329 100644 > > > --- a/include/linux/mtd/rawnand.h > > > +++ b/include/linux/mtd/rawnand.h > > > @@ -87,14 +87,14 @@ struct nand_chip; > > > /* > > > * Constants for ECC_MODES > > > */ > > > -typedef enum { > > > +enum nand_ecc_mode { > > > NAND_ECC_NONE, > > > NAND_ECC_SOFT, > > > NAND_ECC_HW, > > > NAND_ECC_HW_SYNDROME, > > > NAND_ECC_HW_OOB_FIRST, > > > NAND_ECC_ON_DIE, > > > -} nand_ecc_modes_t; > > > +}; > > > > Hm, I'm really not a big fan of this enum because it's mixing 2 > > different concepts: the type of ECC engine to use (on-die, > > hw-controller-side, software, no-ECC) and the layout of > > ECC/FREE bytes (_SYNDROME, _OOB_FIRST). > > > > I'd recommend creating a nand_ecc_engine_type enum: > > > > enum nand_ecc_engine_type { > > NAND_NO_ECC_ENGINE, > > NAND_SOFT_ECC_ENGINE, > > NAND_HW_ECC_ENGINE, > > NAND_ON_DIE_ECC_ENGINE, > > }; > > > > and then convert the raw NAND layer to this enum when the time comes. > > I started something but this goes way too far from what I want to > achieve. I know it would be nice to have it but it has an > increasingly number of side effects which scared me. The way the > series is organized does not allow to easily ignore the raw NAND layer > first and then convert it. I am giving up on this one for now, sorry. I don't think that's a wise choice. Why not focusing on nand_device/spinand with a clean implementation that does not try to convert the rawnand layer to this approach? The reason I initially started over with a new generic NAND layer instead of copying things from nand_base is that I wanted to avoid having to deal with stuff that were not so great in there, and clearly nand_ecc_mode is one of them. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/