Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754199AbbETW0U (ORCPT ); Wed, 20 May 2015 18:26:20 -0400 Received: from mail-pa0-f53.google.com ([209.85.220.53]:33072 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753924AbbETW0O (ORCPT ); Wed, 20 May 2015 18:26:14 -0400 Date: Wed, 20 May 2015 15:26:06 -0700 From: Brian Norris To: Graham Moore Cc: linux-mtd@lists.infradead.org, David Woodhouse , linux-kernel@vger.kernel.org, Alan Tull , Dinh Nguyen , Yves Vandervennet Subject: Re: [PATCH V4 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller. Message-ID: <20150520222606.GQ11598@ld-irv-0074> References: <1427117782-14764-1-git-send-email-grmoore@opensource.altera.com> <1427117782-14764-2-git-send-email-grmoore@opensource.altera.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1427117782-14764-2-git-send-email-grmoore@opensource.altera.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 44514 Lines: 1443 On Mon, Mar 23, 2015 at 08:36:22AM -0500, Graham Moore wrote: > Signed-off-by: Graham Moore > --- > 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. > --- > drivers/mtd/spi-nor/Kconfig | 6 + > drivers/mtd/spi-nor/Makefile | 1 + > drivers/mtd/spi-nor/cadence-quadspi.c | 1297 +++++++++++++++++++++++++++++++++ > 3 files changed, 1304 insertions(+) > create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c > > diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig > index 64a4f0e..9485481 100644 > --- a/drivers/mtd/spi-nor/Kconfig > +++ b/drivers/mtd/spi-nor/Kconfig > @@ -28,4 +28,10 @@ config SPI_FSL_QUADSPI > This enables support for the Quad SPI controller in master mode. > We only connect the NOR to this controller now. > > +config SPI_CADENCE_QUADSPI > + tristate "Cadence Quad SPI controller" > + depends on ARCH_SOCFPGA > + help > + This enables support for the Cadence Quad SPI controller and NOR flash. > + > endif # MTD_SPI_NOR > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile > index 6a7ce14..372628c 100644 > --- a/drivers/mtd/spi-nor/Makefile > +++ b/drivers/mtd/spi-nor/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o > +obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o > obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.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..fa7b421 > --- /dev/null > +++ b/drivers/mtd/spi-nor/cadence-quadspi.c > @@ -0,0 +1,1297 @@ > +/* > + * 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 . > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CQSPI_NAME "cadence-qspi" > +#define CQSPI_MAX_CHIPSELECT 16 > + > +struct cqspi_flash_pdata { > + struct mtd_info mtd; > + struct spi_nor nor; > + u32 clk_rate; > + unsigned int page_size; > + unsigned int block_size; > + unsigned int read_delay; > + unsigned int tshsl_ns; > + unsigned int tsd2d_ns; > + unsigned int tchsh_ns; > + unsigned int tslch_ns; > + int quad_mode_set; > +}; > + > +struct cqspi_st { > + struct platform_device *pdev; > + > + struct clk *clk; > + unsigned int sclk; > + > + void __iomem *iobase; > + void __iomem *ahb_base; > + struct completion transfer_complete; > + unsigned int irq_mask; > + int current_cs; > + unsigned int ahb_phy_addr; > + unsigned int master_ref_clk_hz; > + u32 is_decoded_cs; > + unsigned int fifo_depth; > + struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT]; > +}; > + > +/* Operation timeout value */ > +#define CQSPI_TIMEOUT_MS 500 > +#define CQSPI_READ_TIMEOUT_MS 10 > +#define CQSPI_POLL_IDLE_RETRY 3 > + > +#define CQSPI_FIFO_WIDTH 4 > + > +#define CQSPI_REG_SRAM_THRESHOLD_BYTES 50 > + > +/* 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_STIG_DATA_LEN_MAX 8 > + > +#define CQSPI_INDIRECTTRIGGER_ADDR_MASK 0xFFFFF > + > +/* 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_STATUS_ERR (CQSPI_REG_IRQ_MODE_ERR | \ > + CQSPI_REG_IRQ_IND_RD_REJECT | \ > + CQSPI_REG_IRQ_WR_PROTECTED_ERR | \ > + CQSPI_REG_IRQ_ILLEGAL_AHB_ERR) > + > +#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 > + > +#define CQSPI_REG_IS_IDLE(base) \ > + ((readl(base + CQSPI_REG_CONFIG) >> \ > + CQSPI_REG_CONFIG_IDLE_LSB) & 0x1) > + > +#define CQSPI_GET_RD_SRAM_LEVEL(reg_base) \ > + (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > + CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK) > + > +static unsigned int cqspi_init_timeout(unsigned long timeout_in_ms) > +{ > + return jiffies + msecs_to_jiffies(timeout_in_ms); > +} > + > +static unsigned int cqspi_check_timeout(unsigned long timeout) > +{ > + return time_before(jiffies, timeout); > +} > + > +static void cqspi_fifo_read(void *dest, const void *src_ahb_addr, > + unsigned int bytes) > +{ > + unsigned int temp; > + int remaining = bytes; > + unsigned int *dest_ptr = (unsigned int *)dest; > + unsigned int *src_ptr = (unsigned int *)src_ahb_addr; > + > + while (remaining > CQSPI_FIFO_WIDTH) { > + *dest_ptr = readl(src_ptr); sparse has a lot of complaints: drivers/mtd/spi-nor/cadence-quadspi.c:256:29: warning: incorrect type in argument 1 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:256:29: expected void const volatile [noderef] *addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:256:29: got unsigned int *src_ptr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:262:24: warning: incorrect type in argument 1 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:262:24: expected void const volatile [noderef] *addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:262:24: got unsigned int *src_ptr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:276:17: warning: incorrect type in argument 2 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:276:17: expected void volatile [noderef] *addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:276:17: got unsigned int *dest_ptr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:283:17: warning: incorrect type in argument 2 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:283:17: expected void volatile [noderef] *addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:283:17: got unsigned int *dest_ptr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:595:48: warning: incorrect type in argument 2 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:595:48: expected void const *src_ahb_addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:595:48: got void [noderef] *ahb_base [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:691:26: warning: incorrect type in argument 1 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:691:26: expected void *dest_ahb_addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:691:26: got void [noderef] *ahb_base [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:706:34: warning: incorrect type in argument 1 (different address spaces) [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:706:34: expected void *dest_ahb_addr [sparse] drivers/mtd/spi-nor/cadence-quadspi.c:706:34: got void [noderef] *ahb_base [sparse] You should keep the __iomem type annotation on the void* MMIO addresses. > + dest_ptr++; > + remaining -= CQSPI_FIFO_WIDTH; > + } > + if (remaining > 0) { > + /* dangling bytes */ > + temp = readl(src_ptr); > + memcpy(dest_ptr, &temp, remaining); > + } > +} > + > +static void cqspi_fifo_write(void *dest_ahb_addr, > + const void *src, unsigned int bytes) > +{ > + unsigned int temp; > + int remaining = bytes; > + unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr; > + unsigned int *src_ptr = (unsigned int *)src; > + > + while (remaining > CQSPI_FIFO_WIDTH) { > + writel(*src_ptr, dest_ptr); > + src_ptr++; > + remaining -= CQSPI_FIFO_WIDTH; > + } > + if (remaining > 0) { > + /* dangling bytes */ > + memcpy(&temp, src_ptr, remaining); > + writel(temp, dest_ptr); > + } > +} > + > +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); > + > + if (irq_status & cqspi->irq_mask) > + complete(&cqspi->transfer_complete); > + > + return IRQ_HANDLED; > +} > + > +static int cqspi_find_chipselect(struct spi_nor *nor) > +{ > + int cs = -1; > + struct cqspi_st *cqspi = nor->priv; > + > + for (cs = 0; cs < CQSPI_MAX_CHIPSELECT; cs++) > + if (nor == &cqspi->f_pdata[cs].nor) > + break; > + return cs; > +} > + > +static unsigned int cqspi_calc_rdreg(struct spi_nor *nor, u8 opcode) > +{ > + unsigned int rdreg = 0; > + struct cqspi_st *cqspi = nor->priv; > + struct cqspi_flash_pdata *f_pdata; > + > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + > + if (f_pdata->quad_mode_set) > + rdreg |= (CQSPI_INST_TYPE_QUAD > + << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB); > + else if (nor->flash_read == SPI_NOR_QUAD) > + rdreg |= (CQSPI_INST_TYPE_QUAD > + << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB); > + return rdreg; > +} > + > +static unsigned int cqspi_wait_idle(struct cqspi_st *cqspi) > +{ > + void __iomem *reg_base = cqspi->iobase; > + unsigned int count = 0; > + unsigned timeout; > + > + timeout = cqspi_init_timeout(CQSPI_TIMEOUT_MS); > + while (cqspi_check_timeout(timeout)) { > + if (CQSPI_REG_IS_IDLE(reg_base)) { > + /* > + * Read few times in succession to ensure it does > + * not transition low again > + */ > + count++; > + if (count >= CQSPI_POLL_IDLE_RETRY) > + return 1; > + } else { > + count = 0; > + } > + cpu_relax(); > + } > + > + /* Timeout, in busy mode. */ > + dev_err(&cqspi->pdev->dev, "QSPI is still busy after %dms timeout.\n", > + CQSPI_TIMEOUT_MS); > + return 0; > +} > + > +static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg) > +{ > + void __iomem *reg_base = cqspi->iobase; > + unsigned int timeout; > + > + /* 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. */ > + timeout = cqspi_init_timeout(CQSPI_TIMEOUT_MS); > + while (cqspi_check_timeout(timeout)) { > + reg = readl(reg_base + CQSPI_REG_CMDCTRL) & > + CQSPI_REG_CMDCTRL_INPROGRESS_MASK; > + if (!reg) > + break; > + } > + > + if (reg != 0) { > + dev_err(&cqspi->pdev->dev, "flash cmd execute time out\n"); > + return -EIO; > + } > + > + /* Polling QSPI idle status. */ > + if (!cqspi_wait_idle(cqspi)) > + return -EIO; > + > + return 0; > +} > + > +static int cqspi_command_read(struct spi_nor *nor, > + const u8 *txbuf, unsigned n_tx, > + u8 *rxbuf, unsigned n_rx) > +{ > + unsigned int reg; > + unsigned int read_len; > + int status; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + unsigned int rdreg; > + > + if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { > + dev_err(nor->dev, > + "Invalid input argument, len %d rxbuf 0x%08x\n", n_rx, > + (unsigned int)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 != 0) > + 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, > + u8 opcode, const u8 *txbuf, unsigned n_tx) > +{ > + unsigned int reg; > + unsigned int data; > + int ret; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + > + if (n_tx > 4 || (n_tx && !txbuf)) { > + dev_err(nor->dev, > + "Invalid input argument, cmdlen %d txbuf 0x%08x\n", > + n_tx, (unsigned int)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); > + /* > + * Hack it up, watch for Micron quad mode command so that > + * we use proper controller setup for next command > + */ > + if (n_tx && opcode == SPINOR_OP_WD_EVCR && > + !(txbuf[0] & EVCR_QUAD_EN_MICRON)) { > + struct cqspi_flash_pdata *f_pdata; > + > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + f_pdata->quad_mode_set = 1; No, please don't do this. The whole point of the spi-nor layer is that we don't want low-level drivers having to snoop every type of flash command. We want to have proper feedback between spi-nor.c and your driver, so you *know* when you're using a quad-enabled device. Food for thought: what happens for a non-Micron SPI flash that uses quad mode? Is it sufficient to just check if nor->flash_read == SPI_NOR_QUAD? > + } > + return ret; > +} > + > +static int cqspi_command_write_addr(struct spi_nor *nor, > + u8 opcode, unsigned int addr) > +{ > + unsigned int reg; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + > + 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, > + unsigned int from_addr) > +{ > + unsigned int reg; > + unsigned int dummy_clk = 0; > + unsigned int dummy_bytes; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + unsigned int ahb_phy_addr = cqspi->ahb_phy_addr; > + > + writel((ahb_phy_addr & CQSPI_INDIRECTTRIGGER_ADDR_MASK), > + reg_base + CQSPI_REG_INDIRECTTRIGGER); > + 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_bytes = nor->read_dummy / 8; > + if (dummy_bytes) { > + struct cqspi_flash_pdata *f_pdata; > + > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + > + if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) > + dummy_bytes = CQSPI_DUMMY_BYTES_MAX; > + > + 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); > + > + dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; > + if (!f_pdata->quad_mode_set) { > + /* Need to subtract the mode byte (8 clocks). */ > + dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; > + } Why do you have to subtract? Is nor->read_dummy incorrect? > + > + 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, unsigned n_rx) > +{ > + int ret = 0; > + unsigned int reg = 0; > + unsigned int bytes_to_read = 0; > + unsigned int timeout; > + unsigned int watermark; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + void __iomem *ahb_base = cqspi->ahb_base; > + int remaining = (int)n_rx; > + > + watermark = cqspi->fifo_depth * CQSPI_FIFO_WIDTH / 2; > + writel(watermark, reg_base + CQSPI_REG_INDIRECTRDWATERMARK); > + writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES); > + > + /* Clear all interrupts. */ > + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS); > + > + cqspi->irq_mask = CQSPI_IRQ_MASK_RD; > + writel(cqspi->irq_mask, 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(reg_base); > + > + 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; > + cqspi_fifo_read(rxbuf, ahb_base, bytes_to_read); > + rxbuf += bytes_to_read; > + remaining -= bytes_to_read; > + bytes_to_read = CQSPI_GET_RD_SRAM_LEVEL(reg_base); > + } > + } > + > + /* Check indirect done status */ > + timeout = cqspi_init_timeout(CQSPI_TIMEOUT_MS); > + while (cqspi_check_timeout(timeout)) { > + reg = readl(reg_base + CQSPI_REG_INDIRECTRD); > + if (reg & CQSPI_REG_INDIRECTRD_DONE_MASK) > + break; > + } > + > + if (!(reg & CQSPI_REG_INDIRECTRD_DONE_MASK)) { > + dev_err(nor->dev, > + "Indirect read completion error 0x%08x\n", reg); > + ret = -ETIMEDOUT; > + 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, unsigned int to_addr) > +{ > + unsigned int reg; > + struct cqspi_st *cqspi = nor->priv; > + 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, unsigned n_tx) > +{ > + int ret; > + unsigned int timeout; > + unsigned int reg = 0; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + void __iomem *ahb_base = cqspi->ahb_base; > + int remaining = (int)n_tx; > + struct cqspi_flash_pdata *f_pdata; > + unsigned int page_size; > + unsigned int write_bytes; > + > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + page_size = f_pdata->page_size; > + > + writel(remaining, reg_base + CQSPI_REG_INDIRECTWRBYTES); > + > + writel(CQSPI_REG_SRAM_THRESHOLD_BYTES, reg_base + > + CQSPI_REG_INDIRECTWRWATERMARK); > + > + /* Clear all interrupts. */ > + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS); > + > + cqspi->irq_mask = CQSPI_IRQ_MASK_WR; > + writel(cqspi->irq_mask, reg_base + CQSPI_REG_IRQMASK); > + > + reinit_completion(&cqspi->transfer_complete); > + writel(CQSPI_REG_INDIRECTWR_START_MASK, > + reg_base + CQSPI_REG_INDIRECTWR); > + > + /* Write a page or remaining bytes. */ > + write_bytes = remaining > page_size ? page_size : remaining; > + /* Fill up the data at the beginning */ > + cqspi_fifo_write(ahb_base, txbuf, write_bytes); > + txbuf += write_bytes; > + remaining -= write_bytes; > + > + while (remaining > 0) { > + 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; > + } > + > + write_bytes = remaining > page_size ? page_size : remaining; > + cqspi_fifo_write(ahb_base, txbuf, write_bytes); > + txbuf += write_bytes; > + remaining -= write_bytes; > + } > + > + 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; > + } > + > + /* Check indirect done status */ > + timeout = cqspi_init_timeout(CQSPI_TIMEOUT_MS); > + while (cqspi_check_timeout(timeout)) { > + reg = readl(reg_base + CQSPI_REG_INDIRECTWR); > + if (reg & CQSPI_REG_INDIRECTWR_DONE_MASK) > + break; > + } > + > + if (!(reg & CQSPI_REG_INDIRECTWR_DONE_MASK)) { > + dev_err(nor->dev, > + "Indirect write completion error 0x%08x\n", reg); > + ret = -ETIMEDOUT; > + 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 == 0) { > + ret = cqspi_indirect_write_execute(nor, buf, len); > + if (ret == 0) > + *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 == 0) { > + ret = cqspi_indirect_read_execute(nor, buf, len); > + if (ret == 0) > + *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; drivers/mtd/spi-nor/cadence-quadspi.c:791:1-4: WARNING: end returns can be simpified [coccinelle] > +} > + > +static unsigned int calculate_ticks_for_ns(unsigned int ref_clk_hz, > + unsigned int ns_val) > +{ > + unsigned int ticks; > + > + ticks = ref_clk_hz; > + ticks /= 1000; > + ticks *= ns_val; > + ticks += 999999; > + ticks /= 1000000; > + return ticks; > +} > + > +static void cqspi_delay(struct cqspi_st *cqspi, > + unsigned int ref_clk_hz, unsigned int sclk_hz) > +{ > + void __iomem *iobase = cqspi->iobase; > + struct cqspi_flash_pdata *f_pdata; > + unsigned int ref_clk_ns; > + unsigned int sclk_ns; > + unsigned int tshsl, tchsh, tslch, tsd2d; > + unsigned int reg; > + unsigned int tsclk; > + > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + > + /* Convert to ns. */ > + ref_clk_ns = NSEC_PER_SEC / ref_clk_hz; drivers/mtd/spi-nor/cadence-quadspi.c:816:15: warning: variable ‘ref_clk_ns’ set but not used [-Wunused-but-set-variable] unsigned int ref_clk_ns; ^ > + > + /* Convert to ns. */ > + sclk_ns = NSEC_PER_SEC / sclk_hz; drivers/mtd/spi-nor/cadence-quadspi.c: In function ‘cqspi_delay’: drivers/mtd/spi-nor/cadence-quadspi.c:817:15: warning: variable ‘sclk_ns’ set but not used [-Wunused-but-set-variable] unsigned int sclk_ns; ^ > + > + /* 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, > + unsigned int ref_clk_hz, > + unsigned int sclk_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, > + unsigned int bypass, 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 cqspi_st *cqspi, > + unsigned int chip_select, > + unsigned int decoder_enable) > +{ > + void __iomem *reg_base = cqspi->iobase; > + unsigned int reg; > + > + reg = readl(reg_base + CQSPI_REG_CONFIG); > + if (decoder_enable) { > + 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 cqspi_st *cqspi, unsigned int cs) > +{ > + unsigned int reg; > + struct cqspi_flash_pdata *f_pdata = &cqspi->f_pdata[cs]; > + void __iomem *iobase = cqspi->iobase; > + struct spi_nor *nor = &f_pdata->nor; > + > + 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 |= (f_pdata->page_size << CQSPI_REG_SIZE_PAGE_LSB); > + reg |= (f_pdata->block_size << CQSPI_REG_SIZE_BLOCK_LSB); > + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > + reg |= (nor->addr_width - 1); > + writel(reg, iobase + CQSPI_REG_SIZE); > + > + /* configure the chip select */ > + cqspi_chipselect(cqspi, cs, cqspi->is_decoded_cs); > + > + cqspi_controller_enable(cqspi); > +} > + > +static int cqspi_prep(struct spi_nor *nor, enum spi_nor_ops ops) > +{ > + struct cqspi_st *cqspi = nor->priv; > + int cs = cqspi_find_chipselect(nor); > + struct cqspi_flash_pdata *f_pdata; > + unsigned int sclk; > + > + /* Switch chip select. */ > + if (cqspi->current_cs != cs) { > + cqspi->current_cs = cs; > + cqspi_switch_cs(cqspi, cs); > + } > + > + /* Setup baudrate divisor and delays */ > + f_pdata = &cqspi->f_pdata[cqspi->current_cs]; > + sclk = f_pdata->clk_rate; > + if (cqspi->sclk != sclk) { > + cqspi->sclk = sclk; > + cqspi_controller_disable(cqspi); > + cqspi_config_baudrate_div(cqspi, > + cqspi->master_ref_clk_hz, sclk); > + cqspi_delay(cqspi, cqspi->master_ref_clk_hz, sclk); > + cqspi_readdata_capture(cqspi, 1, f_pdata->read_delay); > + cqspi_controller_enable(cqspi); > + } > + return 0; > +} > + > +static void cqspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops) > +{ > +} Do you really need the empty function? > + > +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,page-size", &f_pdata->page_size)) { Do you really need this property? Can't you use nor->page_size? > + dev_err(&pdev->dev, "couldn't determine page-size\n"); > + return -ENXIO; > + } > + > + if (of_property_read_u32(np, "cdns,block-size", &f_pdata->block_size)) { Same here (mtd->erasesize). > + dev_err(&pdev->dev, "couldn't determine block-size\n"); > + return -ENXIO; > + } > + > + 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); > + > + if (of_property_read_u32(np, "is-decoded-cs", &cqspi->is_decoded_cs)) > + cqspi->is_decoded_cs = 0; > + > + if (of_property_read_u32(np, "fifo-depth", &cqspi->fifo_depth)) { > + dev_err(&pdev->dev, "couldn't determine fifo-depth\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); > + > + cqspi_controller_enable(cqspi); > +} > + > +static int cqspi_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct mtd_part_parser_data ppdata; > + struct device *dev = &pdev->dev; > + struct cqspi_st *cqspi; > + struct spi_nor *nor; > + struct mtd_info *mtd; > + 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); > + > + ret = cqspi_of_get_pdata(pdev); > + if (ret) { > + dev_err(dev, "Get platform data failed.\n"); > + return -ENODEV; > + } > + > + cqspi->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(cqspi->clk)) { > + dev_err(dev, "cannot get qspi clk\n"); > + ret = PTR_ERR(cqspi->clk); > + goto probe_failed; > + } > + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + cqspi->iobase = devm_ioremap_resource(dev, res); > + if (IS_ERR(cqspi->iobase)) { > + dev_err(dev, "devm_ioremap_resource res 0 failed\n"); > + ret = PTR_ERR(cqspi->iobase); > + goto probe_failed; > + } > + > + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + cqspi->ahb_phy_addr = res_ahb->start; > + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb); > + if (IS_ERR(cqspi->ahb_base)) { > + dev_err(dev, "devm_ioremap_resource res 1 failed\n"); > + ret = PTR_ERR(cqspi->ahb_base); > + goto probe_failed; > + } > + > + init_completion(&cqspi->transfer_complete); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(dev, "platform_get_irq failed\n"); > + ret = -ENXIO; > + goto probe_failed; > + } > + ret = devm_request_irq(dev, irq, > + cqspi_irq_handler, 0, pdev->name, cqspi); > + if (ret) { > + dev_err(dev, "devm_request_irq failed\n"); > + goto probe_failed; > + } > + > + cqspi_wait_idle(cqspi); > + cqspi_controller_init(cqspi); > + cqspi->current_cs = -1; > + cqspi->sclk = 0; > + > + /* Get flash device data */ > + for_each_available_child_of_node(dev->of_node, np) { > + unsigned int cs; > + struct cqspi_flash_pdata *f_pdata; > + > + if (of_property_read_u32(np, "reg", &cs)) { > + dev_err(dev, "couldn't determine chip select\n"); > + return -ENXIO; > + } > + if (cs > CQSPI_MAX_CHIPSELECT) { > + dev_err(dev, "chip select %d out of range\n", cs); > + return -ENXIO; > + } > + f_pdata = &cqspi->f_pdata[cs]; smatch catches a tricky one here: drivers/mtd/spi-nor/cadence-quadspi.c:1190 cqspi_probe() warn: buffer overflow 'cqspi->f_pdata' 16 <= 16 [smatch] I think the problem is in the condition a few lines above: if (cs > CQSPI_MAX_CHIPSELECT) { which should actually be: if (cs >= CQSPI_MAX_CHIPSELECT) { > + > + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np); > + if (ret) > + goto probe_failed; > + > + f_pdata->quad_mode_set = 0; > + > + nor = &f_pdata->nor; > + mtd = &f_pdata->mtd; > + > + nor->mtd = mtd; > + nor->dev = dev; > + nor->priv = cqspi; > + mtd->priv = nor; > + > + 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->unprepare = cqspi_unprep; > + > + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD); > + if (ret) > + goto probe_failed; > + > + ppdata.of_node = np; > + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0); > + if (ret) > + goto probe_failed; > + } > + > + dev_info(dev, "Cadence QSPI NOR flash driver\n"); > + return 0; > + > + probe_failed: > + dev_err(dev, "Cadence QSPI NOR probe failed\n"); If you're bothering to print here, you might as well include the error code. > + return ret; If you have more than one chip, and the second one fails, you're not really unwinding the first correctly. > +} > + > +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 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, > + }, Your indentation is a little screwed up for the above 4 lines. Please use only tabs, not spaces, and make sure the closing brace is aligned with '.driver', not '.of_match_table'. > +}; > + > +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 "); > +MODULE_AUTHOR("Graham Moore "); There's likely more to say, but that's all I've got for now... Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/