* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-21 9:20 ` [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller Marek Vasut
@ 2015-08-25 22:09 ` vikas
2015-08-26 6:19 ` Marek Vasut
2015-08-31 17:30 ` Graham Moore
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: vikas @ 2015-08-25 22:09 UTC (permalink / raw)
To: Marek Vasut, linux-mtd@lists.infradead.org
Cc: Graham Moore, Alan Tull, Brian Norris, David Woodhouse,
Dinh Nguyen, Yves Vandervennet, devicetree@vger.kernel.org
Hi,
On 08/21/2015 02:20 AM, Marek Vasut wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
>
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Graham Moore <grmoore@opensource.altera.com>
> Cc: Vikas MANOCHA <vikas.manocha@st.com>
> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> Cc: devicetree@vger.kernel.org
> ---
> drivers/mtd/spi-nor/Kconfig | 11 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/cadence-quadspi.c | 1260 +++++++++++++++++++++++++++++++++
> 3 files changed, 1272 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
>
> V2: use NULL instead of modalias in spi_nor_scan call
> V3: Use existing property is-decoded-cs instead of creating duplicate.
> V4: Support Micron quad mode by snooping command stream for EVCR command
> and subsequently configuring Cadence controller for quad mode.
> V5: Clean up sparse and smatch complaints. Remove snooping of Micron
> quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
> up SRAM partition at 1:1 during init.
> V6: Remove dts patch that was included by mistake. Incorporate Vikas's
> comments regarding fifo width, SRAM partition setting, and trigger
> address. Trigger address was added as an unsigned int, as it is not
> an IO resource per se, and does not need to be mapped. Also add
> Marek Vasut's workaround for picking up OF properties on subnodes.
> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> macros and replace them with functions. Get rid of unused variables.
> - Implement support for nor->set_protocol() to handle Quad-command,
> this patch now depends on the following patch:
> mtd: spi-nor: notify (Q)SPI controller about protocol change
> - Replace that cqspi_fifo_read() disaster with plain old readsl()
> and cqspi_fifo_write() tentacle horror with pretty writesl().
> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> - Get rid of cqspi_find_chipselect() mess, instead just place the
> struct cqspi_st and chipselect number into struct cqspi_flash_pdata
> and set nor->priv to the struct cqspi_flash_pdata of that particular
> chip.
> - Replace the odd math in calculate_ticks_for_ns() with DIV_ROUND_UP().
> - Make variables const where applicable.
> V8: - Implement a function to wait for bit being set/unset for a given
> period of time and use it to replace the ad-hoc bits of code.
> - Configure the write underflow watermark to be 1/8 if FIFO size.
> - Extract out the SPI NOR flash probing code into separate function
> to clearly mark what will soon be considered a boilerplate code.
> - Repair the handling of mode bits, which caused instability in V7.
> - Clean up the interrupt handling
> - Fix Kconfig help text and make the patch depend on OF and COMPILE_TEST.
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89bf4c1..ed253a2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> Flash. Enable this option if you have a device with a SPIFI
> controller and want to access the Flash as a mtd device.
>
> +config SPI_CADENCE_QUADSPI
> + tristate "Cadence Quad SPI controller"
> + depends on OF && COMPILE_TEST
> + help
> + Enable support for the Cadence Quad SPI Flash controller.
> +
> + Cadence QSPI is a specialized controller for connecting an SPI
> + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> + device with a Cadence QSPI controller and want to access the
> + Flash as an MTD device.
> +
the patch failed to apply, please rebase it to master.
Cheers,
Vikas
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e53333e..446c6b9 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> +obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> new file mode 100644
> index 0000000..8e024b8
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -0,0 +1,1260 @@
> +/*
> + * Driver for Cadence QSPI Controller
> + *
> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/spi/spi.h>
> +#include <linux/timer.h>
> +
> +#define CQSPI_NAME "cadence-qspi"
> +#define CQSPI_MAX_CHIPSELECT 16
> +
> +struct cqspi_st;
> +
> +struct cqspi_flash_pdata {
> + struct mtd_info mtd;
> + struct spi_nor nor;
> + struct cqspi_st *cqspi;
> + u32 clk_rate;
> + u32 read_delay;
> + u32 tshsl_ns;
> + u32 tsd2d_ns;
> + u32 tchsh_ns;
> + u32 tslch_ns;
> + u8 inst_width;
> + u8 addr_width;
> + u8 cs;
> +};
> +
> +struct cqspi_st {
> + struct platform_device *pdev;
> +
> + struct clk *clk;
> + unsigned int sclk;
> +
> + void __iomem *iobase;
> + void __iomem *ahb_base;
> + struct completion transfer_complete;
> +
> + int current_cs;
> + unsigned long master_ref_clk_hz;
> + bool is_decoded_cs;
> + u32 fifo_depth;
> + u32 fifo_width;
> + u32 trigger_address;
> + struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> +};
> +
> +/* Operation timeout value */
> +#define CQSPI_TIMEOUT_MS 500
> +#define CQSPI_READ_TIMEOUT_MS 10
> +
> +/* Instruction type */
> +#define CQSPI_INST_TYPE_SINGLE 0
> +#define CQSPI_INST_TYPE_DUAL 1
> +#define CQSPI_INST_TYPE_QUAD 2
> +
> +#define CQSPI_DUMMY_CLKS_PER_BYTE 8
> +#define CQSPI_DUMMY_BYTES_MAX 4
> +#define CQSPI_DUMMY_CLKS_MAX 31
> +
> +#define CQSPI_STIG_DATA_LEN_MAX 8
> +
> +/* Register map */
> +#define CQSPI_REG_CONFIG 0x00
> +#define CQSPI_REG_CONFIG_ENABLE_MASK BIT(0)
> +#define CQSPI_REG_CONFIG_DECODE_MASK BIT(9)
> +#define CQSPI_REG_CONFIG_CHIPSELECT_LSB 10
> +#define CQSPI_REG_CONFIG_DMA_MASK BIT(15)
> +#define CQSPI_REG_CONFIG_BAUD_LSB 19
> +#define CQSPI_REG_CONFIG_IDLE_LSB 31
> +#define CQSPI_REG_CONFIG_CHIPSELECT_MASK 0xF
> +#define CQSPI_REG_CONFIG_BAUD_MASK 0xF
> +
> +#define CQSPI_REG_RD_INSTR 0x04
> +#define CQSPI_REG_RD_INSTR_OPCODE_LSB 0
> +#define CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB 8
> +#define CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB 12
> +#define CQSPI_REG_RD_INSTR_TYPE_DATA_LSB 16
> +#define CQSPI_REG_RD_INSTR_MODE_EN_LSB 20
> +#define CQSPI_REG_RD_INSTR_DUMMY_LSB 24
> +#define CQSPI_REG_RD_INSTR_TYPE_INSTR_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_TYPE_ADDR_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_TYPE_DATA_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_DUMMY_MASK 0x1F
> +
> +#define CQSPI_REG_WR_INSTR 0x08
> +#define CQSPI_REG_WR_INSTR_OPCODE_LSB 0
> +#define CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB 12
> +#define CQSPI_REG_WR_INSTR_TYPE_DATA_LSB 16
> +
> +#define CQSPI_REG_DELAY 0x0C
> +#define CQSPI_REG_DELAY_TSLCH_LSB 0
> +#define CQSPI_REG_DELAY_TCHSH_LSB 8
> +#define CQSPI_REG_DELAY_TSD2D_LSB 16
> +#define CQSPI_REG_DELAY_TSHSL_LSB 24
> +#define CQSPI_REG_DELAY_TSLCH_MASK 0xFF
> +#define CQSPI_REG_DELAY_TCHSH_MASK 0xFF
> +#define CQSPI_REG_DELAY_TSD2D_MASK 0xFF
> +#define CQSPI_REG_DELAY_TSHSL_MASK 0xFF
> +
> +#define CQSPI_REG_READCAPTURE 0x10
> +#define CQSPI_REG_READCAPTURE_BYPASS_LSB 0
> +#define CQSPI_REG_READCAPTURE_DELAY_LSB 1
> +#define CQSPI_REG_READCAPTURE_DELAY_MASK 0xF
> +
> +#define CQSPI_REG_SIZE 0x14
> +#define CQSPI_REG_SIZE_ADDRESS_LSB 0
> +#define CQSPI_REG_SIZE_PAGE_LSB 4
> +#define CQSPI_REG_SIZE_BLOCK_LSB 16
> +#define CQSPI_REG_SIZE_ADDRESS_MASK 0xF
> +#define CQSPI_REG_SIZE_PAGE_MASK 0xFFF
> +#define CQSPI_REG_SIZE_BLOCK_MASK 0x3F
> +
> +#define CQSPI_REG_SRAMPARTITION 0x18
> +#define CQSPI_REG_INDIRECTTRIGGER 0x1C
> +
> +#define CQSPI_REG_DMA 0x20
> +#define CQSPI_REG_DMA_SINGLE_LSB 0
> +#define CQSPI_REG_DMA_BURST_LSB 8
> +#define CQSPI_REG_DMA_SINGLE_MASK 0xFF
> +#define CQSPI_REG_DMA_BURST_MASK 0xFF
> +
> +#define CQSPI_REG_REMAP 0x24
> +#define CQSPI_REG_MODE_BIT 0x28
> +
> +#define CQSPI_REG_SDRAMLEVEL 0x2C
> +#define CQSPI_REG_SDRAMLEVEL_RD_LSB 0
> +#define CQSPI_REG_SDRAMLEVEL_WR_LSB 16
> +#define CQSPI_REG_SDRAMLEVEL_RD_MASK 0xFFFF
> +#define CQSPI_REG_SDRAMLEVEL_WR_MASK 0xFFFF
> +
> +#define CQSPI_REG_IRQSTATUS 0x40
> +#define CQSPI_REG_IRQMASK 0x44
> +
> +#define CQSPI_REG_INDIRECTRD 0x60
> +#define CQSPI_REG_INDIRECTRD_START_MASK BIT(0)
> +#define CQSPI_REG_INDIRECTRD_CANCEL_MASK BIT(1)
> +#define CQSPI_REG_INDIRECTRD_DONE_MASK BIT(5)
> +
> +#define CQSPI_REG_INDIRECTRDWATERMARK 0x64
> +#define CQSPI_REG_INDIRECTRDSTARTADDR 0x68
> +#define CQSPI_REG_INDIRECTRDBYTES 0x6C
> +
> +#define CQSPI_REG_CMDCTRL 0x90
> +#define CQSPI_REG_CMDCTRL_EXECUTE_MASK BIT(0)
> +#define CQSPI_REG_CMDCTRL_INPROGRESS_MASK BIT(1)
> +#define CQSPI_REG_CMDCTRL_WR_BYTES_LSB 12
> +#define CQSPI_REG_CMDCTRL_WR_EN_LSB 15
> +#define CQSPI_REG_CMDCTRL_ADD_BYTES_LSB 16
> +#define CQSPI_REG_CMDCTRL_ADDR_EN_LSB 19
> +#define CQSPI_REG_CMDCTRL_RD_BYTES_LSB 20
> +#define CQSPI_REG_CMDCTRL_RD_EN_LSB 23
> +#define CQSPI_REG_CMDCTRL_OPCODE_LSB 24
> +#define CQSPI_REG_CMDCTRL_WR_BYTES_MASK 0x7
> +#define CQSPI_REG_CMDCTRL_ADD_BYTES_MASK 0x3
> +#define CQSPI_REG_CMDCTRL_RD_BYTES_MASK 0x7
> +
> +#define CQSPI_REG_INDIRECTWR 0x70
> +#define CQSPI_REG_INDIRECTWR_START_MASK BIT(0)
> +#define CQSPI_REG_INDIRECTWR_CANCEL_MASK BIT(1)
> +#define CQSPI_REG_INDIRECTWR_DONE_MASK BIT(5)
> +
> +#define CQSPI_REG_INDIRECTWRWATERMARK 0x74
> +#define CQSPI_REG_INDIRECTWRSTARTADDR 0x78
> +#define CQSPI_REG_INDIRECTWRBYTES 0x7C
> +
> +#define CQSPI_REG_CMDADDRESS 0x94
> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
> +
> +/* Interrupt status bits */
> +#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
> +#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
> +#define CQSPI_REG_IRQ_IND_COMP BIT(2)
> +#define CQSPI_REG_IRQ_IND_RD_REJECT BIT(3)
> +#define CQSPI_REG_IRQ_WR_PROTECTED_ERR BIT(4)
> +#define CQSPI_REG_IRQ_ILLEGAL_AHB_ERR BIT(5)
> +#define CQSPI_REG_IRQ_WATERMARK BIT(6)
> +#define CQSPI_REG_IRQ_IND_RD_OVERFLOW BIT(12)
> +
> +#define CQSPI_IRQ_MASK_RD (CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_IND_RD_OVERFLOW | \
> + CQSPI_REG_IRQ_IND_COMP)
> +
> +#define CQSPI_IRQ_MASK_WR (CQSPI_REG_IRQ_IND_COMP | \
> + CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_UNDERFLOW)
> +
> +#define CQSPI_IRQ_STATUS_MASK 0x1FFFF
> +
> +static int cqspi_wait_for_bit(void __iomem *reg, const u32 mask, bool clear)
> +{
> + unsigned long end = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + u32 val;
> +
> + while (1) {
> + val = readl(reg);
> + if (clear)
> + val = ~val;
> + val &= mask;
> +
> + if (val == mask)
> + return 0;
> +
> + if (time_after(jiffies, end))
> + return -ETIMEDOUT;
> + }
> +}
> +
> +static bool cqspi_is_idle(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
> +
> + return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
> +}
> +
> +static u32 cqspi_get_rd_sram_level(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_SDRAMLEVEL);
> +
> + reg >>= CQSPI_REG_SDRAMLEVEL_RD_LSB;
> + return reg & CQSPI_REG_SDRAMLEVEL_RD_MASK;
> +}
> +
> +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
> +{
> + struct cqspi_st *cqspi = dev;
> + unsigned int irq_status;
> +
> + /* Read interrupt status */
> + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + /* Clear interrupt */
> + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
> +
> + if (irq_status)
> + complete(&cqspi->transfer_complete);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static unsigned int cqspi_calc_rdreg(struct spi_nor *nor, const u8 opcode)
> +{
> + unsigned int rdreg = 0;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + rdreg |= f_pdata->inst_width << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB;
> + rdreg |= f_pdata->addr_width << CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB;
> +
> + if (nor->flash_read == SPI_NOR_QUAD)
> + rdreg |= CQSPI_INST_TYPE_QUAD
> + << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
> + return rdreg;
> +}
> +
> +static int cqspi_wait_idle(struct cqspi_st *cqspi)
> +{
> + const unsigned int poll_idle_retry = 3;
> + unsigned int count = 0;
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + while (1) {
> + /*
> + * Read few times in succession to ensure the controller
> + * is indeed idle, that is, the bit does not transition
> + * low again.
> + */
> + if (cqspi_is_idle(cqspi))
> + count++;
> + else
> + count = 0;
> +
> + if (count >= poll_idle_retry)
> + return 0;
> +
> + if (time_after(jiffies, timeout)) {
> + /* Timeout, in busy mode. */
> + dev_err(&cqspi->pdev->dev,
> + "QSPI is still busy after %dms timeout.\n",
> + CQSPI_TIMEOUT_MS);
> + return -ETIMEDOUT;
> + }
> +
> + cpu_relax();
> + }
> +}
> +
> +static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + int ret;
> +
> + /* Write the CMDCTRL without start execution. */
> + writel(reg, reg_base + CQSPI_REG_CMDCTRL);
> + /* Start execute */
> + reg |= CQSPI_REG_CMDCTRL_EXECUTE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CMDCTRL);
> +
> + /* Polling for completion. */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_CMDCTRL,
> + CQSPI_REG_CMDCTRL_INPROGRESS_MASK, 1);
> + if (ret) {
> + dev_err(&cqspi->pdev->dev,
> + "Flash command execution timed out.\n");
> + return ret;
> + }
> +
> + /* Polling QSPI idle status. */
> + return cqspi_wait_idle(cqspi);
> +}
> +
> +static int cqspi_command_read(struct spi_nor *nor,
> + const u8 *txbuf, const unsigned n_tx,
> + u8 *rxbuf, const unsigned n_rx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int rdreg;
> + unsigned int reg;
> + unsigned int read_len;
> + int status;
> +
> + if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
> + dev_err(nor->dev, "Invalid input argument, len %d rxbuf 0x%p\n",
> + n_rx, rxbuf);
> + return -EINVAL;
> + }
> +
> + reg = txbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> +
> + rdreg = cqspi_calc_rdreg(nor, txbuf[0]);
> + writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
> +
> + /* 0 means 1 byte. */
> + reg |= (((n_rx - 1) & CQSPI_REG_CMDCTRL_RD_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_RD_BYTES_LSB);
> + status = cqspi_exec_flash_cmd(cqspi, reg);
> + if (status)
> + return status;
> +
> + reg = readl(reg_base + CQSPI_REG_CMDREADDATALOWER);
> +
> + /* Put the read value into rx_buf */
> + read_len = (n_rx > 4) ? 4 : n_rx;
> + memcpy(rxbuf, ®, read_len);
> + rxbuf += read_len;
> +
> + if (n_rx > 4) {
> + reg = readl(reg_base + CQSPI_REG_CMDREADDATAUPPER);
> +
> + read_len = n_rx - read_len;
> + memcpy(rxbuf, ®, read_len);
> + }
> +
> + return 0;
> +}
> +
> +static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
> + const u8 *txbuf, const unsigned n_tx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> + unsigned int data;
> + int ret;
> +
> + if (n_tx > 4 || (n_tx && !txbuf)) {
> + dev_err(nor->dev,
> + "Invalid input argument, cmdlen %d txbuf 0x%p\n",
> + n_tx, txbuf);
> + return -EINVAL;
> + }
> +
> + reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> + if (n_tx) {
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB);
> + reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
> + data = 0;
> + memcpy(&data, txbuf, n_tx);
> + writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
> + }
> +
> + ret = cqspi_exec_flash_cmd(cqspi, reg);
> + return ret;
> +}
> +
> +static int cqspi_command_write_addr(struct spi_nor *nor,
> + const u8 opcode, const unsigned int addr)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
> + reg |= ((nor->addr_width - 1) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
> +
> + writel(addr, reg_base + CQSPI_REG_CMDADDRESS);
> +
> + return cqspi_exec_flash_cmd(cqspi, reg);
> +}
> +
> +static int cqspi_indirect_read_setup(struct spi_nor *nor,
> + const unsigned int from_addr)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int dummy_clk = 0;
> + unsigned int reg;
> +
> + writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
> +
> + reg = nor->read_opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
> + reg |= cqspi_calc_rdreg(nor, nor->read_opcode);
> +
> + /* Setup dummy clock cycles */
> + dummy_clk = nor->read_dummy;
> + if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
> + dummy_clk = CQSPI_DUMMY_CLKS_MAX;
> +
> + if (dummy_clk / 8) {
> + reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
> + /* Set mode bits high to ensure chip doesn't enter XIP */
> + writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
> +
> + /* Need to subtract the mode byte (8 clocks). */
> + if (f_pdata->inst_width != CQSPI_INST_TYPE_QUAD)
> + dummy_clk -= 8;
> +
> + if (dummy_clk)
> + reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
> + << CQSPI_REG_RD_INSTR_DUMMY_LSB;
> + }
> +
> + writel(reg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + /* Set address width */
> + reg = readl(reg_base + CQSPI_REG_SIZE);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->addr_width - 1);
> + writel(reg, reg_base + CQSPI_REG_SIZE);
> + return 0;
> +}
> +
> +static int cqspi_indirect_read_execute(struct spi_nor *nor,
> + u8 *rxbuf, const unsigned n_rx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + void __iomem *ahb_base = cqspi->ahb_base;
> + unsigned int remaining = n_rx;
> + unsigned int bytes_to_read = 0;
> + int ret = 0;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> +
> + while (remaining > 0) {
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_READ_TIMEOUT_MS));
> +
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> +
> + if (!ret && bytes_to_read == 0) {
> + dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> + ret = -ETIMEDOUT;
> + goto failrd;
> + }
> +
> + while (bytes_to_read != 0) {
> + bytes_to_read *= cqspi->fifo_width;
> + bytes_to_read = bytes_to_read > remaining ?
> + remaining : bytes_to_read;
> + readsl(ahb_base, rxbuf, DIV_ROUND_UP(bytes_to_read, 4));
> + rxbuf += bytes_to_read;
> + remaining -= bytes_to_read;
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> + }
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
> + CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect read completion error (%i)\n", ret);
> + goto failrd;
> + }
> +
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTRD_DONE_MASK, reg_base + CQSPI_REG_INDIRECTRD);
> +
> + return 0;
> +
> +failrd:
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect read */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> + return ret;
> +}
> +
> +static int cqspi_indirect_write_setup(struct spi_nor *nor,
> + const unsigned int to_addr)
> +{
> + unsigned int reg;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> +
> + /* Set opcode. */
> + reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
> + writel(reg, reg_base + CQSPI_REG_WR_INSTR);
> + reg = cqspi_calc_rdreg(nor, nor->program_opcode);
> + writel(reg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + writel(to_addr, reg_base + CQSPI_REG_INDIRECTWRSTARTADDR);
> +
> + reg = readl(reg_base + CQSPI_REG_SIZE);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->addr_width - 1);
> + writel(reg, reg_base + CQSPI_REG_SIZE);
> + return 0;
> +}
> +
> +static int cqspi_indirect_write_execute(struct spi_nor *nor,
> + const u8 *txbuf, const unsigned n_tx)
> +{
> + const unsigned int page_size = nor->page_size;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int remaining = n_tx;
> + unsigned int write_bytes;
> + int ret;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTWRBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_WR, reg_base + CQSPI_REG_IRQMASK);
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTWR_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> +
> + while (remaining > 0) {
> + write_bytes = remaining > page_size ? page_size : remaining;
> + writesl(cqspi->ahb_base, txbuf, DIV_ROUND_UP(write_bytes, 4));
> +
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_TIMEOUT_MS));
> + if (!ret) {
> + dev_err(nor->dev, "Indirect write timeout\n");
> + ret = -ETIMEDOUT;
> + goto failwr;
> + }
> +
> + txbuf += write_bytes;
> + remaining -= write_bytes;
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTWR,
> + CQSPI_REG_INDIRECTWR_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect write completion error (%i)\n", ret);
> + goto failwr;
> + }
> +
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTWR_DONE_MASK, reg_base + CQSPI_REG_INDIRECTWR);
> +
> + cqspi_wait_idle(cqspi);
> +
> + return 0;
> +
> +failwr:
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect write */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> + return ret;
> +}
> +
> +static void cqspi_write(struct spi_nor *nor, loff_t to,
> + size_t len, size_t *retlen, const u_char *buf)
> +{
> + int ret;
> +
> + ret = cqspi_indirect_write_setup(nor, to);
> + if (ret)
> + return;
> +
> + ret = cqspi_indirect_write_execute(nor, buf, len);
> + if (ret)
> + return;
> +
> + *retlen += len;
> +}
> +
> +static int cqspi_read(struct spi_nor *nor, loff_t from,
> + size_t len, size_t *retlen, u_char *buf)
> +{
> + int ret;
> +
> + ret = cqspi_indirect_read_setup(nor, from);
> + if (ret)
> + return ret;
> +
> + ret = cqspi_indirect_read_execute(nor, buf, len);
> + if (ret)
> + return ret;
> +
> + *retlen += len;
> + return ret;
> +}
> +
> +static int cqspi_erase(struct spi_nor *nor, loff_t offs)
> +{
> + int ret;
> +
> + /* Send write enable, then erase commands. */
> + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0, 0);
> + if (ret)
> + return ret;
> +
> + /* Set up command buffer. */
> + ret = cqspi_command_write_addr(nor, nor->erase_opcode, offs);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int cqspi_set_protocol(struct spi_nor *nor, enum spi_protocol proto)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_1_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_2_2_2:
> + f_pdata->inst_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_4_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_2_2_2:
> + f_pdata->addr_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_1_4_4:
> + case SPI_PROTO_4_4_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned int calculate_ticks_for_ns(const unsigned int ref_clk_hz,
> + const unsigned int ns_val)
> +{
> + unsigned int ticks;
> +
> + ticks = ref_clk_hz / 1000; /* kHz */
> + ticks = DIV_ROUND_UP(ticks * ns_val, 1000000);
> +
> + return ticks;
> +}
> +
> +static void cqspi_delay(struct spi_nor *nor, const unsigned int sclk_hz)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *iobase = cqspi->iobase;
> + const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
> + unsigned int tshsl, tchsh, tslch, tsd2d;
> + unsigned int reg;
> + unsigned int tsclk;
> +
> + /* calculate the number of ref ticks for one sclk tick */
> + tsclk = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
> +
> + tshsl = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tshsl_ns);
> + /* this particular value must be at least one sclk */
> + if (tshsl < tsclk)
> + tshsl = tsclk;
> +
> + tchsh = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tchsh_ns);
> + tslch = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tslch_ns);
> + tsd2d = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tsd2d_ns);
> +
> + reg = (tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
> + << CQSPI_REG_DELAY_TSHSL_LSB;
> + reg |= (tchsh & CQSPI_REG_DELAY_TCHSH_MASK)
> + << CQSPI_REG_DELAY_TCHSH_LSB;
> + reg |= (tslch & CQSPI_REG_DELAY_TSLCH_MASK)
> + << CQSPI_REG_DELAY_TSLCH_LSB;
> + reg |= (tsd2d & CQSPI_REG_DELAY_TSD2D_MASK)
> + << CQSPI_REG_DELAY_TSD2D_LSB;
> + writel(reg, iobase + CQSPI_REG_DELAY);
> +}
> +
> +static void cqspi_config_baudrate_div(struct cqspi_st *cqspi,
> + const unsigned int sclk_hz)
> +{
> + const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> + unsigned int div;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
> +
> + div = ref_clk_hz / sclk_hz;
> +
> + /* Recalculate the baudrate divisor based on QSPI specification. */
> + if (div > 32)
> + div = 32;
> +
> + /* Check if even number. */
> + if (div & 1)
> + div = (div / 2);
> + else
> + div = (div / 2) - 1;
> +
> + div = (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB;
> + reg |= div;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_readdata_capture(struct cqspi_st *cqspi,
> + const unsigned int bypass,
> + const unsigned int delay)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_READCAPTURE);
> +
> + if (bypass)
> + reg |= (1 << CQSPI_REG_READCAPTURE_BYPASS_LSB);
> + else
> + reg &= ~(1 << CQSPI_REG_READCAPTURE_BYPASS_LSB);
> +
> + reg &= ~(CQSPI_REG_READCAPTURE_DELAY_MASK
> + << CQSPI_REG_READCAPTURE_DELAY_LSB);
> +
> + reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
> + << CQSPI_REG_READCAPTURE_DELAY_LSB;
> +
> + writel(reg, reg_base + CQSPI_REG_READCAPTURE);
> +}
> +
> +static void cqspi_chipselect(struct spi_nor *nor)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int chip_select = f_pdata->cs;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + if (cqspi->is_decoded_cs) {
> + reg |= CQSPI_REG_CONFIG_DECODE_MASK;
> + } else {
> + reg &= ~CQSPI_REG_CONFIG_DECODE_MASK;
> +
> + /* Convert CS if without decoder.
> + * CS0 to 4b'1110
> + * CS1 to 4b'1101
> + * CS2 to 4b'1011
> + * CS3 to 4b'0111
> + */
> + chip_select = 0xF & ~(1 << chip_select);
> + }
> +
> + reg &= ~(CQSPI_REG_CONFIG_CHIPSELECT_MASK
> + << CQSPI_REG_CONFIG_CHIPSELECT_LSB);
> + reg |= (chip_select & CQSPI_REG_CONFIG_CHIPSELECT_MASK)
> + << CQSPI_REG_CONFIG_CHIPSELECT_LSB;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_controller_enable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_controller_disable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_switch_cs(struct spi_nor *nor)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *iobase = cqspi->iobase;
> + unsigned int reg;
> +
> + cqspi_controller_disable(cqspi);
> +
> + /* configure page size and block size. */
> + reg = readl(iobase + CQSPI_REG_SIZE);
> + reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB);
> + reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->page_size << CQSPI_REG_SIZE_PAGE_LSB);
> + reg |= (ilog2(nor->mtd->erasesize) << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg |= (nor->addr_width - 1);
> + writel(reg, iobase + CQSPI_REG_SIZE);
> +
> + /* configure the chip select */
> + cqspi_chipselect(nor);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + const unsigned int sclk = f_pdata->clk_rate;
> +
> + /* Switch chip select. */
> + if (cqspi->current_cs != f_pdata->cs) {
> + cqspi->current_cs = f_pdata->cs;
> + cqspi_switch_cs(nor);
> + }
> +
> + /* Setup baudrate divisor and delays */
> + if (cqspi->sclk != sclk) {
> + cqspi->sclk = sclk;
> + cqspi_controller_disable(cqspi);
> + cqspi_config_baudrate_div(cqspi, sclk);
> + cqspi_delay(nor, sclk);
> + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
> + cqspi_controller_enable(cqspi);
> + }
> + return 0;
> +}
> +
> +static int cqspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> +{
> + int ret;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_READ);
> +
> + ret = cqspi_command_read(nor, &opcode, 1, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
> + int write_enable)
> +{
> + int ret = 0;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_WRITE);
> +
> + ret = cqspi_command_write(nor, opcode, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
> + struct cqspi_flash_pdata *f_pdata,
> + struct device_node *np)
> +{
> + if (of_property_read_u32(np, "cdns,read-delay", &f_pdata->read_delay)) {
> + dev_err(&pdev->dev, "couldn't determine read-delay\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tshsl-ns", &f_pdata->tshsl_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tshsl-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tsd2d-ns", &f_pdata->tsd2d_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tsd2d-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tchsh-ns", &f_pdata->tchsh_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tchsh-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tslch-ns", &f_pdata->tslch_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tslch-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "spi-max-frequency", &f_pdata->clk_rate)) {
> + dev_err(&pdev->dev, "couldn't determine spi-max-frequency\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static int cqspi_of_get_pdata(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +
> + cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
> +
> + if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-depth\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,fifo-width", &cqspi->fifo_width)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-width\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,trigger-address",
> + &cqspi->trigger_address)) {
> + dev_err(&pdev->dev, "couldn't determine trigger-address\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void cqspi_controller_init(struct cqspi_st *cqspi)
> +{
> + cqspi_controller_disable(cqspi);
> +
> + /* Configure the remap address register, no remap */
> + writel(0, cqspi->iobase + CQSPI_REG_REMAP);
> +
> + /* Disable all interrupts. */
> + writel(0, cqspi->iobase + CQSPI_REG_IRQMASK);
> +
> + /* Configure the SRAM split to 1:1 . */
> + writel(cqspi->fifo_depth / 2, cqspi->iobase + CQSPI_REG_SRAMPARTITION);
> +
> + /* Load indirect trigger address. */
> + writel(cqspi->trigger_address,
> + cqspi->iobase + CQSPI_REG_INDIRECTTRIGGER);
> +
> + /* Program read watermark -- 1/2 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 2,
> + cqspi->iobase + CQSPI_REG_INDIRECTRDWATERMARK);
> + /* Program write watermark -- 1/8 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
> + cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> +{
> + struct platform_device *pdev = cqspi->pdev;
> + struct device *dev = &pdev->dev;
> + struct mtd_part_parser_data ppdata;
> + struct cqspi_flash_pdata *f_pdata;
> + struct spi_nor *nor;
> + struct mtd_info *mtd;
> + unsigned int cs;
> + int i, ret;
> +
> + /* Get flash device data */
> + for_each_available_child_of_node(dev->of_node, np) {
> + if (of_property_read_u32(np, "reg", &cs)) {
> + dev_err(dev, "Couldn't determine chip select.\n");
> + goto err;
> + }
> +
> + if (cs > CQSPI_MAX_CHIPSELECT) {
> + dev_err(dev, "Chip select %d out of range.\n", cs);
> + goto err;
> + }
> +
> + f_pdata = &cqspi->f_pdata[cs];
> + f_pdata->cqspi = cqspi;
> + f_pdata->cs = cs;
> +
> + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
> + if (ret)
> + goto err;
> +
> + nor = &f_pdata->nor;
> + mtd = &f_pdata->mtd;
> +
> + mtd->priv = nor;
> +
> + nor->mtd = mtd;
> + nor->dev = dev;
> + nor->dn = np;
> + nor->priv = f_pdata;
> +
> + nor->read_reg = cqspi_read_reg;
> + nor->write_reg = cqspi_write_reg;
> + nor->read = cqspi_read;
> + nor->write = cqspi_write;
> + nor->erase = cqspi_erase;
> + nor->prepare = cqspi_prep;
> + nor->set_protocol = cqspi_set_protocol;
> +
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + if (ret)
> + goto err;
> +
> + ppdata.of_node = np;
> + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> + if (ret)
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> + return ret;
> +}
> +
> +static int cqspi_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct cqspi_st *cqspi;
> + struct resource *res;
> + struct resource *res_ahb;
> + int ret;
> + int irq;
> +
> + cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
> + if (!cqspi)
> + return -ENOMEM;
> +
> + cqspi->pdev = pdev;
> + platform_set_drvdata(pdev, cqspi);
> +
> + /* Obtain configuration from OF. */
> + ret = cqspi_of_get_pdata(pdev);
> + if (ret) {
> + dev_err(dev, "Cannot get mandatory OF data.\n");
> + return -ENODEV;
> + }
> +
> + /* Obtain QSPI clock. */
> + cqspi->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(cqspi->clk)) {
> + dev_err(dev, "Cannot claim QSPI clock.\n");
> + return PTR_ERR(cqspi->clk);
> + }
> +
> + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> +
> + /* Obtain and remap controller address. */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + cqspi->iobase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(cqspi->iobase)) {
> + dev_err(dev, "Cannot remap controller address.\n");
> + return PTR_ERR(cqspi->iobase);
> + }
> +
> + /* Obtain and remap AHB address. */
> + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
> + if (IS_ERR(cqspi->ahb_base)) {
> + dev_err(dev, "Cannot remap AHB address.\n");
> + return PTR_ERR(cqspi->ahb_base);
> + }
> +
> + init_completion(&cqspi->transfer_complete);
> +
> + /* Obtain IRQ line. */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "Cannot obtain IRQ.\n");
> + return -ENXIO;
> + }
> +
> + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> + pdev->name, cqspi);
> + if (ret) {
> + dev_err(dev, "Cannot request IRQ.\n");
> + return ret;
> + }
> +
> + cqspi_wait_idle(cqspi);
> + cqspi_controller_init(cqspi);
> + cqspi->current_cs = -1;
> + cqspi->sclk = 0;
> +
> + ret = cqspi_setup_flash(cqspi, np);
> + if (ret) {
> + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> + cqspi_controller_disable(cqspi);
> + }
> +
> + return ret;
> +}
> +
> +static int cqspi_remove(struct platform_device *pdev)
> +{
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> + int i;
> +
> + cqspi_controller_disable(cqspi);
> +
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int cqspi_suspend(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_disable(cqspi);
> + return 0;
> +}
> +
> +static int cqspi_resume(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_enable(cqspi);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops cqspi__dev_pm_ops = {
> + .suspend = cqspi_suspend,
> + .resume = cqspi_resume,
> +};
> +
> +#define CQSPI_DEV_PM_OPS (&cqspi__dev_pm_ops)
> +#else
> +#define CQSPI_DEV_PM_OPS NULL
> +#endif
> +
> +static struct of_device_id const cqspi_dt_ids[] = {
> + {.compatible = "cdns,qspi-nor",},
> + { /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> +
> +static struct platform_driver cqspi_platform_driver = {
> + .probe = cqspi_probe,
> + .remove = cqspi_remove,
> + .driver = {
> + .name = CQSPI_NAME,
> + .pm = CQSPI_DEV_PM_OPS,
> + .of_match_table = cqspi_dt_ids,
> + },
> +};
> +
> +module_platform_driver(cqspi_platform_driver);
> +
> +MODULE_DESCRIPTION("Cadence QSPI Controller Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" CQSPI_NAME);
> +MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
> +MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
> --
> 2.1.4
>
> .
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-25 22:09 ` vikas
@ 2015-08-26 6:19 ` Marek Vasut
2015-08-26 15:47 ` vikas
0 siblings, 1 reply; 23+ messages in thread
From: Marek Vasut @ 2015-08-26 6:19 UTC (permalink / raw)
To: vikas
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
> Hi,
>
> On 08/21/2015 02:20 AM, Marek Vasut wrote:
> > From: Graham Moore <grmoore@opensource.altera.com>
> >
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
> >
> > Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Alan Tull <atull@opensource.altera.com>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Graham Moore <grmoore@opensource.altera.com>
> > Cc: Vikas MANOCHA <vikas.manocha@st.com>
> > Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> > Cc: devicetree@vger.kernel.org
> > ---
> >
> > drivers/mtd/spi-nor/Kconfig | 11 +
> > drivers/mtd/spi-nor/Makefile | 1 +
> > drivers/mtd/spi-nor/cadence-quadspi.c | 1260
> > +++++++++++++++++++++++++++++++++ 3 files changed, 1272 insertions(+)
> > create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
> >
> > V2: use NULL instead of modalias in spi_nor_scan call
> > V3: Use existing property is-decoded-cs instead of creating duplicate.
> > V4: Support Micron quad mode by snooping command stream for EVCR command
> >
> > and subsequently configuring Cadence controller for quad mode.
> >
> > V5: Clean up sparse and smatch complaints. Remove snooping of Micron
> >
> > quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
> > up SRAM partition at 1:1 during init.
> >
> > V6: Remove dts patch that was included by mistake. Incorporate Vikas's
> >
> > comments regarding fifo width, SRAM partition setting, and trigger
> > address. Trigger address was added as an unsigned int, as it is not
> > an IO resource per se, and does not need to be mapped. Also add
> > Marek Vasut's workaround for picking up OF properties on subnodes.
> >
> > V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> >
> > macros and replace them with functions. Get rid of unused
> > variables.
> >
> > - Implement support for nor->set_protocol() to handle Quad-command,
> >
> > this patch now depends on the following patch:
> > mtd: spi-nor: notify (Q)SPI controller about protocol change
> >
> > - Replace that cqspi_fifo_read() disaster with plain old readsl()
> >
> > and cqspi_fifo_write() tentacle horror with pretty writesl().
> >
> > - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> > - Get rid of cqspi_find_chipselect() mess, instead just place the
> >
> > struct cqspi_st and chipselect number into struct cqspi_flash_pdata
> > and set nor->priv to the struct cqspi_flash_pdata of that
> > particular chip.
> >
> > - Replace the odd math in calculate_ticks_for_ns() with
> > DIV_ROUND_UP(). - Make variables const where applicable.
> >
> > V8: - Implement a function to wait for bit being set/unset for a given
> >
> > period of time and use it to replace the ad-hoc bits of code.
> >
> > - Configure the write underflow watermark to be 1/8 if FIFO size.
> > - Extract out the SPI NOR flash probing code into separate function
> >
> > to clearly mark what will soon be considered a boilerplate code.
> >
> > - Repair the handling of mode bits, which caused instability in V7.
> > - Clean up the interrupt handling
> > - Fix Kconfig help text and make the patch depend on OF and
> > COMPILE_TEST.
> >
> > diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> > index 89bf4c1..ed253a2 100644
> > --- a/drivers/mtd/spi-nor/Kconfig
> > +++ b/drivers/mtd/spi-nor/Kconfig
> > @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> >
> > Flash. Enable this option if you have a device with a SPIFI
> > controller and want to access the Flash as a mtd device.
> >
> > +config SPI_CADENCE_QUADSPI
> > + tristate "Cadence Quad SPI controller"
> > + depends on OF && COMPILE_TEST
> > + help
> > + Enable support for the Cadence Quad SPI Flash controller.
> > +
> > + Cadence QSPI is a specialized controller for connecting an SPI
> > + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> > + device with a Cadence QSPI controller and want to access the
> > + Flash as an MTD device.
> > +
>
> the patch failed to apply, please rebase it to master.
it's based on -next, I'm sure you can fix trivial conflicts yourself if you
need to apply it elsewhere. Do you have any other review comments ?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-26 6:19 ` Marek Vasut
@ 2015-08-26 15:47 ` vikas
2015-08-26 16:39 ` Marek Vasut
2015-08-26 18:06 ` Brian Norris
0 siblings, 2 replies; 23+ messages in thread
From: vikas @ 2015-08-26 15:47 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
Hi,
On 08/25/2015 11:19 PM, Marek Vasut wrote:
> On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
>> Hi,
>>
>> On 08/21/2015 02:20 AM, Marek Vasut wrote:
>>> From: Graham Moore <grmoore@opensource.altera.com>
>>>
>>> Add support for the Cadence QSPI controller. This controller is
>>> present in the Altera SoCFPGA SoCs and this driver has been tested
>>> on the Cyclone V SoC.
>>>
>>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Alan Tull <atull@opensource.altera.com>
>>> Cc: Brian Norris <computersforpeace@gmail.com>
>>> Cc: David Woodhouse <dwmw2@infradead.org>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Graham Moore <grmoore@opensource.altera.com>
>>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
>>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>
>>> drivers/mtd/spi-nor/Kconfig | 11 +
>>> drivers/mtd/spi-nor/Makefile | 1 +
>>> drivers/mtd/spi-nor/cadence-quadspi.c | 1260
>>> +++++++++++++++++++++++++++++++++ 3 files changed, 1272 insertions(+)
>>> create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
>>>
>>> V2: use NULL instead of modalias in spi_nor_scan call
>>> V3: Use existing property is-decoded-cs instead of creating duplicate.
>>> V4: Support Micron quad mode by snooping command stream for EVCR command
>>>
>>> and subsequently configuring Cadence controller for quad mode.
>>>
>>> V5: Clean up sparse and smatch complaints. Remove snooping of Micron
>>>
>>> quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
>>> up SRAM partition at 1:1 during init.
>>>
>>> V6: Remove dts patch that was included by mistake. Incorporate Vikas's
>>>
>>> comments regarding fifo width, SRAM partition setting, and trigger
>>> address. Trigger address was added as an unsigned int, as it is not
>>> an IO resource per se, and does not need to be mapped. Also add
>>> Marek Vasut's workaround for picking up OF properties on subnodes.
>>>
>>> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
>>>
>>> macros and replace them with functions. Get rid of unused
>>> variables.
>>>
>>> - Implement support for nor->set_protocol() to handle Quad-command,
>>>
>>> this patch now depends on the following patch:
>>> mtd: spi-nor: notify (Q)SPI controller about protocol change
>>>
>>> - Replace that cqspi_fifo_read() disaster with plain old readsl()
>>>
>>> and cqspi_fifo_write() tentacle horror with pretty writesl().
>>>
>>> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
>>> - Get rid of cqspi_find_chipselect() mess, instead just place the
>>>
>>> struct cqspi_st and chipselect number into struct cqspi_flash_pdata
>>> and set nor->priv to the struct cqspi_flash_pdata of that
>>> particular chip.
>>>
>>> - Replace the odd math in calculate_ticks_for_ns() with
>>> DIV_ROUND_UP(). - Make variables const where applicable.
>>>
>>> V8: - Implement a function to wait for bit being set/unset for a given
>>>
>>> period of time and use it to replace the ad-hoc bits of code.
>>>
>>> - Configure the write underflow watermark to be 1/8 if FIFO size.
>>> - Extract out the SPI NOR flash probing code into separate function
>>>
>>> to clearly mark what will soon be considered a boilerplate code.
>>>
>>> - Repair the handling of mode bits, which caused instability in V7.
>>> - Clean up the interrupt handling
>>> - Fix Kconfig help text and make the patch depend on OF and
>>> COMPILE_TEST.
>>>
>>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>>> index 89bf4c1..ed253a2 100644
>>> --- a/drivers/mtd/spi-nor/Kconfig
>>> +++ b/drivers/mtd/spi-nor/Kconfig
>>> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
>>>
>>> Flash. Enable this option if you have a device with a SPIFI
>>> controller and want to access the Flash as a mtd device.
>>>
>>> +config SPI_CADENCE_QUADSPI
>>> + tristate "Cadence Quad SPI controller"
>>> + depends on OF && COMPILE_TEST
>>> + help
>>> + Enable support for the Cadence Quad SPI Flash controller.
>>> +
>>> + Cadence QSPI is a specialized controller for connecting an SPI
>>> + Flash over 1/2/4-bit wide bus. Enable this option if you have a
>>> + device with a Cadence QSPI controller and want to access the
>>> + Flash as an MTD device.
>>> +
>>
>> the patch failed to apply, please rebase it to master.
>
> it's based on -next, I'm sure you can fix trivial conflicts yourself if you
> need to apply it elsewhere. Do you have any other review comments ?
Always rebase the patch on master.
I will be reviewing the patchset this week but on a quick look it seems many review comments of v7 are not there.
Cheers,
Vikas
> .
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-26 15:47 ` vikas
@ 2015-08-26 16:39 ` Marek Vasut
2015-08-26 18:06 ` Brian Norris
1 sibling, 0 replies; 23+ messages in thread
From: Marek Vasut @ 2015-08-26 16:39 UTC (permalink / raw)
To: vikas
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
On Wednesday, August 26, 2015 at 05:47:37 PM, vikas wrote:
> Hi,
Hi,
> On 08/25/2015 11:19 PM, Marek Vasut wrote:
> > On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
> >> Hi,
> >>
> >> On 08/21/2015 02:20 AM, Marek Vasut wrote:
> >>> From: Graham Moore <grmoore@opensource.altera.com>
> >>>
> >>> Add support for the Cadence QSPI controller. This controller is
> >>> present in the Altera SoCFPGA SoCs and this driver has been tested
> >>> on the Cyclone V SoC.
> >>>
> >>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Alan Tull <atull@opensource.altera.com>
> >>> Cc: Brian Norris <computersforpeace@gmail.com>
> >>> Cc: David Woodhouse <dwmw2@infradead.org>
> >>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> >>> Cc: Graham Moore <grmoore@opensource.altera.com>
> >>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
> >>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> >>> Cc: devicetree@vger.kernel.org
> >>> ---
> >>>
> >>> drivers/mtd/spi-nor/Kconfig | 11 +
> >>> drivers/mtd/spi-nor/Makefile | 1 +
> >>> drivers/mtd/spi-nor/cadence-quadspi.c | 1260
> >>> +++++++++++++++++++++++++++++++++ 3 files changed, 1272 insertions(+)
> >>> create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
> >>>
> >>> V2: use NULL instead of modalias in spi_nor_scan call
> >>> V3: Use existing property is-decoded-cs instead of creating duplicate.
> >>> V4: Support Micron quad mode by snooping command stream for EVCR
> >>> command
> >>>
> >>> and subsequently configuring Cadence controller for quad mode.
> >>>
> >>> V5: Clean up sparse and smatch complaints. Remove snooping of Micron
> >>>
> >>> quad mode. Add comment on XIP mode bit and dummy clock cycles.
> >>> Set up SRAM partition at 1:1 during init.
> >>>
> >>> V6: Remove dts patch that was included by mistake. Incorporate Vikas's
> >>>
> >>> comments regarding fifo width, SRAM partition setting, and trigger
> >>> address. Trigger address was added as an unsigned int, as it is
> >>> not an IO resource per se, and does not need to be mapped. Also
> >>> add Marek Vasut's workaround for picking up OF properties on
> >>> subnodes.
> >>>
> >>> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> >>>
> >>> macros and replace them with functions. Get rid of unused
> >>> variables.
> >>>
> >>> - Implement support for nor->set_protocol() to handle Quad-command,
> >>>
> >>> this patch now depends on the following patch:
> >>> mtd: spi-nor: notify (Q)SPI controller about protocol change
> >>>
> >>> - Replace that cqspi_fifo_read() disaster with plain old readsl()
> >>>
> >>> and cqspi_fifo_write() tentacle horror with pretty writesl().
> >>>
> >>> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> >>> - Get rid of cqspi_find_chipselect() mess, instead just place the
> >>>
> >>> struct cqspi_st and chipselect number into struct
> >>> cqspi_flash_pdata and set nor->priv to the struct
> >>> cqspi_flash_pdata of that particular chip.
> >>>
> >>> - Replace the odd math in calculate_ticks_for_ns() with
> >>> DIV_ROUND_UP(). - Make variables const where applicable.
> >>>
> >>> V8: - Implement a function to wait for bit being set/unset for a given
> >>>
> >>> period of time and use it to replace the ad-hoc bits of code.
> >>>
> >>> - Configure the write underflow watermark to be 1/8 if FIFO size.
> >>> - Extract out the SPI NOR flash probing code into separate function
> >>>
> >>> to clearly mark what will soon be considered a boilerplate code.
> >>>
> >>> - Repair the handling of mode bits, which caused instability in V7.
> >>> - Clean up the interrupt handling
> >>> - Fix Kconfig help text and make the patch depend on OF and
> >>> COMPILE_TEST.
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> >>> index 89bf4c1..ed253a2 100644
> >>> --- a/drivers/mtd/spi-nor/Kconfig
> >>> +++ b/drivers/mtd/spi-nor/Kconfig
> >>> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> >>>
> >>> Flash. Enable this option if you have a device with a SPIFI
> >>> controller and want to access the Flash as a mtd device.
> >>>
> >>> +config SPI_CADENCE_QUADSPI
> >>> + tristate "Cadence Quad SPI controller"
> >>> + depends on OF && COMPILE_TEST
> >>> + help
> >>> + Enable support for the Cadence Quad SPI Flash controller.
> >>> +
> >>> + Cadence QSPI is a specialized controller for connecting an
> >>> SPI + Flash over 1/2/4-bit wide bus. Enable this option if you
> >>> have a + device with a Cadence QSPI controller and want to
> >>> access the + Flash as an MTD device.
> >>> +
> >>
> >> the patch failed to apply, please rebase it to master.
> >
> > it's based on -next, I'm sure you can fix trivial conflicts yourself if
> > you need to apply it elsewhere. Do you have any other review comments ?
>
> Always rebase the patch on master.
The patch applies correctly both on next/master and l2-mtd/master.
I think the problem is on your end.
> I will be reviewing the patchset this week but on a quick look it seems
> many review comments of v7 are not there.
Aha.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-26 15:47 ` vikas
2015-08-26 16:39 ` Marek Vasut
@ 2015-08-26 18:06 ` Brian Norris
2015-08-26 23:05 ` vikas
1 sibling, 1 reply; 23+ messages in thread
From: Brian Norris @ 2015-08-26 18:06 UTC (permalink / raw)
To: vikas
Cc: Marek Vasut, linux-mtd@lists.infradead.org, Graham Moore,
Alan Tull, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
On Wed, Aug 26, 2015 at 08:47:37AM -0700, vikas wrote:
> On 08/25/2015 11:19 PM, Marek Vasut wrote:
> > On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
> >> the patch failed to apply, please rebase it to master.
> >
> > it's based on -next, I'm sure you can fix trivial conflicts yourself if you
> > need to apply it elsewhere. Do you have any other review comments ?
>
> Always rebase the patch on master.
Development works against -next, not Linus' master:
https://lwn.net/Articles/289013/
This is also noted here:
https://www.kernel.org/doc/man-pages/linux-next.html
Technically, you should work off of the respective subsystem tree (see
entries in MAINTAINERS). For MTD, this is also documented here:
http://linux-mtd.infradead.org/source.html
In the end, most MTD patches get applied to l2-mtd.git, but l2-mtd.git
and linux-next.git should provide pretty similar MTD patching
experiences, as the former is included in the latter.
Regards,
Brian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-26 18:06 ` Brian Norris
@ 2015-08-26 23:05 ` vikas
0 siblings, 0 replies; 23+ messages in thread
From: vikas @ 2015-08-26 23:05 UTC (permalink / raw)
To: Brian Norris
Cc: Marek Vasut, linux-mtd@lists.infradead.org, Graham Moore,
Alan Tull, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
Hi,
On 08/26/2015 11:06 AM, Brian Norris wrote:
> On Wed, Aug 26, 2015 at 08:47:37AM -0700, vikas wrote:
>> On 08/25/2015 11:19 PM, Marek Vasut wrote:
>>> On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
>>>> the patch failed to apply, please rebase it to master.
>>>
>>> it's based on -next, I'm sure you can fix trivial conflicts yourself if you
>>> need to apply it elsewhere. Do you have any other review comments ?
>>
>> Always rebase the patch on master.
>
> Development works against -next, not Linus' master:
>
> https://lwn.net/Articles/289013/
>
> This is also noted here:
>
> https://www.kernel.org/doc/man-pages/linux-next.html
>
> Technically, you should work off of the respective subsystem tree (see
> entries in MAINTAINERS). For MTD, this is also documented here:
>
> http://linux-mtd.infradead.org/source.html
>
> In the end, most MTD patches get applied to l2-mtd.git, but l2-mtd.git
> and linux-next.git should provide pretty similar MTD patching
> experiences, as the former is included in the latter.
Thanks Brian for these useful links, appreciate it.
Rgds,
Vikas
>
> Regards,
> Brian
> .
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-21 9:20 ` [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller Marek Vasut
2015-08-25 22:09 ` vikas
@ 2015-08-31 17:30 ` Graham Moore
2015-08-31 22:36 ` Marek Vasut
2015-09-04 23:45 ` vikas
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Graham Moore @ 2015-08-31 17:30 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-mtd, Alan Tull, Brian Norris, David Woodhouse, Dinh Nguyen,
Vikas MANOCHA, Yves Vandervennet, devicetree
Hi Marek,
Having some compile issues...see below
On 08/21/2015 04:20 AM, Marek Vasut wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
>
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
[...]
> +static int cqspi_set_protocol(struct spi_nor *nor, enum spi_protocol proto)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_1_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_2_2_2:
> + f_pdata->inst_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_4_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_2_2_2:
> + f_pdata->addr_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_1_4_4:
> + case SPI_PROTO_4_4_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
I think you have some other patches in your tree, the above doesn't
compile on l2-mtd/master:
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:57: warning: ‘enum
spi_protocol’ declared inside parameter list [enabled by default]
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:57: warning: its
scope is only this definition or declaration, which is probably not what
you want [enabled by default]
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:70: error: parameter
2 (‘proto’) has incomplete type
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:12: error: function
declaration isn’t a prototype [-Werror=strict-prototypes]
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c: In function
‘cqspi_set_protocol’:
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:706:7: error:
‘SPI_PROTO_1_1_1’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:706:7: note: each
undeclared identifier is reported only once for each function it appears in
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:707:7: error:
‘SPI_PROTO_1_1_2’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:708:7: error:
‘SPI_PROTO_1_1_4’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:709:7: error:
‘SPI_PROTO_1_2_2’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:710:7: error:
‘SPI_PROTO_1_4_4’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:713:7: error:
‘SPI_PROTO_2_2_2’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:716:7: error:
‘SPI_PROTO_4_4_4’ undeclared (first use in this function)
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c: In function
‘cqspi_setup_flash’:
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:1087:6: error: ‘struct
spi_nor’ has no member named ‘dn’
~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:1096:6: error: ‘struct
spi_nor’ has no member named ‘set_protocol’
[...]
Regards,
Graham
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-31 17:30 ` Graham Moore
@ 2015-08-31 22:36 ` Marek Vasut
0 siblings, 0 replies; 23+ messages in thread
From: Marek Vasut @ 2015-08-31 22:36 UTC (permalink / raw)
To: Graham Moore
Cc: linux-mtd, Alan Tull, Brian Norris, David Woodhouse, Dinh Nguyen,
Vikas MANOCHA, Yves Vandervennet, devicetree
On Monday, August 31, 2015 at 07:30:37 PM, Graham Moore wrote:
> Hi Marek,
Hi Graham,
> Having some compile issues...see below
>
> On 08/21/2015 04:20 AM, Marek Vasut wrote:
> > From: Graham Moore <grmoore@opensource.altera.com>
> >
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
> >
> > Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> > Signed-off-by: Marek Vasut <marex@denx.de>
>
> [...]
>
> > +static int cqspi_set_protocol(struct spi_nor *nor, enum spi_protocol
> > proto) +{
> > + struct cqspi_flash_pdata *f_pdata = nor->priv;
> > +
> > + switch (proto) {
> > + case SPI_PROTO_1_1_1:
> > + case SPI_PROTO_1_1_2:
> > + case SPI_PROTO_1_1_4:
> > + case SPI_PROTO_1_2_2:
> > + case SPI_PROTO_1_4_4:
> > + f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
> > + break;
> > + case SPI_PROTO_2_2_2:
> > + f_pdata->inst_width = CQSPI_INST_TYPE_DUAL;
> > + break;
> > + case SPI_PROTO_4_4_4:
> > + f_pdata->inst_width = CQSPI_INST_TYPE_QUAD;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + switch (proto) {
> > + case SPI_PROTO_1_1_1:
> > + case SPI_PROTO_1_1_2:
> > + case SPI_PROTO_1_1_4:
> > + f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
> > + break;
> > + case SPI_PROTO_1_2_2:
> > + case SPI_PROTO_2_2_2:
> > + f_pdata->addr_width = CQSPI_INST_TYPE_DUAL;
> > + break;
> > + case SPI_PROTO_1_4_4:
> > + case SPI_PROTO_4_4_4:
> > + f_pdata->addr_width = CQSPI_INST_TYPE_QUAD;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> I think you have some other patches in your tree, the above doesn't
> compile on l2-mtd/master:
>
> ~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:57: warning: ‘enum
> spi_protocol’ declared inside parameter list [enabled by default]
> ~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:57: warning: its
> scope is only this definition or declaration, which is probably not what
> you want [enabled by default]
> ~/l2-mtd/drivers/mtd/spi-nor/cadence-quadspi.c:701:70: error: parameter
> 2 (‘proto’) has incomplete type
It's in the V7 changelog, you need:
mtd: spi-nor: notify (Q)SPI controller about protocol change
Also, since V8, you will need:
mtd: spi-nor: Decouple SPI NOR's device_node from controller device
They're both in the linux-mtd list, so feel free to pick them from there.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-21 9:20 ` [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller Marek Vasut
2015-08-25 22:09 ` vikas
2015-08-31 17:30 ` Graham Moore
@ 2015-09-04 23:45 ` vikas
2015-09-06 15:16 ` Marek Vasut
2015-10-15 14:10 ` Graham Moore
2016-01-11 4:14 ` [2/2] " R, Vignesh
4 siblings, 1 reply; 23+ messages in thread
From: vikas @ 2015-09-04 23:45 UTC (permalink / raw)
To: Marek Vasut, linux-mtd@lists.infradead.org
Cc: Graham Moore, Alan Tull, Brian Norris, David Woodhouse,
Dinh Nguyen, Yves Vandervennet, devicetree@vger.kernel.org
Hi,
On 08/21/2015 02:20 AM, Marek Vasut wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
can we add info about the modes supported/not supported like direct mode, indirect etc.
>
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Graham Moore <grmoore@opensource.altera.com>
> Cc: Vikas MANOCHA <vikas.manocha@st.com>
> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> Cc: devicetree@vger.kernel.org
> ---
> drivers/mtd/spi-nor/Kconfig | 11 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/cadence-quadspi.c | 1260 +++++++++++++++++++++++++++++++++
> 3 files changed, 1272 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
>
> V2: use NULL instead of modalias in spi_nor_scan call
> V3: Use existing property is-decoded-cs instead of creating duplicate.
> V4: Support Micron quad mode by snooping command stream for EVCR command
> and subsequently configuring Cadence controller for quad mode.
> V5: Clean up sparse and smatch complaints. Remove snooping of Micron
> quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
> up SRAM partition at 1:1 during init.
> V6: Remove dts patch that was included by mistake. Incorporate Vikas's
> comments regarding fifo width, SRAM partition setting, and trigger
> address. Trigger address was added as an unsigned int, as it is not
> an IO resource per se, and does not need to be mapped. Also add
> Marek Vasut's workaround for picking up OF properties on subnodes.
> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> macros and replace them with functions. Get rid of unused variables.
> - Implement support for nor->set_protocol() to handle Quad-command,
> this patch now depends on the following patch:
> mtd: spi-nor: notify (Q)SPI controller about protocol change
> - Replace that cqspi_fifo_read() disaster with plain old readsl()
> and cqspi_fifo_write() tentacle horror with pretty writesl().
> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> - Get rid of cqspi_find_chipselect() mess, instead just place the
> struct cqspi_st and chipselect number into struct cqspi_flash_pdata
> and set nor->priv to the struct cqspi_flash_pdata of that particular
> chip.
> - Replace the odd math in calculate_ticks_for_ns() with DIV_ROUND_UP().
> - Make variables const where applicable.
> V8: - Implement a function to wait for bit being set/unset for a given
> period of time and use it to replace the ad-hoc bits of code.
> - Configure the write underflow watermark to be 1/8 if FIFO size.
> - Extract out the SPI NOR flash probing code into separate function
> to clearly mark what will soon be considered a boilerplate code.
> - Repair the handling of mode bits, which caused instability in V7.
> - Clean up the interrupt handling
> - Fix Kconfig help text and make the patch depend on OF and COMPILE_TEST.
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89bf4c1..ed253a2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> Flash. Enable this option if you have a device with a SPIFI
> controller and want to access the Flash as a mtd device.
>
> +config SPI_CADENCE_QUADSPI
> + tristate "Cadence Quad SPI controller"
> + depends on OF && COMPILE_TEST
> + help
> + Enable support for the Cadence Quad SPI Flash controller.
> +
> + Cadence QSPI is a specialized controller for connecting an SPI
> + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> + device with a Cadence QSPI controller and want to access the
> + Flash as an MTD device.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e53333e..446c6b9 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> +obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> new file mode 100644
> index 0000000..8e024b8
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -0,0 +1,1260 @@
> +/*
> + * Driver for Cadence QSPI Controller
> + *
> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/spi/spi.h>
> +#include <linux/timer.h>
> +
> +#define CQSPI_NAME "cadence-qspi"
> +#define CQSPI_MAX_CHIPSELECT 16
> +
> +struct cqspi_st;
> +
> +struct cqspi_flash_pdata {
> + struct mtd_info mtd;
> + struct spi_nor nor;
> + struct cqspi_st *cqspi;
> + u32 clk_rate;
> + u32 read_delay;
> + u32 tshsl_ns;
> + u32 tsd2d_ns;
> + u32 tchsh_ns;
> + u32 tslch_ns;
> + u8 inst_width;
> + u8 addr_width;
> + u8 cs;
> +};
> +
> +struct cqspi_st {
> + struct platform_device *pdev;
> +
> + struct clk *clk;
> + unsigned int sclk;
> +
> + void __iomem *iobase;
> + void __iomem *ahb_base;
> + struct completion transfer_complete;
> +
> + int current_cs;
> + unsigned long master_ref_clk_hz;
> + bool is_decoded_cs;
> + u32 fifo_depth;
> + u32 fifo_width;
> + u32 trigger_address;
> + struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> +};
[...]
> +#define CQSPI_REG_CMDADDRESS 0x94
> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
> +
> +/* Interrupt status bits */
> +#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
> +#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
> +#define CQSPI_REG_IRQ_IND_COMP BIT(2)
> +#define CQSPI_REG_IRQ_IND_RD_REJECT BIT(3)
> +#define CQSPI_REG_IRQ_WR_PROTECTED_ERR BIT(4)
> +#define CQSPI_REG_IRQ_ILLEGAL_AHB_ERR BIT(5)
remove unused MACROs.
> +#define CQSPI_REG_IRQ_WATERMARK BIT(6)
> +#define CQSPI_REG_IRQ_IND_RD_OVERFLOW BIT(12)
Correct this bit name to ..IND_RD_SRAM_FULL.
> +
> +#define CQSPI_IRQ_MASK_RD (CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_IND_RD_OVERFLOW | \
> + CQSPI_REG_IRQ_IND_COMP)
> +
> +#define CQSPI_IRQ_MASK_WR (CQSPI_REG_IRQ_IND_COMP | \
> + CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_UNDERFLOW)
> +
> +#define CQSPI_IRQ_STATUS_MASK 0x1FFFF
> +
> +static int cqspi_wait_for_bit(void __iomem *reg, const u32 mask, bool clear)
> +{
> + unsigned long end = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + u32 val;
> +
> + while (1) {
> + val = readl(reg);
> + if (clear)
> + val = ~val;
> + val &= mask;
> +
> + if (val == mask)
> + return 0;
> +
> + if (time_after(jiffies, end))
> + return -ETIMEDOUT;
> + }
> +}
> +
> +static bool cqspi_is_idle(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
> +
> + return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
> +}
> +
> +static u32 cqspi_get_rd_sram_level(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_SDRAMLEVEL);
> +
> + reg >>= CQSPI_REG_SDRAMLEVEL_RD_LSB;
> + return reg & CQSPI_REG_SDRAMLEVEL_RD_MASK;
> +}
> +
> +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
> +{
> + struct cqspi_st *cqspi = dev;
> + unsigned int irq_status;
> +
> + /* Read interrupt status */
no need of this comment.
> + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + /* Clear interrupt */
> + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
> +
> + if (irq_status)
> + complete(&cqspi->transfer_complete);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static unsigned int cqspi_calc_rdreg(struct spi_nor *nor, const u8 opcode)
> +{
> + unsigned int rdreg = 0;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + rdreg |= f_pdata->inst_width << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB;
> + rdreg |= f_pdata->addr_width << CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB;
> +
> + if (nor->flash_read == SPI_NOR_QUAD)
> + rdreg |= CQSPI_INST_TYPE_QUAD
> + << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
> + return rdreg;
> +}
> +
> +static int cqspi_wait_idle(struct cqspi_st *cqspi)
> +{
> + const unsigned int poll_idle_retry = 3;
> + unsigned int count = 0;
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + while (1) {
> + /*
> + * Read few times in succession to ensure the controller
> + * is indeed idle, that is, the bit does not transition
> + * low again.
> + */
> + if (cqspi_is_idle(cqspi))
> + count++;
> + else
> + count = 0;
> +
> + if (count >= poll_idle_retry)
> + return 0;
why do we need to check it 3 times ?
>
> +
> + if (time_after(jiffies, timeout)) {
> + /* Timeout, in busy mode. */
> + dev_err(&cqspi->pdev->dev,
> + "QSPI is still busy after %dms timeout.\n",
> + CQSPI_TIMEOUT_MS);
> + return -ETIMEDOUT;
> + }
> +
> + cpu_relax();
> + }
> +}
> +
[...]
> +
> +static int cqspi_indirect_read_execute(struct spi_nor *nor,
> + u8 *rxbuf, const unsigned n_rx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + void __iomem *ahb_base = cqspi->ahb_base;
> + unsigned int remaining = n_rx;
> + unsigned int bytes_to_read = 0;
> + int ret = 0;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
I think there is no need for separate masks for read & write. Use one mask &
configure it once in the init rather than configuring each time for every
read/write. Then in the ISR, take action as per the interrupt source: read/write/error condition etc.
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> +
> + while (remaining > 0) {
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_READ_TIMEOUT_MS));
> +
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> +
> + if (!ret && bytes_to_read == 0) {
> + dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> + ret = -ETIMEDOUT;
> + goto failrd;
> + }
> +
> + while (bytes_to_read != 0) {
> + bytes_to_read *= cqspi->fifo_width;
> + bytes_to_read = bytes_to_read > remaining ?
> + remaining : bytes_to_read;
> + readsl(ahb_base, rxbuf, DIV_ROUND_UP(bytes_to_read, 4));
> + rxbuf += bytes_to_read;
> + remaining -= bytes_to_read;
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> + }
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
> + CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect read completion error (%i)\n", ret);
> + goto failrd;
> + }
> +
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTRD_DONE_MASK, reg_base + CQSPI_REG_INDIRECTRD);
> +
> + return 0;
> +
> +failrd:
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect read */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> + return ret;
> +}
> +
> +static int cqspi_indirect_write_setup(struct spi_nor *nor,
> + const unsigned int to_addr)
> +{
> + unsigned int reg;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> +
> + /* Set opcode. */
> + reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
> + writel(reg, reg_base + CQSPI_REG_WR_INSTR);
> + reg = cqspi_calc_rdreg(nor, nor->program_opcode);
> + writel(reg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + writel(to_addr, reg_base + CQSPI_REG_INDIRECTWRSTARTADDR);
> +
> + reg = readl(reg_base + CQSPI_REG_SIZE);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->addr_width - 1);
> + writel(reg, reg_base + CQSPI_REG_SIZE);
> + return 0;
> +}
> +
> +static int cqspi_indirect_write_execute(struct spi_nor *nor,
> + const u8 *txbuf, const unsigned n_tx)
> +{
> + const unsigned int page_size = nor->page_size;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int remaining = n_tx;
> + unsigned int write_bytes;
> + int ret;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTWRBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_WR, reg_base + CQSPI_REG_IRQMASK);
as above, no need for separate masks for read & write. Use one mask & configure it
once in the init rather than configuring each time for every read/write.
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTWR_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> +
> + while (remaining > 0) {
> + write_bytes = remaining > page_size ? page_size : remaining;
> + writesl(cqspi->ahb_base, txbuf, DIV_ROUND_UP(write_bytes, 4));
> +
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_TIMEOUT_MS));
> + if (!ret) {
> + dev_err(nor->dev, "Indirect write timeout\n");
> + ret = -ETIMEDOUT;
> + goto failwr;
> + }
> +
> + txbuf += write_bytes;
> + remaining -= write_bytes;
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTWR,
> + CQSPI_REG_INDIRECTWR_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect write completion error (%i)\n", ret);
> + goto failwr;
> + }
> +
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTWR_DONE_MASK, reg_base + CQSPI_REG_INDIRECTWR);
> +
> + cqspi_wait_idle(cqspi);
> +
> + return 0;
> +
> +failwr:
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect write */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> + return ret;
> +}
> +
[...]
> +
> +static void cqspi_controller_enable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_controller_disable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
two above function are almost same, we can have one function by adding
one bool arg.
> +
> +static void cqspi_switch_cs(struct spi_nor *nor)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *iobase = cqspi->iobase;
> + unsigned int reg;
> +
> + cqspi_controller_disable(cqspi);
> +
> + /* configure page size and block size. */
> + reg = readl(iobase + CQSPI_REG_SIZE);
> + reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB);
> + reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->page_size << CQSPI_REG_SIZE_PAGE_LSB);
> + reg |= (ilog2(nor->mtd->erasesize) << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg |= (nor->addr_width - 1);
> + writel(reg, iobase + CQSPI_REG_SIZE);
> +
> + /* configure the chip select */
> + cqspi_chipselect(nor);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + const unsigned int sclk = f_pdata->clk_rate;
> +
> + /* Switch chip select. */
> + if (cqspi->current_cs != f_pdata->cs) {
> + cqspi->current_cs = f_pdata->cs;
> + cqspi_switch_cs(nor);
> + }
> +
> + /* Setup baudrate divisor and delays */
> + if (cqspi->sclk != sclk) {
> + cqspi->sclk = sclk;
> + cqspi_controller_disable(cqspi);
> + cqspi_config_baudrate_div(cqspi, sclk);
> + cqspi_delay(nor, sclk);
> + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
> + cqspi_controller_enable(cqspi);
> + }
> + return 0;
> +}
> +
> +static int cqspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> +{
> + int ret;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_READ);
> +
> + ret = cqspi_command_read(nor, &opcode, 1, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
> + int write_enable)
> +{
> + int ret = 0;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_WRITE);
> +
> + ret = cqspi_command_write(nor, opcode, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
> + struct cqspi_flash_pdata *f_pdata,
> + struct device_node *np)
> +{
> + if (of_property_read_u32(np, "cdns,read-delay", &f_pdata->read_delay)) {
> + dev_err(&pdev->dev, "couldn't determine read-delay\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tshsl-ns", &f_pdata->tshsl_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tshsl-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tsd2d-ns", &f_pdata->tsd2d_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tsd2d-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tchsh-ns", &f_pdata->tchsh_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tchsh-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tslch-ns", &f_pdata->tslch_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tslch-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "spi-max-frequency", &f_pdata->clk_rate)) {
> + dev_err(&pdev->dev, "couldn't determine spi-max-frequency\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static int cqspi_of_get_pdata(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +
> + cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
> +
> + if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-depth\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,fifo-width", &cqspi->fifo_width)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-width\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,trigger-address",
> + &cqspi->trigger_address)) {
> + dev_err(&pdev->dev, "couldn't determine trigger-address\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void cqspi_controller_init(struct cqspi_st *cqspi)
> +{
> + cqspi_controller_disable(cqspi);
> +
> + /* Configure the remap address register, no remap */
> + writel(0, cqspi->iobase + CQSPI_REG_REMAP);
> +
> + /* Disable all interrupts. */
> + writel(0, cqspi->iobase + CQSPI_REG_IRQMASK);
> +
> + /* Configure the SRAM split to 1:1 . */
The comment is rightly using "SRAM" but the variable name "fifo_depth" is misleading,
change it to sram_depth.
> + writel(cqspi->fifo_depth / 2, cqspi->iobase + CQSPI_REG_SRAMPARTITION);
> +
> + /* Load indirect trigger address. */
remove this comment & review other comments of this function. I would
remove at least first two comments also of the routine.
> + writel(cqspi->trigger_address,
> + cqspi->iobase + CQSPI_REG_INDIRECTTRIGGER);
> +
> + /* Program read watermark -- 1/2 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 2,
> + cqspi->iobase + CQSPI_REG_INDIRECTRDWATERMARK);
> + /* Program write watermark -- 1/8 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
> + cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> +{
> + struct platform_device *pdev = cqspi->pdev;
> + struct device *dev = &pdev->dev;
> + struct mtd_part_parser_data ppdata;
> + struct cqspi_flash_pdata *f_pdata;
> + struct spi_nor *nor;
> + struct mtd_info *mtd;
> + unsigned int cs;
> + int i, ret;
> +
> + /* Get flash device data */
> + for_each_available_child_of_node(dev->of_node, np) {
> + if (of_property_read_u32(np, "reg", &cs)) {
> + dev_err(dev, "Couldn't determine chip select.\n");
> + goto err;
> + }
> +
> + if (cs > CQSPI_MAX_CHIPSELECT) {
> + dev_err(dev, "Chip select %d out of range.\n", cs);
> + goto err;
> + }
> +
> + f_pdata = &cqspi->f_pdata[cs];
> + f_pdata->cqspi = cqspi;
> + f_pdata->cs = cs;
> +
> + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
> + if (ret)
> + goto err;
> +
> + nor = &f_pdata->nor;
> + mtd = &f_pdata->mtd;
> +
> + mtd->priv = nor;
> +
> + nor->mtd = mtd;
> + nor->dev = dev;
> + nor->dn = np;
> + nor->priv = f_pdata;
> +
> + nor->read_reg = cqspi_read_reg;
> + nor->write_reg = cqspi_write_reg;
> + nor->read = cqspi_read;
> + nor->write = cqspi_write;
> + nor->erase = cqspi_erase;
> + nor->prepare = cqspi_prep;
> + nor->set_protocol = cqspi_set_protocol;
> +
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + if (ret)
> + goto err;
> +
> + ppdata.of_node = np;
> + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> + if (ret)
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> + return ret;
> +}
> +
> +static int cqspi_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct cqspi_st *cqspi;
> + struct resource *res;
> + struct resource *res_ahb;
> + int ret;
> + int irq;
> +
> + cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
> + if (!cqspi)
> + return -ENOMEM;
> +
> + cqspi->pdev = pdev;
> + platform_set_drvdata(pdev, cqspi);
> +
> + /* Obtain configuration from OF. */
> + ret = cqspi_of_get_pdata(pdev);
> + if (ret) {
> + dev_err(dev, "Cannot get mandatory OF data.\n");
> + return -ENODEV;
> + }
> +
> + /* Obtain QSPI clock. */
> + cqspi->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(cqspi->clk)) {
> + dev_err(dev, "Cannot claim QSPI clock.\n");
> + return PTR_ERR(cqspi->clk);
> + }
> +
> + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> +
> + /* Obtain and remap controller address. */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + cqspi->iobase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(cqspi->iobase)) {
> + dev_err(dev, "Cannot remap controller address.\n");
> + return PTR_ERR(cqspi->iobase);
> + }
> +
> + /* Obtain and remap AHB address. */
> + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
> + if (IS_ERR(cqspi->ahb_base)) {
> + dev_err(dev, "Cannot remap AHB address.\n");
> + return PTR_ERR(cqspi->ahb_base);
> + }
> +
> + init_completion(&cqspi->transfer_complete);
> +
> + /* Obtain IRQ line. */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "Cannot obtain IRQ.\n");
> + return -ENXIO;
> + }
> +
> + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> + pdev->name, cqspi);
> + if (ret) {
> + dev_err(dev, "Cannot request IRQ.\n");
> + return ret;
> + }
> +
> + cqspi_wait_idle(cqspi);
why do we need it here ?
Cheers,
Vikas
> + cqspi_controller_init(cqspi);
> + cqspi->current_cs = -1;
> + cqspi->sclk = 0;
> +
> + ret = cqspi_setup_flash(cqspi, np);
> + if (ret) {
> + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> + cqspi_controller_disable(cqspi);
> + }
> +
> + return ret;
> +}
> +
> +static int cqspi_remove(struct platform_device *pdev)
> +{
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> + int i;
> +
> + cqspi_controller_disable(cqspi);
> +
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int cqspi_suspend(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_disable(cqspi);
> + return 0;
> +}
> +
> +static int cqspi_resume(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_enable(cqspi);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops cqspi__dev_pm_ops = {
> + .suspend = cqspi_suspend,
> + .resume = cqspi_resume,
> +};
> +
> +#define CQSPI_DEV_PM_OPS (&cqspi__dev_pm_ops)
> +#else
> +#define CQSPI_DEV_PM_OPS NULL
> +#endif
> +
> +static struct of_device_id const cqspi_dt_ids[] = {
> + {.compatible = "cdns,qspi-nor",},
> + { /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> +
> +static struct platform_driver cqspi_platform_driver = {
> + .probe = cqspi_probe,
> + .remove = cqspi_remove,
> + .driver = {
> + .name = CQSPI_NAME,
> + .pm = CQSPI_DEV_PM_OPS,
> + .of_match_table = cqspi_dt_ids,
> + },
> +};
> +
> +module_platform_driver(cqspi_platform_driver);
> +
> +MODULE_DESCRIPTION("Cadence QSPI Controller Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" CQSPI_NAME);
> +MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
> +MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
> --
> 2.1.4
>
> .
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-09-04 23:45 ` vikas
@ 2015-09-06 15:16 ` Marek Vasut
2015-09-07 18:56 ` vikas
0 siblings, 1 reply; 23+ messages in thread
From: Marek Vasut @ 2015-09-06 15:16 UTC (permalink / raw)
To: vikas
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
On Saturday, September 05, 2015 at 01:45:01 AM, vikas wrote:
> Hi,
>
> On 08/21/2015 02:20 AM, Marek Vasut wrote:
> > From: Graham Moore <grmoore@opensource.altera.com>
> >
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
>
> can we add info about the modes supported/not supported like direct mode,
> indirect etc.
It's already part of the documentation.
> > Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Alan Tull <atull@opensource.altera.com>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Graham Moore <grmoore@opensource.altera.com>
> > Cc: Vikas MANOCHA <vikas.manocha@st.com>
> > Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> > Cc: devicetree@vger.kernel.org
> > ---
[...]
> > +#define CQSPI_REG_CMDADDRESS 0x94
> > +#define CQSPI_REG_CMDREADDATALOWER 0xA0
> > +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
> > +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
> > +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
> > +
> > +/* Interrupt status bits */
> > +#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
> > +#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
> > +#define CQSPI_REG_IRQ_IND_COMP BIT(2)
> > +#define CQSPI_REG_IRQ_IND_RD_REJECT BIT(3)
> > +#define CQSPI_REG_IRQ_WR_PROTECTED_ERR BIT(4)
> > +#define CQSPI_REG_IRQ_ILLEGAL_AHB_ERR BIT(5)
>
> remove unused MACROs.
You mean you prefer to have incomplete definition of bits in the register
instead of a complete one ? I cannot agree to this, sorry.
> > +#define CQSPI_REG_IRQ_WATERMARK BIT(6)
> > +#define CQSPI_REG_IRQ_IND_RD_OVERFLOW BIT(12)
>
> Correct this bit name to ..IND_RD_SRAM_FULL.
Good catch.
[...]
> > +static int cqspi_wait_idle(struct cqspi_st *cqspi)
> > +{
> > + const unsigned int poll_idle_retry = 3;
> > + unsigned int count = 0;
> > + unsigned long timeout;
> > +
> > + timeout = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> > + while (1) {
> > + /*
> > + * Read few times in succession to ensure the controller
> > + * is indeed idle, that is, the bit does not transition
> > + * low again.
> > + */
> > + if (cqspi_is_idle(cqspi))
> > + count++;
> > + else
> > + count = 0;
> > +
> > + if (count >= poll_idle_retry)
> > + return 0;
>
> why do we need to check it 3 times ?
Please see the comment above.
> > +
> > + if (time_after(jiffies, timeout)) {
> > + /* Timeout, in busy mode. */
> > + dev_err(&cqspi->pdev->dev,
> > + "QSPI is still busy after %dms
> > timeout.\n", + CQSPI_TIMEOUT_MS);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + cpu_relax();
> > + }
> > +}
> > +
>
> [...]
>
> > +
> > +static int cqspi_indirect_read_execute(struct spi_nor *nor,
> > + u8 *rxbuf, const unsigned n_rx)
> > +{
> > + struct cqspi_flash_pdata *f_pdata = nor->priv;
> > + struct cqspi_st *cqspi = f_pdata->cqspi;
> > + void __iomem *reg_base = cqspi->iobase;
> > + void __iomem *ahb_base = cqspi->ahb_base;
> > + unsigned int remaining = n_rx;
> > + unsigned int bytes_to_read = 0;
> > + int ret = 0;
> > +
> > + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
> > +
> > + /* Clear all interrupts. */
> > + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> > +
> > + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
>
> I think there is no need for separate masks for read & write. Use one mask
> & configure it once in the init rather than configuring each time for
> every read/write. Then in the ISR, take action as per the interrupt
> source: read/write/error condition etc.
Setting up the specific IRQ mask prevents spurious interrupts during the
particular IO operation, so this solution looks more precise to me.
> > +
> > + reinit_completion(&cqspi->transfer_complete);
> > + writel(CQSPI_REG_INDIRECTRD_START_MASK,
> > + reg_base + CQSPI_REG_INDIRECTRD);
> > +
> > + while (remaining > 0) {
> > + ret =
> > wait_for_completion_timeout(&cqspi->transfer_complete, +
> > msecs_to_jiffies +
> > (CQSPI_READ_TIMEOUT_MS)); +
> > + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> > +
> > + if (!ret && bytes_to_read == 0) {
> > + dev_err(nor->dev, "Indirect read timeout, no
> > bytes\n"); + ret = -ETIMEDOUT;
> > + goto failrd;
> > + }
> > +
> > + while (bytes_to_read != 0) {
> > + bytes_to_read *= cqspi->fifo_width;
> > + bytes_to_read = bytes_to_read > remaining ?
> > + remaining : bytes_to_read;
> > + readsl(ahb_base, rxbuf,
> > DIV_ROUND_UP(bytes_to_read, 4)); + rxbuf +=
> > bytes_to_read;
> > + remaining -= bytes_to_read;
> > + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> > + }
> > + }
> > +
> > + /* Check indirect done status */
> > + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
> > + CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
> > + if (ret) {
> > + dev_err(nor->dev,
> > + "Indirect read completion error (%i)\n", ret);
> > + goto failrd;
> > + }
> > +
> > + /* Disable interrupt */
> > + writel(0, reg_base + CQSPI_REG_IRQMASK);
> > +
> > + /* Clear indirect completion status */
> > + writel(CQSPI_REG_INDIRECTRD_DONE_MASK, reg_base +
> > CQSPI_REG_INDIRECTRD); +
> > + return 0;
> > +
> > +failrd:
> > + /* Disable interrupt */
> > + writel(0, reg_base + CQSPI_REG_IRQMASK);
> > +
> > + /* Cancel the indirect read */
> > + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> > + reg_base + CQSPI_REG_INDIRECTRD);
> > + return ret;
> > +}
[...]
> > +static void cqspi_controller_enable(struct cqspi_st *cqspi)
> > +{
> > + void __iomem *reg_base = cqspi->iobase;
> > + unsigned int reg;
> > +
> > + reg = readl(reg_base + CQSPI_REG_CONFIG);
> > + reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
> > + writel(reg, reg_base + CQSPI_REG_CONFIG);
> > +}
> > +
> > +static void cqspi_controller_disable(struct cqspi_st *cqspi)
> > +{
> > + void __iomem *reg_base = cqspi->iobase;
> > + unsigned int reg;
> > +
> > + reg = readl(reg_base + CQSPI_REG_CONFIG);
> > + reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
> > + writel(reg, reg_base + CQSPI_REG_CONFIG);
> > +}
>
> two above function are almost same, we can have one function by adding
> one bool arg.
It makes the code less readable, but whatever.
[...]
> > +static void cqspi_controller_init(struct cqspi_st *cqspi)
> > +{
> > + cqspi_controller_disable(cqspi);
> > +
> > + /* Configure the remap address register, no remap */
> > + writel(0, cqspi->iobase + CQSPI_REG_REMAP);
> > +
> > + /* Disable all interrupts. */
> > + writel(0, cqspi->iobase + CQSPI_REG_IRQMASK);
> > +
> > + /* Configure the SRAM split to 1:1 . */
>
> The comment is rightly using "SRAM" but the variable name "fifo_depth" is
> misleading, change it to sram_depth.
Quote from the datasheet:
Defines the size of the indirect read partition in the
SRAM, in units of SRAM locations. By default, half of
the SRAM is reserved for indirect read operations and
half for indirect write operations.
> > + writel(cqspi->fifo_depth / 2, cqspi->iobase +
> > CQSPI_REG_SRAMPARTITION); +
> > + /* Load indirect trigger address. */
>
> remove this comment & review other comments of this function. I would
> remove at least first two comments also of the routine.
I already answered this in the previous iteration.
> > + writel(cqspi->trigger_address,
> > + cqspi->iobase + CQSPI_REG_INDIRECTTRIGGER);
> > +
> > + /* Program read watermark -- 1/2 of the FIFO. */
> > + writel(cqspi->fifo_depth * cqspi->fifo_width / 2,
> > + cqspi->iobase + CQSPI_REG_INDIRECTRDWATERMARK);
> > + /* Program write watermark -- 1/8 of the FIFO. */
> > + writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
> > + cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
> > +
> > + cqspi_controller_enable(cqspi);
> > +}
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-09-06 15:16 ` Marek Vasut
@ 2015-09-07 18:56 ` vikas
2015-09-07 20:27 ` vikas
0 siblings, 1 reply; 23+ messages in thread
From: vikas @ 2015-09-07 18:56 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
Hi,
On 09/06/2015 08:16 AM, Marek Vasut wrote:
> On Saturday, September 05, 2015 at 01:45:01 AM, vikas wrote:
>> Hi,
>>
>> On 08/21/2015 02:20 AM, Marek Vasut wrote:
>>> From: Graham Moore <grmoore@opensource.altera.com>
>>>
>>> Add support for the Cadence QSPI controller. This controller is
>>> present in the Altera SoCFPGA SoCs and this driver has been tested
>>> on the Cyclone V SoC.
>>
>> can we add info about the modes supported/not supported like direct mode,
>> indirect etc.
>
> It's already part of the documentation.
To be clear, add info for modes supported in the driver. e.g. Direct mode is not supported in the driver.
Lets add this info to help users.
>
>>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Alan Tull <atull@opensource.altera.com>
>>> Cc: Brian Norris <computersforpeace@gmail.com>
>>> Cc: David Woodhouse <dwmw2@infradead.org>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Graham Moore <grmoore@opensource.altera.com>
>>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
>>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
>>> Cc: devicetree@vger.kernel.org
>>> ---
>
> [...]
>
>>> +#define CQSPI_REG_CMDADDRESS 0x94
>>> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
>>> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
>>> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
>>> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
>>> +
>>> +/* Interrupt status bits */
>>> +#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
>>> +#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
>>> +#define CQSPI_REG_IRQ_IND_COMP BIT(2)
>>> +#define CQSPI_REG_IRQ_IND_RD_REJECT BIT(3)
>>> +#define CQSPI_REG_IRQ_WR_PROTECTED_ERR BIT(4)
>>> +#define CQSPI_REG_IRQ_ILLEGAL_AHB_ERR BIT(5)
>>
>> remove unused MACROs.
>
> You mean you prefer to have incomplete definition of bits in the register
> instead of a complete one ? I cannot agree to this, sorry.
>
>>> +#define CQSPI_REG_IRQ_WATERMARK BIT(6)
>>> +#define CQSPI_REG_IRQ_IND_RD_OVERFLOW BIT(12)
>>
>> Correct this bit name to ..IND_RD_SRAM_FULL.
>
> Good catch.
>
> [...]
>
>>> +static int cqspi_wait_idle(struct cqspi_st *cqspi)
>>> +{
>>> + const unsigned int poll_idle_retry = 3;
>>> + unsigned int count = 0;
>>> + unsigned long timeout;
>>> +
>>> + timeout = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
>>> + while (1) {
>>> + /*
>>> + * Read few times in succession to ensure the controller
>>> + * is indeed idle, that is, the bit does not transition
>>> + * low again.
>>> + */
>>> + if (cqspi_is_idle(cqspi))
>>> + count++;
>>> + else
>>> + count = 0;
>>> +
>>> + if (count >= poll_idle_retry)
>>> + return 0;
>>
>> why do we need to check it 3 times ?
>
> Please see the comment above.
don't use the comments to explain what code is doing. Add info why it is being done & how this solution fixes it.
The point is if is some soc bug, it should be handled differently.
>
>>> +
>>> + if (time_after(jiffies, timeout)) {
>>> + /* Timeout, in busy mode. */
>>> + dev_err(&cqspi->pdev->dev,
>>> + "QSPI is still busy after %dms
>>> timeout.\n", + CQSPI_TIMEOUT_MS);
>>> + return -ETIMEDOUT;
>>> + }
>>> +
>>> + cpu_relax();
>>> + }
>>> +}
>>> +
>>
>> [...]
>>
>>> +
>>> +static int cqspi_indirect_read_execute(struct spi_nor *nor,
>>> + u8 *rxbuf, const unsigned n_rx)
>>> +{
>>> + struct cqspi_flash_pdata *f_pdata = nor->priv;
>>> + struct cqspi_st *cqspi = f_pdata->cqspi;
>>> + void __iomem *reg_base = cqspi->iobase;
>>> + void __iomem *ahb_base = cqspi->ahb_base;
>>> + unsigned int remaining = n_rx;
>>> + unsigned int bytes_to_read = 0;
>>> + int ret = 0;
>>> +
>>> + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
>>> +
>>> + /* Clear all interrupts. */
>>> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
>>> +
>>> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
>>
>> I think there is no need for separate masks for read & write. Use one mask
>> & configure it once in the init rather than configuring each time for
>> every read/write. Then in the ISR, take action as per the interrupt
>> source: read/write/error condition etc.
>
> Setting up the specific IRQ mask prevents spurious interrupts during the
> particular IO operation, so this solution looks more precise to me.
spurious interrupt ? like ?
Configuring interrupt at one time for read/write (preferably in init) is better software design
then breaking it in for every read/write.
>
>>> +
>>> + reinit_completion(&cqspi->transfer_complete);
>>> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
>>> + reg_base + CQSPI_REG_INDIRECTRD);
>>> +
>>> + while (remaining > 0) {
>>> + ret =
>>> wait_for_completion_timeout(&cqspi->transfer_complete, +
>>> msecs_to_jiffies +
>>> (CQSPI_READ_TIMEOUT_MS)); +
>>> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
>>> +
>>> + if (!ret && bytes_to_read == 0) {
>>> + dev_err(nor->dev, "Indirect read timeout, no
>>> bytes\n"); + ret = -ETIMEDOUT;
>>> + goto failrd;
>>> + }
>>> +
>>> + while (bytes_to_read != 0) {
>>> + bytes_to_read *= cqspi->fifo_width;
>>> + bytes_to_read = bytes_to_read > remaining ?
>>> + remaining : bytes_to_read;
>>> + readsl(ahb_base, rxbuf,
>>> DIV_ROUND_UP(bytes_to_read, 4)); + rxbuf +=
>>> bytes_to_read;
>>> + remaining -= bytes_to_read;
>>> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
>>> + }
>>> + }
>>> +
>>> + /* Check indirect done status */
>>> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
>>> + CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
>>> + if (ret) {
>>> + dev_err(nor->dev,
>>> + "Indirect read completion error (%i)\n", ret);
>>> + goto failrd;
>>> + }
>>> +
>>> + /* Disable interrupt */
>>> + writel(0, reg_base + CQSPI_REG_IRQMASK);
>>> +
>>> + /* Clear indirect completion status */
>>> + writel(CQSPI_REG_INDIRECTRD_DONE_MASK, reg_base +
>>> CQSPI_REG_INDIRECTRD); +
>>> + return 0;
>>> +
>>> +failrd:
>>> + /* Disable interrupt */
>>> + writel(0, reg_base + CQSPI_REG_IRQMASK);
>>> +
>>> + /* Cancel the indirect read */
>>> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
>>> + reg_base + CQSPI_REG_INDIRECTRD);
>>> + return ret;
>>> +}
>
> [...]
>
>>> +static void cqspi_controller_enable(struct cqspi_st *cqspi)
>>> +{
>>> + void __iomem *reg_base = cqspi->iobase;
>>> + unsigned int reg;
>>> +
>>> + reg = readl(reg_base + CQSPI_REG_CONFIG);
>>> + reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
>>> + writel(reg, reg_base + CQSPI_REG_CONFIG);
>>> +}
>>> +
>>> +static void cqspi_controller_disable(struct cqspi_st *cqspi)
>>> +{
>>> + void __iomem *reg_base = cqspi->iobase;
>>> + unsigned int reg;
>>> +
>>> + reg = readl(reg_base + CQSPI_REG_CONFIG);
>>> + reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
>>> + writel(reg, reg_base + CQSPI_REG_CONFIG);
>>> +}
>>
>> two above function are almost same, we can have one function by adding
>> one bool arg.
>
> It makes the code less readable, but whatever.
simple code is good but not simpler :-)
>
> [...]
>
>>> +static void cqspi_controller_init(struct cqspi_st *cqspi)
>>> +{
>>> + cqspi_controller_disable(cqspi);
>>> +
>>> + /* Configure the remap address register, no remap */
>>> + writel(0, cqspi->iobase + CQSPI_REG_REMAP);
>>> +
>>> + /* Disable all interrupts. */
>>> + writel(0, cqspi->iobase + CQSPI_REG_IRQMASK);
>>> +
>>> + /* Configure the SRAM split to 1:1 . */
>>
>> The comment is rightly using "SRAM" but the variable name "fifo_depth" is
>> misleading, change it to sram_depth.
>
> Quote from the datasheet:
>
> Defines the size of the indirect read partition in the
> SRAM, in units of SRAM locations. By default, half of
> the SRAM is reserved for indirect read operations and
> half for indirect write operations.
good, that explains it is sram depth not fifo depth.
>
>
>>> + writel(cqspi->fifo_depth / 2, cqspi->iobase +
>>> CQSPI_REG_SRAMPARTITION); +
>>> + /* Load indirect trigger address. */
>>
>> remove this comment & review other comments of this function. I would
>> remove at least first two comments also of the routine.
>
> I already answered this in the previous iteration.
I don't agree on this commentary of code, loading a value in register & then adding "loading value in register".
I stop here for this point, not worth more discussion.
Cheers,
Vikas
>
>>> + writel(cqspi->trigger_address,
>>> + cqspi->iobase + CQSPI_REG_INDIRECTTRIGGER);
>>> +
>>> + /* Program read watermark -- 1/2 of the FIFO. */
>>> + writel(cqspi->fifo_depth * cqspi->fifo_width / 2,
>>> + cqspi->iobase + CQSPI_REG_INDIRECTRDWATERMARK);
>>> + /* Program write watermark -- 1/8 of the FIFO. */
>>> + writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
>>> + cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
>>> +
>>> + cqspi_controller_enable(cqspi);
>>> +}
>
> [...]
> .
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-09-07 18:56 ` vikas
@ 2015-09-07 20:27 ` vikas
0 siblings, 0 replies; 23+ messages in thread
From: vikas @ 2015-09-07 20:27 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-mtd@lists.infradead.org, Graham Moore, Alan Tull,
Brian Norris, David Woodhouse, Dinh Nguyen, Yves Vandervennet,
devicetree@vger.kernel.org
Hi,
On 09/07/2015 11:56 AM, vikas wrote:
> Hi,
>
> On 09/06/2015 08:16 AM, Marek Vasut wrote:
>> On Saturday, September 05, 2015 at 01:45:01 AM, vikas wrote:
>>> Hi,
>>>
>>> On 08/21/2015 02:20 AM, Marek Vasut wrote:
>>>> From: Graham Moore <grmoore@opensource.altera.com>
>>>>
>>>> Add support for the Cadence QSPI controller. This controller is
>>>> present in the Altera SoCFPGA SoCs and this driver has been tested
>>>> on the Cyclone V SoC.
>>>
>>> can we add info about the modes supported/not supported like direct mode,
>>> indirect etc.
>>
>> It's already part of the documentation.
>
> To be clear, add info for modes supported in the driver. e.g. Direct mode is not supported in the driver.
> Lets add this info to help users.
>
>>
>>>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Alan Tull <atull@opensource.altera.com>
>>>> Cc: Brian Norris <computersforpeace@gmail.com>
>>>> Cc: David Woodhouse <dwmw2@infradead.org>
>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>> Cc: Graham Moore <grmoore@opensource.altera.com>
>>>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
>>>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
>>>> Cc: devicetree@vger.kernel.org
>>>> ---
>>
>> [...]
>>
>>>> +#define CQSPI_REG_CMDADDRESS 0x94
>>>> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
>>>> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
>>>> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
>>>> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
>>>> +
[...]
>>>> +
>>>> + /* Clear all interrupts. */
>>>> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
>>>> +
>>>> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
>>>
>>> I think there is no need for separate masks for read & write. Use one mask
>>> & configure it once in the init rather than configuring each time for
>>> every read/write. Then in the ISR, take action as per the interrupt
>>> source: read/write/error condition etc.
>>
>> Setting up the specific IRQ mask prevents spurious interrupts during the
>> particular IO operation, so this solution looks more precise to me.
>
> spurious interrupt ? like ?
>
> Configuring interrupt at one time for read/write (preferably in init) is better software design
> then breaking it in for every read/write.
just to clarify "preferably in init" not always but yes in this case.
Cheers,
Vikas
>
>>
>>>> +
>>>> + reinit_completion(&cqspi->transfer_complete);
>>>> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
>>>> + reg_base + CQSPI_REG_INDIRECTRD);
>>>> +
>>>> + while (remaining > 0) {
>>>> + ret =
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-21 9:20 ` [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller Marek Vasut
` (2 preceding siblings ...)
2015-09-04 23:45 ` vikas
@ 2015-10-15 14:10 ` Graham Moore
2015-10-15 14:27 ` Marek Vasut
2016-01-11 4:14 ` [2/2] " R, Vignesh
4 siblings, 1 reply; 23+ messages in thread
From: Graham Moore @ 2015-10-15 14:10 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-mtd, Alan Tull, Brian Norris, David Woodhouse, Dinh Nguyen,
Vikas MANOCHA, Yves Vandervennet, devicetree
On 08/21/2015 04:20 AM, Marek Vasut wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
>
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
[...]
Hi, just checking in. Are there any more changes to this driver?
Best Regards,
Graham
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-10-15 14:10 ` Graham Moore
@ 2015-10-15 14:27 ` Marek Vasut
0 siblings, 0 replies; 23+ messages in thread
From: Marek Vasut @ 2015-10-15 14:27 UTC (permalink / raw)
To: Graham Moore
Cc: linux-mtd, Alan Tull, Brian Norris, David Woodhouse, Dinh Nguyen,
Yves Vandervennet, devicetree, Cyrille Pitchen
On Thursday, October 15, 2015 at 04:10:08 PM, Graham Moore wrote:
> On 08/21/2015 04:20 AM, Marek Vasut wrote:
> > From: Graham Moore <grmoore@opensource.altera.com>
> >
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
> >
> > Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> > Signed-off-by: Marek Vasut <marex@denx.de>
>
> [...]
>
> Hi, just checking in. Are there any more changes to this driver?
This will need to be ported on top of the MTD changes by Cyrille , I have
it in my pipeline, but it wasn't done yet.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2015-08-21 9:20 ` [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller Marek Vasut
` (3 preceding siblings ...)
2015-10-15 14:10 ` Graham Moore
@ 2016-01-11 4:14 ` R, Vignesh
2016-01-11 4:50 ` Marek Vasut
4 siblings, 1 reply; 23+ messages in thread
From: R, Vignesh @ 2016-01-11 4:14 UTC (permalink / raw)
To: Marek Vasut, linux-mtd
Cc: devicetree, Alan Tull, Yves Vandervennet, Dinh Nguyen,
Brian Norris, David Woodhouse, Vikas MANOCHA, Graham Moore
Hi Marek,
On 08/21/2015 02:50 PM, Marek Vasut wrote:
> From: Graham Moore <grmoore@opensource.altera.com>
>
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
>
Thanks for the patch.
I have a TI EVM with Cadence QSPI controller connected to Spansion
flash(s25fl512s), and I gave this series a try. It worked with couple of
changes, please see comments inline.
> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Graham Moore <grmoore@opensource.altera.com>
> Cc: Vikas MANOCHA <vikas.manocha@st.com>
> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> Cc: devicetree@vger.kernel.org
> ---
> drivers/mtd/spi-nor/Kconfig | 11 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/cadence-quadspi.c | 1260 +++++++++++++++++++++++++++++++++
> 3 files changed, 1272 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
>
> V2: use NULL instead of modalias in spi_nor_scan call
> V3: Use existing property is-decoded-cs instead of creating duplicate.
> V4: Support Micron quad mode by snooping command stream for EVCR command
> and subsequently configuring Cadence controller for quad mode.
> V5: Clean up sparse and smatch complaints. Remove snooping of Micron
> quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
> up SRAM partition at 1:1 during init.
> V6: Remove dts patch that was included by mistake. Incorporate Vikas's
> comments regarding fifo width, SRAM partition setting, and trigger
> address. Trigger address was added as an unsigned int, as it is not
> an IO resource per se, and does not need to be mapped. Also add
> Marek Vasut's workaround for picking up OF properties on subnodes.
> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> macros and replace them with functions. Get rid of unused variables.
> - Implement support for nor->set_protocol() to handle Quad-command,
> this patch now depends on the following patch:
> mtd: spi-nor: notify (Q)SPI controller about protocol change
> - Replace that cqspi_fifo_read() disaster with plain old readsl()
> and cqspi_fifo_write() tentacle horror with pretty writesl().
> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> - Get rid of cqspi_find_chipselect() mess, instead just place the
> struct cqspi_st and chipselect number into struct cqspi_flash_pdata
> and set nor->priv to the struct cqspi_flash_pdata of that particular
> chip.
> - Replace the odd math in calculate_ticks_for_ns() with DIV_ROUND_UP().
> - Make variables const where applicable.
> V8: - Implement a function to wait for bit being set/unset for a given
> period of time and use it to replace the ad-hoc bits of code.
> - Configure the write underflow watermark to be 1/8 if FIFO size.
> - Extract out the SPI NOR flash probing code into separate function
> to clearly mark what will soon be considered a boilerplate code.
> - Repair the handling of mode bits, which caused instability in V7.
> - Clean up the interrupt handling
> - Fix Kconfig help text and make the patch depend on OF and COMPILE_TEST.
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89bf4c1..ed253a2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> Flash. Enable this option if you have a device with a SPIFI
> controller and want to access the Flash as a mtd device.
>
> +config SPI_CADENCE_QUADSPI
> + tristate "Cadence Quad SPI controller"
> + depends on OF && COMPILE_TEST
> + help
> + Enable support for the Cadence Quad SPI Flash controller.
> +
> + Cadence QSPI is a specialized controller for connecting an SPI
> + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> + device with a Cadence QSPI controller and want to access the
> + Flash as an MTD device.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e53333e..446c6b9 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> +obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> new file mode 100644
> index 0000000..8e024b8
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -0,0 +1,1260 @@
> +/*
> + * Driver for Cadence QSPI Controller
> + *
> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/spi/spi.h>
> +#include <linux/timer.h>
> +
> +#define CQSPI_NAME "cadence-qspi"
> +#define CQSPI_MAX_CHIPSELECT 16
> +
> +struct cqspi_st;
> +
> +struct cqspi_flash_pdata {
> + struct mtd_info mtd;
> + struct spi_nor nor;
> + struct cqspi_st *cqspi;
> + u32 clk_rate;
> + u32 read_delay;
> + u32 tshsl_ns;
> + u32 tsd2d_ns;
> + u32 tchsh_ns;
> + u32 tslch_ns;
> + u8 inst_width;
> + u8 addr_width;
> + u8 cs;
> +};
> +
> +struct cqspi_st {
> + struct platform_device *pdev;
> +
> + struct clk *clk;
> + unsigned int sclk;
> +
> + void __iomem *iobase;
> + void __iomem *ahb_base;
> + struct completion transfer_complete;
> +
> + int current_cs;
> + unsigned long master_ref_clk_hz;
> + bool is_decoded_cs;
> + u32 fifo_depth;
> + u32 fifo_width;
> + u32 trigger_address;
> + struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> +};
> +
> +/* Operation timeout value */
> +#define CQSPI_TIMEOUT_MS 500
> +#define CQSPI_READ_TIMEOUT_MS 10
> +
> +/* Instruction type */
> +#define CQSPI_INST_TYPE_SINGLE 0
> +#define CQSPI_INST_TYPE_DUAL 1
> +#define CQSPI_INST_TYPE_QUAD 2
> +
> +#define CQSPI_DUMMY_CLKS_PER_BYTE 8
> +#define CQSPI_DUMMY_BYTES_MAX 4
> +#define CQSPI_DUMMY_CLKS_MAX 31
> +
> +#define CQSPI_STIG_DATA_LEN_MAX 8
> +
> +/* Register map */
> +#define CQSPI_REG_CONFIG 0x00
> +#define CQSPI_REG_CONFIG_ENABLE_MASK BIT(0)
> +#define CQSPI_REG_CONFIG_DECODE_MASK BIT(9)
> +#define CQSPI_REG_CONFIG_CHIPSELECT_LSB 10
> +#define CQSPI_REG_CONFIG_DMA_MASK BIT(15)
> +#define CQSPI_REG_CONFIG_BAUD_LSB 19
> +#define CQSPI_REG_CONFIG_IDLE_LSB 31
> +#define CQSPI_REG_CONFIG_CHIPSELECT_MASK 0xF
> +#define CQSPI_REG_CONFIG_BAUD_MASK 0xF
> +
> +#define CQSPI_REG_RD_INSTR 0x04
> +#define CQSPI_REG_RD_INSTR_OPCODE_LSB 0
> +#define CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB 8
> +#define CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB 12
> +#define CQSPI_REG_RD_INSTR_TYPE_DATA_LSB 16
> +#define CQSPI_REG_RD_INSTR_MODE_EN_LSB 20
> +#define CQSPI_REG_RD_INSTR_DUMMY_LSB 24
> +#define CQSPI_REG_RD_INSTR_TYPE_INSTR_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_TYPE_ADDR_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_TYPE_DATA_MASK 0x3
> +#define CQSPI_REG_RD_INSTR_DUMMY_MASK 0x1F
> +
> +#define CQSPI_REG_WR_INSTR 0x08
> +#define CQSPI_REG_WR_INSTR_OPCODE_LSB 0
> +#define CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB 12
> +#define CQSPI_REG_WR_INSTR_TYPE_DATA_LSB 16
> +
> +#define CQSPI_REG_DELAY 0x0C
> +#define CQSPI_REG_DELAY_TSLCH_LSB 0
> +#define CQSPI_REG_DELAY_TCHSH_LSB 8
> +#define CQSPI_REG_DELAY_TSD2D_LSB 16
> +#define CQSPI_REG_DELAY_TSHSL_LSB 24
> +#define CQSPI_REG_DELAY_TSLCH_MASK 0xFF
> +#define CQSPI_REG_DELAY_TCHSH_MASK 0xFF
> +#define CQSPI_REG_DELAY_TSD2D_MASK 0xFF
> +#define CQSPI_REG_DELAY_TSHSL_MASK 0xFF
> +
> +#define CQSPI_REG_READCAPTURE 0x10
> +#define CQSPI_REG_READCAPTURE_BYPASS_LSB 0
> +#define CQSPI_REG_READCAPTURE_DELAY_LSB 1
> +#define CQSPI_REG_READCAPTURE_DELAY_MASK 0xF
> +
> +#define CQSPI_REG_SIZE 0x14
> +#define CQSPI_REG_SIZE_ADDRESS_LSB 0
> +#define CQSPI_REG_SIZE_PAGE_LSB 4
> +#define CQSPI_REG_SIZE_BLOCK_LSB 16
> +#define CQSPI_REG_SIZE_ADDRESS_MASK 0xF
> +#define CQSPI_REG_SIZE_PAGE_MASK 0xFFF
> +#define CQSPI_REG_SIZE_BLOCK_MASK 0x3F
> +
> +#define CQSPI_REG_SRAMPARTITION 0x18
> +#define CQSPI_REG_INDIRECTTRIGGER 0x1C
> +
> +#define CQSPI_REG_DMA 0x20
> +#define CQSPI_REG_DMA_SINGLE_LSB 0
> +#define CQSPI_REG_DMA_BURST_LSB 8
> +#define CQSPI_REG_DMA_SINGLE_MASK 0xFF
> +#define CQSPI_REG_DMA_BURST_MASK 0xFF
> +
> +#define CQSPI_REG_REMAP 0x24
> +#define CQSPI_REG_MODE_BIT 0x28
> +
> +#define CQSPI_REG_SDRAMLEVEL 0x2C
> +#define CQSPI_REG_SDRAMLEVEL_RD_LSB 0
> +#define CQSPI_REG_SDRAMLEVEL_WR_LSB 16
> +#define CQSPI_REG_SDRAMLEVEL_RD_MASK 0xFFFF
> +#define CQSPI_REG_SDRAMLEVEL_WR_MASK 0xFFFF
> +
> +#define CQSPI_REG_IRQSTATUS 0x40
> +#define CQSPI_REG_IRQMASK 0x44
> +
> +#define CQSPI_REG_INDIRECTRD 0x60
> +#define CQSPI_REG_INDIRECTRD_START_MASK BIT(0)
> +#define CQSPI_REG_INDIRECTRD_CANCEL_MASK BIT(1)
> +#define CQSPI_REG_INDIRECTRD_DONE_MASK BIT(5)
> +
> +#define CQSPI_REG_INDIRECTRDWATERMARK 0x64
> +#define CQSPI_REG_INDIRECTRDSTARTADDR 0x68
> +#define CQSPI_REG_INDIRECTRDBYTES 0x6C
> +
> +#define CQSPI_REG_CMDCTRL 0x90
> +#define CQSPI_REG_CMDCTRL_EXECUTE_MASK BIT(0)
> +#define CQSPI_REG_CMDCTRL_INPROGRESS_MASK BIT(1)
> +#define CQSPI_REG_CMDCTRL_WR_BYTES_LSB 12
> +#define CQSPI_REG_CMDCTRL_WR_EN_LSB 15
> +#define CQSPI_REG_CMDCTRL_ADD_BYTES_LSB 16
> +#define CQSPI_REG_CMDCTRL_ADDR_EN_LSB 19
> +#define CQSPI_REG_CMDCTRL_RD_BYTES_LSB 20
> +#define CQSPI_REG_CMDCTRL_RD_EN_LSB 23
> +#define CQSPI_REG_CMDCTRL_OPCODE_LSB 24
> +#define CQSPI_REG_CMDCTRL_WR_BYTES_MASK 0x7
> +#define CQSPI_REG_CMDCTRL_ADD_BYTES_MASK 0x3
> +#define CQSPI_REG_CMDCTRL_RD_BYTES_MASK 0x7
> +
> +#define CQSPI_REG_INDIRECTWR 0x70
> +#define CQSPI_REG_INDIRECTWR_START_MASK BIT(0)
> +#define CQSPI_REG_INDIRECTWR_CANCEL_MASK BIT(1)
> +#define CQSPI_REG_INDIRECTWR_DONE_MASK BIT(5)
> +
> +#define CQSPI_REG_INDIRECTWRWATERMARK 0x74
> +#define CQSPI_REG_INDIRECTWRSTARTADDR 0x78
> +#define CQSPI_REG_INDIRECTWRBYTES 0x7C
> +
> +#define CQSPI_REG_CMDADDRESS 0x94
> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
> +
> +/* Interrupt status bits */
> +#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
> +#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
> +#define CQSPI_REG_IRQ_IND_COMP BIT(2)
> +#define CQSPI_REG_IRQ_IND_RD_REJECT BIT(3)
> +#define CQSPI_REG_IRQ_WR_PROTECTED_ERR BIT(4)
> +#define CQSPI_REG_IRQ_ILLEGAL_AHB_ERR BIT(5)
> +#define CQSPI_REG_IRQ_WATERMARK BIT(6)
> +#define CQSPI_REG_IRQ_IND_RD_OVERFLOW BIT(12)
> +
> +#define CQSPI_IRQ_MASK_RD (CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_IND_RD_OVERFLOW | \
> + CQSPI_REG_IRQ_IND_COMP)
> +
> +#define CQSPI_IRQ_MASK_WR (CQSPI_REG_IRQ_IND_COMP | \
> + CQSPI_REG_IRQ_WATERMARK | \
> + CQSPI_REG_IRQ_UNDERFLOW)
> +
> +#define CQSPI_IRQ_STATUS_MASK 0x1FFFF
> +
> +static int cqspi_wait_for_bit(void __iomem *reg, const u32 mask, bool clear)
> +{
> + unsigned long end = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + u32 val;
> +
> + while (1) {
> + val = readl(reg);
> + if (clear)
> + val = ~val;
> + val &= mask;
> +
> + if (val == mask)
> + return 0;
> +
> + if (time_after(jiffies, end))
> + return -ETIMEDOUT;
> + }
> +}
> +
> +static bool cqspi_is_idle(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
> +
> + return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
> +}
> +
> +static u32 cqspi_get_rd_sram_level(struct cqspi_st *cqspi)
> +{
> + u32 reg = readl(cqspi->iobase + CQSPI_REG_SDRAMLEVEL);
> +
> + reg >>= CQSPI_REG_SDRAMLEVEL_RD_LSB;
> + return reg & CQSPI_REG_SDRAMLEVEL_RD_MASK;
> +}
> +
> +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
> +{
> + struct cqspi_st *cqspi = dev;
> + unsigned int irq_status;
> +
> + /* Read interrupt status */
> + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + /* Clear interrupt */
> + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
> +
> + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
> +
> + if (irq_status)
> + complete(&cqspi->transfer_complete);
> +
You seem to signal completion even if the interrupt is
CQSPI_REG_IRQ_IND_(WR|RD)_OVERFLOW. Doesn't this lead to data loss on
overflow?
> + return IRQ_HANDLED;
> +}
> +
> +static unsigned int cqspi_calc_rdreg(struct spi_nor *nor, const u8 opcode)
> +{
> + unsigned int rdreg = 0;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + rdreg |= f_pdata->inst_width << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB;
> + rdreg |= f_pdata->addr_width << CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB;
> +
> + if (nor->flash_read == SPI_NOR_QUAD)
> + rdreg |= CQSPI_INST_TYPE_QUAD
> + << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
> + return rdreg;
> +}
> +
> +static int cqspi_wait_idle(struct cqspi_st *cqspi)
> +{
> + const unsigned int poll_idle_retry = 3;
> + unsigned int count = 0;
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(CQSPI_TIMEOUT_MS);
> + while (1) {
> + /*
> + * Read few times in succession to ensure the controller
> + * is indeed idle, that is, the bit does not transition
> + * low again.
> + */
> + if (cqspi_is_idle(cqspi))
> + count++;
> + else
> + count = 0;
> +
> + if (count >= poll_idle_retry)
> + return 0;
> +
> + if (time_after(jiffies, timeout)) {
> + /* Timeout, in busy mode. */
> + dev_err(&cqspi->pdev->dev,
> + "QSPI is still busy after %dms timeout.\n",
> + CQSPI_TIMEOUT_MS);
> + return -ETIMEDOUT;
> + }
> +
> + cpu_relax();
> + }
> +}
> +
> +static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + int ret;
> +
> + /* Write the CMDCTRL without start execution. */
> + writel(reg, reg_base + CQSPI_REG_CMDCTRL);
> + /* Start execute */
> + reg |= CQSPI_REG_CMDCTRL_EXECUTE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CMDCTRL);
> +
> + /* Polling for completion. */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_CMDCTRL,
> + CQSPI_REG_CMDCTRL_INPROGRESS_MASK, 1);
> + if (ret) {
> + dev_err(&cqspi->pdev->dev,
> + "Flash command execution timed out.\n");
> + return ret;
> + }
> +
> + /* Polling QSPI idle status. */
> + return cqspi_wait_idle(cqspi);
> +}
> +
> +static int cqspi_command_read(struct spi_nor *nor,
> + const u8 *txbuf, const unsigned n_tx,
> + u8 *rxbuf, const unsigned n_rx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int rdreg;
> + unsigned int reg;
> + unsigned int read_len;
> + int status;
> +
> + if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
> + dev_err(nor->dev, "Invalid input argument, len %d rxbuf 0x%p\n",
> + n_rx, rxbuf);
> + return -EINVAL;
> + }
> +
> + reg = txbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> +
> + rdreg = cqspi_calc_rdreg(nor, txbuf[0]);
> + writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
> +
> + /* 0 means 1 byte. */
> + reg |= (((n_rx - 1) & CQSPI_REG_CMDCTRL_RD_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_RD_BYTES_LSB);
> + status = cqspi_exec_flash_cmd(cqspi, reg);
> + if (status)
> + return status;
> +
> + reg = readl(reg_base + CQSPI_REG_CMDREADDATALOWER);
> +
> + /* Put the read value into rx_buf */
> + read_len = (n_rx > 4) ? 4 : n_rx;
> + memcpy(rxbuf, ®, read_len);
> + rxbuf += read_len;
> +
> + if (n_rx > 4) {
> + reg = readl(reg_base + CQSPI_REG_CMDREADDATAUPPER);
> +
> + read_len = n_rx - read_len;
> + memcpy(rxbuf, ®, read_len);
> + }
> +
> + return 0;
> +}
> +
> +static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
> + const u8 *txbuf, const unsigned n_tx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> + unsigned int data;
> + int ret;
> +
> + if (n_tx > 4 || (n_tx && !txbuf)) {
> + dev_err(nor->dev,
> + "Invalid input argument, cmdlen %d txbuf 0x%p\n",
> + n_tx, txbuf);
> + return -EINVAL;
> + }
> +
> + reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> + if (n_tx) {
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB);
> + reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
> + data = 0;
> + memcpy(&data, txbuf, n_tx);
> + writel(data, reg_base + CQSPI_REG_CMDWRITEDATALOWER);
> + }
> +
> + ret = cqspi_exec_flash_cmd(cqspi, reg);
> + return ret;
> +}
> +
> +static int cqspi_command_write_addr(struct spi_nor *nor,
> + const u8 opcode, const unsigned int addr)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> + reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
> + reg |= ((nor->addr_width - 1) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
> + << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
> +
> + writel(addr, reg_base + CQSPI_REG_CMDADDRESS);
> +
> + return cqspi_exec_flash_cmd(cqspi, reg);
> +}
> +
> +static int cqspi_indirect_read_setup(struct spi_nor *nor,
> + const unsigned int from_addr)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int dummy_clk = 0;
> + unsigned int reg;
> +
> + writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
> +
> + reg = nor->read_opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
> + reg |= cqspi_calc_rdreg(nor, nor->read_opcode);
> +
> + /* Setup dummy clock cycles */
> + dummy_clk = nor->read_dummy;
> + if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
> + dummy_clk = CQSPI_DUMMY_CLKS_MAX;
> +
> + if (dummy_clk / 8) {
> + reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
> + /* Set mode bits high to ensure chip doesn't enter XIP */
> + writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
> +
> + /* Need to subtract the mode byte (8 clocks). */
> + if (f_pdata->inst_width != CQSPI_INST_TYPE_QUAD)
> + dummy_clk -= 8;
> +
> + if (dummy_clk)
> + reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
> + << CQSPI_REG_RD_INSTR_DUMMY_LSB;
> + }
> +
> + writel(reg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + /* Set address width */
> + reg = readl(reg_base + CQSPI_REG_SIZE);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->addr_width - 1);
> + writel(reg, reg_base + CQSPI_REG_SIZE);
> + return 0;
> +}
> +
> +static int cqspi_indirect_read_execute(struct spi_nor *nor,
> + u8 *rxbuf, const unsigned n_rx)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + void __iomem *ahb_base = cqspi->ahb_base;
> + unsigned int remaining = n_rx;
> + unsigned int bytes_to_read = 0;
> + int ret = 0;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> +
> + while (remaining > 0) {
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_READ_TIMEOUT_MS));
> +
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> +
> + if (!ret && bytes_to_read == 0) {
> + dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> + ret = -ETIMEDOUT;
> + goto failrd;
> + }
> +
> + while (bytes_to_read != 0) {
> + bytes_to_read *= cqspi->fifo_width;
> + bytes_to_read = bytes_to_read > remaining ?
> + remaining : bytes_to_read;
> + readsl(ahb_base, rxbuf, DIV_ROUND_UP(bytes_to_read, 4));
> + rxbuf += bytes_to_read;
> + remaining -= bytes_to_read;
> + bytes_to_read = cqspi_get_rd_sram_level(cqspi);
> + }
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
> + CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect read completion error (%i)\n", ret);
> + goto failrd;
> + }
> +
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTRD_DONE_MASK, reg_base + CQSPI_REG_INDIRECTRD);
> +
> + return 0;
> +
> +failrd:
> + /* Disable interrupt */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect read */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTRD);
> + return ret;
> +}
> +
> +static int cqspi_indirect_write_setup(struct spi_nor *nor,
> + const unsigned int to_addr)
> +{
> + unsigned int reg;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> +
> + /* Set opcode. */
> + reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
> + writel(reg, reg_base + CQSPI_REG_WR_INSTR);
> + reg = cqspi_calc_rdreg(nor, nor->program_opcode);
> + writel(reg, reg_base + CQSPI_REG_RD_INSTR);
> +
> + writel(to_addr, reg_base + CQSPI_REG_INDIRECTWRSTARTADDR);
> +
> + reg = readl(reg_base + CQSPI_REG_SIZE);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->addr_width - 1);
> + writel(reg, reg_base + CQSPI_REG_SIZE);
> + return 0;
> +}
> +
> +static int cqspi_indirect_write_execute(struct spi_nor *nor,
> + const u8 *txbuf, const unsigned n_tx)
> +{
> + const unsigned int page_size = nor->page_size;
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int remaining = n_tx;
> + unsigned int write_bytes;
> + int ret;
> +
> + writel(remaining, reg_base + CQSPI_REG_INDIRECTWRBYTES);
> +
> + /* Clear all interrupts. */
> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
> +
> + writel(CQSPI_IRQ_MASK_WR, reg_base + CQSPI_REG_IRQMASK);
> +
> + reinit_completion(&cqspi->transfer_complete);
> + writel(CQSPI_REG_INDIRECTWR_START_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> +
> + while (remaining > 0) {
> + write_bytes = remaining > page_size ? page_size : remaining;
> + writesl(cqspi->ahb_base, txbuf, DIV_ROUND_UP(write_bytes, 4));
> +
> + ret = wait_for_completion_timeout(&cqspi->transfer_complete,
> + msecs_to_jiffies
> + (CQSPI_TIMEOUT_MS));
> + if (!ret) {
> + dev_err(nor->dev, "Indirect write timeout\n");
> + ret = -ETIMEDOUT;
> + goto failwr;
> + }
> +
> + txbuf += write_bytes;
> + remaining -= write_bytes;
> + }
> +
> + /* Check indirect done status */
> + ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTWR,
> + CQSPI_REG_INDIRECTWR_DONE_MASK, 0);
> + if (ret) {
> + dev_err(nor->dev,
> + "Indirect write completion error (%i)\n", ret);
> + goto failwr;
> + }
> +
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Clear indirect completion status */
> + writel(CQSPI_REG_INDIRECTWR_DONE_MASK, reg_base + CQSPI_REG_INDIRECTWR);
> +
> + cqspi_wait_idle(cqspi);
> +
> + return 0;
> +
> +failwr:
> + /* Disable interrupt. */
> + writel(0, reg_base + CQSPI_REG_IRQMASK);
> +
> + /* Cancel the indirect write */
> + writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
> + reg_base + CQSPI_REG_INDIRECTWR);
> + return ret;
> +}
> +
> +static void cqspi_write(struct spi_nor *nor, loff_t to,
> + size_t len, size_t *retlen, const u_char *buf)
> +{
> + int ret;
> +
> + ret = cqspi_indirect_write_setup(nor, to);
> + if (ret)
> + return;
> +
> + ret = cqspi_indirect_write_execute(nor, buf, len);
> + if (ret)
> + return;
> +
> + *retlen += len;
> +}
> +
> +static int cqspi_read(struct spi_nor *nor, loff_t from,
> + size_t len, size_t *retlen, u_char *buf)
> +{
> + int ret;
> +
> + ret = cqspi_indirect_read_setup(nor, from);
> + if (ret)
> + return ret;
> +
> + ret = cqspi_indirect_read_execute(nor, buf, len);
> + if (ret)
> + return ret;
> +
> + *retlen += len;
> + return ret;
> +}
> +
> +static int cqspi_erase(struct spi_nor *nor, loff_t offs)
> +{
> + int ret;
> +
> + /* Send write enable, then erase commands. */
> + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0, 0);
> + if (ret)
> + return ret;
> +
> + /* Set up command buffer. */
> + ret = cqspi_command_write_addr(nor, nor->erase_opcode, offs);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int cqspi_set_protocol(struct spi_nor *nor, enum spi_protocol proto)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_1_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_2_2_2:
> + f_pdata->inst_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_4_4_4:
> + f_pdata->inst_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (proto) {
> + case SPI_PROTO_1_1_1:
> + case SPI_PROTO_1_1_2:
> + case SPI_PROTO_1_1_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
> + break;
> + case SPI_PROTO_1_2_2:
> + case SPI_PROTO_2_2_2:
> + f_pdata->addr_width = CQSPI_INST_TYPE_DUAL;
> + break;
> + case SPI_PROTO_1_4_4:
> + case SPI_PROTO_4_4_4:
> + f_pdata->addr_width = CQSPI_INST_TYPE_QUAD;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned int calculate_ticks_for_ns(const unsigned int ref_clk_hz,
> + const unsigned int ns_val)
> +{
> + unsigned int ticks;
> +
> + ticks = ref_clk_hz / 1000; /* kHz */
> + ticks = DIV_ROUND_UP(ticks * ns_val, 1000000);
> +
> + return ticks;
> +}
> +
> +static void cqspi_delay(struct spi_nor *nor, const unsigned int sclk_hz)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *iobase = cqspi->iobase;
> + const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
> + unsigned int tshsl, tchsh, tslch, tsd2d;
> + unsigned int reg;
> + unsigned int tsclk;
> +
> + /* calculate the number of ref ticks for one sclk tick */
> + tsclk = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
> +
> + tshsl = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tshsl_ns);
> + /* this particular value must be at least one sclk */
> + if (tshsl < tsclk)
> + tshsl = tsclk;
> +
> + tchsh = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tchsh_ns);
> + tslch = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tslch_ns);
> + tsd2d = calculate_ticks_for_ns(ref_clk_hz, f_pdata->tsd2d_ns);
> +
> + reg = (tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
> + << CQSPI_REG_DELAY_TSHSL_LSB;
> + reg |= (tchsh & CQSPI_REG_DELAY_TCHSH_MASK)
> + << CQSPI_REG_DELAY_TCHSH_LSB;
> + reg |= (tslch & CQSPI_REG_DELAY_TSLCH_MASK)
> + << CQSPI_REG_DELAY_TSLCH_LSB;
> + reg |= (tsd2d & CQSPI_REG_DELAY_TSD2D_MASK)
> + << CQSPI_REG_DELAY_TSD2D_LSB;
> + writel(reg, iobase + CQSPI_REG_DELAY);
> +}
> +
> +static void cqspi_config_baudrate_div(struct cqspi_st *cqspi,
> + const unsigned int sclk_hz)
> +{
> + const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> + unsigned int div;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
> +
> + div = ref_clk_hz / sclk_hz;
> +
> + /* Recalculate the baudrate divisor based on QSPI specification. */
> + if (div > 32)
> + div = 32;
> +
> + /* Check if even number. */
> + if (div & 1)
> + div = (div / 2);
> + else
> + div = (div / 2) - 1;
> +
> + div = (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB;
> + reg |= div;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_readdata_capture(struct cqspi_st *cqspi,
> + const unsigned int bypass,
> + const unsigned int delay)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_READCAPTURE);
> +
> + if (bypass)
> + reg |= (1 << CQSPI_REG_READCAPTURE_BYPASS_LSB);
> + else
> + reg &= ~(1 << CQSPI_REG_READCAPTURE_BYPASS_LSB);
> +
> + reg &= ~(CQSPI_REG_READCAPTURE_DELAY_MASK
> + << CQSPI_REG_READCAPTURE_DELAY_LSB);
> +
> + reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
> + << CQSPI_REG_READCAPTURE_DELAY_LSB;
> +
> + writel(reg, reg_base + CQSPI_REG_READCAPTURE);
> +}
> +
> +static void cqspi_chipselect(struct spi_nor *nor)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int chip_select = f_pdata->cs;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + if (cqspi->is_decoded_cs) {
> + reg |= CQSPI_REG_CONFIG_DECODE_MASK;
> + } else {
> + reg &= ~CQSPI_REG_CONFIG_DECODE_MASK;
> +
> + /* Convert CS if without decoder.
> + * CS0 to 4b'1110
> + * CS1 to 4b'1101
> + * CS2 to 4b'1011
> + * CS3 to 4b'0111
> + */
> + chip_select = 0xF & ~(1 << chip_select);
> + }
> +
> + reg &= ~(CQSPI_REG_CONFIG_CHIPSELECT_MASK
> + << CQSPI_REG_CONFIG_CHIPSELECT_LSB);
> + reg |= (chip_select & CQSPI_REG_CONFIG_CHIPSELECT_MASK)
> + << CQSPI_REG_CONFIG_CHIPSELECT_LSB;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_controller_enable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_controller_disable(struct cqspi_st *cqspi)
> +{
> + void __iomem *reg_base = cqspi->iobase;
> + unsigned int reg;
> +
> + reg = readl(reg_base + CQSPI_REG_CONFIG);
> + reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
> + writel(reg, reg_base + CQSPI_REG_CONFIG);
> +}
> +
> +static void cqspi_switch_cs(struct spi_nor *nor)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + void __iomem *iobase = cqspi->iobase;
> + unsigned int reg;
> +
> + cqspi_controller_disable(cqspi);
> +
> + /* configure page size and block size. */
> + reg = readl(iobase + CQSPI_REG_SIZE);
> + reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB);
> + reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> + reg |= (nor->page_size << CQSPI_REG_SIZE_PAGE_LSB);
> + reg |= (ilog2(nor->mtd->erasesize) << CQSPI_REG_SIZE_BLOCK_LSB);
> + reg |= (nor->addr_width - 1);
> + writel(reg, iobase + CQSPI_REG_SIZE);
> +
> + /* configure the chip select */
> + cqspi_chipselect(nor);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + const unsigned int sclk = f_pdata->clk_rate;
> +
> + /* Switch chip select. */
> + if (cqspi->current_cs != f_pdata->cs) {
> + cqspi->current_cs = f_pdata->cs;
> + cqspi_switch_cs(nor);
> + }
> +
> + /* Setup baudrate divisor and delays */
> + if (cqspi->sclk != sclk) {
> + cqspi->sclk = sclk;
> + cqspi_controller_disable(cqspi);
> + cqspi_config_baudrate_div(cqspi, sclk);
> + cqspi_delay(nor, sclk);
> + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
I see bypass field value is being hard coded to 1. Atleast on TI SoC,
bypass=0 when SPI bus frequency is >= 50MHz. Therefore, is it possible
to provide a way to configure bypass bit from DT?
Or if bypass=0 for >=50MHz on your SoC too then, its better to configure
bypass bit based on SPI bus frequency.
> + cqspi_controller_enable(cqspi);
> + }
> + return 0;
> +}
> +
> +static int cqspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> +{
> + int ret;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_READ);
> +
> + ret = cqspi_command_read(nor, &opcode, 1, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
> + int write_enable)
> +{
> + int ret = 0;
> +
> + cqspi_prep(nor, SPI_NOR_OPS_WRITE);
> +
> + ret = cqspi_command_write(nor, opcode, buf, len);
> + return ret;
> +}
> +
> +static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
> + struct cqspi_flash_pdata *f_pdata,
> + struct device_node *np)
> +{
> + if (of_property_read_u32(np, "cdns,read-delay", &f_pdata->read_delay)) {
> + dev_err(&pdev->dev, "couldn't determine read-delay\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tshsl-ns", &f_pdata->tshsl_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tshsl-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tsd2d-ns", &f_pdata->tsd2d_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tsd2d-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tchsh-ns", &f_pdata->tchsh_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tchsh-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,tslch-ns", &f_pdata->tslch_ns)) {
> + dev_err(&pdev->dev, "couldn't determine tslch-ns\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "spi-max-frequency", &f_pdata->clk_rate)) {
> + dev_err(&pdev->dev, "couldn't determine spi-max-frequency\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static int cqspi_of_get_pdata(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +
> + cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
> +
> + if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-depth\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,fifo-width", &cqspi->fifo_width)) {
> + dev_err(&pdev->dev, "couldn't determine fifo-width\n");
> + return -ENXIO;
> + }
> +
> + if (of_property_read_u32(np, "cdns,trigger-address",
> + &cqspi->trigger_address)) {
> + dev_err(&pdev->dev, "couldn't determine trigger-address\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static void cqspi_controller_init(struct cqspi_st *cqspi)
> +{
> + cqspi_controller_disable(cqspi);
> +
> + /* Configure the remap address register, no remap */
> + writel(0, cqspi->iobase + CQSPI_REG_REMAP);
> +
> + /* Disable all interrupts. */
> + writel(0, cqspi->iobase + CQSPI_REG_IRQMASK);
> +
> + /* Configure the SRAM split to 1:1 . */
> + writel(cqspi->fifo_depth / 2, cqspi->iobase + CQSPI_REG_SRAMPARTITION);
> +
> + /* Load indirect trigger address. */
> + writel(cqspi->trigger_address,
> + cqspi->iobase + CQSPI_REG_INDIRECTTRIGGER);
> +
> + /* Program read watermark -- 1/2 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 2,
> + cqspi->iobase + CQSPI_REG_INDIRECTRDWATERMARK);
> + /* Program write watermark -- 1/8 of the FIFO. */
> + writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
> + cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
> +
> + cqspi_controller_enable(cqspi);
> +}
> +
> +static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> +{
> + struct platform_device *pdev = cqspi->pdev;
> + struct device *dev = &pdev->dev;
> + struct mtd_part_parser_data ppdata;
> + struct cqspi_flash_pdata *f_pdata;
> + struct spi_nor *nor;
> + struct mtd_info *mtd;
> + unsigned int cs;
> + int i, ret;
> +
> + /* Get flash device data */
> + for_each_available_child_of_node(dev->of_node, np) {
> + if (of_property_read_u32(np, "reg", &cs)) {
> + dev_err(dev, "Couldn't determine chip select.\n");
> + goto err;
> + }
> +
> + if (cs > CQSPI_MAX_CHIPSELECT) {
> + dev_err(dev, "Chip select %d out of range.\n", cs);
> + goto err;
> + }
> +
> + f_pdata = &cqspi->f_pdata[cs];
> + f_pdata->cqspi = cqspi;
> + f_pdata->cs = cs;
> +
> + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
> + if (ret)
> + goto err;
> +
> + nor = &f_pdata->nor;
> + mtd = &f_pdata->mtd;
> +
> + mtd->priv = nor;
> +
> + nor->mtd = mtd;
> + nor->dev = dev;
> + nor->dn = np;
> + nor->priv = f_pdata;
> +
> + nor->read_reg = cqspi_read_reg;
> + nor->write_reg = cqspi_write_reg;
> + nor->read = cqspi_read;
> + nor->write = cqspi_write;
> + nor->erase = cqspi_erase;
> + nor->prepare = cqspi_prep;
> + nor->set_protocol = cqspi_set_protocol;
IMO, this functionality can be added once dependent patches have been
merged. This will enable this driver to be picked up early. (just a
suggestion)
> +
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + if (ret)
> + goto err;
> +
> + ppdata.of_node = np;
> + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> + if (ret)
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> + return ret;
> +}
> +
> +static int cqspi_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct cqspi_st *cqspi;
> + struct resource *res;
> + struct resource *res_ahb;
> + int ret;
> + int irq;
> +
> + cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
> + if (!cqspi)
> + return -ENOMEM;
> +
> + cqspi->pdev = pdev;
> + platform_set_drvdata(pdev, cqspi);
> +
> + /* Obtain configuration from OF. */
> + ret = cqspi_of_get_pdata(pdev);
> + if (ret) {
> + dev_err(dev, "Cannot get mandatory OF data.\n");
> + return -ENODEV;
> + }
> +
> + /* Obtain QSPI clock. */
> + cqspi->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(cqspi->clk)) {
> + dev_err(dev, "Cannot claim QSPI clock.\n");
> + return PTR_ERR(cqspi->clk);
> + }
> +
> + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> +
> + /* Obtain and remap controller address. */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + cqspi->iobase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(cqspi->iobase)) {
> + dev_err(dev, "Cannot remap controller address.\n");
> + return PTR_ERR(cqspi->iobase);
> + }
> +
> + /* Obtain and remap AHB address. */
> + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
> + if (IS_ERR(cqspi->ahb_base)) {
> + dev_err(dev, "Cannot remap AHB address.\n");
> + return PTR_ERR(cqspi->ahb_base);
> + }
> +
> + init_completion(&cqspi->transfer_complete);
> +
> + /* Obtain IRQ line. */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "Cannot obtain IRQ.\n");
> + return -ENXIO;
> + }
> +
> + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> + pdev->name, cqspi);
> + if (ret) {
> + dev_err(dev, "Cannot request IRQ.\n");
> + return ret;
> + }
> +
pm_runtime_enable(), pm_runtime_get_sync() might be needed here as QSPI
clocks may not be enabled by default on all platforms.
> + cqspi_wait_idle(cqspi);
> + cqspi_controller_init(cqspi);
> + cqspi->current_cs = -1;
> + cqspi->sclk = 0;
> +
> + ret = cqspi_setup_flash(cqspi, np);
> + if (ret) {
> + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> + cqspi_controller_disable(cqspi);
> + }
> +
> + return ret;
> +}
> +
> +static int cqspi_remove(struct platform_device *pdev)
> +{
> + struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> + int i;
> +
> + cqspi_controller_disable(cqspi);
> +
> + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> + if (cqspi->f_pdata[i].mtd.name)
> + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int cqspi_suspend(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_disable(cqspi);
> + return 0;
> +}
> +
> +static int cqspi_resume(struct device *dev)
> +{
> + struct cqspi_st *cqspi = dev_get_drvdata(dev);
> +
> + cqspi_controller_enable(cqspi);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops cqspi__dev_pm_ops = {
> + .suspend = cqspi_suspend,
> + .resume = cqspi_resume,
> +};
> +
> +#define CQSPI_DEV_PM_OPS (&cqspi__dev_pm_ops)
> +#else
> +#define CQSPI_DEV_PM_OPS NULL
> +#endif
> +
> +static struct of_device_id const cqspi_dt_ids[] = {
> + {.compatible = "cdns,qspi-nor",},
> + { /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> +
> +static struct platform_driver cqspi_platform_driver = {
> + .probe = cqspi_probe,
> + .remove = cqspi_remove,
> + .driver = {
> + .name = CQSPI_NAME,
> + .pm = CQSPI_DEV_PM_OPS,
> + .of_match_table = cqspi_dt_ids,
> + },
> +};
> +
> +module_platform_driver(cqspi_platform_driver);
> +
> +MODULE_DESCRIPTION("Cadence QSPI Controller Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" CQSPI_NAME);
> +MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
> +MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2016-01-11 4:14 ` [2/2] " R, Vignesh
@ 2016-01-11 4:50 ` Marek Vasut
2016-01-12 4:59 ` Vignesh R
0 siblings, 1 reply; 23+ messages in thread
From: Marek Vasut @ 2016-01-11 4:50 UTC (permalink / raw)
To: R, Vignesh, Graham Moore
Cc: linux-mtd, devicetree, Alan Tull, Yves Vandervennet, Dinh Nguyen,
Brian Norris, David Woodhouse
On Monday, January 11, 2016 at 05:14:17 AM, R, Vignesh wrote:
> Hi Marek,
Hi!
> On 08/21/2015 02:50 PM, Marek Vasut wrote:
> > From: Graham Moore <grmoore@opensource.altera.com>
> >
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
>
> Thanks for the patch.
> I have a TI EVM with Cadence QSPI controller connected to Spansion
> flash(s25fl512s), and I gave this series a try. It worked with couple of
> changes, please see comments inline.
I just posted the latest greatest version I have in tree, could you give it
a spin please ?
> > Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Alan Tull <atull@opensource.altera.com>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Graham Moore <grmoore@opensource.altera.com>
> > Cc: Vikas MANOCHA <vikas.manocha@st.com>
> > Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> > Cc: devicetree@vger.kernel.org
[...]
> > +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
> > +{
> > + struct cqspi_st *cqspi = dev;
> > + unsigned int irq_status;
> > +
> > + /* Read interrupt status */
> > + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
> > +
> > + /* Clear interrupt */
> > + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
> > +
> > + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
> > +
> > + if (irq_status)
> > + complete(&cqspi->transfer_complete);
> > +
>
> You seem to signal completion even if the interrupt is
> CQSPI_REG_IRQ_IND_(WR|RD)_OVERFLOW. Doesn't this lead to data loss on
> overflow?
That's a good question. The write routine, cqspi_indirect_write_execute(),
always writes a page and then waits for completion, so the overflow should
never happen.
What would you suggest to do about write overflow interrupt anyway ? :(
> > + return IRQ_HANDLED;
> > +}
[...]
> > +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> > +{
> > + struct cqspi_flash_pdata *f_pdata = nor->priv;
> > + struct cqspi_st *cqspi = f_pdata->cqspi;
> > + const unsigned int sclk = f_pdata->clk_rate;
> > +
> > + /* Switch chip select. */
> > + if (cqspi->current_cs != f_pdata->cs) {
> > + cqspi->current_cs = f_pdata->cs;
> > + cqspi_switch_cs(nor);
> > + }
> > +
> > + /* Setup baudrate divisor and delays */
> > + if (cqspi->sclk != sclk) {
> > + cqspi->sclk = sclk;
> > + cqspi_controller_disable(cqspi);
> > + cqspi_config_baudrate_div(cqspi, sclk);
> > + cqspi_delay(nor, sclk);
> > + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
>
> I see bypass field value is being hard coded to 1. Atleast on TI SoC,
> bypass=0 when SPI bus frequency is >= 50MHz. Therefore, is it possible
> to provide a way to configure bypass bit from DT?
> Or if bypass=0 for >=50MHz on your SoC too then, its better to configure
> bypass bit based on SPI bus frequency.
You can have multiple SPI peripherals attached to the QSPI block, each running
at different bus speed, so I think the bypass should be configured based on the
frequency of the active peripheral, not from OF.
I don't know exactly what impact configuring this bit has, the socfpga datasheet
is not very clear about the meaning of this bit, so it'd be better if someone
from Altera answered this question.
> > + cqspi_controller_enable(cqspi);
> > + }
> > + return 0;
> > +}
[...]
> > +static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node
> > *np) +{
> > + struct platform_device *pdev = cqspi->pdev;
> > + struct device *dev = &pdev->dev;
> > + struct mtd_part_parser_data ppdata;
> > + struct cqspi_flash_pdata *f_pdata;
> > + struct spi_nor *nor;
> > + struct mtd_info *mtd;
> > + unsigned int cs;
> > + int i, ret;
> > +
> > + /* Get flash device data */
> > + for_each_available_child_of_node(dev->of_node, np) {
> > + if (of_property_read_u32(np, "reg", &cs)) {
> > + dev_err(dev, "Couldn't determine chip select.\n");
> > + goto err;
> > + }
> > +
> > + if (cs > CQSPI_MAX_CHIPSELECT) {
> > + dev_err(dev, "Chip select %d out of range.\n", cs);
> > + goto err;
> > + }
> > +
> > + f_pdata = &cqspi->f_pdata[cs];
> > + f_pdata->cqspi = cqspi;
> > + f_pdata->cs = cs;
> > +
> > + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
> > + if (ret)
> > + goto err;
> > +
> > + nor = &f_pdata->nor;
> > + mtd = &f_pdata->mtd;
> > +
> > + mtd->priv = nor;
> > +
> > + nor->mtd = mtd;
> > + nor->dev = dev;
> > + nor->dn = np;
> > + nor->priv = f_pdata;
> > +
> > + nor->read_reg = cqspi_read_reg;
> > + nor->write_reg = cqspi_write_reg;
> > + nor->read = cqspi_read;
> > + nor->write = cqspi_write;
> > + nor->erase = cqspi_erase;
> > + nor->prepare = cqspi_prep;
> >
> >
> > + nor->set_protocol = cqspi_set_protocol;
>
> IMO, this functionality can be added once dependent patches have been
> merged. This will enable this driver to be picked up early. (just a
> suggestion)
Yeah, it's a struggle to get this driver mainlined and the same problem is
with the Atmel driver ;-) But something tells me the patches will be applied
soon, so no problem here.
> > +
> > + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> > + if (ret)
> > + goto err;
> > +
> > + ppdata.of_node = np;
> > + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> > + if (ret)
> > + goto err;
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> > + if (cqspi->f_pdata[i].mtd.name)
> > + mtd_device_unregister(&cqspi->f_pdata[i].mtd);
> > + return ret;
> > +}
> > +
> > +static int cqspi_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device *dev = &pdev->dev;
> > + struct cqspi_st *cqspi;
> > + struct resource *res;
> > + struct resource *res_ahb;
> > + int ret;
> > + int irq;
> > +
> > + cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
> > + if (!cqspi)
> > + return -ENOMEM;
> > +
> > + cqspi->pdev = pdev;
> > + platform_set_drvdata(pdev, cqspi);
> > +
> > + /* Obtain configuration from OF. */
> > + ret = cqspi_of_get_pdata(pdev);
> > + if (ret) {
> > + dev_err(dev, "Cannot get mandatory OF data.\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Obtain QSPI clock. */
> > + cqspi->clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(cqspi->clk)) {
> > + dev_err(dev, "Cannot claim QSPI clock.\n");
> > + return PTR_ERR(cqspi->clk);
> > + }
> > +
> > + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> > +
> > + /* Obtain and remap controller address. */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + cqspi->iobase = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(cqspi->iobase)) {
> > + dev_err(dev, "Cannot remap controller address.\n");
> > + return PTR_ERR(cqspi->iobase);
> > + }
> > +
> > + /* Obtain and remap AHB address. */
> > + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
> > + if (IS_ERR(cqspi->ahb_base)) {
> > + dev_err(dev, "Cannot remap AHB address.\n");
> > + return PTR_ERR(cqspi->ahb_base);
> > + }
> > +
> > + init_completion(&cqspi->transfer_complete);
> > +
> > + /* Obtain IRQ line. */
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(dev, "Cannot obtain IRQ.\n");
> > + return -ENXIO;
> > + }
> > +
> > + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> > + pdev->name, cqspi);
> > + if (ret) {
> > + dev_err(dev, "Cannot request IRQ.\n");
> > + return ret;
> > + }
> > +
>
> pm_runtime_enable(), pm_runtime_get_sync() might be needed here as QSPI
> clocks may not be enabled by default on all platforms.
Can you whip up a bit of code and test it on your platform ? I dont think
socfpga has runtime PM yet. I'd like to add it into the driver or you can
send a small patch later, whichever way you prefer.
> > + cqspi_wait_idle(cqspi);
> > + cqspi_controller_init(cqspi);
> > + cqspi->current_cs = -1;
> > + cqspi->sclk = 0;
> > +
> > + ret = cqspi_setup_flash(cqspi, np);
> > + if (ret) {
> > + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> > + cqspi_controller_disable(cqspi);
> > + }
> > +
> > + return ret;
> > +}
Thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2016-01-11 4:50 ` Marek Vasut
@ 2016-01-12 4:59 ` Vignesh R
2016-01-12 13:50 ` Marek Vasut
0 siblings, 1 reply; 23+ messages in thread
From: Vignesh R @ 2016-01-12 4:59 UTC (permalink / raw)
To: Marek Vasut, Graham Moore
Cc: linux-mtd, devicetree, Alan Tull, Yves Vandervennet, Dinh Nguyen,
Brian Norris, David Woodhouse
Hi,
On 01/11/2016 10:20 AM, Marek Vasut wrote:
> On Monday, January 11, 2016 at 05:14:17 AM, R, Vignesh wrote:
>> Hi Marek,
>
> Hi!
>
>> On 08/21/2015 02:50 PM, Marek Vasut wrote:
>>> From: Graham Moore <grmoore@opensource.altera.com>
>>>
>>> Add support for the Cadence QSPI controller. This controller is
>>> present in the Altera SoCFPGA SoCs and this driver has been tested
>>> on the Cyclone V SoC.
>>
>> Thanks for the patch.
>> I have a TI EVM with Cadence QSPI controller connected to Spansion
>> flash(s25fl512s), and I gave this series a try. It worked with couple of
>> changes, please see comments inline.
>
> I just posted the latest greatest version I have in tree, could you give it
> a spin please ?
>
Thanks, I will test it soon.
>>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Alan Tull <atull@opensource.altera.com>
>>> Cc: Brian Norris <computersforpeace@gmail.com>
>>> Cc: David Woodhouse <dwmw2@infradead.org>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Graham Moore <grmoore@opensource.altera.com>
>>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
>>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
>>> Cc: devicetree@vger.kernel.org
>
> [...]
>
>>> +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
>>> +{
>>> + struct cqspi_st *cqspi = dev;
>>> + unsigned int irq_status;
>>> +
>>> + /* Read interrupt status */
>>> + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
>>> +
>>> + /* Clear interrupt */
>>> + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
>>> +
>>> + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
>>> +
>>> + if (irq_status)
>>> + complete(&cqspi->transfer_complete);
>>> +
>>
>> You seem to signal completion even if the interrupt is
>> CQSPI_REG_IRQ_IND_(WR|RD)_OVERFLOW. Doesn't this lead to data loss on
>> overflow?
>
> That's a good question. The write routine, cqspi_indirect_write_execute(),
> always writes a page and then waits for completion, so the overflow should
> never happen.
>
> What would you suggest to do about write overflow interrupt anyway ? :(
>
My concern was signalling completion on CQSPI_REG_IRQ_IND_WR_OVERFLOW
interrupt which meant data loss.
But, I see you have disabled CQSPI_REG_IRQ_IND_WR_OVERFLOW in v10. This
addresses my concern.
>>> + return IRQ_HANDLED;
>>> +}
>
> [...]
>
>>> +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>> +{
>>> + struct cqspi_flash_pdata *f_pdata = nor->priv;
>>> + struct cqspi_st *cqspi = f_pdata->cqspi;
>>> + const unsigned int sclk = f_pdata->clk_rate;
>>> +
>>> + /* Switch chip select. */
>>> + if (cqspi->current_cs != f_pdata->cs) {
>>> + cqspi->current_cs = f_pdata->cs;
>>> + cqspi_switch_cs(nor);
>>> + }
>>> +
>>> + /* Setup baudrate divisor and delays */
>>> + if (cqspi->sclk != sclk) {
>>> + cqspi->sclk = sclk;
>>> + cqspi_controller_disable(cqspi);
>>> + cqspi_config_baudrate_div(cqspi, sclk);
>>> + cqspi_delay(nor, sclk);
>>> + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
>>
>> I see bypass field value is being hard coded to 1. Atleast on TI SoC,
>> bypass=0 when SPI bus frequency is >= 50MHz. Therefore, is it possible
>> to provide a way to configure bypass bit from DT?
>> Or if bypass=0 for >=50MHz on your SoC too then, its better to configure
>> bypass bit based on SPI bus frequency.
>
> You can have multiple SPI peripherals attached to the QSPI block, each running
> at different bus speed, so I think the bypass should be configured based on the
> frequency of the active peripheral, not from OF.
I agree, if the dependency is uniform across SoCs, then bypass should be
configured based on the frequency of the active peripheral.
>
> I don't know exactly what impact configuring this bit has, the socfpga datasheet
> is not very clear about the meaning of this bit, so it'd be better if someone
> from Altera answered this question.
>
Have you tested the driver at SPI bus frequency >50MHz?
>>> + cqspi_controller_enable(cqspi);
>>> + }
>>> + return 0;
>>> +}
>
> [...]
>
[...]
>>> + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>> + pdev->name, cqspi);
>>> + if (ret) {
>>> + dev_err(dev, "Cannot request IRQ.\n");
>>> + return ret;
>>> + }
>>> +
>>
>> pm_runtime_enable(), pm_runtime_get_sync() might be needed here as QSPI
>> clocks may not be enabled by default on all platforms.
>
> Can you whip up a bit of code and test it on your platform ? I dont think
> socfpga has runtime PM yet. I'd like to add it into the driver or you can
> send a small patch later, whichever way you prefer.
>
Ok, I can send a patch once the driver is accepted.
>>> + cqspi_wait_idle(cqspi);
>>> + cqspi_controller_init(cqspi);
>>> + cqspi->current_cs = -1;
>>> + cqspi->sclk = 0;
>>> +
>>> + ret = cqspi_setup_flash(cqspi, np);
>>> + if (ret) {
>>> + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
>>> + cqspi_controller_disable(cqspi);
>>> + }
>>> +
>>> + return ret;
>>> +}
>
> Thanks!
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
2016-01-12 4:59 ` Vignesh R
@ 2016-01-12 13:50 ` Marek Vasut
0 siblings, 0 replies; 23+ messages in thread
From: Marek Vasut @ 2016-01-12 13:50 UTC (permalink / raw)
To: Vignesh R
Cc: Graham Moore, linux-mtd, devicetree, Alan Tull, Yves Vandervennet,
Dinh Nguyen, Brian Norris, David Woodhouse
On Tuesday, January 12, 2016 at 05:59:11 AM, Vignesh R wrote:
> Hi,
Hi!
> On 01/11/2016 10:20 AM, Marek Vasut wrote:
> > On Monday, January 11, 2016 at 05:14:17 AM, R, Vignesh wrote:
> >> Hi Marek,
> >
> > Hi!
> >
> >> On 08/21/2015 02:50 PM, Marek Vasut wrote:
> >>> From: Graham Moore <grmoore@opensource.altera.com>
> >>>
> >>> Add support for the Cadence QSPI controller. This controller is
> >>> present in the Altera SoCFPGA SoCs and this driver has been tested
> >>> on the Cyclone V SoC.
> >>
> >> Thanks for the patch.
> >> I have a TI EVM with Cadence QSPI controller connected to Spansion
> >> flash(s25fl512s), and I gave this series a try. It worked with couple of
> >> changes, please see comments inline.
> >
> > I just posted the latest greatest version I have in tree, could you give
> > it a spin please ?
>
> Thanks, I will test it soon.
Cool
> >>> Signed-off-by: Graham Moore <grmoore@opensource.altera.com>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Alan Tull <atull@opensource.altera.com>
> >>> Cc: Brian Norris <computersforpeace@gmail.com>
> >>> Cc: David Woodhouse <dwmw2@infradead.org>
> >>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> >>> Cc: Graham Moore <grmoore@opensource.altera.com>
> >>> Cc: Vikas MANOCHA <vikas.manocha@st.com>
> >>> Cc: Yves Vandervennet <yvanderv@opensource.altera.com>
> >>> Cc: devicetree@vger.kernel.org
> >
> > [...]
> >
> >>> +static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
> >>> +{
> >>> + struct cqspi_st *cqspi = dev;
> >>> + unsigned int irq_status;
> >>> +
> >>> + /* Read interrupt status */
> >>> + irq_status = readl(cqspi->iobase + CQSPI_REG_IRQSTATUS);
> >>> +
> >>> + /* Clear interrupt */
> >>> + writel(irq_status, cqspi->iobase + CQSPI_REG_IRQSTATUS);
> >>> +
> >>> + irq_status &= CQSPI_IRQ_MASK_RD | CQSPI_IRQ_MASK_WR;
> >>> +
> >>> + if (irq_status)
> >>> + complete(&cqspi->transfer_complete);
> >>> +
> >>
> >> You seem to signal completion even if the interrupt is
> >> CQSPI_REG_IRQ_IND_(WR|RD)_OVERFLOW. Doesn't this lead to data loss on
> >> overflow?
> >
> > That's a good question. The write routine,
> > cqspi_indirect_write_execute(), always writes a page and then waits for
> > completion, so the overflow should never happen.
> >
> > What would you suggest to do about write overflow interrupt anyway ? :(
>
> My concern was signalling completion on CQSPI_REG_IRQ_IND_WR_OVERFLOW
> interrupt which meant data loss.
> But, I see you have disabled CQSPI_REG_IRQ_IND_WR_OVERFLOW in v10. This
> addresses my concern.
All right :)
> >>> + return IRQ_HANDLED;
> >>> +}
> >
> > [...]
> >
> >>> +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> >>> +{
> >>> + struct cqspi_flash_pdata *f_pdata = nor->priv;
> >>> + struct cqspi_st *cqspi = f_pdata->cqspi;
> >>> + const unsigned int sclk = f_pdata->clk_rate;
> >>> +
> >>> + /* Switch chip select. */
> >>> + if (cqspi->current_cs != f_pdata->cs) {
> >>> + cqspi->current_cs = f_pdata->cs;
> >>> + cqspi_switch_cs(nor);
> >>> + }
> >>> +
> >>> + /* Setup baudrate divisor and delays */
> >>> + if (cqspi->sclk != sclk) {
> >>> + cqspi->sclk = sclk;
> >>> + cqspi_controller_disable(cqspi);
> >>> + cqspi_config_baudrate_div(cqspi, sclk);
> >>> + cqspi_delay(nor, sclk);
> >>> + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay);
> >>
> >> I see bypass field value is being hard coded to 1. Atleast on TI SoC,
> >> bypass=0 when SPI bus frequency is >= 50MHz. Therefore, is it possible
> >> to provide a way to configure bypass bit from DT?
> >> Or if bypass=0 for >=50MHz on your SoC too then, its better to configure
> >> bypass bit based on SPI bus frequency.
> >
> > You can have multiple SPI peripherals attached to the QSPI block, each
> > running at different bus speed, so I think the bypass should be
> > configured based on the frequency of the active peripheral, not from OF.
>
> I agree, if the dependency is uniform across SoCs, then bypass should be
> configured based on the frequency of the active peripheral.
>
> > I don't know exactly what impact configuring this bit has, the socfpga
> > datasheet is not very clear about the meaning of this bit, so it'd be
> > better if someone from Altera answered this question.
>
> Have you tested the driver at SPI bus frequency >50MHz?
Yeah, I run it at 100MHz .
> >>> + cqspi_controller_enable(cqspi);
> >>> + }
> >>> + return 0;
> >>> +}
> >
> > [...]
>
> [...]
>
> >>> + ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> >>> + pdev->name, cqspi);
> >>> + if (ret) {
> >>> + dev_err(dev, "Cannot request IRQ.\n");
> >>> + return ret;
> >>> + }
> >>> +
> >>
> >> pm_runtime_enable(), pm_runtime_get_sync() might be needed here as QSPI
> >> clocks may not be enabled by default on all platforms.
> >
> > Can you whip up a bit of code and test it on your platform ? I dont think
> > socfpga has runtime PM yet. I'd like to add it into the driver or you can
> > send a small patch later, whichever way you prefer.
>
> Ok, I can send a patch once the driver is accepted.
Thanks!
> >>> + cqspi_wait_idle(cqspi);
> >>> + cqspi_controller_init(cqspi);
> >>> + cqspi->current_cs = -1;
> >>> + cqspi->sclk = 0;
> >>> +
> >>> + ret = cqspi_setup_flash(cqspi, np);
> >>> + if (ret) {
> >>> + dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> >>> + cqspi_controller_disable(cqspi);
> >>> + }
> >>> +
> >>> + return ret;
> >>> +}
> >
> > Thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread