[U-Boot] [PATCH v5 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
Jagan Teki
jagan at amarulasolutions.com
Wed Jun 27 08:12:56 UTC 2018
On Wed, Jun 27, 2018 at 1:13 PM, Siva Durga Prasad Paladugu
<sivadur at xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> Sent: Wednesday, June 27, 2018 12:52 PM
>> To: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
>> Cc: U-Boot-Denx <u-boot at lists.denx.de>; Jagan Teki
>> <jagan at openedev.com>; Michal Simek <michals at xilinx.com>
>> Subject: Re: [U-Boot] [PATCH v5 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP qspi driver
>>
>> On Tue, Jun 26, 2018 at 3:07 PM, Siva Durga Prasad Paladugu
>> <siva.durga.paladugu at xilinx.com> wrote:
>> > This patch adds qspi driver support for ZynqMP SoC. This driver is
>> > responsible for communicating with qspi flash devices.
>> >
>> > Signed-off-by: Siva Durga Prasad Paladugu
>> > <siva.durga.paladugu at xilinx.com>
>> > ---
>> > Changed for v5:
>> > - Removed zynqm_gqspi.h file which was added
>> > by mistake.
>> >
>> > Changes for v4:
>> > - Moved macro definitions back to .c
>> > - Removed last_cmd and flash command checks in driver
>> > - Used macros and GENMASK as per comments
>> > - Removed debugs wherever commented.
>> > - Modified set_mode routine as per comment
>> >
>> > Changes for v3:
>> > - Renamed all macros, functions, files and configs as per comment
>> > - Used wait_for_bit wherever required
>> > - Removed unnecessary header inclusion
>> >
>> > Changes for v2:
>> > - Rebased on top of latest master
>> > - Moved macro definitions to .h file as per comment
>> > - Fixed magic values with macros as per comment
>> > ---
>> > drivers/spi/Kconfig | 7 +
>> > drivers/spi/Makefile | 1 +
>> > drivers/spi/zynqmp_gqspi.c | 794
>> > +++++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 802 insertions(+)
>> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >
>> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
>> > 3532c2a..c3c424e 100644
>> > --- a/drivers/spi/Kconfig
>> > +++ b/drivers/spi/Kconfig
>> > @@ -223,6 +223,13 @@ config ZYNQ_QSPI
>> > Zynq QSPI IP core. This IP is used to connect the flash in
>> > 4-bit qspi, 8-bit dual stacked and shared 4-bit dual parallel.
>> >
>> > +config ZYNQMP_GQSPI
>> > + bool "Configure ZynqMP Generic QSPI"
>> > + depends on ARCH_ZYNQMP
>> > + help
>> > + This option is used to enable ZynqMP QSPI controller driver which
>> > + is used to communicate with qspi flash devices.
>> > +
>> > endif # if DM_SPI
>> >
>> > config SOFT_SPI
>> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index
>> > 5a2c00e..2187633 100644
>> > --- a/drivers/spi/Makefile
>> > +++ b/drivers/spi/Makefile
>> > @@ -51,3 +51,4 @@ obj-$(CONFIG_TI_QSPI) += ti_qspi.o
>> > obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
>> > obj-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
>> > obj-$(CONFIG_ZYNQ_QSPI) += zynq_qspi.o
>> > +obj-$(CONFIG_ZYNQMP_GQSPI) += zynqmp_gqspi.o
>> > diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c
>> > new file mode 100644 index 0000000..9a04839
>> > --- /dev/null
>> > +++ b/drivers/spi/zynqmp_gqspi.c
>> > @@ -0,0 +1,794 @@
>> > +// SPDX-License-Identifier: GPL-2.0+
>> > +/*
>> > + * (C) Copyright 2018 Xilinx
>> > + *
>> > + * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode
>> > +only) */
>> > +
>> > +#include <common.h>
>> > +#include <asm/arch/clk.h>
>> > +#include <asm/arch/hardware.h>
>> > +#include <asm/arch/sys_proto.h>
>> > +#include <asm/io.h>
>> > +#include <clk.h>
>> > +#include <malloc.h>
>> > +#include <memalign.h>
>> > +#include <spi.h>
>> > +#include <spi_flash.h>
>>
>> remove this header.
> Yes can be removed now
>
>>
>> > +#include <ubi_uboot.h>
>> > +#include <wait_bit.h>
>> > +
>> > +#define GQSPI_GFIFO_STRT_MODE_MASK BIT(29)
>> > +#define GQSPI_CONFIG_MODE_EN_MASK (3 << 30)
>> > +#define GQSPI_CONFIG_DMA_MODE (2 << 30) #define
>> > +GQSPI_CONFIG_CPHA_MASK BIT(2) #define
>> GQSPI_CONFIG_CPOL_MASK BIT(1)
>> > +
>> > +/* QSPI MIO's count for different connection topologies */
>> > +#define GQSPI_MIO_NUM_QSPI0 6
>> > +#define GQSPI_MIO_NUM_QSPI1 5
>> > +#define GQSPI_MIO_NUM_QSPI1_CS 1
>> > +
>> > +/*
>> > + * QSPI Interrupt Registers bit Masks
>> > + *
>> > + * All the four interrupt registers (Status/Mask/Enable/Disable) have
>> > +the same
>> > + * bit definitions.
>> > + */
>> > +#define GQSPI_IXR_TXNFULL_MASK 0x00000004 /* QSPI TX FIFO
>> Overflow */
>> > +#define GQSPI_IXR_TXFULL_MASK 0x00000008 /* QSPI TX FIFO is full
>> */
>> > +#define GQSPI_IXR_RXNEMTY_MASK 0x00000010 /* QSPI RX FIFO Not
>> Empty
>> > +*/ #define GQSPI_IXR_GFEMTY_MASK 0x00000080 /* QSPI Generic
>> FIFO Empty */
>> > +#define GQSPI_IXR_ALL_MASK (GQSPI_IXR_TXNFULL_MASK | \
>> > + GQSPI_IXR_RXNEMTY_MASK)
>> > +
>> > +/*
>> > + * QSPI Enable Register bit Masks
>> > + *
>> > + * This register is used to enable or disable the QSPI controller */
>> > +#define GQSPI_ENABLE_ENABLE_MASK 0x00000001 /* QSPI Enable
>> Bit Mask */
>> > +
>> > +#define GQSPI_GFIFO_LOW_BUS BIT(14)
>> > +#define GQSPI_GFIFO_CS_LOWER BIT(12)
>> > +#define GQSPI_GFIFO_UP_BUS BIT(15)
>> > +#define GQSPI_GFIFO_CS_UPPER BIT(13)
>> > +#define GQSPI_SPI_MODE_QSPI (3 << 10)
>> > +#define GQSPI_SPI_MODE_SPI BIT(10)
>> > +#define GQSPI_SPI_MODE_DUAL_SPI (2 << 10)
>> > +#define GQSPI_IMD_DATA_CS_ASSERT 5
>> > +#define GQSPI_IMD_DATA_CS_DEASSERT 5
>> > +#define GQSPI_GFIFO_TX BIT(16)
>> > +#define GQSPI_GFIFO_RX BIT(17)
>> > +#define GQSPI_GFIFO_STRIPE_MASK BIT(18)
>> > +#define GQSPI_GFIFO_IMD_MASK 0xFF
>> > +#define GQSPI_GFIFO_EXP_MASK BIT(9)
>> > +#define GQSPI_GFIFO_DATA_XFR_MASK BIT(8)
>> > +#define GQSPI_STRT_GEN_FIFO BIT(28)
>> > +#define GQSPI_GEN_FIFO_STRT_MOD BIT(29)
>> > +#define GQSPI_GFIFO_WP_HOLD BIT(19)
>> > +#define GQSPI_BAUD_DIV_MASK (7 << 3)
>> > +#define GQSPI_DFLT_BAUD_RATE_DIV BIT(3)
>> > +#define GQSPI_GFIFO_ALL_INT_MASK 0xFBE
>> > +#define GQSPI_DMA_DST_I_STS_DONE BIT(1)
>> > +#define GQSPI_DMA_DST_I_STS_MASK 0xFE
>> > +#define MODEBITS 0x6
>> > +
>> > +#define QUAD_OUT_READ_CMD 0x6B
>> > +#define QUAD_PAGE_PROGRAM_CMD 0x32
>> > +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
>>
>> remove these as well.
> Same as above
>
>>
>> > +
>> > +#define GQSPI_GFIFO_SELECT BIT(0)
>> > +
>> > +#define GQSPI_FIFO_THRESHOLD 1
>> > +
>> > +#define SPI_XFER_ON_BOTH 0
>> > +#define SPI_XFER_ON_LOWER 1
>> > +#define SPI_XFER_ON_UPPER 2
>> > +
>> > +#define GQSPI_DMA_ALIGN 0x4
>> > +#define GQSPI_MAX_BAUD_RATE_VAL 7
>> > +#define GQSPI_DFLT_BAUD_RATE_VAL 2
>> > +
>> > +#define GQSPI_TIMEOUT 100000000
>> > +
>> > +#define GQSPI_BAUD_DIV_SHIFT 2
>> > +#define GQSPI_LPBK_DLY_ADJ_LPBK_SHIFT 5
>> > +#define GQSPI_LPBK_DLY_ADJ_DLY_1 0x2
>> > +#define GQSPI_LPBK_DLY_ADJ_DLY_1_SHIFT 3
>> > +#define GQSPI_LPBK_DLY_ADJ_DLY_0 0x3
>> > +#define GQSPI_USE_DATA_DLY 0x1
>> > +#define GQSPI_USE_DATA_DLY_SHIFT 31
>> > +#define GQSPI_DATA_DLY_ADJ_VALUE 0x2
>> > +#define GQSPI_DATA_DLY_ADJ_SHIFT 28
>> > +#define TAP_DLY_BYPASS_LQSPI_RX_VALUE 0x1 #define
>> > +TAP_DLY_BYPASS_LQSPI_RX_SHIFT 2
>> > +#define GQSPI_DATA_DLY_ADJ_OFST 0x000001F8
>> > +#define IOU_TAPDLY_BYPASS_OFST 0xFF180390
>> > +#define GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK 0x00000020
>> > +#define GQSPI_FREQ_40MHZ 40000000
>> > +#define GQSPI_FREQ_100MHZ 100000000
>> > +#define GQSPI_FREQ_150MHZ 150000000
>> > +#define IOU_TAPDLY_BYPASS_MASK 0x7
>> > +
>> > +#define GQSPI_REG_OFFSET 0x100
>> > +#define GQSPI_DMA_REG_OFFSET 0x800
>>
>> most of the macros in this file are not tab alignied, please align properly.
> All are tab aligned only, do you mean that there only be a single tab between MACRO and value?
I saw most of macros are missaliged with tab space when I apply in tree.
>
>>
>> > +
>> > +/* QSPI register offsets */
>> > +struct zynqmp_qspi_regs {
>> > + u32 confr; /* 0x00 */
>> > + u32 isr; /* 0x04 */
>> > + u32 ier; /* 0x08 */
>> > + u32 idisr; /* 0x0C */
>> > + u32 imaskr; /* 0x10 */
>> > + u32 enbr; /* 0x14 */
>> > + u32 dr; /* 0x18 */
>> > + u32 txd0r; /* 0x1C */
>> > + u32 drxr; /* 0x20 */
>> > + u32 sicr; /* 0x24 */
>> > + u32 txftr; /* 0x28 */
>> > + u32 rxftr; /* 0x2C */
>> > + u32 gpior; /* 0x30 */
>> > + u32 reserved0; /* 0x34 */
>> > + u32 lpbkdly; /* 0x38 */
>> > + u32 reserved1; /* 0x3C */
>> > + u32 genfifo; /* 0x40 */
>> > + u32 gqspisel; /* 0x44 */
>> > + u32 reserved2; /* 0x48 */
>> > + u32 gqfifoctrl; /* 0x4C */
>> > + u32 gqfthr; /* 0x50 */
>> > + u32 gqpollcfg; /* 0x54 */
>> > + u32 gqpollto; /* 0x58 */
>> > + u32 gqxfersts; /* 0x5C */
>> > + u32 gqfifosnap; /* 0x60 */
>> > + u32 gqrxcpy; /* 0x64 */
>> > + u32 reserved3[36]; /* 0x68 */
>> > + u32 gqspidlyadj; /* 0xF8 */
>> > +};
>> > +
>> > +struct zynqmp_qspi_dma_regs {
>> > + u32 dmadst; /* 0x00 */
>> > + u32 dmasize; /* 0x04 */
>> > + u32 dmasts; /* 0x08 */
>> > + u32 dmactrl; /* 0x0C */
>> > + u32 reserved0; /* 0x10 */
>> > + u32 dmaisr; /* 0x14 */
>> > + u32 dmaier; /* 0x18 */
>> > + u32 dmaidr; /* 0x1C */
>> > + u32 dmaimr; /* 0x20 */
>> > + u32 dmactrl2; /* 0x24 */
>> > + u32 dmadstmsb; /* 0x28 */
>> > +};
>> > +
>> > +DECLARE_GLOBAL_DATA_PTR;
>> > +
>> > +struct zynqmp_qspi_platdata {
>> > + struct zynqmp_qspi_regs *regs;
>> > + struct zynqmp_qspi_dma_regs *dma_regs;
>> > + u32 frequency;
>> > + u32 speed_hz;
>> > + unsigned int tx_rx_mode;
>> > +};
>> > +
>> > +struct zynqmp_qspi_priv {
>> > + struct zynqmp_qspi_regs *regs;
>> > + struct zynqmp_qspi_dma_regs *dma_regs;
>> > + u8 mode;
>> > + const void *tx_buf;
>> > + void *rx_buf;
>> > + unsigned int len;
>> > + int bytes_to_transfer;
>> > + int bytes_to_receive;
>> > + unsigned int is_inst;
>> > + unsigned int cs_change:1;
>> > +};
>> > +
>> > +static int zynqmp_qspi_ofdata_to_platdata(struct udevice *bus) {
>> > + struct zynqmp_qspi_platdata *plat = bus->platdata;
>> > + u32 mode = 0;
>> > + u32 value;
>> > + int ret;
>> > + struct clk clk;
>> > + unsigned long clock;
>> > +
>> > + debug("%s\n", __func__);
>> > +
>> > + plat->regs = (struct zynqmp_qspi_regs *)(devfdt_get_addr(bus) +
>> > + GQSPI_REG_OFFSET);
>> > + plat->dma_regs = (struct zynqmp_qspi_dma_regs *)
>> > + (devfdt_get_addr(bus) +
>> > + GQSPI_DMA_REG_OFFSET);
>> > +
>> > + ret = clk_get_by_index(bus, 0, &clk);
>> > + if (ret < 0) {
>> > + dev_err(dev, "failed to get clock\n");
>> > + return ret;
>> > + }
>> > +
>> > + clock = clk_get_rate(&clk);
>> > + if (IS_ERR_VALUE(clock)) {
>> > + dev_err(dev, "failed to get rate\n");
>> > + return clock;
>> > + }
>> > + debug("%s: CLK %ld\n", __func__, clock);
>> > +
>> > + ret = clk_enable(&clk);
>> > + if (ret && ret != -ENOSYS) {
>> > + dev_err(dev, "failed to enable clock\n");
>> > + return ret;
>> > + }
>>
>> these clock stuff, add directly in probe.
>
> Will check and move it if possible.
>
>>
>> > +
>> > + if (dev_read_bool(bus, "spi-cpol"))
>> > + mode |= SPI_CPOL;
>> > + if (dev_read_bool(bus, "spi-cpha"))
>> > + mode |= SPI_CPHA;
>> > +
>> > + value = dev_read_u32_default(bus, "spi-rx-bus-width", 1);
>> > + switch (value) {
>> > + case 1:
>> > + break;
>> > + case 2:
>> > + mode |= SPI_RX_DUAL;
>> > + break;
>> > + case 4:
>> > + mode |= SPI_RX_QUAD;
>> > + break;
>> > + default:
>> > + printf("Invalid spi-rx-bus-width %d\n", value);
>> > + break;
>> > + }
>> > +
>> > + value = dev_read_u32_default(bus, "spi-tx-bus-width", 1);
>> > + switch (value) {
>> > + case 1:
>> > + break;
>> > + case 2:
>> > + mode |= SPI_TX_DUAL;
>> > + break;
>> > + case 4:
>> > + mode |= SPI_TX_QUAD;
>> > + break;
>> > + default:
>> > + printf("Invalid spi-tx-bus-width %d\n", value);
>> > + break;
>> > + }
>> > +
>> > + plat->tx_rx_mode = mode;
>>
>> So, here I'm confused. tx_rx_mode store the mode from dt and assigned in
>> slave->mode in zynqmp_qspi_child_pre_probe, why we need that? and it's
>> never used anywhere. are you trying to change mode in spi-uclass.c?
>
> I agree. If this is the only place, will check and update slave mode here itself. Then we can remove preprobe.
Ha, if ie the case why we need to update the mode? since the mode is
always come from spi-uclass which is again grabbed in .set_mode. if we
want the mode to use anywhere in private driver we can assign it from
there.
More information about the U-Boot
mailing list