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

Alex Nemirovsky Alex.Nemirovsky at cortina-access.com
Wed Jun 3 09:39:51 CEST 2020


Hi Miquel,



> On Jun 3, 2020, at 12:37 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 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 <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
>> 



More information about the U-Boot mailing list