Hi Miquel,
> On Jun 3, 2020, at 12:37 AM, Miquel Raynal <[email protected]> wrote: > > Hi Alex, > > Alex Nemirovsky <[email protected]> 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. yes, I believe it does. Jason, please review Miquel’s feedback and lets try to modify as requested. Thanks > >> >>> On Jun 3, 2020, at 12:21 AM, Miquel Raynal <[email protected]> >>> wrote: >>> >>> Hi Alex, >>> >>> Alex Nemirovsky <[email protected]> wrote on Wed, 3 >>> Jun 2020 00:25:53 +0000: >>> >>>>> On Jun 2, 2020, at 12:20 AM, Miquel Raynal <[email protected]> >>>>> wrote: >>>>> >>>>> Hi Alex, >>>>> >>>>> Alex Nemirovsky <[email protected]> wrote on Mon, 1 >>>>> Jun 2020 14:26:49 -0700: >>>>> >>>>>> From: Jason Li <[email protected]> >>>>>> >>>>>> 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 <[email protected]> >>>>>> Signed-off-by: Alex Nemirovsky <[email protected]> >>>>>> >>>>>> CC: Miquel Raynal <[email protected]> >>>>>> CC: Simon Glass <[email protected]> >>>>>> CC: Tom Rini <[email protected]> >>>>>> --- >>>>>> >>>>>> 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 >>
