Xilinx Zynq uses a QSPI controller that is based on the Cadence SPI IP.
This controller implements all the functionality required to support
Quad SPI NOR flash devices.
This driver along with the MTD layer is used to support flash devices.
This series is for the following purposes:
- RFC of the Quad SPI driver.
- We currently use a custom MTD layer and would like to get inputs on
dual stacked/dual parallel handling (described below).
The flash device(s) can be connected in the three configurations:
1. Single - One flash device with 1 CS, 1 Clock and 4 IO lines.
2. Dual Parallel - Two flash devices connected with common CS and
separate IO lines (resulting in 8 IO lines).
In this configuration, the controller
a) Duplicates commands, address etc. sent on both sets of 4 IO lines.
b) Stripes data both transmitted and received i.e.
4 bits of data is sent to the first flash and the other 4 bits
to the second flash. Similarly read data is also consolidated.
Due to this, TX and RX data handling in the driver need special
handling for parallel mode.
3. Dual Stacked - Two flash devices connected with separate CS and
4 common IO lines. This is largely similar to single, except for
the slave selection logic.
The above configuration is conveyed to the QSPI driver through a
devicetree property.
The QSPI driver differs from the existing Cadence SPI driver in
the following respects majorly:
1. TX and RX handling: Different TX registers are used to write into
the TX FIFO. TXD0, TXD1, TXD2 and TXD3 are used write 4, 1, 2 and 3
bytes respectively. Depending on the TXD register used, the received
bytes also need to be handled separately.
2. Depending on the configuration in which flash devices are connected
(single, parallel or stacked), QSPI controller configuration registers
need to be modified.
3. There is no support for extended slave select in QSPI, as opposed to
SPI. In case of stacked configuration, the slave select field remains
the same and a different configuration bit is used to select between
the two flash devices.
4. Handling of dual parallel configuration.
MTD layer:
The Xilinx Zynq MTD layer by far makes use of the mainline version with
some differences. The primary flash families supported are
Spansion, Winbond and Micron.
- Probe:
- In dual configurations, both flash devices are recognized as one
continuous memory. (ID is read only from one flash and it is a
pre-stated assumption that both flash devices have the same flash
make and size.)
- Addressing:
a) In dual stacked mode, the address passed to the MTD layer can be
between 0x0 to 2*(one flash size). Hence the MTD layer has to recognize
whether the address belongs to the first flash or the second flash
subtract the offset and indicate the same to the QSPI driver.
b) In dual stacked mode too the address can range between
0 to 2*(one flash size). But, when an 8 bit word is written,
4 bits are written to the first and 4 bits are written to the
second flash. Hence the address sent is always halved and checks
are in place for even address and even length.
- 4 byte addressing is not supported and hence bank selection logic is used
along with the addressing system described above.
- Flash register read/writes, for example, lock/unlock, quad enable etc.
are handled differently in dual stacked and parallel modes.
I'm sorry for the long cover letter. Hope it helps.
Harini Katakam (2):
spi: Add support for Zynq QSPI controller
devicetree: Add devicetree bindings documentation for Zynq QSPI
.../devicetree/bindings/spi/spi-zynq-qspi.txt | 28 +
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-zynq-qspi.c | 854 ++++++++++++++++++++
4 files changed, 889 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
create mode 100644 drivers/spi/spi-zynq-qspi.c
--
1.7.9.5
This patch adds support for QSPI controller used by Zynq.
Signed-off-by: Harini Katakam <[email protected]>
---
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-zynq-qspi.c | 854 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 861 insertions(+)
create mode 100644 drivers/spi/spi-zynq-qspi.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 213b5cb..9642148 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -558,6 +558,12 @@ config SPI_XTENSA_XTFPGA
start and deasserting on end.
+config SPI_ZYNQ_QSPI
+ tristate "Xilinx Zynq QSPI controller"
+ depends on ARCH_ZYNQ
+ help
+ This selects the Xilinx Zynq Quad SPI controller master driver.
+
config SPI_NUC900
tristate "Nuvoton NUC900 series SPI"
depends on ARCH_W90X900
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 929c9f5..0bfe75e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -83,3 +83,4 @@ obj-$(CONFIG_SPI_TXX9) += spi-txx9.o
obj-$(CONFIG_SPI_XCOMM) += spi-xcomm.o
obj-$(CONFIG_SPI_XILINX) += spi-xilinx.o
obj-$(CONFIG_SPI_XTENSA_XTFPGA) += spi-xtensa-xtfpga.o
+obj-$(CONFIG_SPI_ZYNQ_QSPI) += spi-zynq-qspi.o
diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
new file mode 100644
index 0000000..2a352a9
--- /dev/null
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -0,0 +1,854 @@
+/*
+ * Xilinx Zynq Quad-SPI (QSPI) controller driver (master mode only)
+ *
+ * Copyright (C) 2009 - 2014 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/workqueue.h>
+
+/* Name of this driver */
+#define DRIVER_NAME "zynq-qspi"
+
+/* Register offset definitions */
+#define ZYNQ_QSPI_CONFIG_OFFSET 0x00 /* Configuration Register, RW */
+#define ZYNQ_QSPI_STATUS_OFFSET 0x04 /* Interrupt Status Register, RO */
+#define ZYNQ_QSPI_IEN_OFFSET 0x08 /* Interrupt Enable Register, WO */
+#define ZYNQ_QSPI_IDIS_OFFSET 0x0C /* Interrupt Disable Reg, WO */
+#define ZYNQ_QSPI_IMASK_OFFSET 0x10 /* Interrupt Enabled Mask Reg,RO */
+#define ZYNQ_QSPI_ENABLE_OFFSET 0x14 /* Enable/Disable Register, RW */
+#define ZYNQ_QSPI_DELAY_OFFSET 0x18 /* Delay Register, RW */
+#define ZYNQ_QSPI_TXD_00_00_OFFSET 0x1C /* Transmit 4-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_01_OFFSET 0x80 /* Transmit 1-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_10_OFFSET 0x84 /* Transmit 2-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_11_OFFSET 0x88 /* Transmit 3-byte inst, WO */
+#define ZYNQ_QSPI_RXD_OFFSET 0x20 /* Data Receive Register, RO */
+#define ZYNQ_QSPI_SIC_OFFSET 0x24 /* Slave Idle Count Register, RW */
+#define ZYNQ_QSPI_TX_THRESH_OFFSET 0x28 /* TX FIFO Watermark Reg, RW */
+#define ZYNQ_QSPI_RX_THRESH_OFFSET 0x2C /* RX FIFO Watermark Reg, RW */
+#define ZYNQ_QSPI_GPIO_OFFSET 0x30 /* GPIO Register, RW */
+#define ZYNQ_QSPI_LINEAR_CFG_OFFSET 0xA0 /* Linear Adapter Config Ref, RW */
+#define ZYNQ_QSPI_MOD_ID_OFFSET 0xFC /* Module ID Register, RO */
+
+/*
+ * QSPI Configuration Register bit Masks
+ *
+ * This register contains various control bits that effect the operation
+ * of the QSPI controller
+ */
+#define ZYNQ_QSPI_CONFIG_IFMODE_MASK 0x80000000 /* Flash Memory Interface */
+#define ZYNQ_QSPI_CONFIG_MANSRT_MASK 0x00010000 /* Manual TX Start */
+#define ZYNQ_QSPI_CONFIG_MANSRTEN_MASK 0x00008000 /* Enable Manual TX Mode */
+#define ZYNQ_QSPI_CONFIG_SSFORCE_MASK 0x00004000 /* Manual Chip Select */
+#define ZYNQ_QSPI_CONFIG_BDRATE_MASK 0x00000038 /* Baud Rate Divisor Mask */
+#define ZYNQ_QSPI_CONFIG_CPHA_MASK 0x00000004 /* Clock Phase Control */
+#define ZYNQ_QSPI_CONFIG_CPOL_MASK 0x00000002 /* Clock Polarity Control */
+#define ZYNQ_QSPI_CONFIG_SSCTRL_MASK 0x00003C00 /* Slave Select Mask */
+#define ZYNQ_QSPI_CONFIG_FWIDTH_MASK 0x000000C0 /* FIFO width */
+#define ZYNQ_QSPI_CONFIG_MSTREN_MASK 0x00000001 /* Master Mode */
+
+/*
+ * QSPI Configuration Register - Baud rate and slave select
+ *
+ * These are the values used in the calculation of baud rate divisor and
+ * setting the slave select.
+ */
+#define ZYNQ_QSPI_BAUD_DIV_MAX 7 /* Baud rate divisor maximum */
+#define ZYNQ_QSPI_BAUD_DIV_SHIFT 3 /* Baud rate divisor shift in CR */
+#define ZYNQ_QSPI_SS_SHIFT 10 /* Slave Select field shift in CR */
+
+/*
+ * QSPI Interrupt Registers bit Masks
+ *
+ * All the four interrupt registers (Status/Mask/Enable/Disable) have the same
+ * bit definitions.
+ */
+#define ZYNQ_QSPI_IXR_TXNFULL_MASK 0x00000004 /* QSPI TX FIFO Overflow */
+#define ZYNQ_QSPI_IXR_TXFULL_MASK 0x00000008 /* QSPI TX FIFO is full */
+#define ZYNQ_QSPI_IXR_RXNEMTY_MASK 0x00000010 /* QSPI RX FIFO Not Empty */
+#define ZYNQ_QSPI_IXR_ALL_MASK (ZYNQ_QSPI_IXR_TXNFULL_MASK | \
+ ZYNQ_QSPI_IXR_RXNEMTY_MASK)
+
+/*
+ * QSPI Enable Register bit Masks
+ *
+ * This register is used to enable or disable the QSPI controller
+ */
+#define ZYNQ_QSPI_ENABLE_ENABLE_MASK 0x00000001 /* QSPI Enable Bit Mask */
+
+/*
+ * QSPI Linear Configuration Register
+ *
+ * It is named Linear Configuration but it controls other modes when not in
+ * linear mode also.
+ */
+#define ZYNQ_QSPI_LCFG_TWO_MEM_MASK 0x40000000 /* LQSPI Two memories Mask */
+#define ZYNQ_QSPI_LCFG_SEP_BUS_MASK 0x20000000 /* LQSPI Separate bus Mask */
+#define ZYNQ_QSPI_LCFG_U_PAGE_MASK 0x10000000 /* LQSPI Upper Page Mask */
+
+#define ZYNQ_QSPI_LCFG_DUMMY_SHIFT 8
+
+#define ZYNQ_QSPI_FAST_READ_QOUT_CODE 0x6B /* read instruction code */
+#define ZYNQ_QSPI_FIFO_DEPTH 63 /* FIFO depth in words */
+#define ZYNQ_QSPI_RX_THRESHOLD 32 /* Rx FIFO threshold level */
+#define ZYNQ_QSPI_TX_THRESHOLD 1 /* Tx FIFO threshold level */
+
+/* Default number of chip selects */
+#define ZYNQ_QSPI_DEFAULT_NUM_CS 1
+
+/**
+ * struct zynq_qspi - Defines qspi driver instance
+ * @regs: Virtual address of the QSPI controller registers
+ * @refclk: Pointer to the peripheral clock
+ * @pclk: Pointer to the APB clock
+ * @irq: IRQ number
+ * @config_reg_lock: Lock used for accessing configuration register
+ * @txbuf: Pointer to the TX buffer
+ * @rxbuf: Pointer to the RX buffer
+ * @bytes_to_transfer: Number of bytes left to transfer
+ * @bytes_to_receive: Number of bytes left to receive
+ * @is_parallel: Flag to indicate two flash devices are in parallel
+ * @is_stacked: Flag to indicate two flash devices are stacked
+ */
+struct zynq_qspi {
+ void __iomem *regs;
+ struct clk *refclk;
+ struct clk *pclk;
+ int irq;
+ const void *txbuf;
+ void *rxbuf;
+ int bytes_to_transfer;
+ int bytes_to_receive;
+ u32 is_parallel;
+ u32 is_stacked;
+ u8 is_instr;
+};
+
+/*
+ * Inline functions for the QSPI controller read/write
+ */
+static inline u32 zynq_qspi_read(struct zynq_qspi *xqspi, u32 offset)
+{
+ return readl_relaxed(xqspi->regs + offset);
+}
+
+static inline void zynq_qspi_write(struct zynq_qspi *xqspi, u32 offset,
+ u32 val)
+{
+ writel_relaxed(val, xqspi->regs + offset);
+}
+
+/**
+ * zynq_qspi_init_hw - Initialize the hardware
+ * @xqspi: Pointer to the zynq_qspi structure
+ *
+ * The default settings of the QSPI controller's configurable parameters on
+ * reset are
+ * - Master mode
+ * - Baud rate divisor is set to 2
+ * - Tx thresold set to 1l Rx threshold set to 32
+ * - Flash memory interface mode enabled
+ * - Size of the word to be transferred as 8 bit
+ * This function performs the following actions
+ * - Disable and clear all the interrupts
+ * - Enable manual slave select
+ * - Enable manual start
+ * - Deselect all the chip select lines
+ * - Set the size of the word to be transferred as 32 bit
+ * - Set the little endian mode of TX FIFO and
+ * - Enable the QSPI controller
+ */
+static void zynq_qspi_init_hw(struct zynq_qspi *xqspi)
+{
+ u32 config_reg;
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_IDIS_OFFSET, 0x7F);
+
+ /* Disable linear mode as the boot loader may have used it */
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET, 0);
+
+ /* Clear the RX FIFO */
+ while (zynq_qspi_read(xqspi, ZYNQ_QSPI_STATUS_OFFSET) &
+ ZYNQ_QSPI_IXR_RXNEMTY_MASK)
+ zynq_qspi_read(xqspi, ZYNQ_QSPI_RXD_OFFSET);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_STATUS_OFFSET, 0x7F);
+ config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+ config_reg &= ~(ZYNQ_QSPI_CONFIG_MSTREN_MASK |
+ ZYNQ_QSPI_CONFIG_CPOL_MASK |
+ ZYNQ_QSPI_CONFIG_CPHA_MASK |
+ ZYNQ_QSPI_CONFIG_BDRATE_MASK |
+ ZYNQ_QSPI_CONFIG_SSFORCE_MASK |
+ ZYNQ_QSPI_CONFIG_MANSRTEN_MASK |
+ ZYNQ_QSPI_CONFIG_MANSRT_MASK);
+ config_reg |= (ZYNQ_QSPI_CONFIG_MSTREN_MASK |
+ ZYNQ_QSPI_CONFIG_SSFORCE_MASK |
+ ZYNQ_QSPI_CONFIG_FWIDTH_MASK |
+ ZYNQ_QSPI_CONFIG_IFMODE_MASK);
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_RX_THRESH_OFFSET,
+ ZYNQ_QSPI_RX_THRESHOLD);
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_TX_THRESH_OFFSET,
+ ZYNQ_QSPI_TX_THRESHOLD);
+
+ if (xqspi->is_parallel)
+ /* Enable two memories on seperate buses */
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+ (ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
+ ZYNQ_QSPI_LCFG_SEP_BUS_MASK |
+ (1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
+ ZYNQ_QSPI_FAST_READ_QOUT_CODE));
+ if (xqspi->is_stacked)
+ /* Enable two memories on shared bus */
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+ (ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
+ (1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
+ ZYNQ_QSPI_FAST_READ_QOUT_CODE));
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET,
+ ZYNQ_QSPI_ENABLE_ENABLE_MASK);
+}
+
+/**
+ * zynq_qspi_copy_read_data - Copy data to RX buffer
+ * @xqspi: Pointer to the zynq_qspi structure
+ * @data: The 32 bit variable where data is stored
+ * @size: Number of bytes to be copied from data to RX buffer
+ *
+ * Note: In case of dual parallel connection, even number of bytes are read
+ * when odd bytes are requested to avoid transfer of a nibble to each flash.
+ * The receive buffer though, is populated with the number of bytes requested.
+ */
+static void zynq_qspi_copy_read_data(struct zynq_qspi *xqspi, u32 data, u8 size)
+{
+ if (xqspi->rxbuf) {
+ if (!xqspi->is_parallel || xqspi->is_instr) {
+ memcpy(xqspi->rxbuf, ((u8 *) &data) + 4 - size, size);
+ xqspi->rxbuf += size;
+ } else {
+ u8 buff[4], len;
+
+ len = size;
+ size = size % 2 ? size + 1 : size;
+ memcpy(buff, ((u8 *) &data) + 4 - size, size);
+ memcpy(xqspi->rxbuf, buff, len);
+ xqspi->rxbuf += len;
+ }
+ }
+ xqspi->bytes_to_receive -= size;
+ if (xqspi->bytes_to_receive < 0)
+ xqspi->bytes_to_receive = 0;
+}
+
+/**
+ * zynq_qspi_copy_write_data - Copy data from TX buffer
+ * @xqspi: Pointer to the zynq_qspi structure
+ * @data: Pointer to the 32 bit variable where data is to be copied
+ * @size: Number of bytes to be copied from TX buffer to data
+ */
+static void zynq_qspi_copy_write_data(struct zynq_qspi *xqspi, u32 *data,
+ u8 size)
+{
+ if (xqspi->txbuf) {
+ memcpy(data, xqspi->txbuf, size);
+ xqspi->txbuf += size;
+ } else {
+ *data = 0;
+ }
+
+ xqspi->bytes_to_transfer -= size;
+}
+
+/**
+ * zynq_qspi_chipselect - Select or deselect the chip select line
+ * @qspi: Pointer to the spi_device structure
+ * @is_high: Select(0) or deselect (1) the chip select line
+ */
+static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+ u32 config_reg;
+
+ config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+
+ /* Select upper/lower page before asserting CS */
+ if (xqspi->is_stacked) {
+ u32 lqspi_cfg_reg;
+
+ lqspi_cfg_reg = zynq_qspi_read(xqspi,
+ ZYNQ_QSPI_LINEAR_CFG_OFFSET);
+ if (qspi->master->flags & SPI_MASTER_U_PAGE)
+ lqspi_cfg_reg |= ZYNQ_QSPI_LCFG_U_PAGE_MASK;
+ else
+ lqspi_cfg_reg &= ~ZYNQ_QSPI_LCFG_U_PAGE_MASK;
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+ lqspi_cfg_reg);
+ }
+
+ if (is_high) {
+ /* Deselect the slave */
+ config_reg |= ZYNQ_QSPI_CONFIG_SSCTRL_MASK;
+ } else {
+ /* Select the slave */
+ config_reg &= ~ZYNQ_QSPI_CONFIG_SSCTRL_MASK;
+ config_reg |= (((~(BIT(qspi->chip_select))) <<
+ ZYNQ_QSPI_SS_SHIFT) &
+ ZYNQ_QSPI_CONFIG_SSCTRL_MASK);
+ xqspi->is_instr = 1;
+ }
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_qspi_config_clock_mode - Sets clock frequency
+ * @qspi: Pointer to the spi_device structure
+ *
+ * Sets the requested clock polarity and phase.
+ */
+static void zynq_qspi_config_clock_mode(struct spi_device *qspi)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+ u32 config_reg;
+
+ config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+
+ /* Set the QSPI clock phase and clock polarity */
+ config_reg &= (~ZYNQ_QSPI_CONFIG_CPHA_MASK) &
+ (~ZYNQ_QSPI_CONFIG_CPOL_MASK);
+ if (qspi->mode & SPI_CPHA)
+ config_reg |= ZYNQ_QSPI_CONFIG_CPHA_MASK;
+ if (qspi->mode & SPI_CPOL)
+ config_reg |= ZYNQ_QSPI_CONFIG_CPOL_MASK;
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_qspi_config_clock_freq - Sets clock frequency
+ * @qspi: Pointer to the spi_device structure
+ * @transfer: Pointer to the spi_transfer structure which provides
+ * information about next transfer setup parameters
+ *
+ * Sets the requested clock frequency.
+ * Note: If the requested frequency is not an exact match with what can be
+ * obtained using the prescalar value the driver sets the clock frequency which
+ * is lower than the requested frequency (maximum lower) for the transfer. If
+ * the requested frequency is higher or lower than that is supported by the SPI
+ * controller the driver will set the highest or lowest frequency supported by
+ * controller.
+ */
+static void zynq_qspi_config_clock_freq(struct spi_device *qspi,
+ struct spi_transfer *transfer)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+ u32 config_reg, baud_rate_val = 0;
+
+ /* Set the clock frequency */
+ /* If requested frequency is zero, default to lowest speed */
+ while ((baud_rate_val < ZYNQ_QSPI_BAUD_DIV_MAX) &&
+ (clk_get_rate(xqspi->refclk) / (2 << baud_rate_val)) >
+ transfer->speed_hz)
+ baud_rate_val++;
+
+ config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+ config_reg &= ~ZYNQ_QSPI_CONFIG_BDRATE_MASK;
+ config_reg |= (baud_rate_val << ZYNQ_QSPI_BAUD_DIV_SHIFT);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_qspi_setup_transfer - Configure QSPI controller for specified transfer
+ * @qspi: Pointer to the spi_device structure
+ * @transfer: Pointer to the spi_transfer structure which provides information
+ * about next transfer setup parameters
+ *
+ * Sets the operational mode of QSPI controller for the next QSPI transfer and
+ * sets the requested clock frequency.
+ *
+ * Return: Always 0
+ *
+ */
+static int zynq_qspi_setup_transfer(struct spi_device *qspi,
+ struct spi_transfer *transfer)
+{
+ zynq_qspi_config_clock_freq(qspi, transfer);
+
+ return 0;
+}
+
+/**
+ * zynq_qspi_fill_tx_fifo - Fills the TX FIFO with as many bytes as possible
+ * @xqspi: Pointer to the zynq_qspi structure
+ * @size: Size of the fifo to be filled
+ */
+static void zynq_qspi_fill_tx_fifo(struct zynq_qspi *xqspi, u32 size)
+{
+ u32 fifocount;
+
+ for (fifocount = 0; (fifocount < size) &&
+ (xqspi->bytes_to_transfer >= 4); fifocount++) {
+ if (xqspi->txbuf) {
+ zynq_qspi_write(xqspi,
+ ZYNQ_QSPI_TXD_00_00_OFFSET,
+ *((u32 *)xqspi->txbuf));
+ xqspi->txbuf += 4;
+ } else {
+ zynq_qspi_write(xqspi,
+ ZYNQ_QSPI_TXD_00_00_OFFSET, 0x00);
+ }
+ xqspi->bytes_to_transfer -= 4;
+ }
+}
+
+/**
+ * zynq_qspi_tx_dual_parallel - Handles odd byte tx for dual parallel
+ *
+ * In dual parallel configuration, when read/write data operations
+ * are performed, odd data bytes have to be converted to even to
+ * avoid a nibble (of data when programming / dummy when reading)
+ * going to individual flash devices, where a byte is expected.
+ * This check is only for data and will not apply for commands.
+ *
+ * @xqspi: Pointer to the zynq_qspi structure
+ * @data: Data to be transmitted
+ * @len: No. of bytes to be transmitted
+ */
+static inline void zynq_qspi_tx_dual_parallel(struct zynq_qspi *xqspi,
+ u32 data, u32 len)
+{
+ len = len % 2 ? len + 1 : len;
+ if (len == 4)
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_00_OFFSET, data);
+ else
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_01_OFFSET +
+ ((len - 1) * 4), data);
+}
+
+/**
+ * zynq_qspi_irq - Interrupt service routine of the QSPI controller
+ * @irq: IRQ number
+ * @dev_id: Pointer to the xqspi structure
+ *
+ * This function handles TX empty only.
+ * On TX empty interrupt this function reads the received data from RX FIFO and
+ * fills the TX FIFO if there is any data remaining to be transferred.
+ *
+ * Return: IRQ_HANDLED when interrupt is handled; IRQ_NONE otherwise.
+ */
+static irqreturn_t zynq_qspi_irq(int irq, void *dev_id)
+{
+ struct spi_master *master = dev_id;
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+ u32 intr_status, rxcount, rxindex = 0;
+ u8 offset[3] = {ZYNQ_QSPI_TXD_00_01_OFFSET, ZYNQ_QSPI_TXD_00_10_OFFSET,
+ ZYNQ_QSPI_TXD_00_11_OFFSET};
+
+ intr_status = zynq_qspi_read(xqspi, ZYNQ_QSPI_STATUS_OFFSET);
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_STATUS_OFFSET, intr_status);
+
+ if ((intr_status & ZYNQ_QSPI_IXR_TXNFULL_MASK) ||
+ (intr_status & ZYNQ_QSPI_IXR_RXNEMTY_MASK)) {
+ /*
+ * This bit is set when Tx FIFO has < THRESHOLD entries.
+ * We have the THRESHOLD value set to 1,
+ * so this bit indicates Tx FIFO is empty.
+ */
+ u32 data;
+
+ rxcount = xqspi->bytes_to_receive - xqspi->bytes_to_transfer;
+ rxcount = (rxcount % 4) ? ((rxcount/4) + 1) : (rxcount/4);
+
+ /* Read out the data from the RX FIFO */
+ while ((rxindex < rxcount) &&
+ (rxindex < ZYNQ_QSPI_RX_THRESHOLD)) {
+ if (xqspi->bytes_to_receive >= 4) {
+ if (xqspi->rxbuf) {
+ (*(u32 *)xqspi->rxbuf) =
+ zynq_qspi_read(xqspi,
+ ZYNQ_QSPI_RXD_OFFSET);
+ xqspi->rxbuf += 4;
+ } else {
+ data = zynq_qspi_read(xqspi,
+ ZYNQ_QSPI_RXD_OFFSET);
+ }
+ xqspi->bytes_to_receive -= 4;
+ } else {
+ data = zynq_qspi_read(xqspi,
+ ZYNQ_QSPI_RXD_OFFSET);
+ zynq_qspi_copy_read_data(xqspi, data,
+ xqspi->bytes_to_receive);
+ }
+ rxindex++;
+ }
+
+ if (xqspi->bytes_to_transfer) {
+ if (xqspi->bytes_to_transfer >= 4) {
+ /* There is more data to send */
+ zynq_qspi_fill_tx_fifo(xqspi,
+ ZYNQ_QSPI_RX_THRESHOLD);
+ } else if (intr_status & ZYNQ_QSPI_IXR_TXNFULL_MASK) {
+ int tmp;
+
+ tmp = xqspi->bytes_to_transfer;
+ zynq_qspi_copy_write_data(xqspi, &data,
+ xqspi->bytes_to_transfer);
+
+ if (!xqspi->is_parallel || xqspi->is_instr)
+ zynq_qspi_write(xqspi,
+ offset[tmp - 1], data);
+ else {
+ zynq_qspi_tx_dual_parallel(xqspi, data,
+ tmp);
+ }
+ }
+ } else {
+ /*
+ * If transfer and receive is completed then only send
+ * complete signal.
+ */
+ if (!xqspi->bytes_to_receive) {
+ zynq_qspi_write(xqspi,
+ ZYNQ_QSPI_IDIS_OFFSET,
+ ZYNQ_QSPI_IXR_ALL_MASK);
+ spi_finalize_current_transfer(master);
+ xqspi->is_instr = 0;
+ }
+ }
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+/**
+ * zynq_qspi_transfer_one - Initiates the QSPI transfer
+ * @qspi: Pointer to the spi_device structure
+ * @transfer: Pointer to the spi_transfer structure which provide information
+ * about next transfer parameters
+ *
+ * This function fills the TX FIFO, starts the QSPI transfer, and waits for the
+ * transfer to be completed.
+ *
+ * Return: Number of bytes transferred in the last transfer
+ */
+static int zynq_qspi_transfer_one(struct spi_master *master,
+ struct spi_device *qspi,
+ struct spi_transfer *transfer)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+ u32 data;
+
+ xqspi->txbuf = transfer->tx_buf;
+ xqspi->rxbuf = transfer->rx_buf;
+ xqspi->bytes_to_transfer = transfer->len;
+ xqspi->bytes_to_receive = transfer->len;
+
+ zynq_qspi_setup_transfer(qspi, transfer);
+
+ if (transfer->len >= 4) {
+ zynq_qspi_fill_tx_fifo(xqspi, ZYNQ_QSPI_FIFO_DEPTH);
+ } else {
+ zynq_qspi_copy_write_data(xqspi, &data, transfer->len);
+
+ if (!xqspi->is_parallel || xqspi->is_instr)
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_01_OFFSET +
+ ((transfer->len - 1) * 4), data);
+ else {
+ zynq_qspi_tx_dual_parallel(xqspi, data, transfer->len);
+ }
+ }
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
+ ZYNQ_QSPI_IXR_ALL_MASK);
+
+ return transfer->len;
+}
+
+/**
+ * zynq_prepare_transfer_hardware - Prepares hardware for transfer.
+ * @master: Pointer to the spi_master structure which provides
+ * information about the controller.
+ *
+ * This function enables SPI master controller.
+ *
+ * Return: Always 0
+ */
+static int zynq_prepare_transfer_hardware(struct spi_master *master)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+ zynq_qspi_config_clock_mode(master->cur_msg->spi);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET,
+ ZYNQ_QSPI_ENABLE_ENABLE_MASK);
+
+ return 0;
+}
+
+/**
+ * zynq_unprepare_transfer_hardware - Relaxes hardware after transfer
+ * @master: Pointer to the spi_master structure which provides
+ * information about the controller.
+ *
+ * This function disables the SPI master controller.
+ *
+ * Return: Always 0
+ */
+static int zynq_unprepare_transfer_hardware(struct spi_master *master)
+{
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+
+ return 0;
+}
+
+/**
+ * zynq_qspi_suspend - Suspend method for the QSPI driver
+ * @_dev: Address of the platform_device structure
+ *
+ * This function stops the QSPI driver queue and disables the QSPI controller
+ *
+ * Return: Always 0
+ */
+static int __maybe_unused zynq_qspi_suspend(struct device *_dev)
+{
+ struct platform_device *pdev = container_of(_dev,
+ struct platform_device, dev);
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+ spi_master_suspend(master);
+
+ clk_disable_unprepare(xqspi->refclk);
+ clk_disable_unprepare(xqspi->pclk);
+
+ return 0;
+}
+
+/**
+ * zynq_qspi_resume - Resume method for the QSPI driver
+ * @dev: Address of the platform_device structure
+ *
+ * The function starts the QSPI driver queue and initializes the QSPI controller
+ *
+ * Return: 0 on success and error value on error
+ */
+static int __maybe_unused zynq_qspi_resume(struct device *dev)
+{
+ struct platform_device *pdev = container_of(dev,
+ struct platform_device, dev);
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+ int ret = 0;
+
+ ret = clk_prepare_enable(xqspi->pclk);
+ if (ret) {
+ dev_err(dev, "Cannot enable APB clock.\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(xqspi->refclk);
+ if (ret) {
+ dev_err(dev, "Cannot enable device clock.\n");
+ clk_disable(xqspi->pclk);
+ return ret;
+ }
+
+ spi_master_resume(master);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(zynq_qspi_dev_pm_ops, zynq_qspi_suspend,
+ zynq_qspi_resume);
+
+/**
+ * zynq_qspi_probe - Probe method for the QSPI driver
+ * @pdev: Pointer to the platform_device structure
+ *
+ * This function initializes the driver data structures and the hardware.
+ *
+ * Return: 0 on success and error value on failure
+ */
+static int zynq_qspi_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct spi_master *master;
+ struct zynq_qspi *xqspi;
+ struct resource *res;
+ u32 num_cs, qspi_mode;
+
+ master = spi_alloc_master(&pdev->dev, sizeof(*xqspi));
+ if (master == NULL)
+ return -ENOMEM;
+
+ xqspi = spi_master_get_devdata(master);
+ master->dev.of_node = pdev->dev.of_node;
+ platform_set_drvdata(pdev, master);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ xqspi->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(xqspi->regs)) {
+ ret = PTR_ERR(xqspi->regs);
+ goto remove_master;
+ }
+
+ if (of_property_read_u32(pdev->dev.of_node, "xlnx,qspi-mode",
+ &qspi_mode) < 0)
+ dev_warn(&pdev->dev,
+ "qspi-mode not found; defaulting to single\n");
+
+ /* Default single mode */
+ xqspi->is_parallel = 0;
+ xqspi->is_stacked = 0;
+ if (qspi_mode == 1)
+ xqspi->is_stacked = 1;
+ else if (qspi_mode == 2)
+ xqspi->is_parallel = 1;
+
+ xqspi->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(xqspi->pclk)) {
+ dev_err(&pdev->dev, "pclk clock not found.\n");
+ ret = PTR_ERR(xqspi->pclk);
+ goto remove_master;
+ }
+
+ xqspi->refclk = devm_clk_get(&pdev->dev, "ref_clk");
+ if (IS_ERR(xqspi->refclk)) {
+ dev_err(&pdev->dev, "ref_clk clock not found.\n");
+ ret = PTR_ERR(xqspi->refclk);
+ goto remove_master;
+ }
+
+ ret = clk_prepare_enable(xqspi->pclk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable APB clock.\n");
+ goto remove_master;
+ }
+
+ ret = clk_prepare_enable(xqspi->refclk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable device clock.\n");
+ goto clk_dis_pclk;
+ }
+
+ /* QSPI controller initializations */
+ zynq_qspi_init_hw(xqspi);
+
+ xqspi->irq = platform_get_irq(pdev, 0);
+ if (xqspi->irq <= 0) {
+ ret = -ENXIO;
+ dev_err(&pdev->dev, "irq resource not found\n");
+ goto remove_master;
+ }
+ ret = devm_request_irq(&pdev->dev, xqspi->irq, zynq_qspi_irq,
+ 0, pdev->name, master);
+ if (ret != 0) {
+ ret = -ENXIO;
+ dev_err(&pdev->dev, "request_irq failed\n");
+ goto remove_master;
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node, "num-cs",
+ &num_cs);
+ if (ret < 0)
+ master->num_chipselect = ZYNQ_QSPI_DEFAULT_NUM_CS;
+ else
+ master->num_chipselect = num_cs;
+
+ master->set_cs = zynq_qspi_chipselect;
+ master->transfer_one = zynq_qspi_transfer_one;
+ master->prepare_transfer_hardware = zynq_prepare_transfer_hardware;
+ master->unprepare_transfer_hardware = zynq_unprepare_transfer_hardware;
+ master->flags = SPI_MASTER_QUAD_MODE;
+
+ master->max_speed_hz = clk_get_rate(xqspi->refclk) / 2;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
+ SPI_TX_DUAL | SPI_TX_QUAD;
+
+ ret = spi_register_master(master);
+ if (ret) {
+ dev_err(&pdev->dev, "spi_register_master failed\n");
+ goto clk_dis_all;
+ }
+
+ return ret;
+
+clk_dis_all:
+ clk_disable_unprepare(xqspi->refclk);
+clk_dis_pclk:
+ clk_disable_unprepare(xqspi->pclk);
+remove_master:
+ spi_master_put(master);
+ return ret;
+}
+
+/**
+ * zynq_qspi_remove - Remove method for the QSPI driver
+ * @pdev: Pointer to the platform_device structure
+ *
+ * This function is called if a device is physically removed from the system or
+ * if the driver module is being unloaded. It frees all resources allocated to
+ * the device.
+ *
+ * Return: 0 on success and error value on failure
+ */
+static int zynq_qspi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+ zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+
+ clk_disable_unprepare(xqspi->refclk);
+ clk_disable_unprepare(xqspi->pclk);
+
+ spi_unregister_master(master);
+
+ return 0;
+}
+
+static const struct of_device_id zynq_qspi_of_match[] = {
+ { .compatible = "xlnx,zynq-qspi-1.0", },
+ { /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, zynq_qspi_of_match);
+
+/*
+ * zynq_qspi_driver - This structure defines the QSPI platform driver
+ */
+static struct platform_driver zynq_qspi_driver = {
+ .probe = zynq_qspi_probe,
+ .remove = zynq_qspi_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = zynq_qspi_of_match,
+ .pm = &zynq_qspi_dev_pm_ops,
+ },
+};
+
+module_platform_driver(zynq_qspi_driver);
+
+MODULE_AUTHOR("Xilinx, Inc.");
+MODULE_DESCRIPTION("Xilinx Zynq QSPI driver");
+MODULE_LICENSE("GPL");
--
1.7.9.5
Add bindings documentation for Zynq QSPI driver.
Signed-off-by: Harini Katakam <[email protected]>
---
.../devicetree/bindings/spi/spi-zynq-qspi.txt | 28 ++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
new file mode 100644
index 0000000..288c551
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
@@ -0,0 +1,28 @@
+Xilinx Zynq QSPI controller Device Tree Bindings
+-------------------------------------------------
+
+Required properties:
+- compatible : Should be "xlnx,zynq-qspi-1.0".
+- reg : Physical base address and size of QSPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+
+Optional properties:
+- num-cs : Number of chip selects used.
+- xlnx,qspi-mode : Single - 0; Dual Stacked - 1; Dual Parallel - 2.
+
+Example:
+ qspi@e000d000 {
+ compatible = "xlnx,zynq-qspi-1.0";
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 10>, <&clkc 43>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 19 4>;
+ num-cs = <1>;
+ xlnx,qspi-mode = <0x0>;
+ reg = <0xe000d000 0x1000>;
+ } ;
--
1.7.9.5
Hi Harini,
On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam <[email protected]> wrote:
> + master->flags = SPI_MASTER_QUAD_MODE;
SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines
in include/linux/spi/spi.h?
> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
> + SPI_TX_DUAL | SPI_TX_QUAD;
Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Thu, Jul 10, 2014 at 2:48 PM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam <[email protected]> wrote:
>> + master->flags = SPI_MASTER_QUAD_MODE;
>
> SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines
> in include/linux/spi/spi.h?
>
I'm sorry about that. That flag is unused - will remove this statement.
>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>> + SPI_TX_DUAL | SPI_TX_QUAD;
>
> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>
Here the driver is just giving information that the controller support it.
The MTD layer enables dual/quad based on what the flash supports; quad
being the first priority
I understand that the spi core reads rx, tx-bus-width property and
master support flags and
performs the necessary checks.
Regards,
Harini
Hi Harini,
On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
<[email protected]> wrote:
>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>> + SPI_TX_DUAL | SPI_TX_QUAD;
>>
>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>
> Here the driver is just giving information that the controller support it.
> The MTD layer enables dual/quad based on what the flash supports; quad
> being the first priority
> I understand that the spi core reads rx, tx-bus-width property and
> master support flags and
> performs the necessary checks.
That's correct: as long as the rx, tx-bus-width properties do not indicate a
Dual or Quad wiring, it won't be used.
However, based on schematics, someone may set the rx, tx-bus-width properties
to 4, which is correct, as DT describes the hardware. But this will fail to
work.
So I think it's safer not to announce Dual/Quad support in the driver until
the actual driver support is there.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
HI Greet,
>-----Original Message-----
>From: Harini Katakam [mailto:[email protected]]
>Sent: Thursday, July 10, 2014 3:01 PM
>To: Geert Uytterhoeven
>Cc: Mark Brown; Grant Likely; Rob Herring; Pawel Moll; Mark Rutland; Ian
>Campbell; Kumar Gala; linux-spi; [email protected];
>[email protected]; [email protected]; David Woodhouse;
>Brian Norris; Marek Vašut; Artem Bityutskiy; Geert Uytterhoeven; Sascha
>Hauer; Jingoo Han; Sourav Poddar; Michal Simek; Punnaiah Choudary Kalluri
>Subject: Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
>
>Hi Geert,
>
>On Thu, Jul 10, 2014 at 2:48 PM, Geert Uytterhoeven
><[email protected]> wrote:
>> Hi Harini,
>>
>> On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam <[email protected]>
>wrote:
>>> + master->flags = SPI_MASTER_QUAD_MODE;
>>
>> SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines
>> in include/linux/spi/spi.h?
>>
>
>I'm sorry about that. That flag is unused - will remove this statement.
>
>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL |
>SPI_RX_QUAD |
>>> + SPI_TX_DUAL | SPI_TX_QUAD;
>>
>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>> check spi_transfer.[tr]x_nbits? How can it determine when to enable
>Dual/Quad?
>>
>
>Here the driver is just giving information that the controller support it.
>The MTD layer enables dual/quad based on what the flash supports; quad
>being the first priority
>I understand that the spi core reads rx, tx-bus-width property and
>master support flags and
>performs the necessary checks.
Just to add, the zynq qspi controller will automatically select the IO lines based
On the flash command.
Regards,
Punnaiah
>
>Regards,
>Harini
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Hi Geert,
On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
> <[email protected]> wrote:
>>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>> + SPI_TX_DUAL | SPI_TX_QUAD;
>>>
>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>
>> Here the driver is just giving information that the controller support it.
>> The MTD layer enables dual/quad based on what the flash supports; quad
>> being the first priority
>> I understand that the spi core reads rx, tx-bus-width property and
>> master support flags and
>> performs the necessary checks.
>
> That's correct: as long as the rx, tx-bus-width properties do not indicate a
> Dual or Quad wiring, it won't be used.
>
> However, based on schematics, someone may set the rx, tx-bus-width properties
> to 4, which is correct, as DT describes the hardware. But this will fail to
> work.
> So I think it's safer not to announce Dual/Quad support in the driver until
> the actual driver support is there.
>
OK. Correct me if I'm wrong but announcing this support in master->flags is
just to say the controller supports it - Like Punnaiah mentioned in the other
mail, nothing specific needs to be done from the controller driver to enable
dual/quad support. This is at the SOC/IP level.
I agree it might or might not be supported at board-level.
But that's based on the user's hardware. Should master->flags
really take this into consideration?
BTW, I dint see master->mode_bits being used anywhere at the moment.
Regards,
Harini
Hi Harini,
On Thu, Jul 10, 2014 at 12:33 PM, Harini Katakam
<[email protected]> wrote:
> On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
> <[email protected]> wrote:
>> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
>> <[email protected]> wrote:
>>>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>>> + SPI_TX_DUAL | SPI_TX_QUAD;
>>>>
>>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>>
>>> Here the driver is just giving information that the controller support it.
>>> The MTD layer enables dual/quad based on what the flash supports; quad
>>> being the first priority
>>> I understand that the spi core reads rx, tx-bus-width property and
>>> master support flags and
>>> performs the necessary checks.
>>
>> That's correct: as long as the rx, tx-bus-width properties do not indicate a
>> Dual or Quad wiring, it won't be used.
>>
>> However, based on schematics, someone may set the rx, tx-bus-width properties
>> to 4, which is correct, as DT describes the hardware. But this will fail to
>> work.
>> So I think it's safer not to announce Dual/Quad support in the driver until
>> the actual driver support is there.
>
> OK. Correct me if I'm wrong but announcing this support in master->flags is
> just to say the controller supports it - Like Punnaiah mentioned in the other
> mail, nothing specific needs to be done from the controller driver to enable
> dual/quad support. This is at the SOC/IP level.
> I agree it might or might not be supported at board-level.
IC. So this is not a generic SPI controller, but a controller meant for QSPI
FLASHes? I.e. if you would connect a different device, the controller may
unexpectedly use Dual or Quad mode if it sees a byte fly by that looks like
a Quad SPI FLASH read command?
> But that's based on the user's hardware. Should master->flags
> really take this into consideration?
You mean master->mode_bits?
> BTW, I dint see master->mode_bits being used anywhere at the moment.
It is used to match SPI controller and slave features, cfr. spi_setup() in
drivers/spi/spi.c.
If Dual/Quad is supported, the bits should be set. Else spi_setup() will
clear the bits in the SPI slave's mode field, disabling Dual/Quad transfers.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Thu, Jul 10, 2014 at 4:55 PM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 12:33 PM, Harini Katakam
> <[email protected]> wrote:
>> On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
>> <[email protected]> wrote:
>>> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
>>> <[email protected]> wrote:
>>>>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>>>> + SPI_TX_DUAL | SPI_TX_QUAD;
>>>>>
>>>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>>>
>>>> Here the driver is just giving information that the controller support it.
>>>> The MTD layer enables dual/quad based on what the flash supports; quad
>>>> being the first priority
>>>> I understand that the spi core reads rx, tx-bus-width property and
>>>> master support flags and
>>>> performs the necessary checks.
>>>
>>> That's correct: as long as the rx, tx-bus-width properties do not indicate a
>>> Dual or Quad wiring, it won't be used.
>>>
>>> However, based on schematics, someone may set the rx, tx-bus-width properties
>>> to 4, which is correct, as DT describes the hardware. But this will fail to
>>> work.
>>> So I think it's safer not to announce Dual/Quad support in the driver until
>>> the actual driver support is there.
>>
>> OK. Correct me if I'm wrong but announcing this support in master->flags is
>> just to say the controller supports it - Like Punnaiah mentioned in the other
>> mail, nothing specific needs to be done from the controller driver to enable
>> dual/quad support. This is at the SOC/IP level.
>> I agree it might or might not be supported at board-level.
>
> IC. So this is not a generic SPI controller, but a controller meant for QSPI
> FLASHes? I.e. if you would connect a different device, the controller may
> unexpectedly use Dual or Quad mode if it sees a byte fly by that looks like
> a Quad SPI FLASH read command?
>
Yes. It would.
>> But that's based on the user's hardware. Should master->flags
>> really take this into consideration?
>
> You mean master->mode_bits?
Yes, i mean mode_bits. That was typo.
>
>> BTW, I dint see master->mode_bits being used anywhere at the moment.
>
> It is used to match SPI controller and slave features, cfr. spi_setup() in
> drivers/spi/spi.c.
>
> If Dual/Quad is supported, the bits should be set. Else spi_setup() will
> clear the bits in the SPI slave's mode field, disabling Dual/Quad transfers.
>
OK.
Regards,
Harini
On Thu, Jul 10, 2014 at 04:03:19PM +0530, Harini Katakam wrote:
> OK. Correct me if I'm wrong but announcing this support in master->flags is
> just to say the controller supports it - Like Punnaiah mentioned in the other
No, it's broken to set this if there is no ability to use it.
> mail, nothing specific needs to be done from the controller driver to enable
> dual/quad support. This is at the SOC/IP level.
How does the client driver select the width to use for a transfer?
Hi Mark,
On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown <[email protected]> wrote:
> On Thu, Jul 10, 2014 at 04:03:19PM +0530, Harini Katakam wrote:
>
>> OK. Correct me if I'm wrong but announcing this support in master->flags is
>> just to say the controller supports it - Like Punnaiah mentioned in the other
>
> No, it's broken to set this if there is no ability to use it.
>
>> mail, nothing specific needs to be done from the controller driver to enable
>> dual/quad support. This is at the SOC/IP level.
>
> How does the client driver select the width to use for a transfer?
This controller is meant to be used only with flash devices.
The flash devices' supported width will be reflected in a table in MTD layer.
When selecting, priority is given to quad over dual and single in the MTD and
it will send commands using the supported tx/rx bus width accordingly.
About the supported bus width on board, tx-bus-width and rx-bus-width
properties in dts will have the info; which I believe spi core uses.
Regards,
Harini
On Thu, Jul 10, 2014 at 02:20:06PM +0530, Harini Katakam wrote:
> This patch adds support for QSPI controller used by Zynq.
The driver looks pretty clean but there are a couple of issues below,
including a little bit more of the flash specifics.
> +static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
> +{
> + struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
> + u32 config_reg;
> +
> + config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
> +
> + /* Select upper/lower page before asserting CS */
> + if (xqspi->is_stacked) {
> + u32 lqspi_cfg_reg;
Like with the dual and quad mode stuff this looks very much like it's
specific to flash rather than something that applies to a generic SPI
driver. However it does look like it's a generic SPI device which could
be used in other applications which makes things a bit tricky. We don't
have a really good answer for this right now unfortunately, probably we
need some sort of special interface between the SPI and flash subsystems
to allow flash to use the flash specific stuff.
For use as a generic SPI device what I'd suggest is stripping out the
flash specifics, merging the rest of the support and then considering
the flash specifics separately.
> +/**
> + * zynq_prepare_transfer_hardware - Prepares hardware for transfer.
> + * @master: Pointer to the spi_master structure which provides
> + * information about the controller.
> + *
> + * This function enables SPI master controller.
> + *
> + * Return: Always 0
> + */
> +static int zynq_prepare_transfer_hardware(struct spi_master *master)
> +{
> + struct zynq_qspi *xqspi = spi_master_get_devdata(master);
> +
> + zynq_qspi_config_clock_mode(master->cur_msg->spi);
The clock mode needs to be (and is) configured per transfer so I'd
expect it's possible to remove this call.
> + ret = clk_prepare_enable(xqspi->refclk);
> + if (ret) {
> + dev_err(dev, "Cannot enable device clock.\n");
It's better to display the error code.
> + clk_disable(xqspi->pclk);
This needs to be disable_unprepare().
> +static SIMPLE_DEV_PM_OPS(zynq_qspi_dev_pm_ops, zynq_qspi_suspend,
> + zynq_qspi_resume);
It would be better to also implement runtime PM support to disable the
clocks while the device is idle as well, that will save a small amount
of power while the device isn't doing anything.
Hi Mark,
My mail seems to have missed the below reply.
Anyway, please find my answer below:
> -----Original Message-----
> From: Punnaiah Choudary Kalluri
> Sent: Monday, July 14, 2014 12:03 PM
> To: Harini Katakam
> Subject: FW: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
>
>
>
> >-----Original Message-----
> >From: Mark Brown [mailto:[email protected]]
> >Sent: Thursday, July 10, 2014 8:37 PM
> >To: Harini Katakam
> >Cc: Geert Uytterhoeven; Grant Likely; Rob Herring; Pawel Moll; Mark
> Rutland;
> >Ian Campbell; Kumar Gala; linux-spi; [email protected];
> >[email protected]; [email protected]; David
> Woodhouse;
> >Brian Norris; Marek Va?ut; Artem Bityutskiy; Geert Uytterhoeven; Sascha
> >Hauer; Jingoo Han; Sourav Poddar; Michal Simek; Punnaiah Choudary
> Kalluri
> >Subject: Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
> >
> >On Thu, Jul 10, 2014 at 06:09:55PM +0530, Harini Katakam wrote:
> >> On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown <[email protected]>
> wrote:
> >
> >> > How does the client driver select the width to use for a transfer?
> >
> >> This controller is meant to be used only with flash devices.
> >> The flash devices' supported width will be reflected in a table in MTD
> layer.
> >> When selecting, priority is given to quad over dual and single in the MTD
> and
> >> it will send commands using the supported tx/rx bus width accordingly.
> >> About the supported bus width on board, tx-bus-width and rx-bus-width
> >> properties in dts will have the info; which I believe spi core uses.
> >
> >If it's only intended to be used as a flash controller (and might
> >misbehave if used as such, if the command detection false triggers) then
> >it is probably better to support it as a flash controller rather than as
> >a SPI controller. Or can the flash-specific features be disabled?
There is provision to switch to legacy (generic spi) mode but it is not usually used.
As you can can see, the flash related specifics come into play only when two flash devices
are used. For a single slave, it will be generic.
As per your suggestions, I could send this driver in multiple stages -
Initially, qspi driver without flash specifics (this will work straight-away for a single flash)
In the next set, flash specifics changes for dual flash configurations (parallel/stacked)
Is that acceptable?
Regards,
Harini
Hi Mark,
On Fri, Jul 11, 2014 at 7:08 PM, Mark Brown <[email protected]> wrote:
> On Thu, Jul 10, 2014 at 02:20:06PM +0530, Harini Katakam wrote:
>
>> This patch adds support for QSPI controller used by Zynq.
>
> The driver looks pretty clean but there are a couple of issues below,
> including a little bit more of the flash specifics.
>
>> +static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
>> +{
>> + struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
>> + u32 config_reg;
>> +
>> + config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
>> +
>> + /* Select upper/lower page before asserting CS */
>> + if (xqspi->is_stacked) {
>> + u32 lqspi_cfg_reg;
>
> Like with the dual and quad mode stuff this looks very much like it's
> specific to flash rather than something that applies to a generic SPI
> driver. However it does look like it's a generic SPI device which could
> be used in other applications which makes things a bit tricky. We don't
> have a really good answer for this right now unfortunately, probably we
> need some sort of special interface between the SPI and flash subsystems
> to allow flash to use the flash specific stuff.
>
> For use as a generic SPI device what I'd suggest is stripping out the
> flash specifics, merging the rest of the support and then considering
> the flash specifics separately.
Reply in the other thread.
>
>> +/**
>> + * zynq_prepare_transfer_hardware - Prepares hardware for transfer.
>> + * @master: Pointer to the spi_master structure which provides
>> + * information about the controller.
>> + *
>> + * This function enables SPI master controller.
>> + *
>> + * Return: Always 0
>> + */
>> +static int zynq_prepare_transfer_hardware(struct spi_master *master)
>> +{
>> + struct zynq_qspi *xqspi = spi_master_get_devdata(master);
>> +
>> + zynq_qspi_config_clock_mode(master->cur_msg->spi);
>
> The clock mode needs to be (and is) configured per transfer so I'd
> expect it's possible to remove this call.
>
Yeah I understand. It can go away from here and there should be a
prepare_message as per Lars-Peter's patches on spi-cadence.
I'll reflect the same in this driver.
Regards,
Harini
On Mon, Jul 14, 2014 at 07:22:24AM +0000, Harini Katakam wrote:
> As per your suggestions, I could send this driver in multiple stages -
> Initially, qspi driver without flash specifics (this will work straight-away for a single flash)
> In the next set, flash specifics changes for dual flash configurations (parallel/stacked)
> Is that acceptable?
Yes, that should be fine.