Hi Priyanka,
> -----Original Message-----
> From: Priyanka Jain <priyanka.j...@nxp.com>
> Sent: Tuesday, December 17, 2019 10:38 AM
> To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de
> Cc: ja...@amarulasolutions.com; s...@denx.de; Ashish Kumar
> <ashish.ku...@nxp.com>; frieder.schre...@kontron.de; Kuldeep Singh
> <kuldeep.si...@nxp.com>; Ashish Kumar <ashish.ku...@nxp.com>
> Subject: RE: [Patch v2 1/7] spi: Transform the FSL QuadSPI driver to use the
> SPI
> MEM API
>
>
>
> >-----Original Message-----
> >From: Kuldeep Singh <kuldeep.si...@nxp.com>
> >Sent: Monday, December 16, 2019 5:19 PM
> >To: u-boot@lists.denx.de
> >Cc: ja...@amarulasolutions.com; Priyanka Jain <priyanka.j...@nxp.com>;
> >s...@denx.de; Ashish Kumar <ashish.ku...@nxp.com>;
> >frieder.schre...@kontron.de; Kuldeep Singh <kuldeep.si...@nxp.com>;
> >Ashish Kumar <ashish.ku...@nxp.com>
> >Subject: [Patch v2 1/7] spi: Transform the FSL QuadSPI driver to use
> >the SPI MEM API
> >
> >To support the SPI MEM API, instead of modifying the existing U-Boot
> >driver, this patch adds a port of the existing Linux driver.
> >This also has the advantage that porting changes and fixes from Linux
> >will be easier.
> >Porting of driver left most of the functions unchanged while few of the
> >changes are:
> >-Remove lock(mutexes) and irq handler as uboot is a single core execution.
> >-Remove clock support as the changing spi speed is not supported in
> >uboot and nor in linux.
> >
> >Currently tested on LS1088ARDB, LS1012ARDB, LS1046ARDB, LS1046AFRWY,
> >LS1043AQDS, LS1021ATWR, LS2080ARDB
> >
> >Signed-off-by: Frieder Schrempf <frieder.schre...@kontron.de>
> >Signed-off-by: Ashish Kumar <ashish.ku...@nxp.com>
> >Signed-off-by: Kuldeep Singh <kuldeep.si...@nxp.com>
> >---
> > drivers/spi/fsl_qspi.c | 1562 +++++++++++++++-------------------------
> > drivers/spi/fsl_qspi.h | 145 ----
> > 2 files changed, 593 insertions(+), 1114 deletions(-) delete mode
> >100644 drivers/spi/fsl_qspi.h
> >
> >diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index
> >8e2a09df36..96178c06bc 100644
> >--- a/drivers/spi/fsl_qspi.c
> >+++ b/drivers/spi/fsl_qspi.c
> >@@ -1,1142 +1,766 @@
> > // SPDX-License-Identifier: GPL-2.0+
> >+
> > /*
> >- * Copyright 2013-2015 Freescale Semiconductor, Inc.
> >+ * Freescale QuadSPI driver.
> >+ *
> >+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
> >+ * Copyright (C) 2018 Bootlin
> >+ * Copyright (C) 2018 exceet electronics GmbH
> >+ * Copyright (C) 2018 Kontron Electronics GmbH
> >+ * Copyright (C) 2019-2020 NXP
> Please check copyright format
> It seems the changes have been brought from Linux driver.
> If yes, it would be good to mention that in File header
Ok. I will mention that information on top.
> > *
> >- * Freescale Quad Serial Peripheral Interface (QSPI) driver
> >+ * Based on the original fsl-quadspi.c spi-nor driver.
> >+ * Transition to spi-mem in spi-fsl-qspi.c
> > */
> >
> > #include <common.h>
> >-#include <malloc.h>
> >-#include <spi.h>
> > #include <asm/io.h>
> >-#include <linux/sizes.h>
> >-#include <linux/iopoll.h>
> > #include <dm.h>
> >-#include <errno.h>
> >-#include <watchdog.h>
> >-#include <wait_bit.h>
> >-#include "fsl_qspi.h"
> >+#include <linux/iopoll.h>
> >+#include <linux/sizes.h>
> >+#include <spi.h>
> >+#include <spi-mem.h>
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> >-#define OFFSET_BITS_MASK GENMASK(23, 0)
> >-
> >-#define FLASH_STATUS_WEL 0x02
> >-
> >-/* SEQID */
> >-#define SEQID_WREN 1
> >-#define SEQID_FAST_READ 2
> >-#define SEQID_RDSR 3
> >-#define SEQID_SE 4
> >-#define SEQID_CHIP_ERASE 5
> >-#define SEQID_PP 6
> >-#define SEQID_RDID 7
> >-#define SEQID_BE_4K 8
> >-#ifdef CONFIG_SPI_FLASH_BAR
> >-#define SEQID_BRRD 9
> >-#define SEQID_BRWR 10
> >-#define SEQID_RDEAR 11
> >-#define SEQID_WREAR 12
> >-#endif
> >-#define SEQID_WRAR 13
> >-#define SEQID_RDAR 14
> >-
> >-/* QSPI CMD */
> >-#define QSPI_CMD_PP 0x02 /* Page program (up to 256
> bytes) */
> >-#define QSPI_CMD_RDSR 0x05 /* Read status register */
> >-#define QSPI_CMD_WREN 0x06 /* Write enable */
> >-#define QSPI_CMD_FAST_READ 0x0b /* Read data bytes (high
> >frequency) */
> >-#define QSPI_CMD_BE_4K 0x20 /* 4K erase */
> >-#define QSPI_CMD_CHIP_ERASE 0xc7 /* Erase whole flash chip */
> >-#define QSPI_CMD_SE 0xd8 /* Sector erase (usually 64KiB)
> */
> >-#define QSPI_CMD_RDID 0x9f /* Read JEDEC ID */
> >-
> >-/* Used for Micron, winbond and Macronix flashes */
> >-#define QSPI_CMD_WREAR 0xc5 /* EAR register write
> */
> >-#define QSPI_CMD_RDEAR 0xc8 /* EAR reigster read
> */
> >-
> >-/* Used for Spansion flashes only. */
> >-#define QSPI_CMD_BRRD 0x16 /* Bank register read
> >*/
> >-#define QSPI_CMD_BRWR 0x17 /* Bank register
> write
> >*/
> >-
> >-/* Used for Spansion S25FS-S family flash only. */
> >-#define QSPI_CMD_RDAR 0x65 /* Read any device register
> */
> >-#define QSPI_CMD_WRAR 0x71 /* Write any device register
> */
> >-
> >-/* 4-byte address QSPI CMD - used on Spansion and some Macronix flashes
> */
> >-#define QSPI_CMD_FAST_READ_4B 0x0c /* Read data bytes (high
> >frequency) */
> >-#define QSPI_CMD_PP_4B 0x12 /* Page program (up to 256
> >bytes) */
> >-#define QSPI_CMD_SE_4B 0xdc /* Sector erase (usually 64KiB)
> >*/
> >-
> >-/* fsl_qspi_platdata flags */
> >-#define QSPI_FLAG_REGMAP_ENDIAN_BIG BIT(0)
> >-
> >-/* default SCK frequency, unit: HZ */
> >-#define FSL_QSPI_DEFAULT_SCK_FREQ 50000000
> >-
> >-/* QSPI max chipselect signals number */
> >-#define FSL_QSPI_MAX_CHIPSELECT_NUM 4
> >-
> >-/* Controller needs driver to swap endian */
> >+/*
> >+ * 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 (15).
> >+ */
> >+#define SEQID_LUT 15
> >+
> >+/* Registers used by the driver */
> >+#define QUADSPI_MCR 0x00
> >+#define QUADSPI_MCR_RESERVED_MASK GENMASK(19, 16)
> >+#define QUADSPI_MCR_MDIS_MASK BIT(14)
> >+#define QUADSPI_MCR_CLR_TXF_MASK BIT(11)
> >+#define QUADSPI_MCR_CLR_RXF_MASK BIT(10)
> >+#define QUADSPI_MCR_DDR_EN_MASK BIT(7)
> >+#define QUADSPI_MCR_END_CFG_MASK GENMASK(3, 2)
> >+#define QUADSPI_MCR_SWRSTHD_MASK BIT(1)
> >+#define QUADSPI_MCR_SWRSTSD_MASK BIT(0)
> >+
> >+#define QUADSPI_IPCR 0x08
> >+#define QUADSPI_IPCR_SEQID(x) ((x) << 24)
> >+#define QUADSPI_FLSHCR 0x0c
> >+#define QUADSPI_FLSHCR_TCSS_MASK GENMASK(3, 0)
> >+#define QUADSPI_FLSHCR_TCSH_MASK GENMASK(11, 8)
> >+#define QUADSPI_FLSHCR_TDH_MASK GENMASK(17, 16)
> >+
> >+#define QUADSPI_BUF3CR 0x1c
> >+#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31)
> >+#define QUADSPI_BUF3CR_ADATSZ(x) ((x) << 8)
> >+#define QUADSPI_BUF3CR_ADATSZ_MASK GENMASK(15, 8)
> >+
> >+#define QUADSPI_BFGENCR 0x20
> >+#define QUADSPI_BFGENCR_SEQID(x) ((x) << 12)
> >+
> >+#define QUADSPI_BUF0IND 0x30
> >+#define QUADSPI_BUF1IND 0x34
> >+#define QUADSPI_BUF2IND 0x38
> >+#define QUADSPI_SFAR 0x100
> >+
> >+#define QUADSPI_SMPR 0x108
> >+#define QUADSPI_SMPR_DDRSMP_MASK GENMASK(18, 16)
> >+#define QUADSPI_SMPR_FSDLY_MASK BIT(6)
> >+#define QUADSPI_SMPR_FSPHS_MASK BIT(5)
> >+#define QUADSPI_SMPR_HSENA_MASK BIT(0)
> >+
> >+#define QUADSPI_RBCT 0x110
> >+#define QUADSPI_RBCT_WMRK_MASK GENMASK(4, 0)
> >+#define QUADSPI_RBCT_RXBRD_USEIPS BIT(8)
> >+
> >+#define QUADSPI_TBDR 0x154
> >+
> >+#define QUADSPI_SR 0x15c
> >+#define QUADSPI_SR_IP_ACC_MASK BIT(1)
> >+#define QUADSPI_SR_AHB_ACC_MASK BIT(2)
> >+
> >+#define QUADSPI_FR 0x160
> >+#define QUADSPI_FR_TFF_MASK BIT(0)
> >+
> >+#define QUADSPI_RSER 0x164
> >+#define QUADSPI_RSER_TFIE BIT(0)
> >+
> >+#define QUADSPI_SPTRCLR 0x16c
> >+#define QUADSPI_SPTRCLR_IPPTRC BIT(8)
> >+#define QUADSPI_SPTRCLR_BFPTRC BIT(0)
> >+
> >+#define QUADSPI_SFA1AD 0x180
> >+#define QUADSPI_SFA2AD 0x184
> >+#define QUADSPI_SFB1AD 0x188
> >+#define QUADSPI_SFB2AD 0x18c
> >+#define QUADSPI_RBDR(x) (0x200 + ((x) * 4))
> >+
> >+#define QUADSPI_LUTKEY 0x300
> >+#define QUADSPI_LUTKEY_VALUE 0x5AF05AF0
> >+
> >+#define QUADSPI_LCKCR 0x304
> >+#define QUADSPI_LCKER_LOCK BIT(0)
> >+#define QUADSPI_LCKER_UNLOCK BIT(1)
> >+
> >+#define QUADSPI_LUT_BASE 0x310
> >+#define QUADSPI_LUT_OFFSET (SEQID_LUT * 4 * 4)
> >+#define QUADSPI_LUT_REG(idx) \
> >+ (QUADSPI_LUT_BASE + QUADSPI_LUT_OFFSET + (idx) * 4)
> >+
> >+/* Instruction set for the LUT register */
> >+#define LUT_STOP 0
> >+#define LUT_CMD 1
> >+#define LUT_ADDR 2
> >+#define LUT_DUMMY 3
> >+#define LUT_MODE 4
> >+#define LUT_MODE2 5
> >+#define LUT_MODE4 6
> >+#define LUT_FSL_READ 7
> >+#define LUT_FSL_WRITE 8
> >+#define LUT_JMP_ON_CS 9
> >+#define LUT_ADDR_DDR 10
> >+#define LUT_MODE_DDR 11
> >+#define LUT_MODE2_DDR 12
> >+#define LUT_MODE4_DDR 13
> >+#define LUT_FSL_READ_DDR 14
> >+#define LUT_FSL_WRITE_DDR 15
> >+#define LUT_DATA_LEARN 16
> >+
> >+/*
> >+ * The PAD definitions for LUT register.
> >+ *
> >+ * The pad stands for the number of IO lines [0:3].
> >+ * For example, the quad read needs four IO lines,
> >+ * so you should use LUT_PAD(4).
> >+ */
> >+#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 LUT_DEF(idx, ins, pad, opr) \
> >+ ((((ins) << 10) | ((pad) << 8) | (opr)) << (((idx) % 2) * 16))
> >+
> >+/* Controller needs driver to swap endianness */
> > #define QUADSPI_QUIRK_SWAP_ENDIAN BIT(0)
> >
> >-enum fsl_qspi_devtype {
> >- FSL_QUADSPI_VYBRID,
> >- FSL_QUADSPI_IMX6SX,
> >- FSL_QUADSPI_IMX6UL_7D,
> >- FSL_QUADSPI_IMX7ULP,
> >-};
> >+/* Controller needs 4x internal clock */
> >+#define QUADSPI_QUIRK_4X_INT_CLK BIT(1)
> >
> >-struct fsl_qspi_devtype_data {
> >- enum fsl_qspi_devtype devtype;
> >- u32 rxfifo;
> >- u32 txfifo;
> >- u32 ahb_buf_size;
> >- u32 driver_data;
> >-};
> >+/*
> >+ * TKT253890, the controller needs the driver to fill the txfifo with
> >+ * 16 bytes at least to trigger a data transfer, even though the extra
> >+ * data won't be transferred.
> >+ */
> >+#define QUADSPI_QUIRK_TKT253890 BIT(2)
> >
> >-/**
> >- * struct fsl_qspi_platdata - platform data for Freescale QSPI
> >- *
> >- * @flags: Flags for QSPI QSPI_FLAG_...
> >- * @speed_hz: Default SCK frequency
> >- * @reg_base: Base address of QSPI registers
> >- * @amba_base: Base address of QSPI memory mapping
> >- * @amba_total_size: size of QSPI memory mapping
> >- * @flash_num: Number of active slave devices
> >- * @num_chipselect: Number of QSPI chipselect signals
> >+/* TKT245618, the controller cannot wake up from wait mode */
> >+#define QUADSPI_QUIRK_TKT245618 BIT(3)
> >+
> >+/*
> >+ * Controller adds QSPI_AMBA_BASE (base address of the mapped memory)
> >+ * internally. No need to add it when setting SFXXAD and SFAR
> >+registers
> > */
> >-struct fsl_qspi_platdata {
> >- u32 flags;
> >- u32 speed_hz;
> >- fdt_addr_t reg_base;
> >- fdt_addr_t amba_base;
> >- fdt_size_t amba_total_size;
> >- u32 flash_num;
> >- u32 num_chipselect;
> >-};
> >+#define QUADSPI_QUIRK_BASE_INTERNAL BIT(4)
> >
> >-/**
> >- * struct fsl_qspi_priv - private data for Freescale QSPI
> >- *
> >- * @flags: Flags for QSPI QSPI_FLAG_...
> >- * @bus_clk: QSPI input clk frequency
> >- * @speed_hz: Default SCK frequency
> >- * @cur_seqid: current LUT table sequence id
> >- * @sf_addr: flash access offset
> >- * @amba_base: Base address of QSPI memory mapping of every CS
> >- * @amba_total_size: size of QSPI memory mapping
> >- * @cur_amba_base: Base address of QSPI memory mapping of current CS
> >- * @flash_num: Number of active slave devices
> >- * @num_chipselect: Number of QSPI chipselect signals
> >- * @regs: Point to QSPI register structure for I/O access
> >+/*
> >+ * Controller uses TDH bits in register QUADSPI_FLSHCR.
> >+ * They need to be set in accordance with the DDR/SDR mode.
> > */
> >-struct fsl_qspi_priv {
> >- u32 flags;
> >- u32 bus_clk;
> >- u32 speed_hz;
> >- u32 cur_seqid;
> >- u32 sf_addr;
> >- u32 amba_base[FSL_QSPI_MAX_CHIPSELECT_NUM];
> >- u32 amba_total_size;
> >- u32 cur_amba_base;
> >- u32 flash_num;
> >- u32 num_chipselect;
> >- struct fsl_qspi_regs *regs;
> >- struct fsl_qspi_devtype_data *devtype_data;
> >+#define QUADSPI_QUIRK_USE_TDH_SETTING BIT(5)
> >+
> >+struct fsl_qspi_devtype_data {
> >+ unsigned int rxfifo;
> >+ unsigned int txfifo;
> >+ unsigned int ahb_buf_size;
> >+ unsigned int quirks;
> >+ bool little_endian;
> > };
> >
> > static const struct fsl_qspi_devtype_data vybrid_data = {
> >- .devtype = FSL_QUADSPI_VYBRID,
> >- .rxfifo = 128,
> >- .txfifo = 64,
> >- .ahb_buf_size = 1024,
> >- .driver_data = QUADSPI_QUIRK_SWAP_ENDIAN,
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_64,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_SWAP_ENDIAN,
> >+ .little_endian = true,
> > };
> >
> > static const struct fsl_qspi_devtype_data imx6sx_data = {
> >- .devtype = FSL_QUADSPI_IMX6SX,
> >- .rxfifo = 128,
> >- .txfifo = 512,
> >- .ahb_buf_size = 1024,
> >- .driver_data = 0,
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_512,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_4X_INT_CLK | QUADSPI_QUIRK_TKT245618,
> >+ .little_endian = true,
> >+};
> >+
> >+static const struct fsl_qspi_devtype_data imx7d_data = {
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_512,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_QUIRK_4X_INT_CLK
> |
> >+ QUADSPI_QUIRK_USE_TDH_SETTING,
> >+ .little_endian = true,
> >+};
> >+
> >+static const struct fsl_qspi_devtype_data imx6ul_data = {
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_512,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_QUIRK_4X_INT_CLK
> |
> >+ QUADSPI_QUIRK_USE_TDH_SETTING,
> >+ .little_endian = true,
> > };
> >
> >-static const struct fsl_qspi_devtype_data imx6ul_7d_data = {
> >- .devtype = FSL_QUADSPI_IMX6UL_7D,
> >- .rxfifo = 128,
> >- .txfifo = 512,
> >- .ahb_buf_size = 1024,
> >- .driver_data = 0,
> >+static const struct fsl_qspi_devtype_data ls1021a_data = {
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_64,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = 0,
> >+ .little_endian = false,
> > };
> >
> >-static const struct fsl_qspi_devtype_data imx7ulp_data = {
> >- .devtype = FSL_QUADSPI_IMX7ULP,
> >- .rxfifo = 64,
> >- .txfifo = 64,
> >- .ahb_buf_size = 128,
> >- .driver_data = 0,
> >+static const struct fsl_qspi_devtype_data ls1088a_data = {
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_128,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_TKT253890,
> >+ .little_endian = true,
> > };
> >
> >-static u32 qspi_read32(u32 flags, u32 *addr)
> >+static const struct fsl_qspi_devtype_data ls2080a_data = {
> >+ .rxfifo = SZ_128,
> >+ .txfifo = SZ_64,
> >+ .ahb_buf_size = SZ_1K,
> >+ .quirks = QUADSPI_QUIRK_TKT253890 |
> >QUADSPI_QUIRK_BASE_INTERNAL,
> >+ .little_endian = true,
> >+};
> >+
> >+struct fsl_qspi {
> >+ void __iomem *iobase;
> >+ void __iomem *ahb_addr;
> >+ u32 memmap_phy;
> >+ const struct fsl_qspi_devtype_data *devtype_data;
> >+ int selected;
> >+};
> >+
> It would be better to move all declarations to header file.
Current Linux driver has declarations in the same file and that's why the same
idea was applied to u-boot as well.
Thanks
Kuldeep
> <snip>
> -Priyanka
