[PATCH v3 1/2] mtd: rawnand: ca_nand: add Cortina Access Parallel NAND controller support

[Miquel Raynal]

Hi Alex,

Alex Nemirovsky <Alex.Nemirovsky at cortina-access.com> wrote on Wed, 3
Jun 2020 07:23:23 +0000:

> Hi Miquel,
> 
> still not clear what you would like us to do about the length of the driver.

Stop duplicating the whole logic present in the core:
- nand reset
- nand identification
- bad block handling
- ca_nand_command{,_lp}()
- ca_nand_scan_tail()

Plus:
- ca_nand_read/write_page_hwecc is overly complicated and 100 lines to
  assign chip->oob_poi is very suspicious.
- your endless set of definitions in the header is really strange,
  could be reduced to a smaller number of defines (the ones of
  interest, that you use).

Hope this helps.

> 
> > On Jun 3, 2020, at 12:21 AM, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> > 
> > Hi Alex,
> > 
> > Alex Nemirovsky <Alex.Nemirovsky at cortina-access.com> wrote on Wed, 3
> > Jun 2020 00:25:53 +0000:
> > 
> >>> On Jun 2, 2020, at 12:20 AM, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> >>> 
> >>> Hi Alex,
> >>> 
> >>> Alex Nemirovsky <alex.nemirovsky at cortina-access.com> wrote on Mon,  1
> >>> Jun 2020 14:26:49 -0700:
> >>> 
> >>>> From: Jason Li <jason.li at cortina-access.com>
> >>>> 
> >>>> Supports all CAxxxx SoCs which support a parallel nand controller.
> >>>> It should be noted that some CAxxxx Soc also support an separate
> >>>> SPI serial NAND controller.
> >>>> 
> >>>> This driver only supports the parallel NAND controller. A different
> >>>> driver supports the SPI NAND interface controller.
> >>>> 
> >>>> Signed-off-by: Jason Li <jason.li at cortina-access.com>
> >>>> Signed-off-by: Alex Nemirovsky <alex.nemirovsky at cortina-access.com>
> >>>> 
> >>>> CC: Miquel Raynal <miquel.raynal at bootlin.com>
> >>>> CC: Simon Glass <sjg at chromium.org>
> >>>> CC: Tom Rini <trini at konsulko.com>
> >>>> ---
> >>>> 
> >>>> Changes in v3:
> >>>> - Include udelay.h to avoid implicit declaration of udelay()
> >>>> 
> >>>> Changes in v2:
> >>>> - Cleanup code style to pass checkpatch.pl
> >>>> 
> >>>> MAINTAINERS                    |    2 +
> >>>> drivers/mtd/nand/raw/Kconfig   |   31 +
> >>>> drivers/mtd/nand/raw/Makefile  |    1 +
> >>>> drivers/mtd/nand/raw/ca_nand.c | 4943 ++++++++++++++++++++++++++++++++++++++++
> >>>> drivers/mtd/nand/raw/ca_nand.h | 3899 +++++++++++++++++++++++++++++++  
> >>> 
> >>> This is insanely big !  
> >> 
> >> Hi Miquel, could you clarify?  Is there a change request?
> > 
> > 9000 lines for a single NAND driver? You bet I do :)
> > 
> > $ wc -l drivers/mtd/nand/raw/*.c | grep -v spl
> >  1273 drivers/mtd/nand/raw/arasan_nfc.c
> >  1511 drivers/mtd/nand/raw/atmel_nand.c
> >   838 drivers/mtd/nand/raw/davinci_nand.c
> >  1376 drivers/mtd/nand/raw/denali.c
> >   171 drivers/mtd/nand/raw/denali_dt.c
> >   810 drivers/mtd/nand/raw/fsl_elbc_nand.c
> >  1064 drivers/mtd/nand/raw/fsl_ifc_nand.c
> >   184 drivers/mtd/nand/raw/fsl_upm.c
> >   518 drivers/mtd/nand/raw/fsmc_nand.c
> >   133 drivers/mtd/nand/raw/kb9202_nand.c
> >    91 drivers/mtd/nand/raw/kirkwood_nand.c
> >   122 drivers/mtd/nand/raw/kmeter1_nand.c
> >   765 drivers/mtd/nand/raw/lpc32xx_nand_mlc.c
> >   583 drivers/mtd/nand/raw/lpc32xx_nand_slc.c
> >  1307 drivers/mtd/nand/raw/mxc_nand.c
> >  1421 drivers/mtd/nand/raw/mxs_nand.c
> >    94 drivers/mtd/nand/raw/mxs_nand_dt.c
> >  5316 drivers/mtd/nand/raw/nand_base.c
> >  1373 drivers/mtd/nand/raw/nand_bbt.c
> >   231 drivers/mtd/nand/raw/nand_bch.c
> >   175 drivers/mtd/nand/raw/nand.c
> >   174 drivers/mtd/nand/raw/nand_ecc.c
> >   213 drivers/mtd/nand/raw/nand_ids.c
> >    64 drivers/mtd/nand/raw/nand_plat.c
> >   334 drivers/mtd/nand/raw/nand_timings.c
> >   904 drivers/mtd/nand/raw/nand_util.c
> >   193 drivers/mtd/nand/raw/omap_elm.c
> >  1037 drivers/mtd/nand/raw/omap_gpmc.c
> >  1933 drivers/mtd/nand/raw/pxa3xx_nand.c
> >  1061 drivers/mtd/nand/raw/stm32_fmc2_nand.c
> >  1850 drivers/mtd/nand/raw/sunxi_nand.c
> >  1002 drivers/mtd/nand/raw/tegra_nand.c
> >   815 drivers/mtd/nand/raw/vf610_nfc.c
> >  1254 drivers/mtd/nand/raw/zynq_nand.c
> > 
> > So this driver would be almost twice bigger than the NAND core itself
> > (nand_base.c). I did not check in details but I am pretty sure you
> > duplicate a lot of code.
> > 
> > Thanks,
> > Miquèl
>