* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
@ 2018-05-08 10:13 Siva Durga Prasad Paladugu
2018-05-08 10:13 ` [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards Siva Durga Prasad Paladugu
2018-05-09 6:30 ` [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Jagan Teki
0 siblings, 2 replies; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-08 10:13 UTC (permalink / raw)
To: u-boot
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@xilinx.com>
---
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
---
arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/zynqmp_gqspi.c | 670 ++++++++++++++++++++++++
4 files changed, 832 insertions(+)
create mode 100644 arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
create mode 100644 drivers/spi/zynqmp_gqspi.c
diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
new file mode 100644
index 0000000..4b26d80
--- /dev/null
+++ b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
@@ -0,0 +1,154 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * (C) Copyright 2018 Xilinx
+ *
+ * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller
+ * driver (master mode only)
+ */
+
+#ifndef _ASM_ARCH_ZYNQMP_GQSPI_H_
+#define _ASM_ARCH_ZYNQMP_GQSPI_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
+
+#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
+
+/* 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 */
+};
+
+#endif /* _ASM_ARCH_ZYNQMP_GQSPI_H_ */
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6667f73..269af6f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -222,6 +222,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 176bfa0..29c77c3 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -53,3 +53,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..ceb3745
--- /dev/null
+++ b/drivers/spi/zynqmp_gqspi.c
@@ -0,0 +1,670 @@
+// 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 <malloc.h>
+#include <memalign.h>
+#include <ubi_uboot.h>
+#include <spi.h>
+#include <spi_flash.h>
+#include <asm/io.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/clk.h>
+#include <clk.h>
+#include <asm/arch/zynqmp_gqspi.h>
+#include <wait_bit.h>
+
+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 u8 last_cmd;
+
+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;
+ }
+
+ 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;
+
+ plat->frequency = clock;
+ plat->speed_hz = plat->frequency;
+
+ return 0;
+}
+
+static void zynqmp_qspi_init_hw(struct zynqmp_qspi_priv *priv)
+{
+ u32 config_reg;
+ struct zynqmp_qspi_regs *regs = priv->regs;
+
+ writel(GQSPI_GFIFO_SELECT, ®s->gqspisel);
+ writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->idisr);
+ writel(GQSPI_FIFO_THRESHOLD, ®s->txftr);
+ writel(GQSPI_FIFO_THRESHOLD, ®s->rxftr);
+ writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->isr);
+
+ config_reg = readl(®s->confr);
+ config_reg &= ~(GQSPI_GFIFO_STRT_MODE_MASK |
+ GQSPI_CONFIG_MODE_EN_MASK);
+ config_reg |= GQSPI_CONFIG_DMA_MODE |
+ GQSPI_GFIFO_WP_HOLD |
+ GQSPI_DFLT_BAUD_RATE_DIV;
+ writel(config_reg, ®s->confr);
+
+ writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
+}
+
+static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv *priv)
+{
+ u32 gqspi_fifo_reg = 0;
+
+ gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+ GQSPI_GFIFO_CS_LOWER;
+
+ return gqspi_fifo_reg;
+}
+
+static void zynqmp_qspi_fill_gen_fifo(struct zynqmp_qspi_priv *priv,
+ u32 gqspi_fifo_reg)
+{
+ struct zynqmp_qspi_regs *regs = priv->regs;
+ int ret = 0;
+
+ ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_GFEMTY_MASK, 1,
+ GQSPI_TIMEOUT, 1);
+ if (ret)
+ printf("%s Timeout\n", __func__);
+
+ writel(gqspi_fifo_reg, ®s->genfifo);
+}
+
+static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv *priv, int is_on)
+{
+ u32 gqspi_fifo_reg = 0;
+
+ if (is_on) {
+ gqspi_fifo_reg = zynqmp_qspi_bus_select(priv);
+ gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
+ GQSPI_IMD_DATA_CS_ASSERT;
+ } else {
+ gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
+ gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
+ }
+
+ debug("GFIFO_CMD_CS: 0x%x\n", gqspi_fifo_reg);
+
+ zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg);
+}
+
+void zynqmp_qspi_set_tapdelay(struct udevice *bus, u32 baudrateval)
+{
+ struct zynqmp_qspi_platdata *plat = bus->platdata;
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+ struct zynqmp_qspi_regs *regs = priv->regs;
+ u32 tapdlybypass = 0, lpbkdlyadj = 0, datadlyadj = 0, clk_rate;
+ u32 reqhz = 0;
+
+ clk_rate = plat->frequency;
+ reqhz = (clk_rate / (GQSPI_BAUD_DIV_SHIFT << baudrateval));
+
+ debug("%s, req_hz:%d, clk_rate:%d, baudrateval:%d\n",
+ __func__, reqhz, clk_rate, baudrateval);
+
+ if (reqhz < GQSPI_FREQ_40MHZ) {
+ zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST, &tapdlybypass);
+ tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
+ TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
+ } else if (reqhz < GQSPI_FREQ_100MHZ) {
+ zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST, &tapdlybypass);
+ tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
+ TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
+ lpbkdlyadj = readl(®s->lpbkdly);
+ lpbkdlyadj |= (GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK);
+ datadlyadj = readl(®s->gqspidlyadj);
+ datadlyadj |= ((GQSPI_USE_DATA_DLY << GQSPI_USE_DATA_DLY_SHIFT)
+ | (GQSPI_DATA_DLY_ADJ_VALUE <<
+ GQSPI_DATA_DLY_ADJ_SHIFT));
+ } else if (reqhz < GQSPI_FREQ_150MHZ) {
+ lpbkdlyadj = readl(®s->lpbkdly);
+ lpbkdlyadj |= ((GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK) |
+ GQSPI_LPBK_DLY_ADJ_DLY_0);
+ }
+
+ zynqmp_mmio_write(IOU_TAPDLY_BYPASS_OFST, IOU_TAPDLY_BYPASS_MASK,
+ tapdlybypass);
+ writel(lpbkdlyadj, ®s->lpbkdly);
+ writel(datadlyadj, ®s->gqspidlyadj);
+}
+
+static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed)
+{
+ struct zynqmp_qspi_platdata *plat = bus->platdata;
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+ struct zynqmp_qspi_regs *regs = priv->regs;
+ u32 confr;
+ u8 baud_rate_val = 0;
+
+ debug("%s\n", __func__);
+ if (speed > plat->frequency)
+ speed = plat->frequency;
+
+ /* Set the clock frequency */
+ confr = readl(®s->confr);
+ if (speed == 0) {
+ /* Set baudrate x8, if the freq is 0 */
+ baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
+ } else if (plat->speed_hz != speed) {
+ while ((baud_rate_val < 8) &&
+ ((plat->frequency /
+ (2 << baud_rate_val)) > speed))
+ baud_rate_val++;
+
+ if (baud_rate_val > GQSPI_MAX_BAUD_RATE_VAL)
+ baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
+
+ plat->speed_hz = plat->frequency / (2 << baud_rate_val);
+ }
+ confr &= ~GQSPI_BAUD_DIV_MASK;
+ confr |= (baud_rate_val << 3);
+ writel(confr, ®s->confr);
+
+ zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
+ debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
+
+ return 0;
+}
+
+static int zynqmp_qspi_child_pre_probe(struct udevice *bus)
+{
+ struct spi_slave *slave = dev_get_parent_priv(bus);
+ struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus->parent);
+
+ slave->mode = plat->tx_rx_mode;
+
+ return 0;
+}
+
+static int zynqmp_qspi_probe(struct udevice *bus)
+{
+ struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus);
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+
+ debug("%s: bus:%p, priv:%p\n", __func__, bus, priv);
+
+ priv->regs = plat->regs;
+ priv->dma_regs = plat->dma_regs;
+
+ /* init the zynq spi hw */
+ zynqmp_qspi_init_hw(priv);
+
+ return 0;
+}
+
+static int zynqmp_qspi_set_mode(struct udevice *bus, uint mode)
+{
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+ struct zynqmp_qspi_regs *regs = priv->regs;
+ u32 confr;
+
+ debug("%s\n", __func__);
+ /* Set the SPI Clock phase and polarities */
+ confr = readl(®s->confr);
+ confr &= ~(GQSPI_CONFIG_CPHA_MASK |
+ GQSPI_CONFIG_CPOL_MASK);
+
+ if (priv->mode & SPI_CPHA)
+ confr |= GQSPI_CONFIG_CPHA_MASK;
+ if (priv->mode & SPI_CPOL)
+ confr |= GQSPI_CONFIG_CPOL_MASK;
+
+ priv->mode = mode;
+ writel(confr, ®s->confr);
+
+ debug("regs=%p, mode=%d\n", priv->regs, priv->mode);
+
+ return 0;
+}
+
+static int zynqmp_qspi_fill_tx_fifo(struct zynqmp_qspi_priv *priv, u32 size)
+{
+ u32 data;
+ int ret = 0;
+ struct zynqmp_qspi_regs *regs = priv->regs;
+ u32 *buf = (u32 *)priv->tx_buf;
+ u32 len = size;
+
+ debug("TxFIFO: 0x%x, size: 0x%x\n", readl(®s->isr),
+ size);
+
+ while (size) {
+ ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_TXNFULL_MASK, 1,
+ GQSPI_TIMEOUT, 1);
+ if (ret) {
+ printf("%s: Timeout\n", __func__);
+ return ret;
+ }
+
+ if (size >= 4) {
+ writel(*buf, ®s->txd0r);
+ buf++;
+ size -= 4;
+ } else {
+ switch (size) {
+ case 1:
+ data = *((u8 *)buf);
+ buf += 1;
+ data |= 0xFFFFFF00;
+ break;
+ case 2:
+ data = *((u16 *)buf);
+ buf += 2;
+ data |= 0xFFFF0000;
+ break;
+ case 3:
+ data = *((u16 *)buf);
+ buf += 2;
+ data |= (*((u8 *)buf) << 16);
+ buf += 1;
+ data |= 0xFF000000;
+ break;
+ }
+ writel(data, ®s->txd0r);
+ size = 0;
+ }
+ }
+
+ priv->tx_buf += len;
+ return 0;
+}
+
+static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv *priv)
+{
+ u8 command = 1;
+ u32 gen_fifo_cmd;
+ u32 bytecount = 0;
+
+ while (priv->len) {
+ gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
+ gen_fifo_cmd |= GQSPI_GFIFO_TX;
+
+ if (command) {
+ command = 0;
+ last_cmd = *(u8 *)priv->tx_buf;
+ }
+
+ gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
+ gen_fifo_cmd |= *(u8 *)priv->tx_buf;
+ bytecount++;
+ priv->len--;
+ priv->tx_buf = (u8 *)priv->tx_buf + 1;
+
+ debug("GFIFO_CMD_Cmd = 0x%x\n", gen_fifo_cmd);
+
+ zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
+ }
+}
+
+static u32 zynqmp_qspi_calc_exp(struct zynqmp_qspi_priv *priv,
+ u32 *gen_fifo_cmd)
+{
+ u32 expval = 8;
+ u32 len;
+
+ while (1) {
+ if (priv->len > 255) {
+ if (priv->len & (1 << expval)) {
+ *gen_fifo_cmd &= ~GQSPI_GFIFO_IMD_MASK;
+ *gen_fifo_cmd |= GQSPI_GFIFO_EXP_MASK;
+ *gen_fifo_cmd |= expval;
+ priv->len -= (1 << expval);
+ return expval;
+ }
+ expval++;
+ } else {
+ *gen_fifo_cmd &= ~(GQSPI_GFIFO_IMD_MASK |
+ GQSPI_GFIFO_EXP_MASK);
+ *gen_fifo_cmd |= (u8)priv->len;
+ len = (u8)priv->len;
+ priv->len = 0;
+ return len;
+ }
+ }
+}
+
+static int zynqmp_qspi_genfifo_fill_tx(struct zynqmp_qspi_priv *priv)
+{
+ u32 gen_fifo_cmd;
+ u32 len;
+ int ret = 0;
+
+ gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
+ gen_fifo_cmd |= GQSPI_GFIFO_TX |
+ GQSPI_GFIFO_DATA_XFR_MASK;
+
+ if (last_cmd == QUAD_PAGE_PROGRAM_CMD)
+ gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
+ else
+ gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
+
+ while (priv->len) {
+ len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
+ zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
+
+ debug("GFIFO_CMD_TX:0x%x\n", gen_fifo_cmd);
+
+ if (gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK)
+ ret = zynqmp_qspi_fill_tx_fifo(priv,
+ 1 << len);
+ else
+ ret = zynqmp_qspi_fill_tx_fifo(priv,
+ len);
+
+ if (ret)
+ return ret;
+ }
+ return ret;
+}
+
+static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv,
+ u32 gen_fifo_cmd, u32 *buf)
+{
+ u32 addr;
+ u32 size, len;
+ u32 actuallen = priv->len;
+ int ret = 0;
+ struct zynqmp_qspi_dma_regs *dma_regs = priv->dma_regs;
+
+ writel((unsigned long)buf, &dma_regs->dmadst);
+ writel(roundup(priv->len, 4), &dma_regs->dmasize);
+ writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier);
+ addr = (unsigned long)buf;
+ size = roundup(priv->len, ARCH_DMA_MINALIGN);
+ flush_dcache_range(addr, addr + size);
+
+ while (priv->len) {
+ len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
+ if (!(gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK) &&
+ (len % 4)) {
+ gen_fifo_cmd &= ~(0xFF);
+ gen_fifo_cmd |= (len / 4 + 1) * 4;
+ }
+ zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
+
+ debug("GFIFO_CMD_RX:0x%x\n", gen_fifo_cmd);
+ }
+
+ ret = wait_for_bit_le32(&dma_regs->dmaisr, GQSPI_DMA_DST_I_STS_DONE,
+ 1, GQSPI_TIMEOUT, 1);
+ if (ret) {
+ printf("DMA Timeout:0x%x\n", readl(&dma_regs->dmaisr));
+ return -1;
+ }
+
+ writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr);
+
+ debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n",
+ (unsigned long)buf, (unsigned long)priv->rx_buf, *buf,
+ actuallen);
+
+ if (buf != priv->rx_buf)
+ memcpy(priv->rx_buf, buf, actuallen);
+
+ return 0;
+}
+
+static int zynqmp_qspi_genfifo_fill_rx(struct zynqmp_qspi_priv *priv)
+{
+ u32 gen_fifo_cmd;
+ u32 *buf;
+ u32 actuallen = priv->len;
+
+ gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
+ gen_fifo_cmd |= GQSPI_GFIFO_RX |
+ GQSPI_GFIFO_DATA_XFR_MASK;
+
+ if (last_cmd == QUAD_OUT_READ_CMD)
+ gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
+ else if (last_cmd == DUAL_OUTPUT_FASTRD_CMD)
+ gen_fifo_cmd |= GQSPI_SPI_MODE_DUAL_SPI;
+ else
+ gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
+
+ /*
+ * Check if receive buffer is aligned to 4 byte and length
+ * is multiples of four byte as we are using dma to receive.
+ */
+ if (!((unsigned long)priv->rx_buf & (GQSPI_DMA_ALIGN - 1)) &&
+ !(actuallen % GQSPI_DMA_ALIGN)) {
+ buf = (u32 *)priv->rx_buf;
+ return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
+ }
+
+ ALLOC_CACHE_ALIGN_BUFFER(u8, tmp, roundup(priv->len,
+ GQSPI_DMA_ALIGN));
+ buf = (u32 *)tmp;
+ return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
+}
+
+static int zynqmp_qspi_start_transfer(struct zynqmp_qspi_priv *priv)
+{
+ int ret = 0;
+
+ if (priv->is_inst) {
+ if (priv->tx_buf)
+ zynqmp_qspi_genfifo_cmd(priv);
+ else
+ ret = -1;
+ } else {
+ if (priv->tx_buf)
+ ret = zynqmp_qspi_genfifo_fill_tx(priv);
+ else if (priv->rx_buf)
+ ret = zynqmp_qspi_genfifo_fill_rx(priv);
+ else
+ ret = -1;
+ }
+ return ret;
+}
+
+static int zynqmp_qspi_transfer(struct zynqmp_qspi_priv *priv)
+{
+ static unsigned int cs_change = 1;
+ int status = 0;
+
+ debug("%s\n", __func__);
+
+ while (1) {
+ /* Select the chip if required */
+ if (cs_change)
+ zynqmp_qspi_chipselect(priv, 1);
+
+ cs_change = priv->cs_change;
+
+ if (!priv->tx_buf && !priv->rx_buf && priv->len) {
+ status = -1;
+ break;
+ }
+
+ /* Request the transfer */
+ if (priv->len) {
+ status = zynqmp_qspi_start_transfer(priv);
+ priv->is_inst = 0;
+ if (status < 0)
+ break;
+ }
+
+ if (cs_change)
+ /* Deselect the chip */
+ zynqmp_qspi_chipselect(priv, 0);
+ break;
+ }
+
+ return status;
+}
+
+static int zynqmp_qspi_claim_bus(struct udevice *dev)
+{
+ struct udevice *bus = dev->parent;
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+ struct zynqmp_qspi_regs *regs = priv->regs;
+
+ debug("%s\n", __func__);
+ writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
+
+ return 0;
+}
+
+static int zynqmp_qspi_release_bus(struct udevice *dev)
+{
+ struct udevice *bus = dev->parent;
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+ struct zynqmp_qspi_regs *regs = priv->regs;
+
+ debug("%s\n", __func__);
+ writel(~GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
+
+ return 0;
+}
+
+int zynqmp_qspi_xfer(struct udevice *dev, unsigned int bitlen, const void *dout,
+ void *din, unsigned long flags)
+{
+ struct udevice *bus = dev->parent;
+ struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
+
+ debug("%s: priv: 0x%08lx bitlen: %d dout: 0x%08lx ", __func__,
+ (unsigned long)priv, bitlen, (unsigned long)dout);
+ debug("din: 0x%08lx flags: 0x%lx\n", (unsigned long)din, flags);
+
+ priv->tx_buf = dout;
+ priv->rx_buf = din;
+ priv->len = bitlen / 8;
+
+ /*
+ * Festering sore.
+ * Assume that the beginning of a transfer with bits to
+ * transmit must contain a device command.
+ */
+ if (dout && flags & SPI_XFER_BEGIN)
+ priv->is_inst = 1;
+ else
+ priv->is_inst = 0;
+
+ if (flags & SPI_XFER_END)
+ priv->cs_change = 1;
+ else
+ priv->cs_change = 0;
+
+ zynqmp_qspi_transfer(priv);
+
+ return 0;
+}
+
+static const struct dm_spi_ops zynqmp_qspi_ops = {
+ .claim_bus = zynqmp_qspi_claim_bus,
+ .release_bus = zynqmp_qspi_release_bus,
+ .xfer = zynqmp_qspi_xfer,
+ .set_speed = zynqmp_qspi_set_speed,
+ .set_mode = zynqmp_qspi_set_mode,
+};
+
+static const struct udevice_id zynqmp_qspi_ids[] = {
+ { .compatible = "xlnx,zynqmp-qspi-1.0" },
+ { }
+};
+
+U_BOOT_DRIVER(zynqmp_qspi) = {
+ .name = "zynqmp_qspi",
+ .id = UCLASS_SPI,
+ .of_match = zynqmp_qspi_ids,
+ .ops = &zynqmp_qspi_ops,
+ .ofdata_to_platdata = zynqmp_qspi_ofdata_to_platdata,
+ .platdata_auto_alloc_size = sizeof(struct zynqmp_qspi_platdata),
+ .priv_auto_alloc_size = sizeof(struct zynqmp_qspi_priv),
+ .probe = zynqmp_qspi_probe,
+ .child_pre_probe = zynqmp_qspi_child_pre_probe,
+};
--
2.7.4
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards
2018-05-08 10:13 [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Siva Durga Prasad Paladugu
@ 2018-05-08 10:13 ` Siva Durga Prasad Paladugu
2018-05-09 6:32 ` Jagan Teki
2018-05-09 6:30 ` [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Jagan Teki
1 sibling, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-08 10:13 UTC (permalink / raw)
To: u-boot
This patch adds qspi driver support for all ZynqMP ZCU102
boards.
Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
---
Changes for v3:
- Changed as per latest changes in 1/2
Changes for v2:
- Rebased on top of latest master and enabled qspi for
all zcu102 boards.
---
configs/xilinx_zynqmp_zcu102_rev1_0_defconfig | 5 +++++
configs/xilinx_zynqmp_zcu102_revA_defconfig | 5 +++++
configs/xilinx_zynqmp_zcu102_revB_defconfig | 5 +++++
3 files changed, 15 insertions(+)
diff --git a/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig b/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
index 68da9dc..51f4669 100644
--- a/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
@@ -37,6 +37,7 @@ CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
CONFIG_CMD_I2C=y
CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
CONFIG_CMD_USB=y
CONFIG_CMD_TFTPPUT=y
CONFIG_CMD_TIME=y
@@ -64,6 +65,7 @@ CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
CONFIG_DM_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_ZYNQ=y
+CONFIG_DM_SPI_FLASH=y
CONFIG_SPI_FLASH=y
CONFIG_SPI_FLASH_BAR=y
CONFIG_SPI_FLASH_MACRONIX=y
@@ -87,6 +89,9 @@ CONFIG_DEBUG_UART_BASE=0xff000000
CONFIG_DEBUG_UART_CLOCK=100000000
CONFIG_DEBUG_UART_ANNOUNCE=y
CONFIG_ZYNQ_SERIAL=y
+CONFIG_SPI=y
+CONFIG_DM_SPI=y
+CONFIG_ZYNQMP_GQSPI=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_DWC3=y
diff --git a/configs/xilinx_zynqmp_zcu102_revA_defconfig b/configs/xilinx_zynqmp_zcu102_revA_defconfig
index 2adba61..2ae1a0b 100644
--- a/configs/xilinx_zynqmp_zcu102_revA_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_revA_defconfig
@@ -37,6 +37,7 @@ CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
CONFIG_CMD_I2C=y
CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
CONFIG_CMD_USB=y
CONFIG_CMD_TFTPPUT=y
CONFIG_CMD_TIME=y
@@ -64,6 +65,7 @@ CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
CONFIG_DM_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_ZYNQ=y
+CONFIG_DM_SPI_FLASH=y
CONFIG_SPI_FLASH=y
CONFIG_SPI_FLASH_BAR=y
CONFIG_SPI_FLASH_MACRONIX=y
@@ -87,6 +89,9 @@ CONFIG_DEBUG_UART_BASE=0xff000000
CONFIG_DEBUG_UART_CLOCK=100000000
CONFIG_DEBUG_UART_ANNOUNCE=y
CONFIG_ZYNQ_SERIAL=y
+CONFIG_SPI=y
+CONFIG_DM_SPI=y
+CONFIG_ZYNQMP_GQSPI=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_DWC3=y
diff --git a/configs/xilinx_zynqmp_zcu102_revB_defconfig b/configs/xilinx_zynqmp_zcu102_revB_defconfig
index 2310fa8..6f79a68 100644
--- a/configs/xilinx_zynqmp_zcu102_revB_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_revB_defconfig
@@ -37,6 +37,7 @@ CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
CONFIG_CMD_I2C=y
CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
CONFIG_CMD_USB=y
CONFIG_CMD_TFTPPUT=y
CONFIG_CMD_TIME=y
@@ -64,6 +65,7 @@ CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
CONFIG_DM_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_ZYNQ=y
+CONFIG_DM_SPI_FLASH=y
CONFIG_SPI_FLASH=y
CONFIG_SPI_FLASH_BAR=y
CONFIG_SPI_FLASH_MACRONIX=y
@@ -87,6 +89,9 @@ CONFIG_DEBUG_UART_BASE=0xff000000
CONFIG_DEBUG_UART_CLOCK=100000000
CONFIG_DEBUG_UART_ANNOUNCE=y
CONFIG_ZYNQ_SERIAL=y
+CONFIG_SPI=y
+CONFIG_DM_SPI=y
+CONFIG_ZYNQMP_GQSPI=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_DWC3=y
--
2.7.4
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-08 10:13 [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Siva Durga Prasad Paladugu
2018-05-08 10:13 ` [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards Siva Durga Prasad Paladugu
@ 2018-05-09 6:30 ` Jagan Teki
2018-05-09 7:03 ` Siva Durga Prasad Paladugu
1 sibling, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 6:30 UTC (permalink / raw)
To: u-boot
On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
<siva.durga.paladugu@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@xilinx.com>
> ---
> 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
> ---
> arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
> drivers/spi/Kconfig | 7 +
> drivers/spi/Makefile | 1 +
> drivers/spi/zynqmp_gqspi.c | 670 ++++++++++++++++++++++++
> 4 files changed, 832 insertions(+)
> create mode 100644 arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> create mode 100644 drivers/spi/zynqmp_gqspi.c
>
> diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
already asked you to move this header code in driver .c file
> new file mode 100644
> index 0000000..4b26d80
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> @@ -0,0 +1,154 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
[snip]
> + *
> + * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode only)
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <memalign.h>
> +#include <ubi_uboot.h>
> +#include <spi.h>
> +#include <spi_flash.h>
> +#include <asm/io.h>
> +#include <asm/arch/hardware.h>
> +#include <asm/arch/sys_proto.h>
> +#include <asm/arch/clk.h>
> +#include <clk.h>
> +#include <asm/arch/zynqmp_gqspi.h>
> +#include <wait_bit.h>
headers are inorder.
> +
> +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 u8 last_cmd;
> +
> +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;
> + }
> +
> + 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;
> +
> + plat->frequency = clock;
> + plat->speed_hz = plat->frequency;
> +
> + return 0;
> +}
> +
> +static void zynqmp_qspi_init_hw(struct zynqmp_qspi_priv *priv)
> +{
> + u32 config_reg;
> + struct zynqmp_qspi_regs *regs = priv->regs;
> +
> + writel(GQSPI_GFIFO_SELECT, ®s->gqspisel);
> + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->idisr);
> + writel(GQSPI_FIFO_THRESHOLD, ®s->txftr);
> + writel(GQSPI_FIFO_THRESHOLD, ®s->rxftr);
> + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->isr);
> +
> + config_reg = readl(®s->confr);
> + config_reg &= ~(GQSPI_GFIFO_STRT_MODE_MASK |
> + GQSPI_CONFIG_MODE_EN_MASK);
> + config_reg |= GQSPI_CONFIG_DMA_MODE |
> + GQSPI_GFIFO_WP_HOLD |
> + GQSPI_DFLT_BAUD_RATE_DIV;
> + writel(config_reg, ®s->confr);
> +
> + writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
> +}
> +
> +static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv *priv)
> +{
> + u32 gqspi_fifo_reg = 0;
> +
> + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
> + GQSPI_GFIFO_CS_LOWER;
> +
> + return gqspi_fifo_reg;
> +}
> +
> +static void zynqmp_qspi_fill_gen_fifo(struct zynqmp_qspi_priv *priv,
> + u32 gqspi_fifo_reg)
> +{
> + struct zynqmp_qspi_regs *regs = priv->regs;
> + int ret = 0;
> +
> + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_GFEMTY_MASK, 1,
> + GQSPI_TIMEOUT, 1);
> + if (ret)
> + printf("%s Timeout\n", __func__);
> +
> + writel(gqspi_fifo_reg, ®s->genfifo);
> +}
> +
> +static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv *priv, int is_on)
> +{
> + u32 gqspi_fifo_reg = 0;
> +
> + if (is_on) {
> + gqspi_fifo_reg = zynqmp_qspi_bus_select(priv);
> + gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
> + GQSPI_IMD_DATA_CS_ASSERT;
> + } else {
> + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
> + gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
> + }
> +
> + debug("GFIFO_CMD_CS: 0x%x\n", gqspi_fifo_reg);
> +
> + zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg);
> +}
> +
> +void zynqmp_qspi_set_tapdelay(struct udevice *bus, u32 baudrateval)
> +{
> + struct zynqmp_qspi_platdata *plat = bus->platdata;
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> + struct zynqmp_qspi_regs *regs = priv->regs;
> + u32 tapdlybypass = 0, lpbkdlyadj = 0, datadlyadj = 0, clk_rate;
> + u32 reqhz = 0;
> +
> + clk_rate = plat->frequency;
> + reqhz = (clk_rate / (GQSPI_BAUD_DIV_SHIFT << baudrateval));
> +
> + debug("%s, req_hz:%d, clk_rate:%d, baudrateval:%d\n",
> + __func__, reqhz, clk_rate, baudrateval);
> +
> + if (reqhz < GQSPI_FREQ_40MHZ) {
> + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST, &tapdlybypass);
> + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
> + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
> + } else if (reqhz < GQSPI_FREQ_100MHZ) {
> + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST, &tapdlybypass);
> + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
> + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
> + lpbkdlyadj = readl(®s->lpbkdly);
> + lpbkdlyadj |= (GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK);
> + datadlyadj = readl(®s->gqspidlyadj);
> + datadlyadj |= ((GQSPI_USE_DATA_DLY << GQSPI_USE_DATA_DLY_SHIFT)
> + | (GQSPI_DATA_DLY_ADJ_VALUE <<
> + GQSPI_DATA_DLY_ADJ_SHIFT));
> + } else if (reqhz < GQSPI_FREQ_150MHZ) {
> + lpbkdlyadj = readl(®s->lpbkdly);
> + lpbkdlyadj |= ((GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK) |
> + GQSPI_LPBK_DLY_ADJ_DLY_0);
> + }
> +
> + zynqmp_mmio_write(IOU_TAPDLY_BYPASS_OFST, IOU_TAPDLY_BYPASS_MASK,
> + tapdlybypass);
> + writel(lpbkdlyadj, ®s->lpbkdly);
> + writel(datadlyadj, ®s->gqspidlyadj);
> +}
> +
> +static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed)
> +{
> + struct zynqmp_qspi_platdata *plat = bus->platdata;
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> + struct zynqmp_qspi_regs *regs = priv->regs;
> + u32 confr;
> + u8 baud_rate_val = 0;
> +
> + debug("%s\n", __func__);
> + if (speed > plat->frequency)
> + speed = plat->frequency;
> +
> + /* Set the clock frequency */
> + confr = readl(®s->confr);
> + if (speed == 0) {
> + /* Set baudrate x8, if the freq is 0 */
> + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
> + } else if (plat->speed_hz != speed) {
> + while ((baud_rate_val < 8) &&
> + ((plat->frequency /
> + (2 << baud_rate_val)) > speed))
> + baud_rate_val++;
> +
> + if (baud_rate_val > GQSPI_MAX_BAUD_RATE_VAL)
> + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
> +
> + plat->speed_hz = plat->frequency / (2 << baud_rate_val);
> + }
> + confr &= ~GQSPI_BAUD_DIV_MASK;
> + confr |= (baud_rate_val << 3);
> + writel(confr, ®s->confr);
> +
> + zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
> + debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_child_pre_probe(struct udevice *bus)
> +{
> + struct spi_slave *slave = dev_get_parent_priv(bus);
> + struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus->parent);
> +
> + slave->mode = plat->tx_rx_mode;
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_probe(struct udevice *bus)
> +{
> + struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus);
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> +
> + debug("%s: bus:%p, priv:%p\n", __func__, bus, priv);
> +
> + priv->regs = plat->regs;
> + priv->dma_regs = plat->dma_regs;
> +
> + /* init the zynq spi hw */
> + zynqmp_qspi_init_hw(priv);
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_set_mode(struct udevice *bus, uint mode)
what is the mode values your getting here? this mode is what platfdata
configured like what you did in above function.
see drivers/spi/stm32_qspi.c .set_mode
> +{
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> + struct zynqmp_qspi_regs *regs = priv->regs;
> + u32 confr;
> +
> + debug("%s\n", __func__);
> + /* Set the SPI Clock phase and polarities */
> + confr = readl(®s->confr);
> + confr &= ~(GQSPI_CONFIG_CPHA_MASK |
> + GQSPI_CONFIG_CPOL_MASK);
> +
> + if (priv->mode & SPI_CPHA)
> + confr |= GQSPI_CONFIG_CPHA_MASK;
> + if (priv->mode & SPI_CPOL)
> + confr |= GQSPI_CONFIG_CPOL_MASK;
> +
> + priv->mode = mode;
> + writel(confr, ®s->confr);
> +
> + debug("regs=%p, mode=%d\n", priv->regs, priv->mode);
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_fill_tx_fifo(struct zynqmp_qspi_priv *priv, u32 size)
> +{
> + u32 data;
> + int ret = 0;
> + struct zynqmp_qspi_regs *regs = priv->regs;
> + u32 *buf = (u32 *)priv->tx_buf;
> + u32 len = size;
> +
> + debug("TxFIFO: 0x%x, size: 0x%x\n", readl(®s->isr),
> + size);
> +
> + while (size) {
> + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_TXNFULL_MASK, 1,
> + GQSPI_TIMEOUT, 1);
> + if (ret) {
> + printf("%s: Timeout\n", __func__);
> + return ret;
> + }
> +
> + if (size >= 4) {
> + writel(*buf, ®s->txd0r);
> + buf++;
> + size -= 4;
> + } else {
> + switch (size) {
> + case 1:
> + data = *((u8 *)buf);
> + buf += 1;
> + data |= 0xFFFFFF00;
> + break;
> + case 2:
> + data = *((u16 *)buf);
> + buf += 2;
> + data |= 0xFFFF0000;
> + break;
> + case 3:
> + data = *((u16 *)buf);
> + buf += 2;
> + data |= (*((u8 *)buf) << 16);
> + buf += 1;
> + data |= 0xFF000000;
Use GENMASK instead of hexcodes.
> + break;
> + }
> + writel(data, ®s->txd0r);
> + size = 0;
> + }
> + }
> +
> + priv->tx_buf += len;
> + return 0;
> +}
> +
> +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv *priv)
> +{
> + u8 command = 1;
> + u32 gen_fifo_cmd;
> + u32 bytecount = 0;
> +
> + while (priv->len) {
> + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> +
> + if (command) {
> + command = 0;
> + last_cmd = *(u8 *)priv->tx_buf;
> + }
don't understand this code can you explain? command assigned 1 it will
not updated anywhere?
> +
> + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> + gen_fifo_cmd |= *(u8 *)priv->tx_buf;
> + bytecount++;
> + priv->len--;
> + priv->tx_buf = (u8 *)priv->tx_buf + 1;
> +
> + debug("GFIFO_CMD_Cmd = 0x%x\n", gen_fifo_cmd);
> +
> + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> + }
> +}
> +
> +static u32 zynqmp_qspi_calc_exp(struct zynqmp_qspi_priv *priv,
> + u32 *gen_fifo_cmd)
> +{
> + u32 expval = 8;
> + u32 len;
> +
> + while (1) {
> + if (priv->len > 255) {
what is 255 here?
> + if (priv->len & (1 << expval)) {
> + *gen_fifo_cmd &= ~GQSPI_GFIFO_IMD_MASK;
> + *gen_fifo_cmd |= GQSPI_GFIFO_EXP_MASK;
> + *gen_fifo_cmd |= expval;
> + priv->len -= (1 << expval);
> + return expval;
> + }
> + expval++;
> + } else {
> + *gen_fifo_cmd &= ~(GQSPI_GFIFO_IMD_MASK |
> + GQSPI_GFIFO_EXP_MASK);
> + *gen_fifo_cmd |= (u8)priv->len;
> + len = (u8)priv->len;
> + priv->len = 0;
> + return len;
> + }
> + }
> +}
> +
> +static int zynqmp_qspi_genfifo_fill_tx(struct zynqmp_qspi_priv *priv)
> +{
> + u32 gen_fifo_cmd;
> + u32 len;
> + int ret = 0;
> +
> + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> + gen_fifo_cmd |= GQSPI_GFIFO_TX |
> + GQSPI_GFIFO_DATA_XFR_MASK;
> +
> + if (last_cmd == QUAD_PAGE_PROGRAM_CMD)
> + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> + else
> + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> +
> + while (priv->len) {
> + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> +
> + debug("GFIFO_CMD_TX:0x%x\n", gen_fifo_cmd);
> +
> + if (gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK)
> + ret = zynqmp_qspi_fill_tx_fifo(priv,
> + 1 << len);
> + else
> + ret = zynqmp_qspi_fill_tx_fifo(priv,
> + len);
> +
> + if (ret)
> + return ret;
> + }
> + return ret;
> +}
> +
> +static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv,
> + u32 gen_fifo_cmd, u32 *buf)
> +{
> + u32 addr;
> + u32 size, len;
> + u32 actuallen = priv->len;
> + int ret = 0;
> + struct zynqmp_qspi_dma_regs *dma_regs = priv->dma_regs;
> +
> + writel((unsigned long)buf, &dma_regs->dmadst);
> + writel(roundup(priv->len, 4), &dma_regs->dmasize);
> + writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier);
> + addr = (unsigned long)buf;
> + size = roundup(priv->len, ARCH_DMA_MINALIGN);
> + flush_dcache_range(addr, addr + size);
> +
> + while (priv->len) {
> + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> + if (!(gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK) &&
> + (len % 4)) {
> + gen_fifo_cmd &= ~(0xFF);
> + gen_fifo_cmd |= (len / 4 + 1) * 4;
Need macros for these numerics
> + }
> + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> +
> + debug("GFIFO_CMD_RX:0x%x\n", gen_fifo_cmd);
> + }
> +
> + ret = wait_for_bit_le32(&dma_regs->dmaisr, GQSPI_DMA_DST_I_STS_DONE,
> + 1, GQSPI_TIMEOUT, 1);
> + if (ret) {
> + printf("DMA Timeout:0x%x\n", readl(&dma_regs->dmaisr));
> + return -1;
> + }
> +
> + writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr);
> +
> + debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n",
> + (unsigned long)buf, (unsigned long)priv->rx_buf, *buf,
> + actuallen);
> +
> + if (buf != priv->rx_buf)
> + memcpy(priv->rx_buf, buf, actuallen);
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_genfifo_fill_rx(struct zynqmp_qspi_priv *priv)
> +{
> + u32 gen_fifo_cmd;
> + u32 *buf;
> + u32 actuallen = priv->len;
> +
> + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> + gen_fifo_cmd |= GQSPI_GFIFO_RX |
> + GQSPI_GFIFO_DATA_XFR_MASK;
> +
> + if (last_cmd == QUAD_OUT_READ_CMD)
> + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> + else if (last_cmd == DUAL_OUTPUT_FASTRD_CMD)
> + gen_fifo_cmd |= GQSPI_SPI_MODE_DUAL_SPI;
> + else
> + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> +
> + /*
> + * Check if receive buffer is aligned to 4 byte and length
> + * is multiples of four byte as we are using dma to receive.
> + */
> + if (!((unsigned long)priv->rx_buf & (GQSPI_DMA_ALIGN - 1)) &&
> + !(actuallen % GQSPI_DMA_ALIGN)) {
> + buf = (u32 *)priv->rx_buf;
> + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
> + }
> +
> + ALLOC_CACHE_ALIGN_BUFFER(u8, tmp, roundup(priv->len,
> + GQSPI_DMA_ALIGN));
> + buf = (u32 *)tmp;
> + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
> +}
> +
> +static int zynqmp_qspi_start_transfer(struct zynqmp_qspi_priv *priv)
> +{
> + int ret = 0;
> +
> + if (priv->is_inst) {
> + if (priv->tx_buf)
> + zynqmp_qspi_genfifo_cmd(priv);
> + else
> + ret = -1;
> + } else {
> + if (priv->tx_buf)
> + ret = zynqmp_qspi_genfifo_fill_tx(priv);
> + else if (priv->rx_buf)
> + ret = zynqmp_qspi_genfifo_fill_rx(priv);
> + else
> + ret = -1;
> + }
> + return ret;
> +}
> +
> +static int zynqmp_qspi_transfer(struct zynqmp_qspi_priv *priv)
> +{
> + static unsigned int cs_change = 1;
> + int status = 0;
> +
> + debug("%s\n", __func__);
> +
> + while (1) {
> + /* Select the chip if required */
> + if (cs_change)
> + zynqmp_qspi_chipselect(priv, 1);
> +
> + cs_change = priv->cs_change;
> +
> + if (!priv->tx_buf && !priv->rx_buf && priv->len) {
> + status = -1;
> + break;
> + }
> +
> + /* Request the transfer */
> + if (priv->len) {
> + status = zynqmp_qspi_start_transfer(priv);
> + priv->is_inst = 0;
> + if (status < 0)
> + break;
> + }
> +
> + if (cs_change)
> + /* Deselect the chip */
> + zynqmp_qspi_chipselect(priv, 0);
> + break;
> + }
> +
> + return status;
> +}
> +
> +static int zynqmp_qspi_claim_bus(struct udevice *dev)
> +{
> + struct udevice *bus = dev->parent;
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> + struct zynqmp_qspi_regs *regs = priv->regs;
> +
> + debug("%s\n", __func__);
drop this.
> + writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
> +
> + return 0;
> +}
> +
> +static int zynqmp_qspi_release_bus(struct udevice *dev)
> +{
> + struct udevice *bus = dev->parent;
> + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> + struct zynqmp_qspi_regs *regs = priv->regs;
> +
> + debug("%s\n", __func__);
drop this.
--
Jagan Teki
Senior Linux Kernel Engineer | Amarula Solutions
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards
2018-05-08 10:13 ` [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards Siva Durga Prasad Paladugu
@ 2018-05-09 6:32 ` Jagan Teki
0 siblings, 0 replies; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 6:32 UTC (permalink / raw)
To: u-boot
On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
> This patch adds qspi driver support for all ZynqMP ZCU102
> boards.
>
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
Reviewed-by: Jagan Teki <jagan@openedev.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 6:30 ` [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Jagan Teki
@ 2018-05-09 7:03 ` Siva Durga Prasad Paladugu
2018-05-09 7:41 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-09 7:03 UTC (permalink / raw)
To: u-boot
Hi Jagan,
> -----Original Message-----
> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
> Sent: Wednesday, May 09, 2018 12:01 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP
> qspi driver
>
> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
> <siva.durga.paladugu@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@xilinx.com>
> > ---
> > 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
> > ---
> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
> > drivers/spi/Kconfig | 7 +
> > drivers/spi/Makefile | 1 +
> > drivers/spi/zynqmp_gqspi.c | 670
> ++++++++++++++++++++++++
> > 4 files changed, 832 insertions(+)
> > create mode 100644 arch/arm/include/asm/arch-
> zynqmp/zynqmp_gqspi.h
> > create mode 100644 drivers/spi/zynqmp_gqspi.c
> >
> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>
> already asked you to move this header code in driver .c file
You might have missed my reply to your earlier comment on this. These were moved to .h based on comment
from Lukasz in v1.
I don’t have any issue in having them anywhere. Let me know your choice.
>
> > new file mode 100644
> > index 0000000..4b26d80
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> > @@ -0,0 +1,154 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
>
> [snip]
>
> > + *
> > + * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode
> > + only) */
> > +
> > +#include <common.h>
> > +#include <malloc.h>
> > +#include <memalign.h>
> > +#include <ubi_uboot.h>
> > +#include <spi.h>
> > +#include <spi_flash.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/hardware.h>
> > +#include <asm/arch/sys_proto.h>
> > +#include <asm/arch/clk.h>
> > +#include <clk.h>
> > +#include <asm/arch/zynqmp_gqspi.h>
> > +#include <wait_bit.h>
>
> headers are inorder.
Ok.
>
> > +
> > +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 u8 last_cmd;
> > +
> > +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;
> > + }
> > +
> > + 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;
> > +
> > + plat->frequency = clock;
> > + plat->speed_hz = plat->frequency;
> > +
> > + return 0;
> > +}
> > +
> > +static void zynqmp_qspi_init_hw(struct zynqmp_qspi_priv *priv) {
> > + u32 config_reg;
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > +
> > + writel(GQSPI_GFIFO_SELECT, ®s->gqspisel);
> > + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->idisr);
> > + writel(GQSPI_FIFO_THRESHOLD, ®s->txftr);
> > + writel(GQSPI_FIFO_THRESHOLD, ®s->rxftr);
> > + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->isr);
> > +
> > + config_reg = readl(®s->confr);
> > + config_reg &= ~(GQSPI_GFIFO_STRT_MODE_MASK |
> > + GQSPI_CONFIG_MODE_EN_MASK);
> > + config_reg |= GQSPI_CONFIG_DMA_MODE |
> > + GQSPI_GFIFO_WP_HOLD |
> > + GQSPI_DFLT_BAUD_RATE_DIV;
> > + writel(config_reg, ®s->confr);
> > +
> > + writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr); }
> > +
> > +static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv *priv) {
> > + u32 gqspi_fifo_reg = 0;
> > +
> > + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
> > + GQSPI_GFIFO_CS_LOWER;
> > +
> > + return gqspi_fifo_reg;
> > +}
> > +
> > +static void zynqmp_qspi_fill_gen_fifo(struct zynqmp_qspi_priv *priv,
> > + u32 gqspi_fifo_reg) {
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > + int ret = 0;
> > +
> > + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_GFEMTY_MASK, 1,
> > + GQSPI_TIMEOUT, 1);
> > + if (ret)
> > + printf("%s Timeout\n", __func__);
> > +
> > + writel(gqspi_fifo_reg, ®s->genfifo); }
> > +
> > +static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv *priv, int
> > +is_on) {
> > + u32 gqspi_fifo_reg = 0;
> > +
> > + if (is_on) {
> > + gqspi_fifo_reg = zynqmp_qspi_bus_select(priv);
> > + gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
> > + GQSPI_IMD_DATA_CS_ASSERT;
> > + } else {
> > + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
> > + gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
> > + }
> > +
> > + debug("GFIFO_CMD_CS: 0x%x\n", gqspi_fifo_reg);
> > +
> > + zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg); }
> > +
> > +void zynqmp_qspi_set_tapdelay(struct udevice *bus, u32 baudrateval) {
> > + struct zynqmp_qspi_platdata *plat = bus->platdata;
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > + u32 tapdlybypass = 0, lpbkdlyadj = 0, datadlyadj = 0, clk_rate;
> > + u32 reqhz = 0;
> > +
> > + clk_rate = plat->frequency;
> > + reqhz = (clk_rate / (GQSPI_BAUD_DIV_SHIFT << baudrateval));
> > +
> > + debug("%s, req_hz:%d, clk_rate:%d, baudrateval:%d\n",
> > + __func__, reqhz, clk_rate, baudrateval);
> > +
> > + if (reqhz < GQSPI_FREQ_40MHZ) {
> > + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST,
> &tapdlybypass);
> > + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
> > + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
> > + } else if (reqhz < GQSPI_FREQ_100MHZ) {
> > + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST,
> &tapdlybypass);
> > + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
> > + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
> > + lpbkdlyadj = readl(®s->lpbkdly);
> > + lpbkdlyadj |= (GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK);
> > + datadlyadj = readl(®s->gqspidlyadj);
> > + datadlyadj |= ((GQSPI_USE_DATA_DLY <<
> GQSPI_USE_DATA_DLY_SHIFT)
> > + | (GQSPI_DATA_DLY_ADJ_VALUE <<
> > + GQSPI_DATA_DLY_ADJ_SHIFT));
> > + } else if (reqhz < GQSPI_FREQ_150MHZ) {
> > + lpbkdlyadj = readl(®s->lpbkdly);
> > + lpbkdlyadj |= ((GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK) |
> > + GQSPI_LPBK_DLY_ADJ_DLY_0);
> > + }
> > +
> > + zynqmp_mmio_write(IOU_TAPDLY_BYPASS_OFST,
> IOU_TAPDLY_BYPASS_MASK,
> > + tapdlybypass);
> > + writel(lpbkdlyadj, ®s->lpbkdly);
> > + writel(datadlyadj, ®s->gqspidlyadj); }
> > +
> > +static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed) {
> > + struct zynqmp_qspi_platdata *plat = bus->platdata;
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > + u32 confr;
> > + u8 baud_rate_val = 0;
> > +
> > + debug("%s\n", __func__);
> > + if (speed > plat->frequency)
> > + speed = plat->frequency;
> > +
> > + /* Set the clock frequency */
> > + confr = readl(®s->confr);
> > + if (speed == 0) {
> > + /* Set baudrate x8, if the freq is 0 */
> > + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
> > + } else if (plat->speed_hz != speed) {
> > + while ((baud_rate_val < 8) &&
> > + ((plat->frequency /
> > + (2 << baud_rate_val)) > speed))
> > + baud_rate_val++;
> > +
> > + if (baud_rate_val > GQSPI_MAX_BAUD_RATE_VAL)
> > + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
> > +
> > + plat->speed_hz = plat->frequency / (2 << baud_rate_val);
> > + }
> > + confr &= ~GQSPI_BAUD_DIV_MASK;
> > + confr |= (baud_rate_val << 3);
> > + writel(confr, ®s->confr);
> > +
> > + zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
> > + debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_child_pre_probe(struct udevice *bus) {
> > + struct spi_slave *slave = dev_get_parent_priv(bus);
> > + struct zynqmp_qspi_platdata *plat =
> > +dev_get_platdata(bus->parent);
> > +
> > + slave->mode = plat->tx_rx_mode;
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_probe(struct udevice *bus) {
> > + struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus);
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > +
> > + debug("%s: bus:%p, priv:%p\n", __func__, bus, priv);
> > +
> > + priv->regs = plat->regs;
> > + priv->dma_regs = plat->dma_regs;
> > +
> > + /* init the zynq spi hw */
> > + zynqmp_qspi_init_hw(priv);
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_set_mode(struct udevice *bus, uint mode)
>
> what is the mode values your getting here? this mode is what platfdata
> configured like what you did in above function.
> see drivers/spi/stm32_qspi.c .set_mode
Will check.
>
> > +{
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > + u32 confr;
> > +
> > + debug("%s\n", __func__);
> > + /* Set the SPI Clock phase and polarities */
> > + confr = readl(®s->confr);
> > + confr &= ~(GQSPI_CONFIG_CPHA_MASK |
> > + GQSPI_CONFIG_CPOL_MASK);
> > +
> > + if (priv->mode & SPI_CPHA)
> > + confr |= GQSPI_CONFIG_CPHA_MASK;
> > + if (priv->mode & SPI_CPOL)
> > + confr |= GQSPI_CONFIG_CPOL_MASK;
> > +
> > + priv->mode = mode;
> > + writel(confr, ®s->confr);
> > +
> > + debug("regs=%p, mode=%d\n", priv->regs, priv->mode);
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_fill_tx_fifo(struct zynqmp_qspi_priv *priv,
> > +u32 size) {
> > + u32 data;
> > + int ret = 0;
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > + u32 *buf = (u32 *)priv->tx_buf;
> > + u32 len = size;
> > +
> > + debug("TxFIFO: 0x%x, size: 0x%x\n", readl(®s->isr),
> > + size);
> > +
> > + while (size) {
> > + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_TXNFULL_MASK,
> 1,
> > + GQSPI_TIMEOUT, 1);
> > + if (ret) {
> > + printf("%s: Timeout\n", __func__);
> > + return ret;
> > + }
> > +
> > + if (size >= 4) {
> > + writel(*buf, ®s->txd0r);
> > + buf++;
> > + size -= 4;
> > + } else {
> > + switch (size) {
> > + case 1:
> > + data = *((u8 *)buf);
> > + buf += 1;
> > + data |= 0xFFFFFF00;
> > + break;
> > + case 2:
> > + data = *((u16 *)buf);
> > + buf += 2;
> > + data |= 0xFFFF0000;
> > + break;
> > + case 3:
> > + data = *((u16 *)buf);
> > + buf += 2;
> > + data |= (*((u8 *)buf) << 16);
> > + buf += 1;
> > + data |= 0xFF000000;
>
> Use GENMASK instead of hexcodes.
Ok.
>
> > + break;
> > + }
> > + writel(data, ®s->txd0r);
> > + size = 0;
> > + }
> > + }
> > +
> > + priv->tx_buf += len;
> > + return 0;
> > +}
> > +
> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv *priv) {
> > + u8 command = 1;
> > + u32 gen_fifo_cmd;
> > + u32 bytecount = 0;
> > +
> > + while (priv->len) {
> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> > +
> > + if (command) {
> > + command = 0;
> > + last_cmd = *(u8 *)priv->tx_buf;
> > + }
>
> don't understand this code can you explain? command assigned 1 it will not
> updated anywhere?
I want to store last command sent. As the first byte in loop only contains command, it ensures it fills only for one time
and next time it may contain data to be sent along with command.
Command initialized to 1 while declaring it above(u8 command = 1).
>
> > +
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> > + gen_fifo_cmd |= *(u8 *)priv->tx_buf;
> > + bytecount++;
> > + priv->len--;
> > + priv->tx_buf = (u8 *)priv->tx_buf + 1;
> > +
> > + debug("GFIFO_CMD_Cmd = 0x%x\n", gen_fifo_cmd);
> > +
> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> > + }
> > +}
> > +
> > +static u32 zynqmp_qspi_calc_exp(struct zynqmp_qspi_priv *priv,
> > + u32 *gen_fifo_cmd) {
> > + u32 expval = 8;
> > + u32 len;
> > +
> > + while (1) {
> > + if (priv->len > 255) {
>
> what is 255 here?
When length is greater than 2^8 then we should fill length in different way, hence this check.
I will anyway use macro for better readability.
>
> > + if (priv->len & (1 << expval)) {
> > + *gen_fifo_cmd &= ~GQSPI_GFIFO_IMD_MASK;
> > + *gen_fifo_cmd |= GQSPI_GFIFO_EXP_MASK;
> > + *gen_fifo_cmd |= expval;
> > + priv->len -= (1 << expval);
> > + return expval;
> > + }
> > + expval++;
> > + } else {
> > + *gen_fifo_cmd &= ~(GQSPI_GFIFO_IMD_MASK |
> > + GQSPI_GFIFO_EXP_MASK);
> > + *gen_fifo_cmd |= (u8)priv->len;
> > + len = (u8)priv->len;
> > + priv->len = 0;
> > + return len;
> > + }
> > + }
> > +}
> > +
> > +static int zynqmp_qspi_genfifo_fill_tx(struct zynqmp_qspi_priv *priv)
> > +{
> > + u32 gen_fifo_cmd;
> > + u32 len;
> > + int ret = 0;
> > +
> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> > + gen_fifo_cmd |= GQSPI_GFIFO_TX |
> > + GQSPI_GFIFO_DATA_XFR_MASK;
> > +
> > + if (last_cmd == QUAD_PAGE_PROGRAM_CMD)
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> > + else
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> > +
> > + while (priv->len) {
> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> > +
> > + debug("GFIFO_CMD_TX:0x%x\n", gen_fifo_cmd);
> > +
> > + if (gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK)
> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
> > + 1 << len);
> > + else
> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
> > + len);
> > +
> > + if (ret)
> > + return ret;
> > + }
> > + return ret;
> > +}
> > +
> > +static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv,
> > + u32 gen_fifo_cmd, u32 *buf) {
> > + u32 addr;
> > + u32 size, len;
> > + u32 actuallen = priv->len;
> > + int ret = 0;
> > + struct zynqmp_qspi_dma_regs *dma_regs = priv->dma_regs;
> > +
> > + writel((unsigned long)buf, &dma_regs->dmadst);
> > + writel(roundup(priv->len, 4), &dma_regs->dmasize);
> > + writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier);
> > + addr = (unsigned long)buf;
> > + size = roundup(priv->len, ARCH_DMA_MINALIGN);
> > + flush_dcache_range(addr, addr + size);
> > +
> > + while (priv->len) {
> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> > + if (!(gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK) &&
> > + (len % 4)) {
> > + gen_fifo_cmd &= ~(0xFF);
> > + gen_fifo_cmd |= (len / 4 + 1) * 4;
>
> Need macros for these numerics
Ok
>
> > + }
> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> > +
> > + debug("GFIFO_CMD_RX:0x%x\n", gen_fifo_cmd);
> > + }
> > +
> > + ret = wait_for_bit_le32(&dma_regs->dmaisr,
> GQSPI_DMA_DST_I_STS_DONE,
> > + 1, GQSPI_TIMEOUT, 1);
> > + if (ret) {
> > + printf("DMA Timeout:0x%x\n", readl(&dma_regs->dmaisr));
> > + return -1;
> > + }
> > +
> > + writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr);
> > +
> > + debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n",
> > + (unsigned long)buf, (unsigned long)priv->rx_buf, *buf,
> > + actuallen);
> > +
> > + if (buf != priv->rx_buf)
> > + memcpy(priv->rx_buf, buf, actuallen);
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_genfifo_fill_rx(struct zynqmp_qspi_priv *priv)
> > +{
> > + u32 gen_fifo_cmd;
> > + u32 *buf;
> > + u32 actuallen = priv->len;
> > +
> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> > + gen_fifo_cmd |= GQSPI_GFIFO_RX |
> > + GQSPI_GFIFO_DATA_XFR_MASK;
> > +
> > + if (last_cmd == QUAD_OUT_READ_CMD)
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> > + else if (last_cmd == DUAL_OUTPUT_FASTRD_CMD)
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_DUAL_SPI;
> > + else
> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> > +
> > + /*
> > + * Check if receive buffer is aligned to 4 byte and length
> > + * is multiples of four byte as we are using dma to receive.
> > + */
> > + if (!((unsigned long)priv->rx_buf & (GQSPI_DMA_ALIGN - 1)) &&
> > + !(actuallen % GQSPI_DMA_ALIGN)) {
> > + buf = (u32 *)priv->rx_buf;
> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
> > + }
> > +
> > + ALLOC_CACHE_ALIGN_BUFFER(u8, tmp, roundup(priv->len,
> > + GQSPI_DMA_ALIGN));
> > + buf = (u32 *)tmp;
> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf); }
> > +
> > +static int zynqmp_qspi_start_transfer(struct zynqmp_qspi_priv *priv)
> > +{
> > + int ret = 0;
> > +
> > + if (priv->is_inst) {
> > + if (priv->tx_buf)
> > + zynqmp_qspi_genfifo_cmd(priv);
> > + else
> > + ret = -1;
> > + } else {
> > + if (priv->tx_buf)
> > + ret = zynqmp_qspi_genfifo_fill_tx(priv);
> > + else if (priv->rx_buf)
> > + ret = zynqmp_qspi_genfifo_fill_rx(priv);
> > + else
> > + ret = -1;
> > + }
> > + return ret;
> > +}
> > +
> > +static int zynqmp_qspi_transfer(struct zynqmp_qspi_priv *priv) {
> > + static unsigned int cs_change = 1;
> > + int status = 0;
> > +
> > + debug("%s\n", __func__);
> > +
> > + while (1) {
> > + /* Select the chip if required */
> > + if (cs_change)
> > + zynqmp_qspi_chipselect(priv, 1);
> > +
> > + cs_change = priv->cs_change;
> > +
> > + if (!priv->tx_buf && !priv->rx_buf && priv->len) {
> > + status = -1;
> > + break;
> > + }
> > +
> > + /* Request the transfer */
> > + if (priv->len) {
> > + status = zynqmp_qspi_start_transfer(priv);
> > + priv->is_inst = 0;
> > + if (status < 0)
> > + break;
> > + }
> > +
> > + if (cs_change)
> > + /* Deselect the chip */
> > + zynqmp_qspi_chipselect(priv, 0);
> > + break;
> > + }
> > +
> > + return status;
> > +}
> > +
> > +static int zynqmp_qspi_claim_bus(struct udevice *dev) {
> > + struct udevice *bus = dev->parent;
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > +
> > + debug("%s\n", __func__);
>
> drop this.
I didn’t see any problem in having, its just for debugging.
If you don’t like I can remove.
>
> > + writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr);
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_qspi_release_bus(struct udevice *dev) {
> > + struct udevice *bus = dev->parent;
> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> > + struct zynqmp_qspi_regs *regs = priv->regs;
> > +
> > + debug("%s\n", __func__);
>
> drop this.
Same as above.
Thanks,
Siva
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 7:03 ` Siva Durga Prasad Paladugu
@ 2018-05-09 7:41 ` Jagan Teki
2018-05-09 7:50 ` Siva Durga Prasad Paladugu
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 7:41 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
<sivadur@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> Sent: Wednesday, May 09, 2018 12:01 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
>> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP
>> qspi driver
>>
>> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>> <siva.durga.paladugu@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@xilinx.com>
>> > ---
>> > 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
>> > ---
>> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
>> > drivers/spi/Kconfig | 7 +
>> > drivers/spi/Makefile | 1 +
>> > drivers/spi/zynqmp_gqspi.c | 670
>> ++++++++++++++++++++++++
>> > 4 files changed, 832 insertions(+)
>> > create mode 100644 arch/arm/include/asm/arch-
>> zynqmp/zynqmp_gqspi.h
>> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >
>> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>>
>> already asked you to move this header code in driver .c file
>
> You might have missed my reply to your earlier comment on this. These were moved to .h based on comment
> from Lukasz in v1.
> I don’t have any issue in having them anywhere. Let me know your choice.
I'm trying to align Linux code, better to move like that and make sure
to use similar macros.
>
>>
>> > new file mode 100644
>> > index 0000000..4b26d80
>> > --- /dev/null
>> > +++ b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> > @@ -0,0 +1,154 @@
>> > +/* SPDX-License-Identifier: GPL-2.0+ */
>>
>> [snip]
>>
>> > + *
>> > + * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode
>> > + only) */
>> > +
>> > +#include <common.h>
>> > +#include <malloc.h>
>> > +#include <memalign.h>
>> > +#include <ubi_uboot.h>
>> > +#include <spi.h>
>> > +#include <spi_flash.h>
>> > +#include <asm/io.h>
>> > +#include <asm/arch/hardware.h>
>> > +#include <asm/arch/sys_proto.h>
>> > +#include <asm/arch/clk.h>
>> > +#include <clk.h>
>> > +#include <asm/arch/zynqmp_gqspi.h>
>> > +#include <wait_bit.h>
>>
>> headers are inorder.
>
> Ok.
>>
>> > +
>> > +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 u8 last_cmd;
>> > +
>> > +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;
>> > + }
>> > +
>> > + 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;
>> > +
>> > + plat->frequency = clock;
>> > + plat->speed_hz = plat->frequency;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void zynqmp_qspi_init_hw(struct zynqmp_qspi_priv *priv) {
>> > + u32 config_reg;
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > +
>> > + writel(GQSPI_GFIFO_SELECT, ®s->gqspisel);
>> > + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->idisr);
>> > + writel(GQSPI_FIFO_THRESHOLD, ®s->txftr);
>> > + writel(GQSPI_FIFO_THRESHOLD, ®s->rxftr);
>> > + writel(GQSPI_GFIFO_ALL_INT_MASK, ®s->isr);
>> > +
>> > + config_reg = readl(®s->confr);
>> > + config_reg &= ~(GQSPI_GFIFO_STRT_MODE_MASK |
>> > + GQSPI_CONFIG_MODE_EN_MASK);
>> > + config_reg |= GQSPI_CONFIG_DMA_MODE |
>> > + GQSPI_GFIFO_WP_HOLD |
>> > + GQSPI_DFLT_BAUD_RATE_DIV;
>> > + writel(config_reg, ®s->confr);
>> > +
>> > + writel(GQSPI_ENABLE_ENABLE_MASK, ®s->enbr); }
>> > +
>> > +static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv *priv) {
>> > + u32 gqspi_fifo_reg = 0;
>> > +
>> > + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
>> > + GQSPI_GFIFO_CS_LOWER;
>> > +
>> > + return gqspi_fifo_reg;
>> > +}
>> > +
>> > +static void zynqmp_qspi_fill_gen_fifo(struct zynqmp_qspi_priv *priv,
>> > + u32 gqspi_fifo_reg) {
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > + int ret = 0;
>> > +
>> > + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_GFEMTY_MASK, 1,
>> > + GQSPI_TIMEOUT, 1);
>> > + if (ret)
>> > + printf("%s Timeout\n", __func__);
>> > +
>> > + writel(gqspi_fifo_reg, ®s->genfifo); }
>> > +
>> > +static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv *priv, int
>> > +is_on) {
>> > + u32 gqspi_fifo_reg = 0;
>> > +
>> > + if (is_on) {
>> > + gqspi_fifo_reg = zynqmp_qspi_bus_select(priv);
>> > + gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
>> > + GQSPI_IMD_DATA_CS_ASSERT;
>> > + } else {
>> > + gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
>> > + gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
>> > + }
>> > +
>> > + debug("GFIFO_CMD_CS: 0x%x\n", gqspi_fifo_reg);
>> > +
>> > + zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg); }
>> > +
>> > +void zynqmp_qspi_set_tapdelay(struct udevice *bus, u32 baudrateval) {
>> > + struct zynqmp_qspi_platdata *plat = bus->platdata;
>> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > + u32 tapdlybypass = 0, lpbkdlyadj = 0, datadlyadj = 0, clk_rate;
>> > + u32 reqhz = 0;
>> > +
>> > + clk_rate = plat->frequency;
>> > + reqhz = (clk_rate / (GQSPI_BAUD_DIV_SHIFT << baudrateval));
>> > +
>> > + debug("%s, req_hz:%d, clk_rate:%d, baudrateval:%d\n",
>> > + __func__, reqhz, clk_rate, baudrateval);
>> > +
>> > + if (reqhz < GQSPI_FREQ_40MHZ) {
>> > + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST,
>> &tapdlybypass);
>> > + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
>> > + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
>> > + } else if (reqhz < GQSPI_FREQ_100MHZ) {
>> > + zynqmp_mmio_read(IOU_TAPDLY_BYPASS_OFST,
>> &tapdlybypass);
>> > + tapdlybypass |= (TAP_DLY_BYPASS_LQSPI_RX_VALUE <<
>> > + TAP_DLY_BYPASS_LQSPI_RX_SHIFT);
>> > + lpbkdlyadj = readl(®s->lpbkdly);
>> > + lpbkdlyadj |= (GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK);
>> > + datadlyadj = readl(®s->gqspidlyadj);
>> > + datadlyadj |= ((GQSPI_USE_DATA_DLY <<
>> GQSPI_USE_DATA_DLY_SHIFT)
>> > + | (GQSPI_DATA_DLY_ADJ_VALUE <<
>> > + GQSPI_DATA_DLY_ADJ_SHIFT));
>> > + } else if (reqhz < GQSPI_FREQ_150MHZ) {
>> > + lpbkdlyadj = readl(®s->lpbkdly);
>> > + lpbkdlyadj |= ((GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK) |
>> > + GQSPI_LPBK_DLY_ADJ_DLY_0);
>> > + }
>> > +
>> > + zynqmp_mmio_write(IOU_TAPDLY_BYPASS_OFST,
>> IOU_TAPDLY_BYPASS_MASK,
>> > + tapdlybypass);
>> > + writel(lpbkdlyadj, ®s->lpbkdly);
>> > + writel(datadlyadj, ®s->gqspidlyadj); }
>> > +
>> > +static int zynqmp_qspi_set_speed(struct udevice *bus, uint speed) {
>> > + struct zynqmp_qspi_platdata *plat = bus->platdata;
>> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > + u32 confr;
>> > + u8 baud_rate_val = 0;
>> > +
>> > + debug("%s\n", __func__);
>> > + if (speed > plat->frequency)
>> > + speed = plat->frequency;
>> > +
>> > + /* Set the clock frequency */
>> > + confr = readl(®s->confr);
>> > + if (speed == 0) {
>> > + /* Set baudrate x8, if the freq is 0 */
>> > + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
>> > + } else if (plat->speed_hz != speed) {
>> > + while ((baud_rate_val < 8) &&
>> > + ((plat->frequency /
>> > + (2 << baud_rate_val)) > speed))
>> > + baud_rate_val++;
>> > +
>> > + if (baud_rate_val > GQSPI_MAX_BAUD_RATE_VAL)
>> > + baud_rate_val = GQSPI_DFLT_BAUD_RATE_VAL;
>> > +
>> > + plat->speed_hz = plat->frequency / (2 << baud_rate_val);
>> > + }
>> > + confr &= ~GQSPI_BAUD_DIV_MASK;
>> > + confr |= (baud_rate_val << 3);
>> > + writel(confr, ®s->confr);
>> > +
>> > + zynqmp_qspi_set_tapdelay(bus, baud_rate_val);
>> > + debug("regs=%p, speed=%d\n", priv->regs, plat->speed_hz);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int zynqmp_qspi_child_pre_probe(struct udevice *bus) {
>> > + struct spi_slave *slave = dev_get_parent_priv(bus);
>> > + struct zynqmp_qspi_platdata *plat =
>> > +dev_get_platdata(bus->parent);
>> > +
>> > + slave->mode = plat->tx_rx_mode;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int zynqmp_qspi_probe(struct udevice *bus) {
>> > + struct zynqmp_qspi_platdata *plat = dev_get_platdata(bus);
>> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
>> > +
>> > + debug("%s: bus:%p, priv:%p\n", __func__, bus, priv);
>> > +
>> > + priv->regs = plat->regs;
>> > + priv->dma_regs = plat->dma_regs;
>> > +
>> > + /* init the zynq spi hw */
>> > + zynqmp_qspi_init_hw(priv);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int zynqmp_qspi_set_mode(struct udevice *bus, uint mode)
>>
>> what is the mode values your getting here? this mode is what platfdata
>> configured like what you did in above function.
>> see drivers/spi/stm32_qspi.c .set_mode
> Will check.
>>
>> > +{
>> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > + u32 confr;
>> > +
>> > + debug("%s\n", __func__);
>> > + /* Set the SPI Clock phase and polarities */
>> > + confr = readl(®s->confr);
>> > + confr &= ~(GQSPI_CONFIG_CPHA_MASK |
>> > + GQSPI_CONFIG_CPOL_MASK);
>> > +
>> > + if (priv->mode & SPI_CPHA)
>> > + confr |= GQSPI_CONFIG_CPHA_MASK;
>> > + if (priv->mode & SPI_CPOL)
>> > + confr |= GQSPI_CONFIG_CPOL_MASK;
>> > +
>> > + priv->mode = mode;
>> > + writel(confr, ®s->confr);
>> > +
>> > + debug("regs=%p, mode=%d\n", priv->regs, priv->mode);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int zynqmp_qspi_fill_tx_fifo(struct zynqmp_qspi_priv *priv,
>> > +u32 size) {
>> > + u32 data;
>> > + int ret = 0;
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > + u32 *buf = (u32 *)priv->tx_buf;
>> > + u32 len = size;
>> > +
>> > + debug("TxFIFO: 0x%x, size: 0x%x\n", readl(®s->isr),
>> > + size);
>> > +
>> > + while (size) {
>> > + ret = wait_for_bit_le32(®s->isr, GQSPI_IXR_TXNFULL_MASK,
>> 1,
>> > + GQSPI_TIMEOUT, 1);
>> > + if (ret) {
>> > + printf("%s: Timeout\n", __func__);
>> > + return ret;
>> > + }
>> > +
>> > + if (size >= 4) {
>> > + writel(*buf, ®s->txd0r);
>> > + buf++;
>> > + size -= 4;
>> > + } else {
>> > + switch (size) {
>> > + case 1:
>> > + data = *((u8 *)buf);
>> > + buf += 1;
>> > + data |= 0xFFFFFF00;
>> > + break;
>> > + case 2:
>> > + data = *((u16 *)buf);
>> > + buf += 2;
>> > + data |= 0xFFFF0000;
>> > + break;
>> > + case 3:
>> > + data = *((u16 *)buf);
>> > + buf += 2;
>> > + data |= (*((u8 *)buf) << 16);
>> > + buf += 1;
>> > + data |= 0xFF000000;
>>
>> Use GENMASK instead of hexcodes.
>
> Ok.
>>
>> > + break;
>> > + }
>> > + writel(data, ®s->txd0r);
>> > + size = 0;
>> > + }
>> > + }
>> > +
>> > + priv->tx_buf += len;
>> > + return 0;
>> > +}
>> > +
>> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv *priv) {
>> > + u8 command = 1;
>> > + u32 gen_fifo_cmd;
>> > + u32 bytecount = 0;
>> > +
>> > + while (priv->len) {
>> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> > +
>> > + if (command) {
>> > + command = 0;
>> > + last_cmd = *(u8 *)priv->tx_buf;
>> > + }
>>
>> don't understand this code can you explain? command assigned 1 it will not
>> updated anywhere?
>
> I want to store last command sent. As the first byte in loop only contains command, it ensures it fills only for one time
> and next time it may contain data to be sent along with command.
> Command initialized to 1 while declaring it above(u8 command = 1).
Ok the first TX buf has command and reused in tx and rx fifo, how come
to use use same in rx fifo? why is is last_cmd it is first_cmd?
>
>>
>> > +
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
>> > + gen_fifo_cmd |= *(u8 *)priv->tx_buf;
>> > + bytecount++;
>> > + priv->len--;
>> > + priv->tx_buf = (u8 *)priv->tx_buf + 1;
>> > +
>> > + debug("GFIFO_CMD_Cmd = 0x%x\n", gen_fifo_cmd);
>> > +
>> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
>> > + }
>> > +}
>> > +
>> > +static u32 zynqmp_qspi_calc_exp(struct zynqmp_qspi_priv *priv,
>> > + u32 *gen_fifo_cmd) {
>> > + u32 expval = 8;
>> > + u32 len;
>> > +
>> > + while (1) {
>> > + if (priv->len > 255) {
>>
>> what is 255 here?
> When length is greater than 2^8 then we should fill length in different way, hence this check.
> I will anyway use macro for better readability.
make sure to write proper comments on why?
>
>>
>> > + if (priv->len & (1 << expval)) {
>> > + *gen_fifo_cmd &= ~GQSPI_GFIFO_IMD_MASK;
>> > + *gen_fifo_cmd |= GQSPI_GFIFO_EXP_MASK;
>> > + *gen_fifo_cmd |= expval;
>> > + priv->len -= (1 << expval);
>> > + return expval;
>> > + }
>> > + expval++;
>> > + } else {
>> > + *gen_fifo_cmd &= ~(GQSPI_GFIFO_IMD_MASK |
>> > + GQSPI_GFIFO_EXP_MASK);
>> > + *gen_fifo_cmd |= (u8)priv->len;
>> > + len = (u8)priv->len;
>> > + priv->len = 0;
>> > + return len;
>> > + }
>> > + }
>> > +}
>> > +
>> > +static int zynqmp_qspi_genfifo_fill_tx(struct zynqmp_qspi_priv *priv)
>> > +{
>> > + u32 gen_fifo_cmd;
>> > + u32 len;
>> > + int ret = 0;
>> > +
>> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> > + gen_fifo_cmd |= GQSPI_GFIFO_TX |
>> > + GQSPI_GFIFO_DATA_XFR_MASK;
>> > +
>> > + if (last_cmd == QUAD_PAGE_PROGRAM_CMD)
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
>> > + else
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
>> > +
>> > + while (priv->len) {
>> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
>> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
>> > +
>> > + debug("GFIFO_CMD_TX:0x%x\n", gen_fifo_cmd);
>> > +
>> > + if (gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK)
>> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
>> > + 1 << len);
>> > + else
>> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
>> > + len);
>> > +
>> > + if (ret)
>> > + return ret;
>> > + }
>> > + return ret;
>> > +}
>> > +
>> > +static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv,
>> > + u32 gen_fifo_cmd, u32 *buf) {
>> > + u32 addr;
>> > + u32 size, len;
>> > + u32 actuallen = priv->len;
>> > + int ret = 0;
>> > + struct zynqmp_qspi_dma_regs *dma_regs = priv->dma_regs;
>> > +
>> > + writel((unsigned long)buf, &dma_regs->dmadst);
>> > + writel(roundup(priv->len, 4), &dma_regs->dmasize);
>> > + writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier);
>> > + addr = (unsigned long)buf;
>> > + size = roundup(priv->len, ARCH_DMA_MINALIGN);
>> > + flush_dcache_range(addr, addr + size);
>> > +
>> > + while (priv->len) {
>> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
>> > + if (!(gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK) &&
>> > + (len % 4)) {
>> > + gen_fifo_cmd &= ~(0xFF);
>> > + gen_fifo_cmd |= (len / 4 + 1) * 4;
>>
>> Need macros for these numerics
> Ok
>>
>> > + }
>> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
>> > +
>> > + debug("GFIFO_CMD_RX:0x%x\n", gen_fifo_cmd);
>> > + }
>> > +
>> > + ret = wait_for_bit_le32(&dma_regs->dmaisr,
>> GQSPI_DMA_DST_I_STS_DONE,
>> > + 1, GQSPI_TIMEOUT, 1);
>> > + if (ret) {
>> > + printf("DMA Timeout:0x%x\n", readl(&dma_regs->dmaisr));
>> > + return -1;
>> > + }
>> > +
>> > + writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr);
>> > +
>> > + debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n",
>> > + (unsigned long)buf, (unsigned long)priv->rx_buf, *buf,
>> > + actuallen);
>> > +
>> > + if (buf != priv->rx_buf)
>> > + memcpy(priv->rx_buf, buf, actuallen);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int zynqmp_qspi_genfifo_fill_rx(struct zynqmp_qspi_priv *priv)
>> > +{
>> > + u32 gen_fifo_cmd;
>> > + u32 *buf;
>> > + u32 actuallen = priv->len;
>> > +
>> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> > + gen_fifo_cmd |= GQSPI_GFIFO_RX |
>> > + GQSPI_GFIFO_DATA_XFR_MASK;
>> > +
>> > + if (last_cmd == QUAD_OUT_READ_CMD)
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
>> > + else if (last_cmd == DUAL_OUTPUT_FASTRD_CMD)
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_DUAL_SPI;
>> > + else
>> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
>> > +
>> > + /*
>> > + * Check if receive buffer is aligned to 4 byte and length
>> > + * is multiples of four byte as we are using dma to receive.
>> > + */
>> > + if (!((unsigned long)priv->rx_buf & (GQSPI_DMA_ALIGN - 1)) &&
>> > + !(actuallen % GQSPI_DMA_ALIGN)) {
>> > + buf = (u32 *)priv->rx_buf;
>> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
>> > + }
>> > +
>> > + ALLOC_CACHE_ALIGN_BUFFER(u8, tmp, roundup(priv->len,
>> > + GQSPI_DMA_ALIGN));
>> > + buf = (u32 *)tmp;
>> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf); }
>> > +
>> > +static int zynqmp_qspi_start_transfer(struct zynqmp_qspi_priv *priv)
>> > +{
>> > + int ret = 0;
>> > +
>> > + if (priv->is_inst) {
>> > + if (priv->tx_buf)
>> > + zynqmp_qspi_genfifo_cmd(priv);
>> > + else
>> > + ret = -1;
>> > + } else {
>> > + if (priv->tx_buf)
>> > + ret = zynqmp_qspi_genfifo_fill_tx(priv);
>> > + else if (priv->rx_buf)
>> > + ret = zynqmp_qspi_genfifo_fill_rx(priv);
>> > + else
>> > + ret = -1;
>> > + }
>> > + return ret;
>> > +}
>> > +
>> > +static int zynqmp_qspi_transfer(struct zynqmp_qspi_priv *priv) {
>> > + static unsigned int cs_change = 1;
>> > + int status = 0;
>> > +
>> > + debug("%s\n", __func__);
>> > +
>> > + while (1) {
>> > + /* Select the chip if required */
>> > + if (cs_change)
>> > + zynqmp_qspi_chipselect(priv, 1);
>> > +
>> > + cs_change = priv->cs_change;
>> > +
>> > + if (!priv->tx_buf && !priv->rx_buf && priv->len) {
>> > + status = -1;
>> > + break;
>> > + }
>> > +
>> > + /* Request the transfer */
>> > + if (priv->len) {
>> > + status = zynqmp_qspi_start_transfer(priv);
>> > + priv->is_inst = 0;
>> > + if (status < 0)
>> > + break;
>> > + }
>> > +
>> > + if (cs_change)
>> > + /* Deselect the chip */
>> > + zynqmp_qspi_chipselect(priv, 0);
>> > + break;
>> > + }
>> > +
>> > + return status;
>> > +}
>> > +
>> > +static int zynqmp_qspi_claim_bus(struct udevice *dev) {
>> > + struct udevice *bus = dev->parent;
>> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
>> > + struct zynqmp_qspi_regs *regs = priv->regs;
>> > +
>> > + debug("%s\n", __func__);
>>
>> drop this.
> I didn’t see any problem in having, its just for debugging.
> If you don’t like I can remove.
Yes drop it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 7:41 ` Jagan Teki
@ 2018-05-09 7:50 ` Siva Durga Prasad Paladugu
2018-05-09 7:52 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-09 7:50 UTC (permalink / raw)
To: u-boot
Hi,
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, May 09, 2018 1:12 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> boot at lists.denx.de>; michal.simek at xilinx.com
> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> ZynqMP qspi driver
>
> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
> <sivadur@xilinx.com> wrote:
> > Hi Jagan,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
> >> Sent: Wednesday, May 09, 2018 12:01 PM
> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> ZynqMP
> >> qspi driver
> >>
> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
> >> <siva.durga.paladugu@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@xilinx.com>
> >> > ---
> >> > 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
> >> > ---
> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
> >> > drivers/spi/Kconfig | 7 +
> >> > drivers/spi/Makefile | 1 +
> >> > drivers/spi/zynqmp_gqspi.c | 670
> >> ++++++++++++++++++++++++
> >> > 4 files changed, 832 insertions(+) create mode 100644
> >> > arch/arm/include/asm/arch-
> >> zynqmp/zynqmp_gqspi.h
> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
> >> >
> >> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >>
> >> already asked you to move this header code in driver .c file
> >
> > You might have missed my reply to your earlier comment on this. These
> > were moved to .h based on comment from Lukasz in v1.
> > I don’t have any issue in having them anywhere. Let me know your choice.
>
> I'm trying to align Linux code, better to move like that and make sure to use
> similar macros.
>
> >
[snip]
> >> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv
> *priv) {
> >> > + u8 command = 1;
> >> > + u32 gen_fifo_cmd;
> >> > + u32 bytecount = 0;
> >> > +
> >> > + while (priv->len) {
> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >> > +
> >> > + if (command) {
> >> > + command = 0;
> >> > + last_cmd = *(u8 *)priv->tx_buf;
> >> > + }
> >>
> >> don't understand this code can you explain? command assigned 1 it
> >> will not updated anywhere?
> >
> > I want to store last command sent. As the first byte in loop only
> > contains command, it ensures it fills only for one time and next time it
> may contain data to be sent along with command.
> > Command initialized to 1 while declaring it above(u8 command = 1).
>
> Ok the first TX buf has command and reused in tx and rx fifo, how come to
> use use same in rx fifo? why is is last_cmd it is first_cmd?
This holds the command that was sent last to the device before we use in tx/rx fill, hence used this name.
Thanks,
Siva
>
>
> >
> >>
> >> > +
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> >> > + gen_fifo_cmd |= *(u8 *)priv->tx_buf;
> >> > + bytecount++;
> >> > + priv->len--;
> >> > + priv->tx_buf = (u8 *)priv->tx_buf + 1;
> >> > +
> >> > + debug("GFIFO_CMD_Cmd = 0x%x\n", gen_fifo_cmd);
> >> > +
> >> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> >> > + }
> >> > +}
> >> > +
> >> > +static u32 zynqmp_qspi_calc_exp(struct zynqmp_qspi_priv *priv,
> >> > + u32 *gen_fifo_cmd) {
> >> > + u32 expval = 8;
> >> > + u32 len;
> >> > +
> >> > + while (1) {
> >> > + if (priv->len > 255) {
> >>
> >> what is 255 here?
> > When length is greater than 2^8 then we should fill length in different
> way, hence this check.
> > I will anyway use macro for better readability.
>
> make sure to write proper comments on why?
>
> >
> >>
> >> > + if (priv->len & (1 << expval)) {
> >> > + *gen_fifo_cmd &= ~GQSPI_GFIFO_IMD_MASK;
> >> > + *gen_fifo_cmd |= GQSPI_GFIFO_EXP_MASK;
> >> > + *gen_fifo_cmd |= expval;
> >> > + priv->len -= (1 << expval);
> >> > + return expval;
> >> > + }
> >> > + expval++;
> >> > + } else {
> >> > + *gen_fifo_cmd &= ~(GQSPI_GFIFO_IMD_MASK |
> >> > + GQSPI_GFIFO_EXP_MASK);
> >> > + *gen_fifo_cmd |= (u8)priv->len;
> >> > + len = (u8)priv->len;
> >> > + priv->len = 0;
> >> > + return len;
> >> > + }
> >> > + }
> >> > +}
> >> > +
> >> > +static int zynqmp_qspi_genfifo_fill_tx(struct zynqmp_qspi_priv
> >> > +*priv) {
> >> > + u32 gen_fifo_cmd;
> >> > + u32 len;
> >> > + int ret = 0;
> >> > +
> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX |
> >> > + GQSPI_GFIFO_DATA_XFR_MASK;
> >> > +
> >> > + if (last_cmd == QUAD_PAGE_PROGRAM_CMD)
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> >> > + else
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> >> > +
> >> > + while (priv->len) {
> >> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> >> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> >> > +
> >> > + debug("GFIFO_CMD_TX:0x%x\n", gen_fifo_cmd);
> >> > +
> >> > + if (gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK)
> >> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
> >> > + 1 << len);
> >> > + else
> >> > + ret = zynqmp_qspi_fill_tx_fifo(priv,
> >> > + len);
> >> > +
> >> > + if (ret)
> >> > + return ret;
> >> > + }
> >> > + return ret;
> >> > +}
> >> > +
> >> > +static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv,
> >> > + u32 gen_fifo_cmd, u32 *buf) {
> >> > + u32 addr;
> >> > + u32 size, len;
> >> > + u32 actuallen = priv->len;
> >> > + int ret = 0;
> >> > + struct zynqmp_qspi_dma_regs *dma_regs = priv->dma_regs;
> >> > +
> >> > + writel((unsigned long)buf, &dma_regs->dmadst);
> >> > + writel(roundup(priv->len, 4), &dma_regs->dmasize);
> >> > + writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier);
> >> > + addr = (unsigned long)buf;
> >> > + size = roundup(priv->len, ARCH_DMA_MINALIGN);
> >> > + flush_dcache_range(addr, addr + size);
> >> > +
> >> > + while (priv->len) {
> >> > + len = zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd);
> >> > + if (!(gen_fifo_cmd & GQSPI_GFIFO_EXP_MASK) &&
> >> > + (len % 4)) {
> >> > + gen_fifo_cmd &= ~(0xFF);
> >> > + gen_fifo_cmd |= (len / 4 + 1) * 4;
> >>
> >> Need macros for these numerics
> > Ok
> >>
> >> > + }
> >> > + zynqmp_qspi_fill_gen_fifo(priv, gen_fifo_cmd);
> >> > +
> >> > + debug("GFIFO_CMD_RX:0x%x\n", gen_fifo_cmd);
> >> > + }
> >> > +
> >> > + ret = wait_for_bit_le32(&dma_regs->dmaisr,
> >> GQSPI_DMA_DST_I_STS_DONE,
> >> > + 1, GQSPI_TIMEOUT, 1);
> >> > + if (ret) {
> >> > + printf("DMA Timeout:0x%x\n", readl(&dma_regs->dmaisr));
> >> > + return -1;
> >> > + }
> >> > +
> >> > + writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr);
> >> > +
> >> > + debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n",
> >> > + (unsigned long)buf, (unsigned long)priv->rx_buf, *buf,
> >> > + actuallen);
> >> > +
> >> > + if (buf != priv->rx_buf)
> >> > + memcpy(priv->rx_buf, buf, actuallen);
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static int zynqmp_qspi_genfifo_fill_rx(struct zynqmp_qspi_priv
> >> > +*priv) {
> >> > + u32 gen_fifo_cmd;
> >> > + u32 *buf;
> >> > + u32 actuallen = priv->len;
> >> > +
> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> > + gen_fifo_cmd |= GQSPI_GFIFO_RX |
> >> > + GQSPI_GFIFO_DATA_XFR_MASK;
> >> > +
> >> > + if (last_cmd == QUAD_OUT_READ_CMD)
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_QSPI;
> >> > + else if (last_cmd == DUAL_OUTPUT_FASTRD_CMD)
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_DUAL_SPI;
> >> > + else
> >> > + gen_fifo_cmd |= GQSPI_SPI_MODE_SPI;
> >> > +
> >> > + /*
> >> > + * Check if receive buffer is aligned to 4 byte and length
> >> > + * is multiples of four byte as we are using dma to receive.
> >> > + */
> >> > + if (!((unsigned long)priv->rx_buf & (GQSPI_DMA_ALIGN - 1)) &&
> >> > + !(actuallen % GQSPI_DMA_ALIGN)) {
> >> > + buf = (u32 *)priv->rx_buf;
> >> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf);
> >> > + }
> >> > +
> >> > + ALLOC_CACHE_ALIGN_BUFFER(u8, tmp, roundup(priv->len,
> >> > + GQSPI_DMA_ALIGN));
> >> > + buf = (u32 *)tmp;
> >> > + return zynqmp_qspi_start_dma(priv, gen_fifo_cmd, buf); }
> >> > +
> >> > +static int zynqmp_qspi_start_transfer(struct zynqmp_qspi_priv
> >> > +*priv) {
> >> > + int ret = 0;
> >> > +
> >> > + if (priv->is_inst) {
> >> > + if (priv->tx_buf)
> >> > + zynqmp_qspi_genfifo_cmd(priv);
> >> > + else
> >> > + ret = -1;
> >> > + } else {
> >> > + if (priv->tx_buf)
> >> > + ret = zynqmp_qspi_genfifo_fill_tx(priv);
> >> > + else if (priv->rx_buf)
> >> > + ret = zynqmp_qspi_genfifo_fill_rx(priv);
> >> > + else
> >> > + ret = -1;
> >> > + }
> >> > + return ret;
> >> > +}
> >> > +
> >> > +static int zynqmp_qspi_transfer(struct zynqmp_qspi_priv *priv) {
> >> > + static unsigned int cs_change = 1;
> >> > + int status = 0;
> >> > +
> >> > + debug("%s\n", __func__);
> >> > +
> >> > + while (1) {
> >> > + /* Select the chip if required */
> >> > + if (cs_change)
> >> > + zynqmp_qspi_chipselect(priv, 1);
> >> > +
> >> > + cs_change = priv->cs_change;
> >> > +
> >> > + if (!priv->tx_buf && !priv->rx_buf && priv->len) {
> >> > + status = -1;
> >> > + break;
> >> > + }
> >> > +
> >> > + /* Request the transfer */
> >> > + if (priv->len) {
> >> > + status = zynqmp_qspi_start_transfer(priv);
> >> > + priv->is_inst = 0;
> >> > + if (status < 0)
> >> > + break;
> >> > + }
> >> > +
> >> > + if (cs_change)
> >> > + /* Deselect the chip */
> >> > + zynqmp_qspi_chipselect(priv, 0);
> >> > + break;
> >> > + }
> >> > +
> >> > + return status;
> >> > +}
> >> > +
> >> > +static int zynqmp_qspi_claim_bus(struct udevice *dev) {
> >> > + struct udevice *bus = dev->parent;
> >> > + struct zynqmp_qspi_priv *priv = dev_get_priv(bus);
> >> > + struct zynqmp_qspi_regs *regs = priv->regs;
> >> > +
> >> > + debug("%s\n", __func__);
> >>
> >> drop this.
> > I didn’t see any problem in having, its just for debugging.
> > If you don’t like I can remove.
>
> Yes drop it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 7:50 ` Siva Durga Prasad Paladugu
@ 2018-05-09 7:52 ` Jagan Teki
2018-05-09 8:01 ` Siva Durga Prasad Paladugu
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 7:52 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
<sivadur@xilinx.com> wrote:
>
> Hi,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Wednesday, May 09, 2018 1:12 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> boot at lists.denx.de>; michal.simek at xilinx.com
>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP qspi driver
>>
>> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
>> <sivadur@xilinx.com> wrote:
>> > Hi Jagan,
>> >
>> >> -----Original Message-----
>> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> >> Sent: Wednesday, May 09, 2018 12:01 PM
>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
>> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP
>> >> qspi driver
>> >>
>> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>> >> <siva.durga.paladugu@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@xilinx.com>
>> >> > ---
>> >> > 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
>> >> > ---
>> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154 ++++++
>> >> > drivers/spi/Kconfig | 7 +
>> >> > drivers/spi/Makefile | 1 +
>> >> > drivers/spi/zynqmp_gqspi.c | 670
>> >> ++++++++++++++++++++++++
>> >> > 4 files changed, 832 insertions(+) create mode 100644
>> >> > arch/arm/include/asm/arch-
>> >> zynqmp/zynqmp_gqspi.h
>> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >> >
>> >> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >>
>> >> already asked you to move this header code in driver .c file
>> >
>> > You might have missed my reply to your earlier comment on this. These
>> > were moved to .h based on comment from Lukasz in v1.
>> > I don’t have any issue in having them anywhere. Let me know your choice.
>>
>> I'm trying to align Linux code, better to move like that and make sure to use
>> similar macros.
>>
>> >
>
> [snip]
>
>> >> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv
>> *priv) {
>> >> > + u8 command = 1;
>> >> > + u32 gen_fifo_cmd;
>> >> > + u32 bytecount = 0;
>> >> > +
>> >> > + while (priv->len) {
>> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >> > +
>> >> > + if (command) {
>> >> > + command = 0;
>> >> > + last_cmd = *(u8 *)priv->tx_buf;
>> >> > + }
>> >>
>> >> don't understand this code can you explain? command assigned 1 it
>> >> will not updated anywhere?
>> >
>> > I want to store last command sent. As the first byte in loop only
>> > contains command, it ensures it fills only for one time and next time it
>> may contain data to be sent along with command.
>> > Command initialized to 1 while declaring it above(u8 command = 1).
>>
>> Ok the first TX buf has command and reused in tx and rx fifo, how come to
>> use use same in rx fifo? why is is last_cmd it is first_cmd?
>
> This holds the command that was sent last to the device before we use in tx/rx fill, hence used this name.
ie. what I wonder, usually the spi transfer start with cmd + data in
that case it would be the first command, did gqspi work reverse way?
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 7:52 ` Jagan Teki
@ 2018-05-09 8:01 ` Siva Durga Prasad Paladugu
2018-05-09 10:38 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-09 8:01 UTC (permalink / raw)
To: u-boot
Hi Jagan,
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, May 09, 2018 1:22 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> boot at lists.denx.de>; michal.simek at xilinx.com
> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> ZynqMP qspi driver
>
> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
> <sivadur@xilinx.com> wrote:
> >
> > Hi,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >> Sent: Wednesday, May 09, 2018 1:12 PM
> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >> boot at lists.denx.de>; michal.simek at xilinx.com
> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
> >> for ZynqMP qspi driver
> >>
> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
> >> <sivadur@xilinx.com> wrote:
> >> > Hi Jagan,
> >> >
> >> >> -----Original Message-----
> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> >> ZynqMP
> >> >> qspi driver
> >> >>
> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
> >> >> <siva.durga.paladugu@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@xilinx.com>
> >> >> > ---
> >> >> > 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
> >> >> > ---
> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
> ++++++
> >> >> > drivers/spi/Kconfig | 7 +
> >> >> > drivers/spi/Makefile | 1 +
> >> >> > drivers/spi/zynqmp_gqspi.c | 670
> >> >> ++++++++++++++++++++++++
> >> >> > 4 files changed, 832 insertions(+) create mode 100644
> >> >> > arch/arm/include/asm/arch-
> >> >> zynqmp/zynqmp_gqspi.h
> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
> >> >> >
> >> >> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >> >>
> >> >> already asked you to move this header code in driver .c file
> >> >
> >> > You might have missed my reply to your earlier comment on this.
> >> > These were moved to .h based on comment from Lukasz in v1.
> >> > I don’t have any issue in having them anywhere. Let me know your
> choice.
> >>
> >> I'm trying to align Linux code, better to move like that and make
> >> sure to use similar macros.
> >>
> >> >
> >
> > [snip]
> >
> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv
> >> *priv) {
> >> >> > + u8 command = 1;
> >> >> > + u32 gen_fifo_cmd;
> >> >> > + u32 bytecount = 0;
> >> >> > +
> >> >> > + while (priv->len) {
> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >> >> > +
> >> >> > + if (command) {
> >> >> > + command = 0;
> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
> >> >> > + }
> >> >>
> >> >> don't understand this code can you explain? command assigned 1 it
> >> >> will not updated anywhere?
> >> >
> >> > I want to store last command sent. As the first byte in loop only
> >> > contains command, it ensures it fills only for one time and next
> >> > time it
> >> may contain data to be sent along with command.
> >> > Command initialized to 1 while declaring it above(u8 command = 1).
> >>
> >> Ok the first TX buf has command and reused in tx and rx fifo, how
> >> come to use use same in rx fifo? why is is last_cmd it is first_cmd?
> >
> > This holds the command that was sent last to the device before we use in
> tx/rx fill, hence used this name.
>
> ie. what I wonder, usually the spi transfer start with cmd + data in that case
> it would be the first command, did gqspi work reverse way?
It's also same as others :-), the last_cmd holds the recent command that was sent to the device.
Its just name. To avoid confusion, I can use other names like "cmd_sent or cmd_recent". Hope this is ok for you or suggest which
one would be appropriate.
Thanks,
Siva
>
> Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 8:01 ` Siva Durga Prasad Paladugu
@ 2018-05-09 10:38 ` Jagan Teki
2018-05-09 10:48 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 10:38 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
<sivadur@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Wednesday, May 09, 2018 1:22 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> boot at lists.denx.de>; michal.simek at xilinx.com
>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP qspi driver
>>
>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
>> <sivadur@xilinx.com> wrote:
>> >
>> > Hi,
>> >
>> >> -----Original Message-----
>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >> Sent: Wednesday, May 09, 2018 1:12 PM
>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >> boot at lists.denx.de>; michal.simek at xilinx.com
>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
>> >> for ZynqMP qspi driver
>> >>
>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
>> >> <sivadur@xilinx.com> wrote:
>> >> > Hi Jagan,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> >> ZynqMP
>> >> >> qspi driver
>> >> >>
>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>> >> >> <siva.durga.paladugu@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@xilinx.com>
>> >> >> > ---
>> >> >> > 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
>> >> >> > ---
>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
>> ++++++
>> >> >> > drivers/spi/Kconfig | 7 +
>> >> >> > drivers/spi/Makefile | 1 +
>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
>> >> >> ++++++++++++++++++++++++
>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
>> >> >> > arch/arm/include/asm/arch-
>> >> >> zynqmp/zynqmp_gqspi.h
>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >> >> >
>> >> >> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >> >>
>> >> >> already asked you to move this header code in driver .c file
>> >> >
>> >> > You might have missed my reply to your earlier comment on this.
>> >> > These were moved to .h based on comment from Lukasz in v1.
>> >> > I don’t have any issue in having them anywhere. Let me know your
>> choice.
>> >>
>> >> I'm trying to align Linux code, better to move like that and make
>> >> sure to use similar macros.
>> >>
>> >> >
>> >
>> > [snip]
>> >
>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv
>> >> *priv) {
>> >> >> > + u8 command = 1;
>> >> >> > + u32 gen_fifo_cmd;
>> >> >> > + u32 bytecount = 0;
>> >> >> > +
>> >> >> > + while (priv->len) {
>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >> >> > +
>> >> >> > + if (command) {
>> >> >> > + command = 0;
>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
>> >> >> > + }
>> >> >>
>> >> >> don't understand this code can you explain? command assigned 1 it
>> >> >> will not updated anywhere?
>> >> >
>> >> > I want to store last command sent. As the first byte in loop only
>> >> > contains command, it ensures it fills only for one time and next
>> >> > time it
>> >> may contain data to be sent along with command.
>> >> > Command initialized to 1 while declaring it above(u8 command = 1).
>> >>
>> >> Ok the first TX buf has command and reused in tx and rx fifo, how
>> >> come to use use same in rx fifo? why is is last_cmd it is first_cmd?
>> >
>> > This holds the command that was sent last to the device before we use in
>> tx/rx fill, hence used this name.
>>
>> ie. what I wonder, usually the spi transfer start with cmd + data in that case
>> it would be the first command, did gqspi work reverse way?
>
> It's also same as others :-), the last_cmd holds the recent command that was sent to the device.
> Its just name. To avoid confusion, I can use other names like "cmd_sent or cmd_recent". Hope this is ok for you or suggest which
> one would be appropriate.
Let's park the naming aside.
u8 command = 0;
while (priv->len) {
gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
gen_fifo_cmd |= GQSPI_GFIFO_TX;
if (command) {
command = 0;
last_cmd = *(u8 *)priv->tx_buf;
}
......
priv->len--;
}
Why the last_cmd assignment is in loop? say priv->len has non-zero the
command is 1 last_cmd assigned and command reassigned to 0 there is no
way execute the if for next while isn't it?
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 10:38 ` Jagan Teki
@ 2018-05-09 10:48 ` Jagan Teki
2018-05-09 11:14 ` Siva Durga Prasad Paladugu
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 10:48 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 4:08 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
> <sivadur@xilinx.com> wrote:
>> Hi Jagan,
>>
>>> -----Original Message-----
>>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>> Sent: Wednesday, May 09, 2018 1:22 PM
>>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>>> boot at lists.denx.de>; michal.simek at xilinx.com
>>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>>> ZynqMP qspi driver
>>>
>>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
>>> <sivadur@xilinx.com> wrote:
>>> >
>>> > Hi,
>>> >
>>> >> -----Original Message-----
>>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>> >> Sent: Wednesday, May 09, 2018 1:12 PM
>>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>>> >> boot at lists.denx.de>; michal.simek at xilinx.com
>>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
>>> >> for ZynqMP qspi driver
>>> >>
>>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
>>> >> <sivadur@xilinx.com> wrote:
>>> >> > Hi Jagan,
>>> >> >
>>> >> >> -----Original Message-----
>>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
>>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.simek at xilinx.com
>>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>>> >> ZynqMP
>>> >> >> qspi driver
>>> >> >>
>>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>>> >> >> <siva.durga.paladugu@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@xilinx.com>
>>> >> >> > ---
>>> >> >> > 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
>>> >> >> > ---
>>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
>>> ++++++
>>> >> >> > drivers/spi/Kconfig | 7 +
>>> >> >> > drivers/spi/Makefile | 1 +
>>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
>>> >> >> ++++++++++++++++++++++++
>>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
>>> >> >> > arch/arm/include/asm/arch-
>>> >> >> zynqmp/zynqmp_gqspi.h
>>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>>> >> >> >
>>> >> >> > diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>>> >> >>
>>> >> >> already asked you to move this header code in driver .c file
>>> >> >
>>> >> > You might have missed my reply to your earlier comment on this.
>>> >> > These were moved to .h based on comment from Lukasz in v1.
>>> >> > I don’t have any issue in having them anywhere. Let me know your
>>> choice.
>>> >>
>>> >> I'm trying to align Linux code, better to move like that and make
>>> >> sure to use similar macros.
>>> >>
>>> >> >
>>> >
>>> > [snip]
>>> >
>>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct zynqmp_qspi_priv
>>> >> *priv) {
>>> >> >> > + u8 command = 1;
>>> >> >> > + u32 gen_fifo_cmd;
>>> >> >> > + u32 bytecount = 0;
>>> >> >> > +
>>> >> >> > + while (priv->len) {
>>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>>> >> >> > +
>>> >> >> > + if (command) {
>>> >> >> > + command = 0;
>>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
>>> >> >> > + }
>>> >> >>
>>> >> >> don't understand this code can you explain? command assigned 1 it
>>> >> >> will not updated anywhere?
>>> >> >
>>> >> > I want to store last command sent. As the first byte in loop only
>>> >> > contains command, it ensures it fills only for one time and next
>>> >> > time it
>>> >> may contain data to be sent along with command.
>>> >> > Command initialized to 1 while declaring it above(u8 command = 1).
>>> >>
>>> >> Ok the first TX buf has command and reused in tx and rx fifo, how
>>> >> come to use use same in rx fifo? why is is last_cmd it is first_cmd?
>>> >
>>> > This holds the command that was sent last to the device before we use in
>>> tx/rx fill, hence used this name.
>>>
>>> ie. what I wonder, usually the spi transfer start with cmd + data in that case
>>> it would be the first command, did gqspi work reverse way?
>>
>> It's also same as others :-), the last_cmd holds the recent command that was sent to the device.
>> Its just name. To avoid confusion, I can use other names like "cmd_sent or cmd_recent". Hope this is ok for you or suggest which
>> one would be appropriate.
>
> Let's park the naming aside.
>
> u8 command = 0;
>
> while (priv->len) {
> gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> gen_fifo_cmd |= GQSPI_GFIFO_TX;
>
> if (command) {
> command = 0;
> last_cmd = *(u8 *)priv->tx_buf;
> }
> ......
> priv->len--;
> }
>
> Why the last_cmd assignment is in loop? say priv->len has non-zero the
> command is 1 last_cmd assigned and command reassigned to 0 there is no
> way execute the if for next while isn't it?
The main issue with this driver, which I told/commented several times
about using flash specific stuff.
+
+#define QUAD_OUT_READ_CMD 0x6B
+#define QUAD_PAGE_PROGRAM_CMD 0x32
+#define DUAL_OUTPUT_FASTRD_CMD 0x3B
+
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 10:48 ` Jagan Teki
@ 2018-05-09 11:14 ` Siva Durga Prasad Paladugu
2018-05-09 11:16 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-09 11:14 UTC (permalink / raw)
To: u-boot
Hi Jagan,
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, May 09, 2018 4:18 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> boot at lists.denx.de>; michal.simek at xilinx.com
> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> ZynqMP qspi driver
>
> On Wed, May 9, 2018 at 4:08 PM, Jagan Teki <jagannadh.teki@gmail.com>
> wrote:
> > On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
> > <sivadur@xilinx.com> wrote:
> >> Hi Jagan,
> >>
> >>> -----Original Message-----
> >>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >>> Sent: Wednesday, May 09, 2018 1:22 PM
> >>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >>> boot at lists.denx.de>; michal.simek at xilinx.com
> >>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
> >>> for ZynqMP qspi driver
> >>>
> >>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
> >>> <sivadur@xilinx.com> wrote:
> >>> >
> >>> > Hi,
> >>> >
> >>> >> -----Original Message-----
> >>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >>> >> Sent: Wednesday, May 09, 2018 1:12 PM
> >>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >>> >> boot at lists.denx.de>; michal.simek at xilinx.com
> >>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
> >>> >> support for ZynqMP qspi driver
> >>> >>
> >>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
> >>> >> <sivadur@xilinx.com> wrote:
> >>> >> > Hi Jagan,
> >>> >> >
> >>> >> >> -----Original Message-----
> >>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
> >>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
> >>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>;
> >>> >> >> michal.simek at xilinx.com
> >>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> >>> >> ZynqMP
> >>> >> >> qspi driver
> >>> >> >>
> >>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
> >>> >> >> <siva.durga.paladugu@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@xilinx.com>
> >>> >> >> > ---
> >>> >> >> > 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
> >>> >> >> > ---
> >>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
> >>> ++++++
> >>> >> >> > drivers/spi/Kconfig | 7 +
> >>> >> >> > drivers/spi/Makefile | 1 +
> >>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
> >>> >> >> ++++++++++++++++++++++++
> >>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
> >>> >> >> > arch/arm/include/asm/arch-
> >>> >> >> zynqmp/zynqmp_gqspi.h
> >>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
> >>> >> >> >
> >>> >> >> > diff --git a/arch/arm/include/asm/arch-
> zynqmp/zynqmp_gqspi.h
> >>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >>> >> >>
> >>> >> >> already asked you to move this header code in driver .c file
> >>> >> >
> >>> >> > You might have missed my reply to your earlier comment on this.
> >>> >> > These were moved to .h based on comment from Lukasz in v1.
> >>> >> > I don’t have any issue in having them anywhere. Let me know
> >>> >> > your
> >>> choice.
> >>> >>
> >>> >> I'm trying to align Linux code, better to move like that and make
> >>> >> sure to use similar macros.
> >>> >>
> >>> >> >
> >>> >
> >>> > [snip]
> >>> >
> >>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct
> zynqmp_qspi_priv
> >>> >> *priv) {
> >>> >> >> > + u8 command = 1;
> >>> >> >> > + u32 gen_fifo_cmd;
> >>> >> >> > + u32 bytecount = 0;
> >>> >> >> > +
> >>> >> >> > + while (priv->len) {
> >>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >>> >> >> > +
> >>> >> >> > + if (command) {
> >>> >> >> > + command = 0;
> >>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
> >>> >> >> > + }
> >>> >> >>
> >>> >> >> don't understand this code can you explain? command assigned
> 1
> >>> >> >> it will not updated anywhere?
> >>> >> >
> >>> >> > I want to store last command sent. As the first byte in loop
> >>> >> > only contains command, it ensures it fills only for one time
> >>> >> > and next time it
> >>> >> may contain data to be sent along with command.
> >>> >> > Command initialized to 1 while declaring it above(u8 command =
> 1).
> >>> >>
> >>> >> Ok the first TX buf has command and reused in tx and rx fifo, how
> >>> >> come to use use same in rx fifo? why is is last_cmd it is first_cmd?
> >>> >
> >>> > This holds the command that was sent last to the device before we
> >>> > use in
> >>> tx/rx fill, hence used this name.
> >>>
> >>> ie. what I wonder, usually the spi transfer start with cmd + data in
> >>> that case it would be the first command, did gqspi work reverse way?
> >>
> >> It's also same as others :-), the last_cmd holds the recent command that
> was sent to the device.
> >> Its just name. To avoid confusion, I can use other names like
> >> "cmd_sent or cmd_recent". Hope this is ok for you or suggest which one
> would be appropriate.
> >
> > Let's park the naming aside.
> >
> > u8 command = 0;
> >
> > while (priv->len) {
> > gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> > gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >
> > if (command) {
> > command = 0;
> > last_cmd = *(u8 *)priv->tx_buf;
> > }
> > ......
> > priv->len--;
> > }
> >
> > Why the last_cmd assignment is in loop? say priv->len has non-zero the
> > command is 1 last_cmd assigned and command reassigned to 0 there is
> no
> > way execute the if for next while isn't it?
>
> The main issue with this driver, which I told/commented several times
> about using flash specific stuff.
>
> +
> +#define QUAD_OUT_READ_CMD 0x6B
> +#define QUAD_PAGE_PROGRAM_CMD 0x32
> +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
> +
Yeah I know, that’s where our discussion stopped last time and would like to conclude using this series.
https://patchwork.ozlabs.org/patch/829856/
We discussed this sometime back and I mentioned that this controller is not just targeted for spi-nor
but even though none tested/used with devices other than spi-nor AFAIK.
Thanks,
Siva
>
> Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 11:14 ` Siva Durga Prasad Paladugu
@ 2018-05-09 11:16 ` Jagan Teki
2018-05-09 11:52 ` Siva Durga Prasad Paladugu
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 11:16 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 4:44 PM, Siva Durga Prasad Paladugu
<sivadur@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Wednesday, May 09, 2018 4:18 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> boot at lists.denx.de>; michal.simek at xilinx.com
>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP qspi driver
>>
>> On Wed, May 9, 2018 at 4:08 PM, Jagan Teki <jagannadh.teki@gmail.com>
>> wrote:
>> > On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
>> > <sivadur@xilinx.com> wrote:
>> >> Hi Jagan,
>> >>
>> >>> -----Original Message-----
>> >>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >>> Sent: Wednesday, May 09, 2018 1:22 PM
>> >>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >>> boot at lists.denx.de>; michal.simek at xilinx.com
>> >>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
>> >>> for ZynqMP qspi driver
>> >>>
>> >>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
>> >>> <sivadur@xilinx.com> wrote:
>> >>> >
>> >>> > Hi,
>> >>> >
>> >>> >> -----Original Message-----
>> >>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >>> >> Sent: Wednesday, May 09, 2018 1:12 PM
>> >>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >>> >> boot at lists.denx.de>; michal.simek at xilinx.com
>> >>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
>> >>> >> support for ZynqMP qspi driver
>> >>> >>
>> >>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
>> >>> >> <sivadur@xilinx.com> wrote:
>> >>> >> > Hi Jagan,
>> >>> >> >
>> >>> >> >> -----Original Message-----
>> >>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> >>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
>> >>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>;
>> >>> >> >> michal.simek at xilinx.com
>> >>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> >>> >> ZynqMP
>> >>> >> >> qspi driver
>> >>> >> >>
>> >>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>> >>> >> >> <siva.durga.paladugu@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@xilinx.com>
>> >>> >> >> > ---
>> >>> >> >> > 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
>> >>> >> >> > ---
>> >>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
>> >>> ++++++
>> >>> >> >> > drivers/spi/Kconfig | 7 +
>> >>> >> >> > drivers/spi/Makefile | 1 +
>> >>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
>> >>> >> >> ++++++++++++++++++++++++
>> >>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
>> >>> >> >> > arch/arm/include/asm/arch-
>> >>> >> >> zynqmp/zynqmp_gqspi.h
>> >>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >>> >> >> >
>> >>> >> >> > diff --git a/arch/arm/include/asm/arch-
>> zynqmp/zynqmp_gqspi.h
>> >>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >>> >> >>
>> >>> >> >> already asked you to move this header code in driver .c file
>> >>> >> >
>> >>> >> > You might have missed my reply to your earlier comment on this.
>> >>> >> > These were moved to .h based on comment from Lukasz in v1.
>> >>> >> > I don’t have any issue in having them anywhere. Let me know
>> >>> >> > your
>> >>> choice.
>> >>> >>
>> >>> >> I'm trying to align Linux code, better to move like that and make
>> >>> >> sure to use similar macros.
>> >>> >>
>> >>> >> >
>> >>> >
>> >>> > [snip]
>> >>> >
>> >>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct
>> zynqmp_qspi_priv
>> >>> >> *priv) {
>> >>> >> >> > + u8 command = 1;
>> >>> >> >> > + u32 gen_fifo_cmd;
>> >>> >> >> > + u32 bytecount = 0;
>> >>> >> >> > +
>> >>> >> >> > + while (priv->len) {
>> >>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> >>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >>> >> >> > +
>> >>> >> >> > + if (command) {
>> >>> >> >> > + command = 0;
>> >>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
>> >>> >> >> > + }
>> >>> >> >>
>> >>> >> >> don't understand this code can you explain? command assigned
>> 1
>> >>> >> >> it will not updated anywhere?
>> >>> >> >
>> >>> >> > I want to store last command sent. As the first byte in loop
>> >>> >> > only contains command, it ensures it fills only for one time
>> >>> >> > and next time it
>> >>> >> may contain data to be sent along with command.
>> >>> >> > Command initialized to 1 while declaring it above(u8 command =
>> 1).
>> >>> >>
>> >>> >> Ok the first TX buf has command and reused in tx and rx fifo, how
>> >>> >> come to use use same in rx fifo? why is is last_cmd it is first_cmd?
>> >>> >
>> >>> > This holds the command that was sent last to the device before we
>> >>> > use in
>> >>> tx/rx fill, hence used this name.
>> >>>
>> >>> ie. what I wonder, usually the spi transfer start with cmd + data in
>> >>> that case it would be the first command, did gqspi work reverse way?
>> >>
>> >> It's also same as others :-), the last_cmd holds the recent command that
>> was sent to the device.
>> >> Its just name. To avoid confusion, I can use other names like
>> >> "cmd_sent or cmd_recent". Hope this is ok for you or suggest which one
>> would be appropriate.
>> >
>> > Let's park the naming aside.
>> >
>> > u8 command = 0;
>> >
>> > while (priv->len) {
>> > gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> > gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >
>> > if (command) {
>> > command = 0;
>> > last_cmd = *(u8 *)priv->tx_buf;
>> > }
>> > ......
>> > priv->len--;
>> > }
>> >
>> > Why the last_cmd assignment is in loop? say priv->len has non-zero the
>> > command is 1 last_cmd assigned and command reassigned to 0 there is
>> no
>> > way execute the if for next while isn't it?
>>
>> The main issue with this driver, which I told/commented several times
>> about using flash specific stuff.
>>
>> +
>> +#define QUAD_OUT_READ_CMD 0x6B
>> +#define QUAD_PAGE_PROGRAM_CMD 0x32
>> +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
>> +
>
> Yeah I know, that’s where our discussion stopped last time and would like to conclude using this series.
> https://patchwork.ozlabs.org/patch/829856/
> We discussed this sometime back and I mentioned that this controller is not just targeted for spi-nor
> but even though none tested/used with devices other than spi-nor AFAIK.
and you agree to write driver on spi side right? but you still using
flash commands.
#define QUAD_OUT_READ_CMD 0x6B
#define QUAD_PAGE_PROGRAM_CMD 0x32
#define DUAL_OUTPUT_FASTRD_CMD 0x3B
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 11:16 ` Jagan Teki
@ 2018-05-09 11:52 ` Siva Durga Prasad Paladugu
2018-05-09 11:54 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Siva Durga Prasad Paladugu @ 2018-05-09 11:52 UTC (permalink / raw)
To: u-boot
Hi,
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, May 09, 2018 4:47 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> boot at lists.denx.de>; michal.simek at xilinx.com
> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
> ZynqMP qspi driver
>
> On Wed, May 9, 2018 at 4:44 PM, Siva Durga Prasad Paladugu
> <sivadur@xilinx.com> wrote:
> > Hi Jagan,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >> Sent: Wednesday, May 09, 2018 4:18 PM
> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >> boot at lists.denx.de>; michal.simek at xilinx.com
> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
> >> for ZynqMP qspi driver
> >>
> >> On Wed, May 9, 2018 at 4:08 PM, Jagan Teki
> <jagannadh.teki@gmail.com>
> >> wrote:
> >> > On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
> >> > <sivadur@xilinx.com> wrote:
> >> >> Hi Jagan,
> >> >>
> >> >>> -----Original Message-----
> >> >>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >> >>> Sent: Wednesday, May 09, 2018 1:22 PM
> >> >>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >> >>> boot at lists.denx.de>; michal.simek at xilinx.com
> >> >>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
> >> >>> support for ZynqMP qspi driver
> >> >>>
> >> >>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
> >> >>> <sivadur@xilinx.com> wrote:
> >> >>> >
> >> >>> > Hi,
> >> >>> >
> >> >>> >> -----Original Message-----
> >> >>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> >> >>> >> Sent: Wednesday, May 09, 2018 1:12 PM
> >> >>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
> >> >>> >> boot at lists.denx.de>; michal.simek at xilinx.com
> >> >>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
> >> >>> >> support for ZynqMP qspi driver
> >> >>> >>
> >> >>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
> >> >>> >> <sivadur@xilinx.com> wrote:
> >> >>> >> > Hi Jagan,
> >> >>> >> >
> >> >>> >> >> -----Original Message-----
> >> >>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
> >> >>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
> >> >>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>;
> >> >>> >> >> michal.simek at xilinx.com
> >> >>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
> >> >>> >> >> for
> >> >>> >> ZynqMP
> >> >>> >> >> qspi driver
> >> >>> >> >>
> >> >>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
> >> >>> >> >> <siva.durga.paladugu@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@xilinx.com>
> >> >>> >> >> > ---
> >> >>> >> >> > 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
> >> >>> >> >> > ---
> >> >>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h |
> 154
> >> >>> ++++++
> >> >>> >> >> > drivers/spi/Kconfig | 7 +
> >> >>> >> >> > drivers/spi/Makefile | 1 +
> >> >>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
> >> >>> >> >> ++++++++++++++++++++++++
> >> >>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
> >> >>> >> >> > arch/arm/include/asm/arch-
> >> >>> >> >> zynqmp/zynqmp_gqspi.h
> >> >>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
> >> >>> >> >> >
> >> >>> >> >> > diff --git a/arch/arm/include/asm/arch-
> >> zynqmp/zynqmp_gqspi.h
> >> >>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
> >> >>> >> >>
> >> >>> >> >> already asked you to move this header code in driver .c
> >> >>> >> >> file
> >> >>> >> >
> >> >>> >> > You might have missed my reply to your earlier comment on
> this.
> >> >>> >> > These were moved to .h based on comment from Lukasz in v1.
> >> >>> >> > I don’t have any issue in having them anywhere. Let me know
> >> >>> >> > your
> >> >>> choice.
> >> >>> >>
> >> >>> >> I'm trying to align Linux code, better to move like that and
> >> >>> >> make sure to use similar macros.
> >> >>> >>
> >> >>> >> >
> >> >>> >
> >> >>> > [snip]
> >> >>> >
> >> >>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct
> >> zynqmp_qspi_priv
> >> >>> >> *priv) {
> >> >>> >> >> > + u8 command = 1;
> >> >>> >> >> > + u32 gen_fifo_cmd;
> >> >>> >> >> > + u32 bytecount = 0;
> >> >>> >> >> > +
> >> >>> >> >> > + while (priv->len) {
> >> >>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> >>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >> >>> >> >> > +
> >> >>> >> >> > + if (command) {
> >> >>> >> >> > + command = 0;
> >> >>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
> >> >>> >> >> > + }
> >> >>> >> >>
> >> >>> >> >> don't understand this code can you explain? command
> >> >>> >> >> assigned
> >> 1
> >> >>> >> >> it will not updated anywhere?
> >> >>> >> >
> >> >>> >> > I want to store last command sent. As the first byte in loop
> >> >>> >> > only contains command, it ensures it fills only for one time
> >> >>> >> > and next time it
> >> >>> >> may contain data to be sent along with command.
> >> >>> >> > Command initialized to 1 while declaring it above(u8 command
> >> >>> >> > =
> >> 1).
> >> >>> >>
> >> >>> >> Ok the first TX buf has command and reused in tx and rx fifo,
> >> >>> >> how come to use use same in rx fifo? why is is last_cmd it is
> first_cmd?
> >> >>> >
> >> >>> > This holds the command that was sent last to the device before
> >> >>> > we use in
> >> >>> tx/rx fill, hence used this name.
> >> >>>
> >> >>> ie. what I wonder, usually the spi transfer start with cmd + data
> >> >>> in that case it would be the first command, did gqspi work reverse
> way?
> >> >>
> >> >> It's also same as others :-), the last_cmd holds the recent
> >> >> command that
> >> was sent to the device.
> >> >> Its just name. To avoid confusion, I can use other names like
> >> >> "cmd_sent or cmd_recent". Hope this is ok for you or suggest which
> >> >> one
> >> would be appropriate.
> >> >
> >> > Let's park the naming aside.
> >> >
> >> > u8 command = 0;
> >> >
> >> > while (priv->len) {
> >> > gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
> >> > gen_fifo_cmd |= GQSPI_GFIFO_TX;
> >> >
> >> > if (command) {
> >> > command = 0;
> >> > last_cmd = *(u8 *)priv->tx_buf;
> >> > }
> >> > ......
> >> > priv->len--;
> >> > }
> >> >
> >> > Why the last_cmd assignment is in loop? say priv->len has non-zero
> >> > the command is 1 last_cmd assigned and command reassigned to 0
> >> > there is
> >> no
> >> > way execute the if for next while isn't it?
> >>
> >> The main issue with this driver, which I told/commented several times
> >> about using flash specific stuff.
> >>
> >> +
> >> +#define QUAD_OUT_READ_CMD 0x6B
> >> +#define QUAD_PAGE_PROGRAM_CMD 0x32
> >> +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
> >> +
> >
> > Yeah I know, that’s where our discussion stopped last time and would
> like to conclude using this series.
> > https://patchwork.ozlabs.org/patch/829856/
> > We discussed this sometime back and I mentioned that this controller
> > is not just targeted for spi-nor but even though none tested/used with
> devices other than spi-nor AFAIK.
>
> and you agree to write driver on spi side right? but you still using flash
> commands.
Sorry, That’s not true, my understanding was that we agreed to move to driver/mtd/spi in future inorder to have other
controller specific functionality like dual flash, but for now and for single its ok to be here.
Anyway, let me try to address your comment of not using any flash specific stuff here in driver
at drivers/spi/ and will send next version. Hope this works for you.
Thanks,
Siva
>
> #define QUAD_OUT_READ_CMD 0x6B
> #define QUAD_PAGE_PROGRAM_CMD 0x32
> #define DUAL_OUTPUT_FASTRD_CMD 0x3B
>
> Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
2018-05-09 11:52 ` Siva Durga Prasad Paladugu
@ 2018-05-09 11:54 ` Jagan Teki
0 siblings, 0 replies; 15+ messages in thread
From: Jagan Teki @ 2018-05-09 11:54 UTC (permalink / raw)
To: u-boot
On Wed, May 9, 2018 at 5:22 PM, Siva Durga Prasad Paladugu
<sivadur@xilinx.com> wrote:
> Hi,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Wednesday, May 09, 2018 4:47 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> boot at lists.denx.de>; michal.simek at xilinx.com
>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for
>> ZynqMP qspi driver
>>
>> On Wed, May 9, 2018 at 4:44 PM, Siva Durga Prasad Paladugu
>> <sivadur@xilinx.com> wrote:
>> > Hi Jagan,
>> >
>> >> -----Original Message-----
>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >> Sent: Wednesday, May 09, 2018 4:18 PM
>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >> boot at lists.denx.de>; michal.simek at xilinx.com
>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
>> >> for ZynqMP qspi driver
>> >>
>> >> On Wed, May 9, 2018 at 4:08 PM, Jagan Teki
>> <jagannadh.teki@gmail.com>
>> >> wrote:
>> >> > On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu
>> >> > <sivadur@xilinx.com> wrote:
>> >> >> Hi Jagan,
>> >> >>
>> >> >>> -----Original Message-----
>> >> >>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >> >>> Sent: Wednesday, May 09, 2018 1:22 PM
>> >> >>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> >>> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >> >>> boot at lists.denx.de>; michal.simek at xilinx.com
>> >> >>> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
>> >> >>> support for ZynqMP qspi driver
>> >> >>>
>> >> >>> On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu
>> >> >>> <sivadur@xilinx.com> wrote:
>> >> >>> >
>> >> >>> > Hi,
>> >> >>> >
>> >> >>> >> -----Original Message-----
>> >> >>> >> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> >> >>> >> Sent: Wednesday, May 09, 2018 1:12 PM
>> >> >>> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> >>> >> Cc: Jagan Teki <jagan@amarulasolutions.com>; U-Boot-Denx <u-
>> >> >>> >> boot at lists.denx.de>; michal.simek at xilinx.com
>> >> >>> >> Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add
>> >> >>> >> support for ZynqMP qspi driver
>> >> >>> >>
>> >> >>> >> On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu
>> >> >>> >> <sivadur@xilinx.com> wrote:
>> >> >>> >> > Hi Jagan,
>> >> >>> >> >
>> >> >>> >> >> -----Original Message-----
>> >> >>> >> >> From: Jagan Teki [mailto:jagan at amarulasolutions.com]
>> >> >>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM
>> >> >>> >> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> >>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>;
>> >> >>> >> >> michal.simek at xilinx.com
>> >> >>> >> >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support
>> >> >>> >> >> for
>> >> >>> >> ZynqMP
>> >> >>> >> >> qspi driver
>> >> >>> >> >>
>> >> >>> >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu
>> >> >>> >> >> <siva.durga.paladugu@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@xilinx.com>
>> >> >>> >> >> > ---
>> >> >>> >> >> > 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
>> >> >>> >> >> > ---
>> >> >>> >> >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h |
>> 154
>> >> >>> ++++++
>> >> >>> >> >> > drivers/spi/Kconfig | 7 +
>> >> >>> >> >> > drivers/spi/Makefile | 1 +
>> >> >>> >> >> > drivers/spi/zynqmp_gqspi.c | 670
>> >> >>> >> >> ++++++++++++++++++++++++
>> >> >>> >> >> > 4 files changed, 832 insertions(+) create mode 100644
>> >> >>> >> >> > arch/arm/include/asm/arch-
>> >> >>> >> >> zynqmp/zynqmp_gqspi.h
>> >> >>> >> >> > create mode 100644 drivers/spi/zynqmp_gqspi.c
>> >> >>> >> >> >
>> >> >>> >> >> > diff --git a/arch/arm/include/asm/arch-
>> >> zynqmp/zynqmp_gqspi.h
>> >> >>> >> >> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h
>> >> >>> >> >>
>> >> >>> >> >> already asked you to move this header code in driver .c
>> >> >>> >> >> file
>> >> >>> >> >
>> >> >>> >> > You might have missed my reply to your earlier comment on
>> this.
>> >> >>> >> > These were moved to .h based on comment from Lukasz in v1.
>> >> >>> >> > I don’t have any issue in having them anywhere. Let me know
>> >> >>> >> > your
>> >> >>> choice.
>> >> >>> >>
>> >> >>> >> I'm trying to align Linux code, better to move like that and
>> >> >>> >> make sure to use similar macros.
>> >> >>> >>
>> >> >>> >> >
>> >> >>> >
>> >> >>> > [snip]
>> >> >>> >
>> >> >>> >> >> > +static void zynqmp_qspi_genfifo_cmd(struct
>> >> zynqmp_qspi_priv
>> >> >>> >> *priv) {
>> >> >>> >> >> > + u8 command = 1;
>> >> >>> >> >> > + u32 gen_fifo_cmd;
>> >> >>> >> >> > + u32 bytecount = 0;
>> >> >>> >> >> > +
>> >> >>> >> >> > + while (priv->len) {
>> >> >>> >> >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> >> >>> >> >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >> >>> >> >> > +
>> >> >>> >> >> > + if (command) {
>> >> >>> >> >> > + command = 0;
>> >> >>> >> >> > + last_cmd = *(u8 *)priv->tx_buf;
>> >> >>> >> >> > + }
>> >> >>> >> >>
>> >> >>> >> >> don't understand this code can you explain? command
>> >> >>> >> >> assigned
>> >> 1
>> >> >>> >> >> it will not updated anywhere?
>> >> >>> >> >
>> >> >>> >> > I want to store last command sent. As the first byte in loop
>> >> >>> >> > only contains command, it ensures it fills only for one time
>> >> >>> >> > and next time it
>> >> >>> >> may contain data to be sent along with command.
>> >> >>> >> > Command initialized to 1 while declaring it above(u8 command
>> >> >>> >> > =
>> >> 1).
>> >> >>> >>
>> >> >>> >> Ok the first TX buf has command and reused in tx and rx fifo,
>> >> >>> >> how come to use use same in rx fifo? why is is last_cmd it is
>> first_cmd?
>> >> >>> >
>> >> >>> > This holds the command that was sent last to the device before
>> >> >>> > we use in
>> >> >>> tx/rx fill, hence used this name.
>> >> >>>
>> >> >>> ie. what I wonder, usually the spi transfer start with cmd + data
>> >> >>> in that case it would be the first command, did gqspi work reverse
>> way?
>> >> >>
>> >> >> It's also same as others :-), the last_cmd holds the recent
>> >> >> command that
>> >> was sent to the device.
>> >> >> Its just name. To avoid confusion, I can use other names like
>> >> >> "cmd_sent or cmd_recent". Hope this is ok for you or suggest which
>> >> >> one
>> >> would be appropriate.
>> >> >
>> >> > Let's park the naming aside.
>> >> >
>> >> > u8 command = 0;
>> >> >
>> >> > while (priv->len) {
>> >> > gen_fifo_cmd = zynqmp_qspi_bus_select(priv);
>> >> > gen_fifo_cmd |= GQSPI_GFIFO_TX;
>> >> >
>> >> > if (command) {
>> >> > command = 0;
>> >> > last_cmd = *(u8 *)priv->tx_buf;
>> >> > }
>> >> > ......
>> >> > priv->len--;
>> >> > }
>> >> >
>> >> > Why the last_cmd assignment is in loop? say priv->len has non-zero
>> >> > the command is 1 last_cmd assigned and command reassigned to 0
>> >> > there is
>> >> no
>> >> > way execute the if for next while isn't it?
>> >>
>> >> The main issue with this driver, which I told/commented several times
>> >> about using flash specific stuff.
>> >>
>> >> +
>> >> +#define QUAD_OUT_READ_CMD 0x6B
>> >> +#define QUAD_PAGE_PROGRAM_CMD 0x32
>> >> +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
>> >> +
>> >
>> > Yeah I know, that’s where our discussion stopped last time and would
>> like to conclude using this series.
>> > https://patchwork.ozlabs.org/patch/829856/
>> > We discussed this sometime back and I mentioned that this controller
>> > is not just targeted for spi-nor but even though none tested/used with
>> devices other than spi-nor AFAIK.
>>
>> and you agree to write driver on spi side right? but you still using flash
>> commands.
>
> Sorry, That’s not true, my understanding was that we agreed to move to driver/mtd/spi in future inorder to have other
> controller specific functionality like dual flash, but for now and for single its ok to be here.
>
> Anyway, let me try to address your comment of not using any flash specific stuff here in driver
> at drivers/spi/ and will send next version. Hope this works for you.
Yes, please.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-05-09 11:54 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 10:13 [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Siva Durga Prasad Paladugu
2018-05-08 10:13 ` [U-Boot] [PATCH v3 2/2] zynqmp: zcu102: Add qspi driver support for ZynqMP zcu102 boards Siva Durga Prasad Paladugu
2018-05-09 6:32 ` Jagan Teki
2018-05-09 6:30 ` [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver Jagan Teki
2018-05-09 7:03 ` Siva Durga Prasad Paladugu
2018-05-09 7:41 ` Jagan Teki
2018-05-09 7:50 ` Siva Durga Prasad Paladugu
2018-05-09 7:52 ` Jagan Teki
2018-05-09 8:01 ` Siva Durga Prasad Paladugu
2018-05-09 10:38 ` Jagan Teki
2018-05-09 10:48 ` Jagan Teki
2018-05-09 11:14 ` Siva Durga Prasad Paladugu
2018-05-09 11:16 ` Jagan Teki
2018-05-09 11:52 ` Siva Durga Prasad Paladugu
2018-05-09 11:54 ` Jagan Teki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.