Hi Frieder,

Thanks for review.

> -----Original Message-----
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Wednesday, November 7, 2018 9:52 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.g...@nxp.com>; linux-
> m...@lists.infradead.org; boris.brezil...@bootlin.com; marek.va...@gmail.com;
> broo...@kernel.org; linux-...@vger.kernel.org; devicet...@vger.kernel.org
> Cc: r...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; linux-
> arm-ker...@lists.infradead.org; computersforpe...@gmail.com;
> frieder.schre...@exceet.de; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH RESEND v4 1/5] spi: spi-mem: Add driver for NXP FlexSPI
> controller
> 
> Hi Yogesh,
> 
> I didn't have time to look at all of the code, but nevertheless here are some
> comments.
> 
> On 23.10.18 10:56, Yogesh Narayan Gaur wrote:
> > - Add 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
> >   devices (2 buses with 2 CS each).
> >
> > (1) 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]
> >
> > Signed-off-by: Yogesh Gaur <yogeshnarayan.g...@nxp.com>
> > ---
> > Changes for v4:
> > - Incorporate Boris review comments
> >    * Use readl_poll_timeout() instead of busy looping.
> >    * Re-define register masking as per comment.
> >    * Drop fspi_devtype enum.
> > Changes for v3:
> > - Added endianness flag in platform specific structure instead of DTS.
> > - Modified nxp_fspi_read_ahb(), removed remapping code.
> > - Added Boris and Frieder as Author and provided reference of
> > spi-fsl-qspi.c Changes for v2:
> > - Incorporated Boris review comments.
> > - Remove dependency of driver over connected flash device size.
> > - Modified the logic to select requested CS.
> > - Remove SPI-Octal Macros.
> >
> >   drivers/spi/Kconfig        |   10 +
> >   drivers/spi/Makefile       |    1 +
> >   drivers/spi/spi-nxp-fspi.c | 1158
> ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 1169 insertions(+)
> >   create mode 100644 drivers/spi/spi-nxp-fspi.c
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > ad5d68e..68da874 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -251,6 +251,16 @@ config SPI_FSL_LPSPI
> >     help
> >       This enables Freescale i.MX LPSPI controllers in master mode.
> >
> > +config SPI_NXP_FLEXSPI
> > +   tristate "NXP Flex SPI controller"
> > +   depends on ARCH_LAYERSCAPE || HAS_IOMEM
> > +   help
> > +     This enables support for the Flex SPI controller in master mode.
> > +     Up to four slave devices can be connected on two buses with two
> > +     chipselects each.
> > +     This controller does not support generic SPI messages and only
> > +     supports the high-level SPI memory interface.
> > +
> >   config SPI_GPIO
> >     tristate "GPIO-based bitbanging SPI Master"
> >     depends on GPIOLIB || COMPILE_TEST
> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index
> > cb1f437..52c9f18 100644
> > --- a/drivers/spi/Makefile
> > +++ b/drivers/spi/Makefile
> > @@ -59,6 +59,7 @@ obj-$(CONFIG_SPI_MPC52xx)         += spi-
> mpc52xx.o
> >   obj-$(CONFIG_SPI_MT65XX)                += spi-mt65xx.o
> >   obj-$(CONFIG_SPI_MXS)                     += spi-mxs.o
> >   obj-$(CONFIG_SPI_NUC900)          += spi-nuc900.o
> > +obj-$(CONFIG_SPI_NXP_FLEXSPI)              += spi-nxp-fspi.o
> >   obj-$(CONFIG_SPI_OC_TINY)         += spi-oc-tiny.o
> >   spi-octeon-objs                           := spi-cavium.o spi-cavium-
> octeon.o
> >   obj-$(CONFIG_SPI_OCTEON)          += spi-octeon.o
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > new file mode 100644 index 0000000..e5188b2
> > --- /dev/null
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -0,0 +1,1158 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * NXP FlexSPI(FSPI) controller driver.
> > + *
> > + * Copyright 2018 NXP.
> > + *
> > + * 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).
> > + *
> > + * FlexSPI controller is driven by the LUT(Look-up Table) registers
> > + * 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.
> > + *
> > + * LUTs are being created at run-time based on the commands passed
> > + * from the spi-mem framework, thus using single LUT index.
> > + *
> > + * Software triggered Flash read/write access by IP Bus.
> > + *
> > + * Memory mapped read access by AHB Bus.
> > + *
> > + * Based on SPI MEM interface and spi-fsl-qspi.c driver.
> > + *
> > + * Author:
> > + *     Yogesh Gaur <yogeshnarayan.g...@nxp.com>
> > + *     Boris Brezillion <boris.brezil...@bootlin.com>
> > + *     Frieder Schrempf <frieder.schre...@exceet.de>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_qos.h>
> > +#include <linux/sizes.h>
> > +#include <linux/iopoll.h>
> 
> If you would move this below io.h, then all the includes would stay in
> alphabetical order ;)
> 
Ok, would change for next version.

> > +
> > +#include <linux/spi/spi.h>
> > +#include <linux/spi/spi-mem.h>
> > +
> > +/*
> > + * The driver only uses one single LUT entry, that is updated on
> > + * each call of exec_op(). Index 0 is preset at boot with a basic
> > + * read operation, so let's use the last entry (31).
> > + */
> > +#define    SEQID_LUT                       31
> > +
> > +/* Registers used by the driver */
> > +#define FSPI_MCR0                  0x00
> > +#define FSPI_MCR0_AHB_TIMEOUT(x)   ((x) << 24)
> > +#define FSPI_MCR0_IP_TIMEOUT(x)            ((x) << 16)
> > +#define FSPI_MCR0_LEARN_EN         BIT(15)
> > +#define FSPI_MCR0_SCRFRUN_EN               BIT(14)
> > +#define FSPI_MCR0_OCTCOMB_EN               BIT(13)
> > +#define FSPI_MCR0_DOZE_EN          BIT(12)
> > +#define FSPI_MCR0_HSEN                     BIT(11)
> > +#define FSPI_MCR0_SERCLKDIV                BIT(8)
> > +#define FSPI_MCR0_ATDF_EN          BIT(7)
> > +#define FSPI_MCR0_ARDF_EN          BIT(6)
> > +#define FSPI_MCR0_RXCLKSRC(x)              ((x) << 4)
> > +#define FSPI_MCR0_END_CFG(x)               ((x) << 2)
> > +#define FSPI_MCR0_MDIS                     BIT(1)
> > +#define FSPI_MCR0_SWRST                    BIT(0)
> > +
> > +#define FSPI_MCR1                  0x04
> > +#define FSPI_MCR1_SEQ_TIMEOUT(x)   ((x) << 16)
> > +#define FSPI_MCR1_AHB_TIMEOUT(x)   (x)
> > +
> > +#define FSPI_MCR2                  0x08
> > +#define FSPI_MCR2_IDLE_WAIT(x)             ((x) << 24)
> > +#define FSPI_MCR2_SAMEDEVICEEN             BIT(15)
> > +#define FSPI_MCR2_CLRLRPHS         BIT(14)
> > +#define FSPI_MCR2_ABRDATSZ         BIT(8)
> > +#define FSPI_MCR2_ABRLEARN         BIT(7)
> > +#define FSPI_MCR2_ABR_READ         BIT(6)
> > +#define FSPI_MCR2_ABRWRITE         BIT(5)
> > +#define FSPI_MCR2_ABRDUMMY         BIT(4)
> > +#define FSPI_MCR2_ABR_MODE         BIT(3)
> > +#define FSPI_MCR2_ABRCADDR         BIT(2)
> > +#define FSPI_MCR2_ABRRADDR         BIT(1)
> > +#define FSPI_MCR2_ABR_CMD          BIT(0)
> > +
> > +#define FSPI_AHBCR                 0x0c
> > +#define FSPI_AHBCR_RDADDROPT               BIT(6)
> > +#define FSPI_AHBCR_PREF_EN         BIT(5)
> > +#define FSPI_AHBCR_BUFF_EN         BIT(4)
> > +#define FSPI_AHBCR_CACH_EN         BIT(3)
> > +#define FSPI_AHBCR_CLRTXBUF                BIT(2)
> > +#define FSPI_AHBCR_CLRRXBUF                BIT(1)
> > +#define FSPI_AHBCR_PAR_EN          BIT(0)
> > +
> > +#define FSPI_INTEN                 0x10
> > +#define FSPI_INTEN_SCLKSBWR                BIT(9)
> > +#define FSPI_INTEN_SCLKSBRD                BIT(8)
> > +#define FSPI_INTEN_DATALRNFL               BIT(7)
> > +#define FSPI_INTEN_IPTXWE          BIT(6)
> > +#define FSPI_INTEN_IPRXWA          BIT(5)
> > +#define FSPI_INTEN_AHBCMDERR               BIT(4)
> > +#define FSPI_INTEN_IPCMDERR                BIT(3)
> > +#define FSPI_INTEN_AHBCMDGE                BIT(2)
> > +#define FSPI_INTEN_IPCMDGE         BIT(1)
> > +#define FSPI_INTEN_IPCMDDONE               BIT(0)
> > +
> > +#define FSPI_INTR                  0x14
> > +#define FSPI_INTR_SCLKSBWR         BIT(9)
> > +#define FSPI_INTR_SCLKSBRD         BIT(8)
> > +#define FSPI_INTR_DATALRNFL                BIT(7)
> > +#define FSPI_INTR_IPTXWE           BIT(6)
> > +#define FSPI_INTR_IPRXWA           BIT(5)
> > +#define FSPI_INTR_AHBCMDERR                BIT(4)
> > +#define FSPI_INTR_IPCMDERR         BIT(3)
> > +#define FSPI_INTR_AHBCMDGE         BIT(2)
> > +#define FSPI_INTR_IPCMDGE          BIT(1)
> > +#define FSPI_INTR_IPCMDDONE                BIT(0)
> > +
> > +#define FSPI_LUTKEY                        0x18
> > +#define FSPI_LUTKEY_VALUE          0x5AF05AF0
> > +
> > +#define FSPI_LCKCR                 0x1C
> > +
> > +#define FSPI_LCKER_LOCK                    0x1
> > +#define FSPI_LCKER_UNLOCK          0x2
> > +
> > +#define FSPI_BUFXCR_INVALID_MSTRID 0xE
> > +#define FSPI_AHBRX_BUF0CR0         0x20
> > +#define FSPI_AHBRX_BUF1CR0         0x24
> > +#define FSPI_AHBRX_BUF2CR0         0x28
> > +#define FSPI_AHBRX_BUF3CR0         0x2C
> > +#define FSPI_AHBRX_BUF4CR0         0x30
> > +#define FSPI_AHBRX_BUF5CR0         0x34
> > +#define FSPI_AHBRX_BUF6CR0         0x38
> > +#define FSPI_AHBRX_BUF7CR0         0x3C
> > +#define FSPI_AHBRXBUF0CR7_PREF             BIT(31)
> > +
> > +#define FSPI_AHBRX_BUF0CR1         0x40
> > +#define FSPI_AHBRX_BUF1CR1         0x44
> > +#define FSPI_AHBRX_BUF2CR1         0x48
> > +#define FSPI_AHBRX_BUF3CR1         0x4C
> > +#define FSPI_AHBRX_BUF4CR1         0x50
> > +#define FSPI_AHBRX_BUF5CR1         0x54
> > +#define FSPI_AHBRX_BUF6CR1         0x58
> > +#define FSPI_AHBRX_BUF7CR1         0x5C
> > +
> > +#define FSPI_FLSHA1CR0                     0x60
> > +#define FSPI_FLSHA2CR0                     0x64
> > +#define FSPI_FLSHB1CR0                     0x68
> > +#define FSPI_FLSHB2CR0                     0x6C
> > +#define FSPI_FLSHXCR0_SZ_KB                10
> > +#define FSPI_FLSHXCR0_SZ(x)                ((x) >> FSPI_FLSHXCR0_SZ_KB)
> > +
> > +#define FSPI_FLSHA1CR1                     0x70
> > +#define FSPI_FLSHA2CR1                     0x74
> > +#define FSPI_FLSHB1CR1                     0x78
> > +#define FSPI_FLSHB2CR1                     0x7C
> > +#define FSPI_FLSHXCR1_CSINTR(x)            ((x) << 16)
> > +#define FSPI_FLSHXCR1_CAS(x)               ((x) << 11)
> > +#define FSPI_FLSHXCR1_WA           BIT(10)
> > +#define FSPI_FLSHXCR1_TCSH(x)              ((x) << 5)
> > +#define FSPI_FLSHXCR1_TCSS(x)              (x)
> > +
> > +#define FSPI_FLSHA1CR2                     0x80
> > +#define FSPI_FLSHA2CR2                     0x84
> > +#define FSPI_FLSHB1CR2                     0x88
> > +#define FSPI_FLSHB2CR2                     0x8C
> > +#define FSPI_FLSHXCR2_CLRINSP              BIT(24)
> > +#define FSPI_FLSHXCR2_AWRWAIT              BIT(16)
> > +#define FSPI_FLSHXCR2_AWRSEQN_SHIFT        13
> > +#define FSPI_FLSHXCR2_AWRSEQI_SHIFT        8
> > +#define FSPI_FLSHXCR2_ARDSEQN_SHIFT        5
> > +#define FSPI_FLSHXCR2_ARDSEQI_SHIFT        0
> > +
> > +#define FSPI_IPCR0                 0xA0
> > +
> > +#define FSPI_IPCR1                 0xA4
> > +#define FSPI_IPCR1_IPAREN          BIT(31)
> > +#define FSPI_IPCR1_SEQNUM_SHIFT            24
> > +#define FSPI_IPCR1_SEQID_SHIFT             16
> > +#define FSPI_IPCR1_IDATSZ(x)               (x)
> > +
> > +#define FSPI_IPCMD                 0xB0
> > +#define FSPI_IPCMD_TRG                     BIT(0)
> > +
> > +#define FSPI_DLPR                  0xB4
> > +
> > +#define FSPI_IPRXFCR                       0xB8
> > +#define FSPI_IPRXFCR_CLR           BIT(0)
> > +#define FSPI_IPRXFCR_DMA_EN                BIT(1)
> > +#define FSPI_IPRXFCR_WMRK(x)               ((x) << 2)
> > +
> > +#define FSPI_IPTXFCR                       0xBC
> > +#define FSPI_IPTXFCR_CLR           BIT(0)
> > +#define FSPI_IPTXFCR_DMA_EN                BIT(1)
> > +#define FSPI_IPTXFCR_WMRK(x)               ((x) << 2)
> > +
> > +#define FSPI_DLLACR                        0xC0
> > +#define FSPI_DLLACR_OVRDEN         BIT(8)
> > +
> > +#define FSPI_DLLBCR                        0xC4
> > +#define FSPI_DLLBCR_OVRDEN         BIT(8)
> > +
> > +#define FSPI_STS0                  0xE0
> > +#define FSPI_STS0_DLPHB(x)         ((x) << 8)
> > +#define FSPI_STS0_DLPHA(x)         ((x) << 4)
> > +#define FSPI_STS0_CMD_SRC(x)               ((x) << 2)
> > +#define FSPI_STS0_ARB_IDLE         BIT(1)
> > +#define FSPI_STS0_SEQ_IDLE         BIT(0)
> > +
> > +#define FSPI_STS1                  0xE4
> > +#define FSPI_STS1_IP_ERRCD(x)              ((x) << 24)
> > +#define FSPI_STS1_IP_ERRID(x)              ((x) << 16)
> > +#define FSPI_STS1_AHB_ERRCD(x)             ((x) << 8)
> > +#define FSPI_STS1_AHB_ERRID(x)             (x)
> > +
> > +#define FSPI_AHBSPNST                      0xEC
> > +#define FSPI_AHBSPNST_DATLFT(x)            ((x) << 16)
> > +#define FSPI_AHBSPNST_BUFID(x)             ((x) << 1)
> > +#define FSPI_AHBSPNST_ACTIVE               BIT(0)
> > +
> > +#define FSPI_IPRXFSTS                      0xF0
> > +#define FSPI_IPRXFSTS_RDCNTR(x)            ((x) << 16)
> > +#define FSPI_IPRXFSTS_FILL(x)              (x)
> > +
> > +#define FSPI_IPTXFSTS                      0xF4
> > +#define FSPI_IPTXFSTS_WRCNTR(x)            ((x) << 16)
> > +#define FSPI_IPTXFSTS_FILL(x)              (x)
> > +
> > +#define FSPI_RFDR                  0x100
> > +#define FSPI_TFDR                  0x180
> > +
> > +#define FSPI_LUT_BASE                      0x200
> > +#define FSPI_LUT_OFFSET                    (SEQID_LUT * 4 * 4)
> > +#define FSPI_LUT_REG(idx) \
> > +   (FSPI_LUT_BASE + FSPI_LUT_OFFSET + (idx) * 4)
> > +
> > +/* register map end */
> > +
> > +/* Instruction set for the LUT register. */
> > +#define LUT_STOP                   0x00
> > +#define LUT_CMD                            0x01
> > +#define LUT_ADDR                   0x02
> > +#define LUT_CADDR_SDR                      0x03
> > +#define LUT_MODE                   0x04
> > +#define LUT_MODE2                  0x05
> > +#define LUT_MODE4                  0x06
> > +#define LUT_MODE8                  0x07
> > +#define LUT_NXP_WRITE                      0x08
> > +#define LUT_NXP_READ                       0x09
> > +#define LUT_LEARN_SDR                      0x0A
> > +#define LUT_DATSZ_SDR                      0x0B
> > +#define LUT_DUMMY                  0x0C
> > +#define LUT_DUMMY_RWDS_SDR         0x0D
> > +#define LUT_JMP_ON_CS                      0x1F
> > +#define LUT_CMD_DDR                        0x21
> > +#define LUT_ADDR_DDR                       0x22
> > +#define LUT_CADDR_DDR                      0x23
> > +#define LUT_MODE_DDR                       0x24
> > +#define LUT_MODE2_DDR                      0x25
> > +#define LUT_MODE4_DDR                      0x26
> > +#define LUT_MODE8_DDR                      0x27
> > +#define LUT_WRITE_DDR                      0x28
> > +#define LUT_READ_DDR                       0x29
> > +#define LUT_LEARN_DDR                      0x2A
> > +#define LUT_DATSZ_DDR                      0x2B
> > +#define LUT_DUMMY_DDR                      0x2C
> > +#define LUT_DUMMY_RWDS_DDR         0x2D
> > +
> > +/*
> > + * Calculate number of required PAD bits for LUT register.
> > + *
> > + * The pad stands for the number of IO lines [0:7].
> > + * For example, the octal read needs eight IO lines,
> > + * so you should use LUT_PAD(8). This macro
> > + * returns 3 i.e. use eight (2^3) IP lines for read.
> > + */
> > +#define LUT_PAD(x) (fls(x) - 1)
> > +
> > +/*
> > + * Macro for constructing the LUT entries with the following
> > + * register layout:
> > + *
> > + *  ---------------------------------------------------
> > + *  | INSTR1 | PAD1 | OPRND1 | INSTR0 | PAD0 | OPRND0 |
> > + *  ---------------------------------------------------
> > + */
> > +#define PAD_SHIFT          8
> > +#define INSTR_SHIFT                10
> > +#define OPRND_SHIFT                16
> > +
> > +/* Macros for constructing the LUT register. */
> > +#define LUT_DEF(idx, ins, pad, opr)                          \
> > +   ((((ins) << INSTR_SHIFT) | ((pad) << PAD_SHIFT) | \
> > +   (opr)) << (((idx) % 2) * OPRND_SHIFT))
> > +
> > +/* Oprands for the LUT register. */
> 
> s/Oprands/Operands
Ok

> 
> > +#define ADDR8BIT           0x08
> > +#define ADDR16BIT          0x10
> > +#define ADDR24BIT          0x18
> > +#define ADDR32BIT          0x20
> > +
> > +#define POLL_TOUT_US               5000
> > +
> > +struct nxp_fspi_devtype_data {
> > +   unsigned int rxfifo;
> > +   unsigned int txfifo;
> > +   unsigned int ahb_buf_size;
> > +   unsigned int quirks;
> > +   bool little_endian;
> > +};
> > +
> > +static const struct nxp_fspi_devtype_data lx2160a_data = {
> > +   .rxfifo = SZ_512,       /* (64  * 64 bits)  */
> > +   .txfifo = SZ_1K,        /* (128 * 64 bits)  */
> > +   .ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
> > +   .quirks = 0,
> > +   .little_endian = 1,     /* little-endian    */
> 
> I'm not sure if this is really necessary, but for a boolean probably 'true' 
> would be
> better than '1'.
Ok

> 
> > +};
> > +
> > +#define NXP_FSPI_MAX_CHIPSELECT            4
> > +
> > +struct nxp_fspi {
> > +   void __iomem *iobase;
> > +   void __iomem *ahb_addr;
> > +   u32 memmap_phy;
> > +   u32 memmap_phy_size;
> > +   struct clk *clk, *clk_en;
> > +   struct device *dev;
> > +   struct completion c;
> > +   const struct nxp_fspi_devtype_data *devtype_data;
> > +   struct mutex lock;
> > +   struct pm_qos_request pm_qos_req;
> > +   int selected;
> > +   void (*write)(u32 val, void __iomem *addr);
> > +   u32 (*read)(void __iomem *addr);
> > +};
> > +
> > +static void fspi_writel_be(u32 val, void __iomem *addr) {
> > +   iowrite32be(val, addr);
> > +}
> > +
> > +static void fspi_writel(u32 val, void __iomem *addr) {
> > +   iowrite32(val, addr);
> > +}
> > +
> > +static u32 fspi_readl_be(void __iomem *addr) {
> > +   return ioread32be(addr);
> > +}
> > +
> > +static u32 fspi_readl(void __iomem *addr) {
> > +   return ioread32(addr);
> > +}
> > +
> > +static irqreturn_t nxp_fspi_irq_handler(int irq, void *dev_id) {
> > +   struct nxp_fspi *f = dev_id;
> > +   u32 reg;
> > +
> > +   /* clear interrupt */
> > +   reg = f->read(f->iobase + FSPI_INTR);
> > +   f->write(FSPI_INTR_IPCMDDONE, f->iobase + FSPI_INTR);
> > +
> > +   if (reg & FSPI_INTR_IPCMDDONE)
> > +           complete(&f->c);
> > +
> > +   return IRQ_HANDLED;
> > +}
> > +
> > +static int nxp_fspi_check_buswidth(struct nxp_fspi *f, u8 width) {
> > +   switch (width) {
> > +   case 1:
> > +   case 2:
> > +   case 4:
> > +   case 8:
> > +           return 0;
> > +   }
> > +
> > +   return -ENOTSUPP;
> > +}
> > +
> > +static bool nxp_fspi_supports_op(struct spi_mem *mem,
> > +                            const struct spi_mem_op *op)
> > +{
> > +   struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +   int ret;
> > +
> > +   ret = nxp_fspi_check_buswidth(f, op->cmd.buswidth);
> > +
> > +   if (op->addr.nbytes)
> > +           ret |= nxp_fspi_check_buswidth(f, op->addr.buswidth);
> > +
> > +   if (op->dummy.nbytes)
> > +           ret |= nxp_fspi_check_buswidth(f, op->dummy.buswidth);
> > +
> > +   if (op->data.nbytes)
> > +           ret |= nxp_fspi_check_buswidth(f, op->data.buswidth);
> > +
> > +   if (ret)
> > +           return false;
> > +
> > +   /*
> > +    * The number of instructions needed for the op, needs
> > +    * to fit into a single LUT entry.
> > +    */
> > +   if (op->addr.nbytes +
> > +      (op->dummy.nbytes ? 1:0) +
> > +      (op->data.nbytes ? 1:0) > 6)
> > +           return false;
> > +
> > +   /* Max 64 dummy clock cycles supported */
> > +   if (op->dummy.buswidth &&
> > +       (op->dummy.nbytes * 8 / op->dummy.buswidth > 64))
> > +           return false;
> > +
> > +   /* Max data length, check controller limits and alignment */
> > +   if (op->data.dir == SPI_MEM_DATA_IN &&
> > +       (op->data.nbytes > f->devtype_data->ahb_buf_size ||
> > +        (op->data.nbytes > f->devtype_data->rxfifo - 4 &&
> > +         !IS_ALIGNED(op->data.nbytes, 8))))
> > +           return false;
> > +
> > +   if (op->data.dir == SPI_MEM_DATA_OUT &&
> > +       op->data.nbytes > f->devtype_data->txfifo)
> > +           return false;
> > +
> > +   return true;
> > +}
> > +
> > +/* Instead of busy looping invoke readl_poll_timeout functionality.
> > +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base, 
> > u32
> reg,
> > +                           u32 mask, u32 delay_us, u32 timeout_us)
> 
> You do not need to pass 'reg' here, you can just use a local variable.
> 
Ok, let me verify by using local variable instead of passing status in reg 
variable.

> > +{
> > +   u32 l_mask;
> > +
> > +   if (f->devtype_data->little_endian)
> > +           l_mask = mask;
> > +   else
> > +           l_mask = (u32)cpu_to_be32(mask);
> 
> You could shorten this a bit by just reusing 'mask' and drop 'l_mask'.
> See my version of this function in v4 of the FSL QSPI driver.
Ok, would check your implementation.

> 
> > +
> > +   return readl_poll_timeout(base, reg, (reg & l_mask),
> > +                             delay_us, timeout_us);
> > +}
> > +
> > +/*
> > + * If the slave device content being changed by Write/Erase, need to
> > + * invalidate the AHB buffer. This can be achieved by doing the reset
> > + * of controller after setting MCR0[SWRESET] bit.
> > + */
> > +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
> > +   u32 reg;
> > +   int ret;
> > +   u32 l_mask;
> > +
> > +   reg = f->read(f->iobase + FSPI_MCR0);
> > +   f->write(reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
> > +
> > +   if (f->devtype_data->little_endian)
> > +           l_mask = FSPI_MCR0_SWRST;
> > +   else
> > +           l_mask = (u32)cpu_to_be32(FSPI_MCR0_SWRST);
> > +
> > +   /* w1c register, wait unit clear */
> > +   ret = readl_poll_timeout(f->iobase + FSPI_MCR0, reg,
> > +                            !(reg & l_mask), 0, POLL_TOUT_US);
> 
> It would be better to add a parameter to fspi_readl_poll_tout() for inverting 
> the
> break condition and then reuse this function here.
> 
Ok

> > +   WARN_ON(ret);
> > +}
> > +
> > +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> > +                            const struct spi_mem_op *op)
> > +{
> > +   void __iomem *base = f->iobase;
> > +   u32 lutval[4] = {};
> > +   int lutidx = 1, i;
> > +
> > +   /* cmd */
> > +   lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > +                        op->cmd.opcode);
> > +
> > +   /* addr bus width */
> > +   if (op->addr.nbytes) {
> > +           u32 addrlen = 0;
> > +
> > +           if (op->addr.nbytes == 3)
> > +                   addrlen = ADDR24BIT;
> > +           else if (op->addr.nbytes == 4)
> > +                   addrlen = ADDR32BIT;
> 
> I know we already had this  discussion with Boris on v2, but if you keep only
> supporting 3 and 4 byte adresses, then it should be documented somewhere to
> let people know that this driver can't be used to interface SPI NAND and other
> flash chips that use deviant address widths.
> 
> An even better solution would be to make this work somehow. I can't see why
> the approach from the QSPI driver using LUT_MODE shouldn't work, but who
> knows...
Working of LUT_MODE in place of LUT_ADDR in QSPI when passing information of 
address might be an QuadSPI controller issue.
I have tried using LUT_MODE for this controller and it didn't work.
Discussed with HW IP owner of FlexSPI controller and they said we need to pass 
LUT_ADDR when programming for address length.

> 
> It's a pity, that the designers of the new IP still did not really take SPI 
> NAND into
> account.
> 
This controller works for the NAND flash as well. In SW, for now my driver has 
been validated only for the NOR flash devices and thus I have added support for 
3 and 4 bytes in code.
Idea, was to modify the driver code when NAND device being verified using this 
driver.
Meanwhile, I can add switch case for address bytes of length 1, 2, 3 and 4 
instead of just 3 and 4.

> > +
> > +           lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
> > +                                         LUT_PAD(op->addr.buswidth),
> > +                                         addrlen);
> > +           lutidx++;
> > +   }
> > +
> > +   /* dummy bytes, if needed */
> > +   if (op->dummy.nbytes) {
> > +           lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
> > +           /*
> > +            * Due to FlexSPI controller limitation number of PAD for
> dummy
> > +            * buswidth needs to be programmed as equal to data buswidth.
> > +            */
> > +                                         LUT_PAD(op->data.buswidth),
> 
> This seems like a hack for something that should be better handled by the spi
> mem layer. I think you should have a check for data.buswidth ==
> dummy.buswidth in nxp_fspi_supports_op() and if that fails the spi mem layer
> must find a working fallback. But I'm not sure, so we probably need to ask 
> Boris.
> 
This is an issue with the controller and would be published in the 
documentation in upcoming revision.
In any of the 1-1-x mode, needs to be pass dummy information on single PAD but 
due to clock control issue, both controller and flash device start taking 
control of the data line and hence data gets corrupted.
This has been identified and been suggested to use PAD information for dummy 
same as Data PAD for case of 1-1-x protocols.
This would going to be updated in the next release document of the controller.

> > +                                         op->dummy.nbytes * 8 /
> > +                                         op->dummy.buswidth);
> > +           lutidx++;
> > +   }
> > +
> > +   /* read/write data bytes */
> > +   if (op->data.nbytes) {
> > +           lutval[lutidx / 2] |= LUT_DEF(lutidx,
> > +                                         op->data.dir ==
> SPI_MEM_DATA_IN ?
Yes, data is coming from SPI memory i.e. read operation
@SPI_MEM_DATA_IN: data coming from the SPI memory

> > +                                         LUT_NXP_READ : LUT_NXP_WRITE,
> > +                                         LUT_PAD(op->data.buswidth),
> > +                                         0);
> > +           lutidx++;
> > +   }
> > +
> > +   /* stop condition. */
> > +   lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_STOP, 0, 0);
> > +
> > +   /* unlock LUT */
> > +   f->write(FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
> > +   f->write(FSPI_LCKER_UNLOCK, f->iobase + FSPI_LCKCR);
> > +
> > +   /* fill LUT */
> > +   for (i = 0; i < ARRAY_SIZE(lutval); i++)
> > +           f->write(lutval[i], base + FSPI_LUT_REG(i));
> > +
> > +   dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x]\n",
> > +           op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3]);
> > +
> > +   /* lock LUT */
> > +   f->write(FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
> > +   f->write(FSPI_LCKER_LOCK, f->iobase + FSPI_LCKCR); }
> > +
> > +static int nxp_fspi_clk_prep_enable(struct nxp_fspi *f) {
> > +   int ret;
> > +
> > +   ret = clk_prepare_enable(f->clk_en);
> > +   if (ret)
> > +           return ret;
> > +
> > +   ret = clk_prepare_enable(f->clk);
> > +   if (ret) {
> > +           clk_disable_unprepare(f->clk_en);
> > +           return ret;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static void nxp_fspi_clk_disable_unprep(struct nxp_fspi *f) {
> > +   clk_disable_unprepare(f->clk);
> > +   clk_disable_unprepare(f->clk_en);
> > +}
> > +
> > +/*
> > + * In FlexSPI controller, flash access is based on value of
> > +FSPI_FLSHXXCR0
> > + * register and start base address of the slave device.
> > + *
> > + *                                                     (Higher address)
> > + *                         --------    <-- FLSHB2CR0
> > + *                         |  B2  |
> > + *                         |      |
> > + * B2 start address -->    --------    <-- FLSHB1CR0
> > + *                         |  B1  |
> > + *                         |      |
> > + * B1 start address -->    --------    <-- FLSHA2CR0
> > + *                         |  A2  |
> > + *                         |      |
> > + * A2 start address -->    --------    <-- FLSHA1CR0
> > + *                         |  A1  |
> > + *                         |      |
> > + * A1 start address -->    --------                    (Lower address)
> > + *
> > + *
> > + * Start base address defines the starting address range for given CS
> > +and
> > + * FSPI_FLSHXXCR0 defines the size of the slave device connected at given 
> > CS.
> > + *
> > + * But, different targets are having different combinations of number
> > +of CS,
> > + * some targets only have single CS or two CS covering controller's
> > +full
> > + * memory mapped space area.
> > + * Thus, implementation is being done as independent of the size and
> > +number
> > + * of the connected slave device.
> > + * Assign controller memory mapped space size as the size to the
> > +connected
> > + * slave device.
> > + * Mark FLSHxxCR0 as zero initially and then assign value only to the
> > +selected
> > + * chip-select Flash configuration register.
> > + *
> > + * For e.g. to access CS2 (B1), FLSHB1CR0 register would be equal to
> > +the
> > + * memory mapped size of the controller.
> > + * Value for rest of the CS FLSHxxCR0 register would be zero.
> > + *
> > + */
> > +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device
> > +*spi) {
> > +   unsigned long rate = spi->max_speed_hz;
> > +   int ret;
> > +   uint64_t size_kb;
> > +
> > +   /*
> > +    * Return, if previously selected slave device is same as current
> > +    * requested slave device.
> > +    */
> > +   if (f->selected == spi->chip_select)
> > +           return;
> > +
> > +   /* Reset FLSHxxCR0 registers */
> > +   f->write(0, f->iobase + FSPI_FLSHA1CR0);
> > +   f->write(0, f->iobase + FSPI_FLSHA2CR0);
> > +   f->write(0, f->iobase + FSPI_FLSHB1CR0);
> > +   f->write(0, f->iobase + FSPI_FLSHB2CR0);
> > +
> > +   /* Assign controller memory mapped space as size, KBytes, of flash. */
> > +   size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
> > +
> > +   switch (spi->chip_select) {
> > +   case 0:
> > +           f->write(size_kb, f->iobase + FSPI_FLSHA1CR0);
> > +           break;
> > +   case 1:
> > +           f->write(size_kb, f->iobase + FSPI_FLSHA2CR0);
> > +           break;
> > +   case 2:
> > +           f->write(size_kb, f->iobase + FSPI_FLSHB1CR0);
> > +           break;
> > +   case 3:
> > +           f->write(size_kb, f->iobase + FSPI_FLSHB2CR0);
> > +           break;
> > +   default:
> > +           dev_err(f->dev, "In-correct CS provided\n");
> > +           return;
> > +   }
> > +
> > +   dev_dbg(f->dev, "Slave device [CS:%x] selected\n",
> > +spi->chip_select);
> > +
> > +   nxp_fspi_clk_disable_unprep(f);
> > +
> > +   ret = clk_set_rate(f->clk, rate);
> > +   if (ret)
> > +           return;
> > +
> > +   ret = nxp_fspi_clk_prep_enable(f);
> > +   if (ret)
> > +           return;
> > +   f->selected = spi->chip_select;
> > +}
> > +
> > +static void nxp_fspi_read_ahb(struct nxp_fspi *f, const struct
> > +spi_mem_op *op) {
> > +   u32 len = op->data.nbytes;
> > +
> > +   /* Read out the data directly from the AHB buffer. */
> > +   memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len); }
> > +
> > +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
> > +                            const struct spi_mem_op *op)
> > +{
> > +   void __iomem *base = f->iobase;
> > +   int i, j, ret;
> > +   int size, tmp_size, wm_size;
> > +   u32 data = 0, reg;
> > +   u32 *txbuf = (u32 *) op->data.buf.out;
> > +
> > +   /* clear the TX FIFO. */
> > +   f->write(FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
> > +
> > +   /* Default value of water mark level is 8 bytes. */
> > +   wm_size = 8;
> > +   size = op->data.nbytes / wm_size;
> > +   for (i = 0; i < size; i++) {
> > +           /* Wait for TXFIFO empty */
> > +           ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR, reg,
> > +                                      FSPI_INTR_IPTXWE, 0,
> POLL_TOUT_US);
> > +           WARN_ON(ret);
> > +
> > +           j = 0;
> > +           tmp_size = wm_size;
> > +           while (tmp_size > 0) {
> > +                   data = 0;
> > +                   memcpy(&data, txbuf, 4);
> > +                   f->write(data, base + FSPI_TFDR + j * 4);
> > +                   tmp_size -= 4;
> > +                   j++;
> > +                   txbuf += 1;
> > +           }
> > +           f->write(FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > +   }
> > +
> > +   size = op->data.nbytes % wm_size;
> > +   if (size) {
> > +           /* Wait for TXFIFO empty */
> > +           ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR, reg,
> > +                                      FSPI_INTR_IPTXWE, 0,
> POLL_TOUT_US);
> > +           WARN_ON(ret);
> > +
> > +           j = 0;
> > +           tmp_size = 0;
> > +           while (size > 0) {
> > +                   data = 0;
> > +                   tmp_size = (size < 4) ? size : 4;
> > +                   memcpy(&data, txbuf, tmp_size);
> > +                   f->write(data, base + FSPI_TFDR + j * 4);
> > +                   size -= tmp_size;
> > +                   j++;
> > +                   txbuf += 1;
> > +           }
> > +           f->write(FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > +   }
> > +}
> > +
> > +static void nxp_fspi_read_rxfifo(struct nxp_fspi *f,
> > +                     const struct spi_mem_op *op)
> > +{
> > +   void __iomem *base = f->iobase;
> > +   int i, j;
> > +   int size, tmp_size, wm_size, ret;
> > +   u32 tmp = 0, reg;
> > +   u8 *buf = op->data.buf.in;
> > +   u32 len = op->data.nbytes;
> > +
> > +   /* Default value of water mark level is 8 bytes. */
> > +   wm_size = 8;
> > +
> > +   while (len > 0) {
> > +           size = len / wm_size;
> > +
> > +           for (i = 0; i < size; i++) {
> > +                   /* Wait for RXFIFO available */
> > +                   ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +                                              reg, FSPI_INTR_IPRXWA,
> > +                                              0, POLL_TOUT_US);
> > +                   WARN_ON(ret);
> > +
> > +                   j = 0;
> > +                   tmp_size = wm_size;
> > +                   while (tmp_size > 0) {
> > +                           tmp = 0;
> > +                           tmp = f->read(base + FSPI_RFDR + j * 4);
> > +                           memcpy(buf, &tmp, 4);
> > +                           tmp_size -= 4;
> > +                           j++;
> > +                           buf += 4;
> > +                   }
> > +                   /* move the FIFO pointer */
> > +                   f->write(FSPI_INTR_IPRXWA, base + FSPI_INTR);
> > +                   len -= wm_size;
> > +           }
> > +
> > +           size = len % wm_size;
> > +
> > +           j = 0;
> > +           if (size) {
> > +                   /* Wait for RXFIFO available */
> > +                   ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +                                              reg, FSPI_INTR_IPRXWA,
> > +                                              0, POLL_TOUT_US);
> > +                   WARN_ON(ret);
> > +
> > +                   while (len > 0) {
> > +                           tmp = 0;
> > +                           size = (len < 4) ? len : 4;
> > +                           tmp = f->read(base + FSPI_RFDR + j * 4);
> > +                           memcpy(buf, &tmp, size);
> > +                           len -= size;
> > +                           j++;
> > +                           buf += size;
> > +                   }
> > +           }
> > +
> > +           /* invalid the RXFIFO */
> > +           f->write(FSPI_IPRXFCR_CLR, base + FSPI_IPRXFCR);
> > +           /* move the FIFO pointer */
> > +           f->write(FSPI_INTR_IPRXWA, base + FSPI_INTR);
> > +   }
> > +}
> > +
> > +static int nxp_fspi_do_op(struct nxp_fspi *f, const struct spi_mem_op
> > +*op) {
> > +   void __iomem *base = f->iobase;
> > +   int seqnum = 0;
> > +   int err = 0;
> > +   u32 reg;
> > +
> > +   reg = f->read(base + FSPI_IPRXFCR);
> > +   /* invalid RXFIFO first */
> > +   reg &= ~FSPI_IPRXFCR_DMA_EN;
> > +   reg = reg | FSPI_IPRXFCR_CLR;
> > +   f->write(reg, base + FSPI_IPRXFCR);
> > +
> > +   init_completion(&f->c);
> > +
> > +   f->write(op->addr.val, base + FSPI_IPCR0);
> > +   /*
> > +    * Always start the sequence at the same index since we update
> > +    * the LUT at each exec_op() call. And also specify the DATA
> > +    * length, since it's has not been specified in the LUT.
> > +    */
> > +   f->write(op->data.nbytes |
> > +            (SEQID_LUT << FSPI_IPCR1_SEQID_SHIFT) |
> > +            (seqnum << FSPI_IPCR1_SEQNUM_SHIFT),
> > +            base + FSPI_IPCR1);
> > +
> > +   /* Trigger the LUT now. */
> > +   f->write(FSPI_IPCMD_TRG, base + FSPI_IPCMD);
> > +
> > +   /* Wait for the interrupt. */
> > +   if (!wait_for_completion_timeout(&f->c, msecs_to_jiffies(1000)))
> > +           err = -ETIMEDOUT;
> > +
> > +   /* Invoke IP data read, if request is of data read. */
> > +   if (!err && op->data.nbytes && op->data.dir == SPI_MEM_DATA_IN)
> > +           nxp_fspi_read_rxfifo(f, op);
> > +
> > +   return err;
> > +}
> > +
> > +static int nxp_fspi_exec_op(struct spi_mem *mem, const struct
> > +spi_mem_op *op) {
> > +   struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +   void __iomem *base = f->iobase;
> > +   int err = 0;
> > +   u32 status;
> > +
> > +   mutex_lock(&f->lock);
> > +
> > +   /* Wait for controller being ready. */
> > +   status = f->read(base + FSPI_STS0);
> > +   err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0, status,
> > +                              FSPI_STS0_ARB_IDLE, 1, POLL_TOUT_US);
> > +   WARN_ON(err);
> > +
> > +   nxp_fspi_select_mem(f, mem->spi);
> > +
> > +   nxp_fspi_prepare_lut(f, op);
> > +   /*
> > +    * If we have large chunks of data, we read them through the AHB bus
> > +    * by accessing the mapped memory. In all other cases we use
> > +    * IP commands to access the flash.
> > +    */
> > +   if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
> > +       op->data.dir == SPI_MEM_DATA_IN) {
> > +           nxp_fspi_read_ahb(f, op);
> > +   } else {
> > +           if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> > +                   nxp_fspi_fill_txfifo(f, op);
> > +
> > +           err = nxp_fspi_do_op(f, op);
> > +
> > +           /* Invalidate the data in the AHB buffer. */
> > +           if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> > +                   nxp_fspi_invalid(f);
> > +   }
> > +
> > +   mutex_unlock(&f->lock);
> > +
> > +   return err;
> > +}
> > +
> > +static int nxp_fspi_adjust_op_size(struct spi_mem *mem, struct
> > +spi_mem_op *op) {
> > +   struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +
> > +   if (op->data.dir == SPI_MEM_DATA_OUT) {
> > +           if (op->data.nbytes > f->devtype_data->txfifo)
> > +                   op->data.nbytes = f->devtype_data->txfifo;
> > +   } else {
> > +           if (op->data.nbytes > f->devtype_data->ahb_buf_size)
> > +                   op->data.nbytes = f->devtype_data->ahb_buf_size;
> > +           else if (op->data.nbytes > (f->devtype_data->rxfifo - 4))
> > +                   op->data.nbytes = ALIGN_DOWN(op->data.nbytes, 8);
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int nxp_fspi_default_setup(struct nxp_fspi *f) {
> > +   void __iomem *base = f->iobase;
> > +   int ret, i;
> > +   u32 reg, l_mask;
> > +
> > +   /* disable and unprepare clock to avoid glitch pass to controller */
> > +   nxp_fspi_clk_disable_unprep(f);
> > +
> > +   /* the default frequency, we will change it later if necessary. */
> > +   ret = clk_set_rate(f->clk, 20000000);
> > +   if (ret)
> > +           return ret;
> > +
> > +   ret = nxp_fspi_clk_prep_enable(f);
> > +   if (ret)
> > +           return ret;
> > +
> > +   /* Reset the module */
> > +   f->write(FSPI_MCR0_SWRST, base + FSPI_MCR0);
> > +
> > +   if (f->devtype_data->little_endian)
> > +           l_mask = FSPI_MCR0_SWRST;
> > +   else
> > +           l_mask = (u32)cpu_to_be32(FSPI_MCR0_SWRST);
> > +
> > +   /* w1c register, wait unit clear */
> > +   ret = readl_poll_timeout(f->iobase + FSPI_MCR0, reg,
> > +                            !(reg & l_mask), 0, POLL_TOUT_US);
> 
> See above. Reuse fspi_readl_poll_tout() here.
> 
Ok

> Regards,
> Frieder
> 
> > +   WARN_ON(ret);
> > +
> > +   /* Disable the module */
> > +   f->write(FSPI_MCR0_MDIS, base + FSPI_MCR0);
> > +
> > +   /* Reset the DLL register to default value */
> > +   f->write(FSPI_DLLACR_OVRDEN, base + FSPI_DLLACR);
> > +   f->write(FSPI_DLLBCR_OVRDEN, base + FSPI_DLLBCR);
> > +
> > +   /* enable module */
> > +   f->write(FSPI_MCR0_AHB_TIMEOUT(0xFF) |
> FSPI_MCR0_IP_TIMEOUT(0xFF),
> > +            base + FSPI_MCR0);
> > +
> > +   /*
> > +    * Disable same device enable bit and configure all slave devices
> > +    * independently.
> > +    */
> > +   reg = f->read(f->iobase + FSPI_MCR2);
> > +   reg = reg & ~(FSPI_MCR2_SAMEDEVICEEN);
> > +   f->write(reg, base + FSPI_MCR2);
> > +
> > +   /* AHB configuration for access buffer 0~7. */
> > +   for (i = 0; i < 7; i++)
> > +           f->write(0, base + FSPI_AHBRX_BUF0CR0 + 4 * i);
> > +
> > +   /*
> > +    * Set ADATSZ with the maximum AHB buffer size to improve the read
> > +    * performance.
> > +    */
> > +   f->write((f->devtype_data->ahb_buf_size / 8 |
> > +             FSPI_AHBRXBUF0CR7_PREF), base + FSPI_AHBRX_BUF7CR0);
> > +
> > +   /* prefetch and no start address alignment limitation */
> > +   f->write(FSPI_AHBCR_PREF_EN | FSPI_AHBCR_RDADDROPT,
> > +            base + FSPI_AHBCR);
> > +
> > +   /* AHB Read - Set lut sequence ID for all CS. */
> > +   f->write(SEQID_LUT, base + FSPI_FLSHA1CR2);
> > +   f->write(SEQID_LUT, base + FSPI_FLSHA2CR2);
> > +   f->write(SEQID_LUT, base + FSPI_FLSHB1CR2);
> > +   f->write(SEQID_LUT, base + FSPI_FLSHB2CR2);
> > +
> > +   f->selected = -1;
> > +
> > +   /* enable the interrupt */
> > +   f->write(FSPI_INTEN_IPCMDDONE, base + FSPI_INTEN);
> > +
> > +   return 0;
> > +}
> > +
> > +static const struct spi_controller_mem_ops nxp_fspi_mem_ops = {
> > +   .adjust_op_size = nxp_fspi_adjust_op_size,
> > +   .supports_op = nxp_fspi_supports_op,
> > +   .exec_op = nxp_fspi_exec_op,
> > +};
> > +
> > +static int nxp_fspi_probe(struct platform_device *pdev) {
> > +   struct spi_controller *ctlr;
> > +   struct device *dev = &pdev->dev;
> > +   struct device_node *np = dev->of_node;
> > +   struct resource *res;
> > +   struct nxp_fspi *f;
> > +   int ret;
> > +
> > +   ctlr = spi_alloc_master(&pdev->dev, sizeof(*f));
> > +   if (!ctlr)
> > +           return -ENOMEM;
> > +
> > +   ctlr->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD |
> > +                     SPI_TX_DUAL | SPI_TX_QUAD;
> > +
> > +   f = spi_controller_get_devdata(ctlr);
> > +   f->dev = dev;
> > +   f->devtype_data = of_device_get_match_data(dev);
> > +   if (!f->devtype_data) {
> > +           ret = -ENODEV;
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   platform_set_drvdata(pdev, f);
> > +
> > +   /* find the resources - configuration register address space */
> > +   res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "fspi_base");
> > +   f->iobase = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(f->iobase)) {
> > +           ret = PTR_ERR(f->iobase);
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   /* find the resources - controller memory mapped space */
> > +   res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "fspi_mmap");
> > +   f->ahb_addr = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(f->ahb_addr)) {
> > +           ret = PTR_ERR(f->ahb_addr);
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   /* assign memory mapped starting address and mapped size. */
> > +   f->memmap_phy = res->start;
> > +   f->memmap_phy_size = resource_size(res);
> > +
> > +   /* find the clocks */
> > +   f->clk_en = devm_clk_get(dev, "fspi_en");
> > +   if (IS_ERR(f->clk_en)) {
> > +           ret = PTR_ERR(f->clk_en);
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   f->clk = devm_clk_get(dev, "fspi");
> > +   if (IS_ERR(f->clk)) {
> > +           ret = PTR_ERR(f->clk);
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   /*
> > +    * R/W functions for big- or little-endian registers:
> > +    * The FSPI controller's endianness is independent of
> > +    * the CPU core's endianness. So far, although the CPU
> > +    * core is little-endian the FSPI controller can use
> > +    * big-endian or little-endian.
> > +    */
> > +   if (f->devtype_data->little_endian) {
> > +           f->write = fspi_writel;
> > +           f->read = fspi_readl;
> > +   } else {
> > +           f->write = fspi_writel_be;
> > +           f->read = fspi_readl_be;
> > +   }
> > +
> > +   ret = nxp_fspi_clk_prep_enable(f);
> > +   if (ret) {
> > +           dev_err(dev, "can not enable the clock\n");
> > +           goto err_put_ctrl;
> > +   }
> > +
> > +   /* find the irq */
> > +   ret = platform_get_irq(pdev, 0);
> > +   if (ret < 0) {
> > +           dev_err(dev, "failed to get the irq: %d\n", ret);
> > +           goto err_disable_clk;
> > +   }
> > +
> > +   ret = devm_request_irq(dev, ret,
> > +                   nxp_fspi_irq_handler, 0, pdev->name, f);
> > +   if (ret) {
> > +           dev_err(dev, "failed to request irq: %d\n", ret);
> > +           goto err_disable_clk;
> > +   }
> > +
> > +   mutex_init(&f->lock);
> > +
> > +   ctlr->bus_num = -1;
> > +   ctlr->num_chipselect = NXP_FSPI_MAX_CHIPSELECT;
> > +   ctlr->mem_ops = &nxp_fspi_mem_ops;
> > +
> > +   nxp_fspi_default_setup(f);
> > +
> > +   ctlr->dev.of_node = np;
> > +
> > +   ret = spi_register_controller(ctlr);
> > +   if (ret)
> > +           goto err_destroy_mutex;
> > +
> > +   return 0;
> > +
> > +err_destroy_mutex:
> > +   mutex_destroy(&f->lock);
> > +
> > +err_disable_clk:
> > +   nxp_fspi_clk_disable_unprep(f);
> > +
> > +err_put_ctrl:
> > +   spi_controller_put(ctlr);
> > +
> > +   dev_err(dev, "NXP FSPI probe failed\n");
> > +   return ret;
> > +}
> > +
> > +static int nxp_fspi_remove(struct platform_device *pdev) {
> > +   struct nxp_fspi *f = platform_get_drvdata(pdev);
> > +
> > +   /* disable the hardware */
> > +   f->write(FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
> > +
> > +   nxp_fspi_clk_disable_unprep(f);
> > +
> > +   mutex_destroy(&f->lock);
> > +
> > +   return 0;
> > +}
> > +
> > +static int nxp_fspi_suspend(struct device *dev) {
> > +   return 0;
> > +}
> > +
> > +static int nxp_fspi_resume(struct device *dev) {
> > +   struct nxp_fspi *f = dev_get_drvdata(dev);
> > +
> > +   nxp_fspi_default_setup(f);
> > +
> > +   return 0;
> > +}
> > +
> > +static const struct of_device_id nxp_fspi_dt_ids[] = {
> > +   { .compatible = "nxp,lx2160a-fspi", .data = (void *)&lx2160a_data, },
> > +   { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);
> > +
> > +static const struct dev_pm_ops nxp_fspi_pm_ops = {
> > +   .suspend        = nxp_fspi_suspend,
> > +   .resume         = nxp_fspi_resume,
> > +};
> > +
> > +static struct platform_driver nxp_fspi_driver = {
> > +   .driver = {
> > +           .name   = "nxp-fspi",
> > +           .of_match_table = nxp_fspi_dt_ids,
> > +           .pm =   &nxp_fspi_pm_ops,
> > +   },
> > +   .probe          = nxp_fspi_probe,
> > +   .remove         = nxp_fspi_remove,
> > +};
> > +module_platform_driver(nxp_fspi_driver);
> > +
> > +MODULE_DESCRIPTION("NXP FSPI Controller Driver");
> MODULE_AUTHOR("NXP
> > +Semiconductor"); MODULE_LICENSE("GPL v2");
> >

Reply via email to