Re: [PATCH v2 07/36] mtd: rawnand: Avoid a typedef

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

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.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/