Re: [PATCH v9 0/9] Preparation to the generic ECC engine abstraction

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Julien Su <juliensu@mxic.com.tw>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Mason Yang <masonccyang@mxic.com.tw>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v9 0/9] Preparation to the generic ECC engine abstraction
Date: Tue, 2 Jun 2020 18:33:36 +0200	[thread overview]
Message-ID: <20200602183336.5447d34a@collabora.com> (raw)
In-Reply-To: <20200602143124.29553-1-miquel.raynal@bootlin.com>

On Tue,  2 Jun 2020 16:31:15 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> This is a respin of the end of my previous series, just the patches which needed to be fixed.
> 
> Changes in v9:
> * This time sending the additional patchs, not just the old ones with
>   corrections. v8 should be ignored, sorry for the noise.
> 
> Changes in v8:
> * Split "Convert generic NAND bits to ECC framework" into several peaces:
>   > added two helpers
>   > converted SPI-NAND then raw-NAND.  
> * Fixed a comment.
> * Used the _ooblayout suffix instead of _layout.
> 
> 
> Miquel Raynal (9):
>   mtd: nand: Create a helper to extract the ECC configuration
>   mtd: spinand: Use nanddev_get_ecc_conf() when relevant
>   mtd: nand: Create a helper to extract the ECC requirements
>   mtd: rawnand: Use nanddev_get_ecc_requirements() when relevant
>   mtd: nand: Convert generic NAND bits to use the ECC framework
>   mtd: rawnand: Hide the generic OOB layout objects behind helpers
>   mtd: rawnand: Write a compatibility layer
>   mtd: rawnand: Move generic OOB layouts to the ECC framework
>   mtd: rawnand: Move the user input parsing bits to the ECC framework

Sorry, but I keep thinking you should re-order things so we don't have
code blocks introduced and then moved around in the same patchset.
What's the point of introducing new props/helpers in rawnand if the
ultimate goal is to move them to nand.h, especially since none of the
existing rawnand drivers (or the rawnand core) need those new DT props
right now. You should really consider doing that in 3 distinct steps:

1/ Introduce ECC related fields/defs/... at the generic NAND level
2/ Patch rawnand to use those fields/new defs and deprecate the old ones
3/ Move rawnand defs/code that can be made generic and be useful to
   !rawnand users

> 
>  drivers/mtd/nand/ecc.c                        | 314 ++++++++++++++
>  drivers/mtd/nand/raw/Kconfig                  |   1 +
>  drivers/mtd/nand/raw/arasan-nand-controller.c |   2 +-
>  drivers/mtd/nand/raw/atmel/nand-controller.c  |  15 +-
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c      |   8 +-
>  drivers/mtd/nand/raw/davinci_nand.c           |   3 +-
>  drivers/mtd/nand/raw/denali.c                 |   3 +
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c    |  13 +-
>  .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   |   6 +-
>  drivers/mtd/nand/raw/marvell_nand.c           |   8 +-
>  drivers/mtd/nand/raw/mtk_nand.c               |   6 +-
>  drivers/mtd/nand/raw/nand_base.c              | 395 ++++--------------
>  drivers/mtd/nand/raw/nand_esmt.c              |  14 +-
>  drivers/mtd/nand/raw/nand_hynix.c             |  43 +-
>  drivers/mtd/nand/raw/nand_jedec.c             |   7 +-
>  drivers/mtd/nand/raw/nand_micron.c            |  17 +-
>  drivers/mtd/nand/raw/nand_onfi.c              |  14 +-
>  drivers/mtd/nand/raw/nand_samsung.c           |  21 +-
>  drivers/mtd/nand/raw/nand_toshiba.c           |  15 +-
>  drivers/mtd/nand/raw/sunxi_nand.c             |   9 +-
>  drivers/mtd/nand/raw/tegra_nand.c             |  15 +-
>  drivers/mtd/nand/raw/vf610_nfc.c              |   2 +-
>  drivers/mtd/nand/spi/core.c                   |  10 +-
>  drivers/mtd/nand/spi/macronix.c               |   7 +-
>  drivers/mtd/nand/spi/toshiba.c                |   6 +-
>  include/linux/mtd/nand.h                      |  40 +-
>  include/linux/mtd/rawnand.h                   |  17 +-
>  27 files changed, 587 insertions(+), 424 deletions(-)
> 

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