On Wed, Feb 7, 2018 at 3:40 PM, Siva Durga Prasad Paladugu sivadur@xilinx.com wrote:
Hi Jagan,
-----Original Message----- From: Jagan Teki [mailto:jagan@amarulasolutions.com] Sent: Tuesday, January 23, 2018 10:41 PM To: Siva Durga Prasad Paladugu sivadur@xilinx.com Cc: U-Boot-Denx u-boot@lists.denx.de; Siva Durga Prasad Paladugu sivadur@xilinx.com Subject: Re: [U-Boot] [PATCH v2 1/2] spi: zynqmp_qspi: Add support for ZynqMP qspi driver
On Thu, Jan 4, 2018 at 1:07 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 sivadur@xilinx.com
Changes from v1:
- 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/cpu/armv8/zynqmp/Kconfig | 7 + arch/arm/include/asm/arch-zynqmp/zynqmp_qspi.h | 154 ++++++ drivers/spi/Makefile | 1 + drivers/spi/zynqmp_qspi.c | 678
+++++++++++++++++++++++++
Was this gqspi like linux spi-zynqmp-gqspi.c or different?
Yes.
then try to use similar naming conventions in macros, or in function names as well.
4 files changed, 840 insertions(+) create mode 100644 arch/arm/include/asm/arch-
zynqmp/zynqmp_qspi.h
create mode 100644 drivers/spi/zynqmp_qspi.c
diff --git a/arch/arm/cpu/armv8/zynqmp/Kconfig b/arch/arm/cpu/armv8/zynqmp/Kconfig index 3f922b4..2fe4f71 100644 --- a/arch/arm/cpu/armv8/zynqmp/Kconfig +++ b/arch/arm/cpu/armv8/zynqmp/Kconfig @@ -65,6 +65,13 @@ config PMUFW_INIT_FILE Include external PMUFW (Platform Management Unit FirmWare) to a Xilinx bootable image (boot.bin).
+config ZYNQMP_QSPI
bool "Configure ZynqMP QSPI"select DM_SPIhelpThis option is used to enable ZynqMP QSPI controller driver whichis used to communicate with qspi flash devices.I've commented this before, what is the reason for adding spi kconfig entry in arch area instead of drivers/spi?
I might have missed it, Will move to drivers/spi
config ZYNQMP_USB bool "Configure ZynqMP USB"
diff --git a/arch/arm/include/asm/arch-zynqmp/zynqmp_qspi.h b/arch/arm/include/asm/arch-zynqmp/zynqmp_qspi.h new file mode 100644 index 0000000..5e2926e --- /dev/null +++ b/arch/arm/include/asm/arch-zynqmp/zynqmp_qspi.h @@ -0,0 +1,154 @@ +/*
- (C) Copyright 2014 - 2015 Xilinx
- Xilinx ZynqMP Quad-SPI(QSPI) controller driver (master mode only)
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef _ASM_ARCH_ZYNQMP_QSPI_H_ +#define _ASM_ARCH_ZYNQMP_QSPI_H_
+#define ZYNQMP_QSPI_GFIFO_STRT_MODE_MASK BIT(29) +#define ZYNQMP_QSPI_CONFIG_MODE_EN_MASK (3 << 30) +#define ZYNQMP_QSPI_CONFIG_DMA_MODE (2 << 30) +#define ZYNQMP_QSPI_CONFIG_CPHA_MASK BIT(2) +#define ZYNQMP_QSPI_CONFIG_CPOL_MASK BIT(1)
+/* QSPI MIO's count for different connection topologies */ +#define ZYNQMP_QSPI_MIO_NUM_QSPI0 6 +#define ZYNQMP_QSPI_MIO_NUM_QSPI1 5 +#define ZYNQMP_QSPI_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 ZYNQMP_QSPI_IXR_TXNFULL_MASK 0x00000004 /* QSPI TX
FIFO Overflow */
+#define ZYNQMP_QSPI_IXR_TXFULL_MASK 0x00000008 /* QSPI TX
FIFO is full */
+#define ZYNQMP_QSPI_IXR_RXNEMTY_MASK 0x00000010 /* QSPI RX
FIFO Not Empty */
+#define ZYNQMP_QSPI_IXR_GFEMTY_MASK 0x00000080 /* QSPI
Generic FIFO Empty */
+#define ZYNQMP_QSPI_IXR_ALL_MASK
(ZYNQMP_QSPI_IXR_TXNFULL_MASK | \
ZYNQMP_QSPI_IXR_RXNEMTY_MASK)+/*
- QSPI Enable Register bit Masks
- This register is used to enable or disable the QSPI controller */
+#define ZYNQMP_QSPI_ENABLE_ENABLE_MASK 0x00000001 /* QSPI
Enable Bit
+Mask */
+#define ZYNQMP_QSPI_GFIFO_LOW_BUS BIT(14) +#define ZYNQMP_QSPI_GFIFO_CS_LOWER BIT(12) +#define ZYNQMP_QSPI_GFIFO_UP_BUS BIT(15) +#define ZYNQMP_QSPI_GFIFO_CS_UPPER BIT(13) +#define ZYNQMP_QSPI_SPI_MODE_QSPI (3 << 10) +#define ZYNQMP_QSPI_SPI_MODE_SPI BIT(10) +#define ZYNQMP_QSPI_SPI_MODE_DUAL_SPI (2 << 10) +#define ZYNQMP_QSPI_IMD_DATA_CS_ASSERT 5 +#define ZYNQMP_QSPI_IMD_DATA_CS_DEASSERT 5 +#define ZYNQMP_QSPI_GFIFO_TX BIT(16) +#define ZYNQMP_QSPI_GFIFO_RX BIT(17) +#define ZYNQMP_QSPI_GFIFO_STRIPE_MASK BIT(18) +#define ZYNQMP_QSPI_GFIFO_IMD_MASK 0xFF +#define ZYNQMP_QSPI_GFIFO_EXP_MASK BIT(9) +#define ZYNQMP_QSPI_GFIFO_DATA_XFR_MASK BIT(8) +#define ZYNQMP_QSPI_STRT_GEN_FIFO BIT(28) +#define ZYNQMP_QSPI_GEN_FIFO_STRT_MOD BIT(29) +#define ZYNQMP_QSPI_GFIFO_WP_HOLD BIT(19) +#define ZYNQMP_QSPI_BAUD_DIV_MASK (7 << 3) +#define ZYNQMP_QSPI_DFLT_BAUD_RATE_DIV BIT(3) #define +ZYNQMP_QSPI_GFIFO_ALL_INT_MASK 0xFBE #define +ZYNQMP_QSPI_DMA_DST_I_STS_DONE BIT(1) #define +ZYNQMP_QSPI_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 ZYNQMP_QSPI_GFIFO_SELECT BIT(0)
+#define ZYNQMP_QSPI_FIFO_THRESHOLD 1
+#define SPI_XFER_ON_BOTH 0 +#define SPI_XFER_ON_LOWER 1 +#define SPI_XFER_ON_UPPER 2
+#define ZYNQMP_QSPI_DMA_ALIGN 0x4 +#define ZYNQMP_QSPI_MAX_BAUD_RATE_VAL 7 #define +ZYNQMP_QSPI_DFLT_BAUD_RATE_VAL 2
+#define ZYNQMP_QSPI_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 ZYNQMP_GQSPI_REG_OFFSET 0x100 +#define ZYNQMP_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_QSPI_H_ */
Move all these on driver file itself, also try to use short macro names.
I moved to .h based on v1 comments, now vice versa. I didn’t see any problem in having these in .h
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index e3184db..9cdd94a 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -50,3 +50,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_QSPI) += zynqmp_qspi.o diff --git a/drivers/spi/zynqmp_qspi.c b/drivers/spi/zynqmp_qspi.c new file mode 100644 index 0000000..199e704 --- /dev/null +++ b/drivers/spi/zynqmp_qspi.c @@ -0,0 +1,678 @@ +/*
- (C) Copyright 2014 - 2015 Xilinx
- Xilinx ZynqMP Quad-SPI(QSPI) controller driver (master mode only)
- SPDX-License-Identifier: GPL-2.0
- */
+#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 "../mtd/spi/sf_internal.h"
This I've already commented and still there any reason?
+#include <clk.h> +#include <asm/arch/zynqmp_qspi.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__);no need
Any issue in having it with debug..?? I can remove if you don’t like.
plat->regs = (struct zynqmp_qspi_regs *)(devfdt_get_addr(bus) +ZYNQMP_GQSPI_REG_OFFSET);plat->dma_regs = (struct zynqmp_qspi_dma_regs *)(devfdt_get_addr(bus) +- ZYNQMP_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;why we need this? all these are generic stuff which is available at spi- uclass.c
Somehow I am not able to get these from spi-uclass.c , the routine which reads all these info from dt in spi-uclass.c is never getting invoked in my flow. I checked other driver as well,. Do you have an idea on why is it so?
All these attributes are from platdata which were initialized by spi-uclass.c so if you need any of these we can get the dm_spi_slave_platdata from your driver using dev_get_parent_platdata() function.
Remeber let not add spi-nor features in this driver, you may ended-up more adding more redundent code in driver if you do so.
Fix, what ever comments you agree and resend we will review again.
Jagan.