Hi Yogesh, On Fri, 31 Aug 2018 16:00:00 +0530 Yogesh Gaur <yogeshnarayan.gaur@xxxxxxx> wrote: > - Add a driver for NXP FlexSPI host controller > > (0) What is the FlexSPI controller? > FlexSPI is a flexsible SPI host controller which supports two SPI > channels and up to 4 external devices. > Each channel supports Single/Dual/Quad/Octal mode data > transfer (1/2/4/8 bidirectional data lines) > i.e. FlexSPI acts as an interface to external devices, > maximum 4, each with up to 8 bidirectional data lines. > > It uses new SPI memory interface of the SPI framework to issue flash > memory operations to up to four connected flash chips (2 buses with > 2 CS each). > Chipselect for each flash can be selected as per address assigned in > controller specific registers. > > FlexSPI controller is similar to the existing Freescale/NXP QuadSPI > controller with advanced features. Yep, I had a quick look at the code and they really look similar. Why not extending the existing driver instead of creating a new one from scratch? > > (1) The FlexSPI controller is driven by the LUT(Look-up Table) > registers. > The LUT registers are a look-up-table for sequences of instructions. > A valid sequence consists of four LUT registers. > Maximum 32 LUT sequences can be programmed simultaneously. > > (2) The definition of the LUT register shows below: > --------------------------------------------------- > | INSTR1 | PAD1 | OPRND1 | INSTR0 | PAD0 | OPRND0 | > --------------------------------------------------- > > There are several types of INSTRx, such as: > CMD : the SPI NOR command. > ADDR : the address for the SPI NOR command. > DUMMY : the dummy cycles needed by the SPI NOR command. > .... > > There are several types of PADx, such as: > PAD1 : use a singe I/O line. > PAD2 : use two I/O lines. > PAD4 : use quad I/O lines. > PAD8 : use octal I/O lines. > .... > > (3) LUTs are being created at run-time based on the commands passed > from the spi-mem framework. Thus, using single LUT index. > > (4) Software triggered Flash read/write access by IP Bus. > > (5) Memory mapped read access by AHB Bus. Do we really want to have this level of details in the commit message? I mean, this is something you can document in the driver, but not here. BTW, you might want to have a look at [1]. I think we can get rid of the ->size field you're adding if this driver implements the dirmap hooks. > > (6) Tested this driver with the mtd_debug and JFFS2 filesystem utility > on NXP LX2160ARDB and LX2160AQDS targets. > LX2160ARDB is having two NOR slave device connected on single bus A > i.e. A0 and A1 (CS0 and CS1). > LX2160AQDS is having two NOR slave device connected on separate buses > one flash on A0 and second on B1 i.e. (CS0 and CS3). > Verified this driver on following SPI NOR flashes: > Micron, mt35xu512ab, [Read - 1 bit mode] > Cypress, s25fl512s, [Read - 1/2/4 bit mode] Ok, that's good to have in the commit message. > > - Add config option entry in 'spi-nor/Kconfig' for FlexSPI driver. But this one is useless. If you add a new driver, you have no other choice but to add a new entry in the Kconfig file. > > - Add entry in the 'spi-nor/Makefile'. > Ditto. Before you re-send a new version, I'd like to understand why you think you need to create a new driver, and I want you to try to implement the dirmap hook and check if you can get rid of the ->size field when doing that. I also seem one extra benefit to having a single driver for both FlexSPI and QuadSPI IPs: you'll help Frieder debug the last remaining problems you reported on the new QuadSPI driver :-P. Thanks, Boris
