* [U-Boot] [PATCH 0/2] spi: cadence-qspi: Move to spi-mem APIs @ 2019-10-14 13:27 Vignesh Raghavendra 2019-10-14 13:27 ` [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework Vignesh Raghavendra 2019-10-14 13:27 ` [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support Vignesh Raghavendra 0 siblings, 2 replies; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-14 13:27 UTC (permalink / raw) To: u-boot First patch moves driver over to spi-mem framework and implement spi_mem_ops. This is require to support more SPI Flash opcodes like SFDP parsing etc. Series is in prepartion to add Octal mode for support for the same driver to support OSPI version of the controller. Second patch adds DAC mode that provide memory mapped access to flash. This greatly increases the read throughput. Tested with mt25qu512 flash and s25fl512 flash Simon, Would greatly appreciate if you could test this series on platforms that you have access to? Vignesh Raghavendra (2): spi: cadence_qspi: Move to spi-mem framework spi: cadence-qspi: Add direct mode support drivers/spi/cadence_qspi.c | 148 ++++++++++++--------------- drivers/spi/cadence_qspi.h | 24 +++-- drivers/spi/cadence_qspi_apb.c | 181 +++++++++++++++------------------ 3 files changed, 160 insertions(+), 193 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-14 13:27 [U-Boot] [PATCH 0/2] spi: cadence-qspi: Move to spi-mem APIs Vignesh Raghavendra @ 2019-10-14 13:27 ` Vignesh Raghavendra 2019-10-17 11:20 ` Simon Goldschmidt 2019-10-14 13:27 ` [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support Vignesh Raghavendra 1 sibling, 1 reply; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-14 13:27 UTC (permalink / raw) To: u-boot Current Cadence QSPI driver has few limitations. It assumes all read operations to be in Quad mode and thus does not support SFDP parsing. Also, adding support for new mode such as Octal mode would not be possible with current configuration. Therefore move the driver over to spi-mem framework. This has added advantage that driver can be used to support SPI NAND memories too. Hence, move driver over to new spi-mem APIs. Please note that this gets rid of mode bit setting done when CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to that config option. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> --- drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- drivers/spi/cadence_qspi.h | 9 +-- drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- 3 files changed, 91 insertions(+), 178 deletions(-) diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index e2e54cd27723..673a2e9a6c4c 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -10,6 +10,7 @@ #include <malloc.h> #include <reset.h> #include <spi.h> +#include <spi-mem.h> #include <linux/errno.h> #include "cadence_qspi.h" @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) return 0; } +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) +{ + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_IN(len, idcode, 1)); + + return cadence_qspi_apb_command_read(reg_base, &op); +} + /* Calibration sequence to determine the read data capture delay register */ static int spi_calibration(struct udevice *bus, uint hz) { struct cadence_spi_priv *priv = dev_get_priv(bus); void *base = priv->regbase; - u8 opcode_rdid = 0x9F; unsigned int idcode = 0, temp = 0; int err = 0, i, range_lo = -1, range_hi = -1; @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) cadence_qspi_apb_controller_enable(base); /* read the ID which will be our golden value */ - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, - 3, (u8 *)&idcode); + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); if (err) { puts("SF: Calibration failed (read)\n"); return err; @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) cadence_qspi_apb_controller_enable(base); /* issue a RDID to get the ID value */ - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, - 3, (u8 *)&temp); + err = cadence_spi_read_id(base, 3, (u8 *)&temp); if (err) { puts("SF: Calibration failed (read)\n"); return err; @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) return 0; } -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, - const void *dout, void *din, unsigned long flags) +static int cadence_spi_mem_exec_op(struct spi_slave *spi, + const struct spi_mem_op *op) { - struct udevice *bus = dev->parent; + struct udevice *bus = spi->dev->parent; struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus); - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); void *base = priv->regbase; - u8 *cmd_buf = priv->cmd_buf; - size_t data_bytes; int err = 0; - u32 mode = CQSPI_STIG_WRITE; - - if (flags & SPI_XFER_BEGIN) { - /* copy command to local buffer */ - priv->cmd_len = bitlen / 8; - memcpy(cmd_buf, dout, priv->cmd_len); - } - - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { - /* if start and end bit are set, the data bytes is 0. */ - data_bytes = 0; - } else { - data_bytes = bitlen / 8; - } - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); + u32 mode; /* Set Chip select */ - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), plat->is_decoded_cs); - if ((flags & SPI_XFER_END) || (flags == 0)) { - if (priv->cmd_len == 0) { - printf("QSPI: Error, command is empty.\n"); - return -1; - } - - if (din && data_bytes) { - /* read */ - /* Use STIG if no address. */ - if (!CQSPI_IS_ADDR(priv->cmd_len)) - mode = CQSPI_STIG_READ; - else - mode = CQSPI_INDIRECT_READ; - } else if (dout && !(flags & SPI_XFER_BEGIN)) { - /* write */ - if (!CQSPI_IS_ADDR(priv->cmd_len)) - mode = CQSPI_STIG_WRITE; - else - mode = CQSPI_INDIRECT_WRITE; - } - - switch (mode) { - case CQSPI_STIG_READ: - err = cadence_qspi_apb_command_read( - base, priv->cmd_len, cmd_buf, - data_bytes, din); + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { + if (!op->addr.nbytes) + mode = CQSPI_STIG_READ; + else + mode = CQSPI_INDIRECT_READ; + } else { + if (!op->addr.nbytes || !op->data.buf.out) + mode = CQSPI_STIG_WRITE; + else + mode = CQSPI_INDIRECT_WRITE; + } + switch (mode) { + case CQSPI_STIG_READ: + err = cadence_qspi_apb_command_read(base, op); break; - case CQSPI_STIG_WRITE: - err = cadence_qspi_apb_command_write(base, - priv->cmd_len, cmd_buf, - data_bytes, dout); + case CQSPI_STIG_WRITE: + err = cadence_qspi_apb_command_write(base, op); break; - case CQSPI_INDIRECT_READ: - err = cadence_qspi_apb_indirect_read_setup(plat, - priv->cmd_len, dm_plat->mode, cmd_buf); - if (!err) { - err = cadence_qspi_apb_indirect_read_execute - (plat, data_bytes, din); - } - break; - case CQSPI_INDIRECT_WRITE: - err = cadence_qspi_apb_indirect_write_setup - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); - if (!err) { - err = cadence_qspi_apb_indirect_write_execute - (plat, data_bytes, dout); - } - break; - default: - err = -1; - break; + case CQSPI_INDIRECT_READ: + err = cadence_qspi_apb_indirect_read_setup(plat, op); + if (!err) { + err = cadence_qspi_apb_indirect_read_execute + (plat, op->data.nbytes, op->data.buf.in); } - - if (flags & SPI_XFER_END) { - /* clear command buffer */ - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); - priv->cmd_len = 0; + break; + case CQSPI_INDIRECT_WRITE: + err = cadence_qspi_apb_indirect_write_setup(plat, op); + if (!err) { + err = cadence_qspi_apb_indirect_write_execute + (plat, op->data.nbytes, op->data.buf.out); } + break; + default: + err = -1; + break; } return err; @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; } +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { + .exec_op = cadence_spi_mem_exec_op, +}; + static const struct dm_spi_ops cadence_spi_ops = { - .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, .set_mode = cadence_spi_set_mode, + .mem_ops = &cadence_spi_mem_ops, /* * cs_info is not needed, since we require all chip selects to be * in the device tree explicitly diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 20cceca239f8..e655b027d788 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); void cadence_qspi_apb_controller_disable(void *reg_base_addr); int cadence_qspi_apb_command_read(void *reg_base_addr, - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); + const struct spi_mem_op *op); int cadence_qspi_apb_command_write(void *reg_base_addr, - unsigned int cmdlen, const u8 *cmdbuf, - unsigned int txlen, const u8 *txbuf); + const struct spi_mem_op *op); int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); + const struct spi_mem_op *op); int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, unsigned int rxlen, u8 *rxbuf); int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); + const struct spi_mem_op *op); int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, unsigned int txlen, const u8 *txbuf); diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 55a7501913a8..8dd0495dfcf4 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -30,6 +30,7 @@ #include <linux/errno.h> #include <wait_bit.h> #include <spi.h> +#include <spi-mem.h> #include <malloc.h> #include "cadence_qspi.h" @@ -172,19 +173,6 @@ (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, - unsigned int addr_width) -{ - unsigned int addr; - - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; - - if (addr_width == 4) - addr = (addr << 8) | addr_buf[3]; - - return addr; -} - void cadence_qspi_apb_controller_enable(void *reg_base) { unsigned int reg; @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, } /* For command RDID, RDSR. */ -int cadence_qspi_apb_command_read(void *reg_base, - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, - u8 *rxbuf) +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) { unsigned int reg; unsigned int read_len; int status; + unsigned int rxlen = op->data.nbytes; + void *rxbuf = op->data.buf.in; - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", - cmdlen, rxlen); + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); return -EINVAL; } - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, } /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) { unsigned int reg = 0; - unsigned int addr_value; unsigned int wr_data; unsigned int wr_len; + unsigned int txlen = op->data.nbytes; + const void *txbuf = op->data.buf.out; + u32 addr; + + /* Reorder address to SPI bus order if only transferring address */ + if (!txlen) { + addr = cpu_to_be32(op->addr.val); + if (op->addr.nbytes == 3) + addr >>= 8; + txbuf = &addr; + txlen = op->addr.nbytes; + } - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", - cmdlen, txlen); + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { + printf("QSPI: Invalid input arguments txlen %u\n", txlen); return -EINVAL; } - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; - - if (cmdlen == 4 || cmdlen == 5) { - /* Command with address */ - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); - /* Number of bytes to write. */ - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; - /* Get address */ - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], - cmdlen >= 5 ? 4 : 3); - - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); - } + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; if (txlen) { /* writing data = yes */ @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) + const struct spi_mem_op *op) { unsigned int reg; unsigned int rd_reg; - unsigned int addr_value; unsigned int dummy_clk; - unsigned int dummy_bytes; - unsigned int addr_bytes; - - /* - * Identify addr_byte. All NOR flash device drivers are using fast read - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. - * With that, the length is in value of 5 or 6. Only FRAM chip from - * ramtron using normal read (which won't need dummy byte). - * Unlikely NOR flash using normal read due to performance issue. - */ - if (cmdlen >= 5) - /* to cater fast read where cmd + addr + dummy */ - addr_bytes = cmdlen - 2; - else - /* for normal read (only ramtron as of now) */ - addr_bytes = cmdlen - 1; + unsigned int dummy_bytes = op->dummy.nbytes; /* Setup the indirect trigger address */ writel(plat->trigger_address, plat->regbase + CQSPI_REG_INDIRECTTRIGGER); /* Configure the opcode */ - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; - if (rx_width & SPI_RX_QUAD) + if (op->data.buswidth == 4) /* Instruction and address at DQ0, data at DQ0-3. */ rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; - /* Get address */ - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); - /* The remaining lenght is dummy bytes. */ - dummy_bytes = cmdlen - addr_bytes - 1; if (dummy_bytes) { if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) dummy_bytes = CQSPI_DUMMY_BYTES_MAX; - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); -#else - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); -#endif - /* Convert to clock cycles. */ dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; - /* Need to minus the mode byte (8 clocks). */ - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; if (dummy_clk) rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, /* set device size */ reg = readl(plat->regbase + CQSPI_REG_SIZE); reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; - reg |= (addr_bytes - 1); + reg |= (op->addr.nbytes - 1); writel(reg, plat->regbase + CQSPI_REG_SIZE); return 0; } @@ -687,35 +641,23 @@ failrd: /* Opcode + Address (3/4 bytes) */ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) + const struct spi_mem_op *op) { unsigned int reg; - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; - if (cmdlen < 4 || cmdbuf == NULL) { - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", - cmdlen, cmdbuf); - return -EINVAL; - } /* Setup the indirect trigger address */ writel(plat->trigger_address, plat->regbase + CQSPI_REG_INDIRECTTRIGGER); /* Configure the opcode */ - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; - - if (tx_width & SPI_TX_QUAD) - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; - + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); - /* Setup write address. */ - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); reg = readl(plat->regbase + CQSPI_REG_SIZE); reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; - reg |= (addr_bytes - 1); + reg |= (op->addr.nbytes - 1); writel(reg, plat->regbase + CQSPI_REG_SIZE); return 0; } -- 2.23.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-14 13:27 ` [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework Vignesh Raghavendra @ 2019-10-17 11:20 ` Simon Goldschmidt 2019-10-17 12:31 ` Vignesh Raghavendra 2019-10-17 13:48 ` [U-Boot] " Tudor.Ambarus at microchip.com 0 siblings, 2 replies; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-17 11:20 UTC (permalink / raw) To: u-boot On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Current Cadence QSPI driver has few limitations. It assumes all read > operations to be in Quad mode and thus does not support SFDP parsing. > Also, adding support for new mode such as Octal mode would not be > possible with current configuration. Therefore move the driver over to spi-mem > framework. This has added advantage that driver can be used to support > SPI NAND memories too. > Hence, move driver over to new spi-mem APIs. > > Please note that this gets rid of mode bit setting done when > CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to > that config option. I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width at 4 in my devicetree, SFDP parsing works, but reading data afterwards produces invalid results (I haven't tested what's wrong there). It works as expected when not parsing SFDP or setting the bus-width to 1. So the change itself probably works, but SFDP parsing is broken? I did the tests with this applied first: https://patchwork.ozlabs.org/project/uboot/list/?series=135505 Regards, Simon > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > --- > drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- > drivers/spi/cadence_qspi.h | 9 +-- > drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- > 3 files changed, 91 insertions(+), 178 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index e2e54cd27723..673a2e9a6c4c 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -10,6 +10,7 @@ > #include <malloc.h> > #include <reset.h> > #include <spi.h> > +#include <spi-mem.h> > #include <linux/errno.h> > #include "cadence_qspi.h" > > @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) > return 0; > } > > +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) > +{ > + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), > + SPI_MEM_OP_NO_ADDR, > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_IN(len, idcode, 1)); > + > + return cadence_qspi_apb_command_read(reg_base, &op); > +} > + > /* Calibration sequence to determine the read data capture delay register */ > static int spi_calibration(struct udevice *bus, uint hz) > { > struct cadence_spi_priv *priv = dev_get_priv(bus); > void *base = priv->regbase; > - u8 opcode_rdid = 0x9F; > unsigned int idcode = 0, temp = 0; > int err = 0, i, range_lo = -1, range_hi = -1; > > @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > cadence_qspi_apb_controller_enable(base); > > /* read the ID which will be our golden value */ > - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > - 3, (u8 *)&idcode); > + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); > if (err) { > puts("SF: Calibration failed (read)\n"); > return err; > @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > cadence_qspi_apb_controller_enable(base); > > /* issue a RDID to get the ID value */ > - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > - 3, (u8 *)&temp); > + err = cadence_spi_read_id(base, 3, (u8 *)&temp); > if (err) { > puts("SF: Calibration failed (read)\n"); > return err; > @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > return 0; > } > > -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, > - const void *dout, void *din, unsigned long flags) > +static int cadence_spi_mem_exec_op(struct spi_slave *spi, > + const struct spi_mem_op *op) > { > - struct udevice *bus = dev->parent; > + struct udevice *bus = spi->dev->parent; > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); > void *base = priv->regbase; > - u8 *cmd_buf = priv->cmd_buf; > - size_t data_bytes; > int err = 0; > - u32 mode = CQSPI_STIG_WRITE; > - > - if (flags & SPI_XFER_BEGIN) { > - /* copy command to local buffer */ > - priv->cmd_len = bitlen / 8; > - memcpy(cmd_buf, dout, priv->cmd_len); > - } > - > - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { > - /* if start and end bit are set, the data bytes is 0. */ > - data_bytes = 0; > - } else { > - data_bytes = bitlen / 8; > - } > - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); > + u32 mode; > > /* Set Chip select */ > - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), > + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), > plat->is_decoded_cs); > > - if ((flags & SPI_XFER_END) || (flags == 0)) { > - if (priv->cmd_len == 0) { > - printf("QSPI: Error, command is empty.\n"); > - return -1; > - } > - > - if (din && data_bytes) { > - /* read */ > - /* Use STIG if no address. */ > - if (!CQSPI_IS_ADDR(priv->cmd_len)) > - mode = CQSPI_STIG_READ; > - else > - mode = CQSPI_INDIRECT_READ; > - } else if (dout && !(flags & SPI_XFER_BEGIN)) { > - /* write */ > - if (!CQSPI_IS_ADDR(priv->cmd_len)) > - mode = CQSPI_STIG_WRITE; > - else > - mode = CQSPI_INDIRECT_WRITE; > - } > - > - switch (mode) { > - case CQSPI_STIG_READ: > - err = cadence_qspi_apb_command_read( > - base, priv->cmd_len, cmd_buf, > - data_bytes, din); > + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { > + if (!op->addr.nbytes) > + mode = CQSPI_STIG_READ; > + else > + mode = CQSPI_INDIRECT_READ; > + } else { > + if (!op->addr.nbytes || !op->data.buf.out) > + mode = CQSPI_STIG_WRITE; > + else > + mode = CQSPI_INDIRECT_WRITE; > + } > > + switch (mode) { > + case CQSPI_STIG_READ: > + err = cadence_qspi_apb_command_read(base, op); > break; > - case CQSPI_STIG_WRITE: > - err = cadence_qspi_apb_command_write(base, > - priv->cmd_len, cmd_buf, > - data_bytes, dout); > + case CQSPI_STIG_WRITE: > + err = cadence_qspi_apb_command_write(base, op); > break; > - case CQSPI_INDIRECT_READ: > - err = cadence_qspi_apb_indirect_read_setup(plat, > - priv->cmd_len, dm_plat->mode, cmd_buf); > - if (!err) { > - err = cadence_qspi_apb_indirect_read_execute > - (plat, data_bytes, din); > - } > - break; > - case CQSPI_INDIRECT_WRITE: > - err = cadence_qspi_apb_indirect_write_setup > - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); > - if (!err) { > - err = cadence_qspi_apb_indirect_write_execute > - (plat, data_bytes, dout); > - } > - break; > - default: > - err = -1; > - break; > + case CQSPI_INDIRECT_READ: > + err = cadence_qspi_apb_indirect_read_setup(plat, op); > + if (!err) { > + err = cadence_qspi_apb_indirect_read_execute > + (plat, op->data.nbytes, op->data.buf.in); > } > - > - if (flags & SPI_XFER_END) { > - /* clear command buffer */ > - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); > - priv->cmd_len = 0; > + break; > + case CQSPI_INDIRECT_WRITE: > + err = cadence_qspi_apb_indirect_write_setup(plat, op); > + if (!err) { > + err = cadence_qspi_apb_indirect_write_execute > + (plat, op->data.nbytes, op->data.buf.out); > } > + break; > + default: > + err = -1; > + break; > } > > return err; > @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > return 0; > } > > +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { > + .exec_op = cadence_spi_mem_exec_op, > +}; > + > static const struct dm_spi_ops cadence_spi_ops = { > - .xfer = cadence_spi_xfer, > .set_speed = cadence_spi_set_speed, > .set_mode = cadence_spi_set_mode, > + .mem_ops = &cadence_spi_mem_ops, > /* > * cs_info is not needed, since we require all chip selects to be > * in the device tree explicitly > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > index 20cceca239f8..e655b027d788 100644 > --- a/drivers/spi/cadence_qspi.h > +++ b/drivers/spi/cadence_qspi.h > @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); > void cadence_qspi_apb_controller_disable(void *reg_base_addr); > > int cadence_qspi_apb_command_read(void *reg_base_addr, > - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); > + const struct spi_mem_op *op); > int cadence_qspi_apb_command_write(void *reg_base_addr, > - unsigned int cmdlen, const u8 *cmdbuf, > - unsigned int txlen, const u8 *txbuf); > + const struct spi_mem_op *op); > > int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); > + const struct spi_mem_op *op); > int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > unsigned int rxlen, u8 *rxbuf); > int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); > + const struct spi_mem_op *op); > int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > unsigned int txlen, const u8 *txbuf); > > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > index 55a7501913a8..8dd0495dfcf4 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -30,6 +30,7 @@ > #include <linux/errno.h> > #include <wait_bit.h> > #include <spi.h> > +#include <spi-mem.h> > #include <malloc.h> > #include "cadence_qspi.h" > > @@ -172,19 +173,6 @@ > (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) > > -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, > - unsigned int addr_width) > -{ > - unsigned int addr; > - > - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; > - > - if (addr_width == 4) > - addr = (addr << 8) | addr_buf[3]; > - > - return addr; > -} > - > void cadence_qspi_apb_controller_enable(void *reg_base) > { > unsigned int reg; > @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, > } > > /* For command RDID, RDSR. */ > -int cadence_qspi_apb_command_read(void *reg_base, > - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, > - u8 *rxbuf) > +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) > { > unsigned int reg; > unsigned int read_len; > int status; > + unsigned int rxlen = op->data.nbytes; > + void *rxbuf = op->data.buf.in; > > - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { > - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", > - cmdlen, rxlen); > + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { > + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); > return -EINVAL; > } > > - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > > reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); > > @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, > } > > /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ > -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) > +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > { > unsigned int reg = 0; > - unsigned int addr_value; > unsigned int wr_data; > unsigned int wr_len; > + unsigned int txlen = op->data.nbytes; > + const void *txbuf = op->data.buf.out; > + u32 addr; > + > + /* Reorder address to SPI bus order if only transferring address */ > + if (!txlen) { > + addr = cpu_to_be32(op->addr.val); > + if (op->addr.nbytes == 3) > + addr >>= 8; > + txbuf = &addr; > + txlen = op->addr.nbytes; > + } > > - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { > - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", > - cmdlen, txlen); > + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { > + printf("QSPI: Invalid input arguments txlen %u\n", txlen); > return -EINVAL; > } > > - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > - > - if (cmdlen == 4 || cmdlen == 5) { > - /* Command with address */ > - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); > - /* Number of bytes to write. */ > - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) > - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; > - /* Get address */ > - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], > - cmdlen >= 5 ? 4 : 3); > - > - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); > - } > + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > > if (txlen) { > /* writing data = yes */ > @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > > /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) > + const struct spi_mem_op *op) > { > unsigned int reg; > unsigned int rd_reg; > - unsigned int addr_value; > unsigned int dummy_clk; > - unsigned int dummy_bytes; > - unsigned int addr_bytes; > - > - /* > - * Identify addr_byte. All NOR flash device drivers are using fast read > - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. > - * With that, the length is in value of 5 or 6. Only FRAM chip from > - * ramtron using normal read (which won't need dummy byte). > - * Unlikely NOR flash using normal read due to performance issue. > - */ > - if (cmdlen >= 5) > - /* to cater fast read where cmd + addr + dummy */ > - addr_bytes = cmdlen - 2; > - else > - /* for normal read (only ramtron as of now) */ > - addr_bytes = cmdlen - 1; > + unsigned int dummy_bytes = op->dummy.nbytes; > > /* Setup the indirect trigger address */ > writel(plat->trigger_address, > plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > > /* Configure the opcode */ > - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; > + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; > > - if (rx_width & SPI_RX_QUAD) > + if (op->data.buswidth == 4) > /* Instruction and address at DQ0, data at DQ0-3. */ > rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > > - /* Get address */ > - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > > - /* The remaining lenght is dummy bytes. */ > - dummy_bytes = cmdlen - addr_bytes - 1; > if (dummy_bytes) { > if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) > dummy_bytes = CQSPI_DUMMY_BYTES_MAX; > > - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); > -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) > - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); > -#else > - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); > -#endif > - > /* Convert to clock cycles. */ > dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; > - /* Need to minus the mode byte (8 clocks). */ > - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; > > if (dummy_clk) > rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) > @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > /* set device size */ > reg = readl(plat->regbase + CQSPI_REG_SIZE); > reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > - reg |= (addr_bytes - 1); > + reg |= (op->addr.nbytes - 1); > writel(reg, plat->regbase + CQSPI_REG_SIZE); > return 0; > } > @@ -687,35 +641,23 @@ failrd: > > /* Opcode + Address (3/4 bytes) */ > int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) > + const struct spi_mem_op *op) > { > unsigned int reg; > - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; > > - if (cmdlen < 4 || cmdbuf == NULL) { > - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", > - cmdlen, cmdbuf); > - return -EINVAL; > - } > /* Setup the indirect trigger address */ > writel(plat->trigger_address, > plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > > /* Configure the opcode */ > - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; > - > - if (tx_width & SPI_TX_QUAD) > - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; > - > + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; > writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); > > - /* Setup write address. */ > - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > > reg = readl(plat->regbase + CQSPI_REG_SIZE); > reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > - reg |= (addr_bytes - 1); > + reg |= (op->addr.nbytes - 1); > writel(reg, plat->regbase + CQSPI_REG_SIZE); > return 0; > } > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-17 11:20 ` Simon Goldschmidt @ 2019-10-17 12:31 ` Vignesh Raghavendra 2019-11-06 19:55 ` Simon Goldschmidt 2020-01-17 15:33 ` Simon Goldschmidt 2019-10-17 13:48 ` [U-Boot] " Tudor.Ambarus at microchip.com 1 sibling, 2 replies; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-17 12:31 UTC (permalink / raw) To: u-boot Hi Simon, On 17/10/19 4:50 PM, Simon Goldschmidt wrote: > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> Current Cadence QSPI driver has few limitations. It assumes all read >> operations to be in Quad mode and thus does not support SFDP parsing. >> Also, adding support for new mode such as Octal mode would not be >> possible with current configuration. Therefore move the driver over to spi-mem >> framework. This has added advantage that driver can be used to support >> SPI NAND memories too. >> Hence, move driver over to new spi-mem APIs. >> >> Please note that this gets rid of mode bit setting done when >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to >> that config option. > > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > produces invalid results (I haven't tested what's wrong there). > Thanks for testing! spi-tx-bus-width = 4 was not supported before so I haven't added support for that mode in this series. That change will be a separate series. Could you try with spi-tx-bus-width = 1 and spi-rx-bus-width = 4 and see if that works? If that does not work then could you disable SFDP parsing (but keep spi-rx-bus-width = 4) and see if that works. This should narrow down whether SFDP parsing is broken or if driver has an issue. Regards Vignesh > It works as expected when not parsing SFDP or setting the bus-width to 1. > So the change itself probably works, but SFDP parsing is broken? > > I did the tests with this applied first: > https://patchwork.ozlabs.org/project/uboot/list/?series=135505 > > Regards, > Simon > > >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> --- >> drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- >> drivers/spi/cadence_qspi.h | 9 +-- >> drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- >> 3 files changed, 91 insertions(+), 178 deletions(-) >> >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c >> index e2e54cd27723..673a2e9a6c4c 100644 >> --- a/drivers/spi/cadence_qspi.c >> +++ b/drivers/spi/cadence_qspi.c >> @@ -10,6 +10,7 @@ >> #include <malloc.h> >> #include <reset.h> >> #include <spi.h> >> +#include <spi-mem.h> >> #include <linux/errno.h> >> #include "cadence_qspi.h" >> >> @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) >> return 0; >> } >> >> +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) >> +{ >> + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), >> + SPI_MEM_OP_NO_ADDR, >> + SPI_MEM_OP_NO_DUMMY, >> + SPI_MEM_OP_DATA_IN(len, idcode, 1)); >> + >> + return cadence_qspi_apb_command_read(reg_base, &op); >> +} >> + >> /* Calibration sequence to determine the read data capture delay register */ >> static int spi_calibration(struct udevice *bus, uint hz) >> { >> struct cadence_spi_priv *priv = dev_get_priv(bus); >> void *base = priv->regbase; >> - u8 opcode_rdid = 0x9F; >> unsigned int idcode = 0, temp = 0; >> int err = 0, i, range_lo = -1, range_hi = -1; >> >> @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) >> cadence_qspi_apb_controller_enable(base); >> >> /* read the ID which will be our golden value */ >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, >> - 3, (u8 *)&idcode); >> + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); >> if (err) { >> puts("SF: Calibration failed (read)\n"); >> return err; >> @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) >> cadence_qspi_apb_controller_enable(base); >> >> /* issue a RDID to get the ID value */ >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, >> - 3, (u8 *)&temp); >> + err = cadence_spi_read_id(base, 3, (u8 *)&temp); >> if (err) { >> puts("SF: Calibration failed (read)\n"); >> return err; >> @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) >> return 0; >> } >> >> -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, >> - const void *dout, void *din, unsigned long flags) >> +static int cadence_spi_mem_exec_op(struct spi_slave *spi, >> + const struct spi_mem_op *op) >> { >> - struct udevice *bus = dev->parent; >> + struct udevice *bus = spi->dev->parent; >> struct cadence_spi_platdata *plat = bus->platdata; >> struct cadence_spi_priv *priv = dev_get_priv(bus); >> - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); >> void *base = priv->regbase; >> - u8 *cmd_buf = priv->cmd_buf; >> - size_t data_bytes; >> int err = 0; >> - u32 mode = CQSPI_STIG_WRITE; >> - >> - if (flags & SPI_XFER_BEGIN) { >> - /* copy command to local buffer */ >> - priv->cmd_len = bitlen / 8; >> - memcpy(cmd_buf, dout, priv->cmd_len); >> - } >> - >> - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { >> - /* if start and end bit are set, the data bytes is 0. */ >> - data_bytes = 0; >> - } else { >> - data_bytes = bitlen / 8; >> - } >> - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); >> + u32 mode; >> >> /* Set Chip select */ >> - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), >> + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), >> plat->is_decoded_cs); >> >> - if ((flags & SPI_XFER_END) || (flags == 0)) { >> - if (priv->cmd_len == 0) { >> - printf("QSPI: Error, command is empty.\n"); >> - return -1; >> - } >> - >> - if (din && data_bytes) { >> - /* read */ >> - /* Use STIG if no address. */ >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) >> - mode = CQSPI_STIG_READ; >> - else >> - mode = CQSPI_INDIRECT_READ; >> - } else if (dout && !(flags & SPI_XFER_BEGIN)) { >> - /* write */ >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) >> - mode = CQSPI_STIG_WRITE; >> - else >> - mode = CQSPI_INDIRECT_WRITE; >> - } >> - >> - switch (mode) { >> - case CQSPI_STIG_READ: >> - err = cadence_qspi_apb_command_read( >> - base, priv->cmd_len, cmd_buf, >> - data_bytes, din); >> + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { >> + if (!op->addr.nbytes) >> + mode = CQSPI_STIG_READ; >> + else >> + mode = CQSPI_INDIRECT_READ; >> + } else { >> + if (!op->addr.nbytes || !op->data.buf.out) >> + mode = CQSPI_STIG_WRITE; >> + else >> + mode = CQSPI_INDIRECT_WRITE; >> + } >> >> + switch (mode) { >> + case CQSPI_STIG_READ: >> + err = cadence_qspi_apb_command_read(base, op); >> break; >> - case CQSPI_STIG_WRITE: >> - err = cadence_qspi_apb_command_write(base, >> - priv->cmd_len, cmd_buf, >> - data_bytes, dout); >> + case CQSPI_STIG_WRITE: >> + err = cadence_qspi_apb_command_write(base, op); >> break; >> - case CQSPI_INDIRECT_READ: >> - err = cadence_qspi_apb_indirect_read_setup(plat, >> - priv->cmd_len, dm_plat->mode, cmd_buf); >> - if (!err) { >> - err = cadence_qspi_apb_indirect_read_execute >> - (plat, data_bytes, din); >> - } >> - break; >> - case CQSPI_INDIRECT_WRITE: >> - err = cadence_qspi_apb_indirect_write_setup >> - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); >> - if (!err) { >> - err = cadence_qspi_apb_indirect_write_execute >> - (plat, data_bytes, dout); >> - } >> - break; >> - default: >> - err = -1; >> - break; >> + case CQSPI_INDIRECT_READ: >> + err = cadence_qspi_apb_indirect_read_setup(plat, op); >> + if (!err) { >> + err = cadence_qspi_apb_indirect_read_execute >> + (plat, op->data.nbytes, op->data.buf.in); >> } >> - >> - if (flags & SPI_XFER_END) { >> - /* clear command buffer */ >> - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); >> - priv->cmd_len = 0; >> + break; >> + case CQSPI_INDIRECT_WRITE: >> + err = cadence_qspi_apb_indirect_write_setup(plat, op); >> + if (!err) { >> + err = cadence_qspi_apb_indirect_write_execute >> + (plat, op->data.nbytes, op->data.buf.out); >> } >> + break; >> + default: >> + err = -1; >> + break; >> } >> >> return err; >> @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) >> return 0; >> } >> >> +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { >> + .exec_op = cadence_spi_mem_exec_op, >> +}; >> + >> static const struct dm_spi_ops cadence_spi_ops = { >> - .xfer = cadence_spi_xfer, >> .set_speed = cadence_spi_set_speed, >> .set_mode = cadence_spi_set_mode, >> + .mem_ops = &cadence_spi_mem_ops, >> /* >> * cs_info is not needed, since we require all chip selects to be >> * in the device tree explicitly >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >> index 20cceca239f8..e655b027d788 100644 >> --- a/drivers/spi/cadence_qspi.h >> +++ b/drivers/spi/cadence_qspi.h >> @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); >> >> int cadence_qspi_apb_command_read(void *reg_base_addr, >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); >> + const struct spi_mem_op *op); >> int cadence_qspi_apb_command_write(void *reg_base_addr, >> - unsigned int cmdlen, const u8 *cmdbuf, >> - unsigned int txlen, const u8 *txbuf); >> + const struct spi_mem_op *op); >> >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); >> + const struct spi_mem_op *op); >> int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >> unsigned int rxlen, u8 *rxbuf); >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); >> + const struct spi_mem_op *op); >> int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >> unsigned int txlen, const u8 *txbuf); >> >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >> index 55a7501913a8..8dd0495dfcf4 100644 >> --- a/drivers/spi/cadence_qspi_apb.c >> +++ b/drivers/spi/cadence_qspi_apb.c >> @@ -30,6 +30,7 @@ >> #include <linux/errno.h> >> #include <wait_bit.h> >> #include <spi.h> >> +#include <spi-mem.h> >> #include <malloc.h> >> #include "cadence_qspi.h" >> >> @@ -172,19 +173,6 @@ >> (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ >> CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) >> >> -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, >> - unsigned int addr_width) >> -{ >> - unsigned int addr; >> - >> - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; >> - >> - if (addr_width == 4) >> - addr = (addr << 8) | addr_buf[3]; >> - >> - return addr; >> -} >> - >> void cadence_qspi_apb_controller_enable(void *reg_base) >> { >> unsigned int reg; >> @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, >> } >> >> /* For command RDID, RDSR. */ >> -int cadence_qspi_apb_command_read(void *reg_base, >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, >> - u8 *rxbuf) >> +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) >> { >> unsigned int reg; >> unsigned int read_len; >> int status; >> + unsigned int rxlen = op->data.nbytes; >> + void *rxbuf = op->data.buf.in; >> >> - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { >> - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", >> - cmdlen, rxlen); >> + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { >> + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); >> return -EINVAL; >> } >> >> - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; >> + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; >> >> reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); >> >> @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, >> } >> >> /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ >> -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, >> - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) >> +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) >> { >> unsigned int reg = 0; >> - unsigned int addr_value; >> unsigned int wr_data; >> unsigned int wr_len; >> + unsigned int txlen = op->data.nbytes; >> + const void *txbuf = op->data.buf.out; >> + u32 addr; >> + >> + /* Reorder address to SPI bus order if only transferring address */ >> + if (!txlen) { >> + addr = cpu_to_be32(op->addr.val); >> + if (op->addr.nbytes == 3) >> + addr >>= 8; >> + txbuf = &addr; >> + txlen = op->addr.nbytes; >> + } >> >> - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { >> - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", >> - cmdlen, txlen); >> + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { >> + printf("QSPI: Invalid input arguments txlen %u\n", txlen); >> return -EINVAL; >> } >> >> - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; >> - >> - if (cmdlen == 4 || cmdlen == 5) { >> - /* Command with address */ >> - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); >> - /* Number of bytes to write. */ >> - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) >> - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; >> - /* Get address */ >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], >> - cmdlen >= 5 ? 4 : 3); >> - >> - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); >> - } >> + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; >> >> if (txlen) { >> /* writing data = yes */ >> @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, >> >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) >> + const struct spi_mem_op *op) >> { >> unsigned int reg; >> unsigned int rd_reg; >> - unsigned int addr_value; >> unsigned int dummy_clk; >> - unsigned int dummy_bytes; >> - unsigned int addr_bytes; >> - >> - /* >> - * Identify addr_byte. All NOR flash device drivers are using fast read >> - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. >> - * With that, the length is in value of 5 or 6. Only FRAM chip from >> - * ramtron using normal read (which won't need dummy byte). >> - * Unlikely NOR flash using normal read due to performance issue. >> - */ >> - if (cmdlen >= 5) >> - /* to cater fast read where cmd + addr + dummy */ >> - addr_bytes = cmdlen - 2; >> - else >> - /* for normal read (only ramtron as of now) */ >> - addr_bytes = cmdlen - 1; >> + unsigned int dummy_bytes = op->dummy.nbytes; >> >> /* Setup the indirect trigger address */ >> writel(plat->trigger_address, >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); >> >> /* Configure the opcode */ >> - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; >> + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; >> >> - if (rx_width & SPI_RX_QUAD) >> + if (op->data.buswidth == 4) >> /* Instruction and address at DQ0, data at DQ0-3. */ >> rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; >> >> - /* Get address */ >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); >> - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); >> >> - /* The remaining lenght is dummy bytes. */ >> - dummy_bytes = cmdlen - addr_bytes - 1; >> if (dummy_bytes) { >> if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) >> dummy_bytes = CQSPI_DUMMY_BYTES_MAX; >> >> - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); >> -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) >> - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); >> -#else >> - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); >> -#endif >> - >> /* Convert to clock cycles. */ >> dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; >> - /* Need to minus the mode byte (8 clocks). */ >> - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; >> >> if (dummy_clk) >> rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) >> @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> /* set device size */ >> reg = readl(plat->regbase + CQSPI_REG_SIZE); >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; >> - reg |= (addr_bytes - 1); >> + reg |= (op->addr.nbytes - 1); >> writel(reg, plat->regbase + CQSPI_REG_SIZE); >> return 0; >> } >> @@ -687,35 +641,23 @@ failrd: >> >> /* Opcode + Address (3/4 bytes) */ >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) >> + const struct spi_mem_op *op) >> { >> unsigned int reg; >> - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; >> >> - if (cmdlen < 4 || cmdbuf == NULL) { >> - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", >> - cmdlen, cmdbuf); >> - return -EINVAL; >> - } >> /* Setup the indirect trigger address */ >> writel(plat->trigger_address, >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); >> >> /* Configure the opcode */ >> - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; >> - >> - if (tx_width & SPI_TX_QUAD) >> - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; >> - >> + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; >> writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); >> >> - /* Setup write address. */ >> - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); >> - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); >> >> reg = readl(plat->regbase + CQSPI_REG_SIZE); >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; >> - reg |= (addr_bytes - 1); >> + reg |= (op->addr.nbytes - 1); >> writel(reg, plat->regbase + CQSPI_REG_SIZE); >> return 0; >> } >> -- >> 2.23.0 >> -- Regards Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-17 12:31 ` Vignesh Raghavendra @ 2019-11-06 19:55 ` Simon Goldschmidt 2019-11-07 6:09 ` Vignesh Raghavendra 2020-01-17 15:33 ` Simon Goldschmidt 1 sibling, 1 reply; 18+ messages in thread From: Simon Goldschmidt @ 2019-11-06 19:55 UTC (permalink / raw) To: u-boot Hi Vignesh, On Thu, Oct 17, 2019 at 2:31 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi Simon, > > On 17/10/19 4:50 PM, Simon Goldschmidt wrote: > > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> Current Cadence QSPI driver has few limitations. It assumes all read > >> operations to be in Quad mode and thus does not support SFDP parsing. > >> Also, adding support for new mode such as Octal mode would not be > >> possible with current configuration. Therefore move the driver over to spi-mem > >> framework. This has added advantage that driver can be used to support > >> SPI NAND memories too. > >> Hence, move driver over to new spi-mem APIs. > >> > >> Please note that this gets rid of mode bit setting done when > >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to > >> that config option. > > > > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > > produces invalid results (I haven't tested what's wrong there). > > > > Thanks for testing! > > spi-tx-bus-width = 4 was not supported before so I haven't added support > for that mode in this series. That change will be a separate series. > > Could you try with spi-tx-bus-width = 1 and spi-rx-bus-width = 4 and see > if that works? > > If that does not work then could you disable SFDP parsing (but keep > spi-rx-bus-width = 4) and see if that works. This should narrow down > whether SFDP parsing is broken or if driver has an issue. Sorry, I still haven't found the time at work to test this. I'll keep trying. Keeping that aside, if this driver is converted to spi-mem, is there any chance of getting this into SPL while not getting standard SPI drivers in? On socfpga, I have a standard SPI driver (designware_spi) enabled but that results in both the cadence_qspi and designware_spi being included in SPL aside well when boot-from-SPI is enabled. Does that somehow change when cadence_qspi is an spi-mem driver? Thanks, Simon > > Regards > Vignesh > > > It works as expected when not parsing SFDP or setting the bus-width to 1. > > So the change itself probably works, but SFDP parsing is broken? > > > > I did the tests with this applied first: > > https://patchwork.ozlabs.org/project/uboot/list/?series=135505 > > > > Regards, > > Simon > > > > > >> > >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > >> --- > >> drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- > >> drivers/spi/cadence_qspi.h | 9 +-- > >> drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- > >> 3 files changed, 91 insertions(+), 178 deletions(-) > >> > >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > >> index e2e54cd27723..673a2e9a6c4c 100644 > >> --- a/drivers/spi/cadence_qspi.c > >> +++ b/drivers/spi/cadence_qspi.c > >> @@ -10,6 +10,7 @@ > >> #include <malloc.h> > >> #include <reset.h> > >> #include <spi.h> > >> +#include <spi-mem.h> > >> #include <linux/errno.h> > >> #include "cadence_qspi.h" > >> > >> @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) > >> return 0; > >> } > >> > >> +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) > >> +{ > >> + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), > >> + SPI_MEM_OP_NO_ADDR, > >> + SPI_MEM_OP_NO_DUMMY, > >> + SPI_MEM_OP_DATA_IN(len, idcode, 1)); > >> + > >> + return cadence_qspi_apb_command_read(reg_base, &op); > >> +} > >> + > >> /* Calibration sequence to determine the read data capture delay register */ > >> static int spi_calibration(struct udevice *bus, uint hz) > >> { > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> void *base = priv->regbase; > >> - u8 opcode_rdid = 0x9F; > >> unsigned int idcode = 0, temp = 0; > >> int err = 0, i, range_lo = -1, range_hi = -1; > >> > >> @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > >> cadence_qspi_apb_controller_enable(base); > >> > >> /* read the ID which will be our golden value */ > >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > >> - 3, (u8 *)&idcode); > >> + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); > >> if (err) { > >> puts("SF: Calibration failed (read)\n"); > >> return err; > >> @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > >> cadence_qspi_apb_controller_enable(base); > >> > >> /* issue a RDID to get the ID value */ > >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > >> - 3, (u8 *)&temp); > >> + err = cadence_spi_read_id(base, 3, (u8 *)&temp); > >> if (err) { > >> puts("SF: Calibration failed (read)\n"); > >> return err; > >> @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >> return 0; > >> } > >> > >> -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> - const void *dout, void *din, unsigned long flags) > >> +static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >> + const struct spi_mem_op *op) > >> { > >> - struct udevice *bus = dev->parent; > >> + struct udevice *bus = spi->dev->parent; > >> struct cadence_spi_platdata *plat = bus->platdata; > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); > >> void *base = priv->regbase; > >> - u8 *cmd_buf = priv->cmd_buf; > >> - size_t data_bytes; > >> int err = 0; > >> - u32 mode = CQSPI_STIG_WRITE; > >> - > >> - if (flags & SPI_XFER_BEGIN) { > >> - /* copy command to local buffer */ > >> - priv->cmd_len = bitlen / 8; > >> - memcpy(cmd_buf, dout, priv->cmd_len); > >> - } > >> - > >> - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { > >> - /* if start and end bit are set, the data bytes is 0. */ > >> - data_bytes = 0; > >> - } else { > >> - data_bytes = bitlen / 8; > >> - } > >> - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); > >> + u32 mode; > >> > >> /* Set Chip select */ > >> - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), > >> + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), > >> plat->is_decoded_cs); > >> > >> - if ((flags & SPI_XFER_END) || (flags == 0)) { > >> - if (priv->cmd_len == 0) { > >> - printf("QSPI: Error, command is empty.\n"); > >> - return -1; > >> - } > >> - > >> - if (din && data_bytes) { > >> - /* read */ > >> - /* Use STIG if no address. */ > >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) > >> - mode = CQSPI_STIG_READ; > >> - else > >> - mode = CQSPI_INDIRECT_READ; > >> - } else if (dout && !(flags & SPI_XFER_BEGIN)) { > >> - /* write */ > >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) > >> - mode = CQSPI_STIG_WRITE; > >> - else > >> - mode = CQSPI_INDIRECT_WRITE; > >> - } > >> - > >> - switch (mode) { > >> - case CQSPI_STIG_READ: > >> - err = cadence_qspi_apb_command_read( > >> - base, priv->cmd_len, cmd_buf, > >> - data_bytes, din); > >> + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { > >> + if (!op->addr.nbytes) > >> + mode = CQSPI_STIG_READ; > >> + else > >> + mode = CQSPI_INDIRECT_READ; > >> + } else { > >> + if (!op->addr.nbytes || !op->data.buf.out) > >> + mode = CQSPI_STIG_WRITE; > >> + else > >> + mode = CQSPI_INDIRECT_WRITE; > >> + } > >> > >> + switch (mode) { > >> + case CQSPI_STIG_READ: > >> + err = cadence_qspi_apb_command_read(base, op); > >> break; > >> - case CQSPI_STIG_WRITE: > >> - err = cadence_qspi_apb_command_write(base, > >> - priv->cmd_len, cmd_buf, > >> - data_bytes, dout); > >> + case CQSPI_STIG_WRITE: > >> + err = cadence_qspi_apb_command_write(base, op); > >> break; > >> - case CQSPI_INDIRECT_READ: > >> - err = cadence_qspi_apb_indirect_read_setup(plat, > >> - priv->cmd_len, dm_plat->mode, cmd_buf); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_read_execute > >> - (plat, data_bytes, din); > >> - } > >> - break; > >> - case CQSPI_INDIRECT_WRITE: > >> - err = cadence_qspi_apb_indirect_write_setup > >> - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_write_execute > >> - (plat, data_bytes, dout); > >> - } > >> - break; > >> - default: > >> - err = -1; > >> - break; > >> + case CQSPI_INDIRECT_READ: > >> + err = cadence_qspi_apb_indirect_read_setup(plat, op); > >> + if (!err) { > >> + err = cadence_qspi_apb_indirect_read_execute > >> + (plat, op->data.nbytes, op->data.buf.in); > >> } > >> - > >> - if (flags & SPI_XFER_END) { > >> - /* clear command buffer */ > >> - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); > >> - priv->cmd_len = 0; > >> + break; > >> + case CQSPI_INDIRECT_WRITE: > >> + err = cadence_qspi_apb_indirect_write_setup(plat, op); > >> + if (!err) { > >> + err = cadence_qspi_apb_indirect_write_execute > >> + (plat, op->data.nbytes, op->data.buf.out); > >> } > >> + break; > >> + default: > >> + err = -1; > >> + break; > >> } > >> > >> return err; > >> @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > >> return 0; > >> } > >> > >> +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { > >> + .exec_op = cadence_spi_mem_exec_op, > >> +}; > >> + > >> static const struct dm_spi_ops cadence_spi_ops = { > >> - .xfer = cadence_spi_xfer, > >> .set_speed = cadence_spi_set_speed, > >> .set_mode = cadence_spi_set_mode, > >> + .mem_ops = &cadence_spi_mem_ops, > >> /* > >> * cs_info is not needed, since we require all chip selects to be > >> * in the device tree explicitly > >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > >> index 20cceca239f8..e655b027d788 100644 > >> --- a/drivers/spi/cadence_qspi.h > >> +++ b/drivers/spi/cadence_qspi.h > >> @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); > >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > >> > >> int cadence_qspi_apb_command_read(void *reg_base_addr, > >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_command_write(void *reg_base_addr, > >> - unsigned int cmdlen, const u8 *cmdbuf, > >> - unsigned int txlen, const u8 *txbuf); > >> + const struct spi_mem_op *op); > >> > >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >> unsigned int rxlen, u8 *rxbuf); > >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >> unsigned int txlen, const u8 *txbuf); > >> > >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >> index 55a7501913a8..8dd0495dfcf4 100644 > >> --- a/drivers/spi/cadence_qspi_apb.c > >> +++ b/drivers/spi/cadence_qspi_apb.c > >> @@ -30,6 +30,7 @@ > >> #include <linux/errno.h> > >> #include <wait_bit.h> > >> #include <spi.h> > >> +#include <spi-mem.h> > >> #include <malloc.h> > >> #include "cadence_qspi.h" > >> > >> @@ -172,19 +173,6 @@ > >> (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > >> CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) > >> > >> -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, > >> - unsigned int addr_width) > >> -{ > >> - unsigned int addr; > >> - > >> - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; > >> - > >> - if (addr_width == 4) > >> - addr = (addr << 8) | addr_buf[3]; > >> - > >> - return addr; > >> -} > >> - > >> void cadence_qspi_apb_controller_enable(void *reg_base) > >> { > >> unsigned int reg; > >> @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, > >> } > >> > >> /* For command RDID, RDSR. */ > >> -int cadence_qspi_apb_command_read(void *reg_base, > >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, > >> - u8 *rxbuf) > >> +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> unsigned int read_len; > >> int status; > >> + unsigned int rxlen = op->data.nbytes; > >> + void *rxbuf = op->data.buf.in; > >> > >> - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { > >> - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", > >> - cmdlen, rxlen); > >> + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { > >> + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); > >> return -EINVAL; > >> } > >> > >> - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> > >> reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); > >> > >> @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, > >> } > >> > >> /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ > >> -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > >> - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) > >> +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > >> { > >> unsigned int reg = 0; > >> - unsigned int addr_value; > >> unsigned int wr_data; > >> unsigned int wr_len; > >> + unsigned int txlen = op->data.nbytes; > >> + const void *txbuf = op->data.buf.out; > >> + u32 addr; > >> + > >> + /* Reorder address to SPI bus order if only transferring address */ > >> + if (!txlen) { > >> + addr = cpu_to_be32(op->addr.val); > >> + if (op->addr.nbytes == 3) > >> + addr >>= 8; > >> + txbuf = &addr; > >> + txlen = op->addr.nbytes; > >> + } > >> > >> - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { > >> - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", > >> - cmdlen, txlen); > >> + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { > >> + printf("QSPI: Invalid input arguments txlen %u\n", txlen); > >> return -EINVAL; > >> } > >> > >> - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> - > >> - if (cmdlen == 4 || cmdlen == 5) { > >> - /* Command with address */ > >> - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); > >> - /* Number of bytes to write. */ > >> - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) > >> - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; > >> - /* Get address */ > >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], > >> - cmdlen >= 5 ? 4 : 3); > >> - > >> - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); > >> - } > >> + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> > >> if (txlen) { > >> /* writing data = yes */ > >> @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > >> > >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> unsigned int rd_reg; > >> - unsigned int addr_value; > >> unsigned int dummy_clk; > >> - unsigned int dummy_bytes; > >> - unsigned int addr_bytes; > >> - > >> - /* > >> - * Identify addr_byte. All NOR flash device drivers are using fast read > >> - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. > >> - * With that, the length is in value of 5 or 6. Only FRAM chip from > >> - * ramtron using normal read (which won't need dummy byte). > >> - * Unlikely NOR flash using normal read due to performance issue. > >> - */ > >> - if (cmdlen >= 5) > >> - /* to cater fast read where cmd + addr + dummy */ > >> - addr_bytes = cmdlen - 2; > >> - else > >> - /* for normal read (only ramtron as of now) */ > >> - addr_bytes = cmdlen - 1; > >> + unsigned int dummy_bytes = op->dummy.nbytes; > >> > >> /* Setup the indirect trigger address */ > >> writel(plat->trigger_address, > >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > >> > >> /* Configure the opcode */ > >> - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; > >> + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; > >> > >> - if (rx_width & SPI_RX_QUAD) > >> + if (op->data.buswidth == 4) > >> /* Instruction and address at DQ0, data at DQ0-3. */ > >> rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > >> > >> - /* Get address */ > >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > >> - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > >> > >> - /* The remaining lenght is dummy bytes. */ > >> - dummy_bytes = cmdlen - addr_bytes - 1; > >> if (dummy_bytes) { > >> if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) > >> dummy_bytes = CQSPI_DUMMY_BYTES_MAX; > >> > >> - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); > >> -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) > >> - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); > >> -#else > >> - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); > >> -#endif > >> - > >> /* Convert to clock cycles. */ > >> dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; > >> - /* Need to minus the mode byte (8 clocks). */ > >> - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; > >> > >> if (dummy_clk) > >> rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) > >> @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> /* set device size */ > >> reg = readl(plat->regbase + CQSPI_REG_SIZE); > >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > >> - reg |= (addr_bytes - 1); > >> + reg |= (op->addr.nbytes - 1); > >> writel(reg, plat->regbase + CQSPI_REG_SIZE); > >> return 0; > >> } > >> @@ -687,35 +641,23 @@ failrd: > >> > >> /* Opcode + Address (3/4 bytes) */ > >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; > >> > >> - if (cmdlen < 4 || cmdbuf == NULL) { > >> - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", > >> - cmdlen, cmdbuf); > >> - return -EINVAL; > >> - } > >> /* Setup the indirect trigger address */ > >> writel(plat->trigger_address, > >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > >> > >> /* Configure the opcode */ > >> - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; > >> - > >> - if (tx_width & SPI_TX_QUAD) > >> - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; > >> - > >> + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; > >> writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); > >> > >> - /* Setup write address. */ > >> - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > >> - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > >> > >> reg = readl(plat->regbase + CQSPI_REG_SIZE); > >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > >> - reg |= (addr_bytes - 1); > >> + reg |= (op->addr.nbytes - 1); > >> writel(reg, plat->regbase + CQSPI_REG_SIZE); > >> return 0; > >> } > >> -- > >> 2.23.0 > >> > > -- > Regards > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-11-06 19:55 ` Simon Goldschmidt @ 2019-11-07 6:09 ` Vignesh Raghavendra 0 siblings, 0 replies; 18+ messages in thread From: Vignesh Raghavendra @ 2019-11-07 6:09 UTC (permalink / raw) To: u-boot Hi Simon, On 07/11/19 1:25 AM, Simon Goldschmidt wrote: > Hi Vignesh, > > On Thu, Oct 17, 2019 at 2:31 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> Hi Simon, >> >> On 17/10/19 4:50 PM, Simon Goldschmidt wrote: >>> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>>> >>>> Current Cadence QSPI driver has few limitations. It assumes all read >>>> operations to be in Quad mode and thus does not support SFDP parsing. >>>> Also, adding support for new mode such as Octal mode would not be >>>> possible with current configuration. Therefore move the driver over to spi-mem >>>> framework. This has added advantage that driver can be used to support >>>> SPI NAND memories too. >>>> Hence, move driver over to new spi-mem APIs. >>>> >>>> Please note that this gets rid of mode bit setting done when >>>> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to >>>> that config option. >>> >>> I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but >>> it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width >>> at 4 in my devicetree, SFDP parsing works, but reading data afterwards >>> produces invalid results (I haven't tested what's wrong there). >>> >> >> Thanks for testing! >> >> spi-tx-bus-width = 4 was not supported before so I haven't added support >> for that mode in this series. That change will be a separate series. >> >> Could you try with spi-tx-bus-width = 1 and spi-rx-bus-width = 4 and see >> if that works? >> >> If that does not work then could you disable SFDP parsing (but keep >> spi-rx-bus-width = 4) and see if that works. This should narrow down >> whether SFDP parsing is broken or if driver has an issue. > > Sorry, I still haven't found the time at work to test this. I'll keep trying. > Keeping that aside, if this driver is converted to spi-mem, is there any > chance of getting this into SPL while not getting standard SPI drivers in? > > On socfpga, I have a standard SPI driver (designware_spi) enabled but that > results in both the cadence_qspi and designware_spi being included in SPL aside > well when boot-from-SPI is enabled. > > Does that somehow change when cadence_qspi is an spi-mem driver? > I dont think so. Only solution I can think of is to create a separate SPL specific config for designware_spi and disable that for socfpga. Regards Vignesh > Thanks, > Simon > >> >> Regards >> Vignesh >> >>> It works as expected when not parsing SFDP or setting the bus-width to 1. >>> So the change itself probably works, but SFDP parsing is broken? >>> >>> I did the tests with this applied first: >>> https://patchwork.ozlabs.org/project/uboot/list/?series=135505 >>> >>> Regards, >>> Simon >>> >>> >>>> >>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>>> --- >>>> drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- >>>> drivers/spi/cadence_qspi.h | 9 +-- >>>> drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- >>>> 3 files changed, 91 insertions(+), 178 deletions(-) >>>> >>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c >>>> index e2e54cd27723..673a2e9a6c4c 100644 >>>> --- a/drivers/spi/cadence_qspi.c >>>> +++ b/drivers/spi/cadence_qspi.c >>>> @@ -10,6 +10,7 @@ >>>> #include <malloc.h> >>>> #include <reset.h> >>>> #include <spi.h> >>>> +#include <spi-mem.h> >>>> #include <linux/errno.h> >>>> #include "cadence_qspi.h" >>>> >>>> @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) >>>> return 0; >>>> } >>>> >>>> +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) >>>> +{ >>>> + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), >>>> + SPI_MEM_OP_NO_ADDR, >>>> + SPI_MEM_OP_NO_DUMMY, >>>> + SPI_MEM_OP_DATA_IN(len, idcode, 1)); >>>> + >>>> + return cadence_qspi_apb_command_read(reg_base, &op); >>>> +} >>>> + >>>> /* Calibration sequence to determine the read data capture delay register */ >>>> static int spi_calibration(struct udevice *bus, uint hz) >>>> { >>>> struct cadence_spi_priv *priv = dev_get_priv(bus); >>>> void *base = priv->regbase; >>>> - u8 opcode_rdid = 0x9F; >>>> unsigned int idcode = 0, temp = 0; >>>> int err = 0, i, range_lo = -1, range_hi = -1; >>>> >>>> @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) >>>> cadence_qspi_apb_controller_enable(base); >>>> >>>> /* read the ID which will be our golden value */ >>>> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, >>>> - 3, (u8 *)&idcode); >>>> + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); >>>> if (err) { >>>> puts("SF: Calibration failed (read)\n"); >>>> return err; >>>> @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) >>>> cadence_qspi_apb_controller_enable(base); >>>> >>>> /* issue a RDID to get the ID value */ >>>> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, >>>> - 3, (u8 *)&temp); >>>> + err = cadence_spi_read_id(base, 3, (u8 *)&temp); >>>> if (err) { >>>> puts("SF: Calibration failed (read)\n"); >>>> return err; >>>> @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) >>>> return 0; >>>> } >>>> >>>> -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, >>>> - const void *dout, void *din, unsigned long flags) >>>> +static int cadence_spi_mem_exec_op(struct spi_slave *spi, >>>> + const struct spi_mem_op *op) >>>> { >>>> - struct udevice *bus = dev->parent; >>>> + struct udevice *bus = spi->dev->parent; >>>> struct cadence_spi_platdata *plat = bus->platdata; >>>> struct cadence_spi_priv *priv = dev_get_priv(bus); >>>> - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); >>>> void *base = priv->regbase; >>>> - u8 *cmd_buf = priv->cmd_buf; >>>> - size_t data_bytes; >>>> int err = 0; >>>> - u32 mode = CQSPI_STIG_WRITE; >>>> - >>>> - if (flags & SPI_XFER_BEGIN) { >>>> - /* copy command to local buffer */ >>>> - priv->cmd_len = bitlen / 8; >>>> - memcpy(cmd_buf, dout, priv->cmd_len); >>>> - } >>>> - >>>> - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { >>>> - /* if start and end bit are set, the data bytes is 0. */ >>>> - data_bytes = 0; >>>> - } else { >>>> - data_bytes = bitlen / 8; >>>> - } >>>> - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); >>>> + u32 mode; >>>> >>>> /* Set Chip select */ >>>> - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), >>>> + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), >>>> plat->is_decoded_cs); >>>> >>>> - if ((flags & SPI_XFER_END) || (flags == 0)) { >>>> - if (priv->cmd_len == 0) { >>>> - printf("QSPI: Error, command is empty.\n"); >>>> - return -1; >>>> - } >>>> - >>>> - if (din && data_bytes) { >>>> - /* read */ >>>> - /* Use STIG if no address. */ >>>> - if (!CQSPI_IS_ADDR(priv->cmd_len)) >>>> - mode = CQSPI_STIG_READ; >>>> - else >>>> - mode = CQSPI_INDIRECT_READ; >>>> - } else if (dout && !(flags & SPI_XFER_BEGIN)) { >>>> - /* write */ >>>> - if (!CQSPI_IS_ADDR(priv->cmd_len)) >>>> - mode = CQSPI_STIG_WRITE; >>>> - else >>>> - mode = CQSPI_INDIRECT_WRITE; >>>> - } >>>> - >>>> - switch (mode) { >>>> - case CQSPI_STIG_READ: >>>> - err = cadence_qspi_apb_command_read( >>>> - base, priv->cmd_len, cmd_buf, >>>> - data_bytes, din); >>>> + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { >>>> + if (!op->addr.nbytes) >>>> + mode = CQSPI_STIG_READ; >>>> + else >>>> + mode = CQSPI_INDIRECT_READ; >>>> + } else { >>>> + if (!op->addr.nbytes || !op->data.buf.out) >>>> + mode = CQSPI_STIG_WRITE; >>>> + else >>>> + mode = CQSPI_INDIRECT_WRITE; >>>> + } >>>> >>>> + switch (mode) { >>>> + case CQSPI_STIG_READ: >>>> + err = cadence_qspi_apb_command_read(base, op); >>>> break; >>>> - case CQSPI_STIG_WRITE: >>>> - err = cadence_qspi_apb_command_write(base, >>>> - priv->cmd_len, cmd_buf, >>>> - data_bytes, dout); >>>> + case CQSPI_STIG_WRITE: >>>> + err = cadence_qspi_apb_command_write(base, op); >>>> break; >>>> - case CQSPI_INDIRECT_READ: >>>> - err = cadence_qspi_apb_indirect_read_setup(plat, >>>> - priv->cmd_len, dm_plat->mode, cmd_buf); >>>> - if (!err) { >>>> - err = cadence_qspi_apb_indirect_read_execute >>>> - (plat, data_bytes, din); >>>> - } >>>> - break; >>>> - case CQSPI_INDIRECT_WRITE: >>>> - err = cadence_qspi_apb_indirect_write_setup >>>> - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); >>>> - if (!err) { >>>> - err = cadence_qspi_apb_indirect_write_execute >>>> - (plat, data_bytes, dout); >>>> - } >>>> - break; >>>> - default: >>>> - err = -1; >>>> - break; >>>> + case CQSPI_INDIRECT_READ: >>>> + err = cadence_qspi_apb_indirect_read_setup(plat, op); >>>> + if (!err) { >>>> + err = cadence_qspi_apb_indirect_read_execute >>>> + (plat, op->data.nbytes, op->data.buf.in); >>>> } >>>> - >>>> - if (flags & SPI_XFER_END) { >>>> - /* clear command buffer */ >>>> - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); >>>> - priv->cmd_len = 0; >>>> + break; >>>> + case CQSPI_INDIRECT_WRITE: >>>> + err = cadence_qspi_apb_indirect_write_setup(plat, op); >>>> + if (!err) { >>>> + err = cadence_qspi_apb_indirect_write_execute >>>> + (plat, op->data.nbytes, op->data.buf.out); >>>> } >>>> + break; >>>> + default: >>>> + err = -1; >>>> + break; >>>> } >>>> >>>> return err; >>>> @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) >>>> return 0; >>>> } >>>> >>>> +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { >>>> + .exec_op = cadence_spi_mem_exec_op, >>>> +}; >>>> + >>>> static const struct dm_spi_ops cadence_spi_ops = { >>>> - .xfer = cadence_spi_xfer, >>>> .set_speed = cadence_spi_set_speed, >>>> .set_mode = cadence_spi_set_mode, >>>> + .mem_ops = &cadence_spi_mem_ops, >>>> /* >>>> * cs_info is not needed, since we require all chip selects to be >>>> * in the device tree explicitly >>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >>>> index 20cceca239f8..e655b027d788 100644 >>>> --- a/drivers/spi/cadence_qspi.h >>>> +++ b/drivers/spi/cadence_qspi.h >>>> @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); >>>> void cadence_qspi_apb_controller_disable(void *reg_base_addr); >>>> >>>> int cadence_qspi_apb_command_read(void *reg_base_addr, >>>> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); >>>> + const struct spi_mem_op *op); >>>> int cadence_qspi_apb_command_write(void *reg_base_addr, >>>> - unsigned int cmdlen, const u8 *cmdbuf, >>>> - unsigned int txlen, const u8 *txbuf); >>>> + const struct spi_mem_op *op); >>>> >>>> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); >>>> + const struct spi_mem_op *op); >>>> int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>> unsigned int rxlen, u8 *rxbuf); >>>> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); >>>> + const struct spi_mem_op *op); >>>> int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>> unsigned int txlen, const u8 *txbuf); >>>> >>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >>>> index 55a7501913a8..8dd0495dfcf4 100644 >>>> --- a/drivers/spi/cadence_qspi_apb.c >>>> +++ b/drivers/spi/cadence_qspi_apb.c >>>> @@ -30,6 +30,7 @@ >>>> #include <linux/errno.h> >>>> #include <wait_bit.h> >>>> #include <spi.h> >>>> +#include <spi-mem.h> >>>> #include <malloc.h> >>>> #include "cadence_qspi.h" >>>> >>>> @@ -172,19 +173,6 @@ >>>> (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ >>>> CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) >>>> >>>> -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, >>>> - unsigned int addr_width) >>>> -{ >>>> - unsigned int addr; >>>> - >>>> - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; >>>> - >>>> - if (addr_width == 4) >>>> - addr = (addr << 8) | addr_buf[3]; >>>> - >>>> - return addr; >>>> -} >>>> - >>>> void cadence_qspi_apb_controller_enable(void *reg_base) >>>> { >>>> unsigned int reg; >>>> @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, >>>> } >>>> >>>> /* For command RDID, RDSR. */ >>>> -int cadence_qspi_apb_command_read(void *reg_base, >>>> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, >>>> - u8 *rxbuf) >>>> +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) >>>> { >>>> unsigned int reg; >>>> unsigned int read_len; >>>> int status; >>>> + unsigned int rxlen = op->data.nbytes; >>>> + void *rxbuf = op->data.buf.in; >>>> >>>> - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { >>>> - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", >>>> - cmdlen, rxlen); >>>> + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { >>>> + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); >>>> return -EINVAL; >>>> } >>>> >>>> - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; >>>> + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; >>>> >>>> reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); >>>> >>>> @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, >>>> } >>>> >>>> /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ >>>> -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, >>>> - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) >>>> +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) >>>> { >>>> unsigned int reg = 0; >>>> - unsigned int addr_value; >>>> unsigned int wr_data; >>>> unsigned int wr_len; >>>> + unsigned int txlen = op->data.nbytes; >>>> + const void *txbuf = op->data.buf.out; >>>> + u32 addr; >>>> + >>>> + /* Reorder address to SPI bus order if only transferring address */ >>>> + if (!txlen) { >>>> + addr = cpu_to_be32(op->addr.val); >>>> + if (op->addr.nbytes == 3) >>>> + addr >>= 8; >>>> + txbuf = &addr; >>>> + txlen = op->addr.nbytes; >>>> + } >>>> >>>> - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { >>>> - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", >>>> - cmdlen, txlen); >>>> + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { >>>> + printf("QSPI: Invalid input arguments txlen %u\n", txlen); >>>> return -EINVAL; >>>> } >>>> >>>> - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; >>>> - >>>> - if (cmdlen == 4 || cmdlen == 5) { >>>> - /* Command with address */ >>>> - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); >>>> - /* Number of bytes to write. */ >>>> - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) >>>> - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; >>>> - /* Get address */ >>>> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], >>>> - cmdlen >= 5 ? 4 : 3); >>>> - >>>> - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); >>>> - } >>>> + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; >>>> >>>> if (txlen) { >>>> /* writing data = yes */ >>>> @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, >>>> >>>> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ >>>> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) >>>> + const struct spi_mem_op *op) >>>> { >>>> unsigned int reg; >>>> unsigned int rd_reg; >>>> - unsigned int addr_value; >>>> unsigned int dummy_clk; >>>> - unsigned int dummy_bytes; >>>> - unsigned int addr_bytes; >>>> - >>>> - /* >>>> - * Identify addr_byte. All NOR flash device drivers are using fast read >>>> - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. >>>> - * With that, the length is in value of 5 or 6. Only FRAM chip from >>>> - * ramtron using normal read (which won't need dummy byte). >>>> - * Unlikely NOR flash using normal read due to performance issue. >>>> - */ >>>> - if (cmdlen >= 5) >>>> - /* to cater fast read where cmd + addr + dummy */ >>>> - addr_bytes = cmdlen - 2; >>>> - else >>>> - /* for normal read (only ramtron as of now) */ >>>> - addr_bytes = cmdlen - 1; >>>> + unsigned int dummy_bytes = op->dummy.nbytes; >>>> >>>> /* Setup the indirect trigger address */ >>>> writel(plat->trigger_address, >>>> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); >>>> >>>> /* Configure the opcode */ >>>> - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; >>>> + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; >>>> >>>> - if (rx_width & SPI_RX_QUAD) >>>> + if (op->data.buswidth == 4) >>>> /* Instruction and address at DQ0, data at DQ0-3. */ >>>> rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; >>>> >>>> - /* Get address */ >>>> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); >>>> - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); >>>> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); >>>> >>>> - /* The remaining lenght is dummy bytes. */ >>>> - dummy_bytes = cmdlen - addr_bytes - 1; >>>> if (dummy_bytes) { >>>> if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) >>>> dummy_bytes = CQSPI_DUMMY_BYTES_MAX; >>>> >>>> - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); >>>> -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) >>>> - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); >>>> -#else >>>> - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); >>>> -#endif >>>> - >>>> /* Convert to clock cycles. */ >>>> dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; >>>> - /* Need to minus the mode byte (8 clocks). */ >>>> - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; >>>> >>>> if (dummy_clk) >>>> rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) >>>> @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>> /* set device size */ >>>> reg = readl(plat->regbase + CQSPI_REG_SIZE); >>>> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; >>>> - reg |= (addr_bytes - 1); >>>> + reg |= (op->addr.nbytes - 1); >>>> writel(reg, plat->regbase + CQSPI_REG_SIZE); >>>> return 0; >>>> } >>>> @@ -687,35 +641,23 @@ failrd: >>>> >>>> /* Opcode + Address (3/4 bytes) */ >>>> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) >>>> + const struct spi_mem_op *op) >>>> { >>>> unsigned int reg; >>>> - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; >>>> >>>> - if (cmdlen < 4 || cmdbuf == NULL) { >>>> - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", >>>> - cmdlen, cmdbuf); >>>> - return -EINVAL; >>>> - } >>>> /* Setup the indirect trigger address */ >>>> writel(plat->trigger_address, >>>> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); >>>> >>>> /* Configure the opcode */ >>>> - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; >>>> - >>>> - if (tx_width & SPI_TX_QUAD) >>>> - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; >>>> - >>>> + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; >>>> writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); >>>> >>>> - /* Setup write address. */ >>>> - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); >>>> - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); >>>> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); >>>> >>>> reg = readl(plat->regbase + CQSPI_REG_SIZE); >>>> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; >>>> - reg |= (addr_bytes - 1); >>>> + reg |= (op->addr.nbytes - 1); >>>> writel(reg, plat->regbase + CQSPI_REG_SIZE); >>>> return 0; >>>> } >>>> -- >>>> 2.23.0 >>>> >> >> -- >> Regards >> Vignesh -- Regards Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-17 12:31 ` Vignesh Raghavendra 2019-11-06 19:55 ` Simon Goldschmidt @ 2020-01-17 15:33 ` Simon Goldschmidt 1 sibling, 0 replies; 18+ messages in thread From: Simon Goldschmidt @ 2020-01-17 15:33 UTC (permalink / raw) To: u-boot On Thu, Oct 17, 2019 at 2:31 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi Simon, > > On 17/10/19 4:50 PM, Simon Goldschmidt wrote: > > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> Current Cadence QSPI driver has few limitations. It assumes all read > >> operations to be in Quad mode and thus does not support SFDP parsing. > >> Also, adding support for new mode such as Octal mode would not be > >> possible with current configuration. Therefore move the driver over to spi-mem > >> framework. This has added advantage that driver can be used to support > >> SPI NAND memories too. > >> Hence, move driver over to new spi-mem APIs. > >> > >> Please note that this gets rid of mode bit setting done when > >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to > >> that config option. > > > > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > > produces invalid results (I haven't tested what's wrong there). > > > > Thanks for testing! > > spi-tx-bus-width = 4 was not supported before so I haven't added support > for that mode in this series. That change will be a separate series. > > Could you try with spi-tx-bus-width = 1 and spi-rx-bus-width = 4 and see > if that works? Yes, that works. So from my side, v2 of this patch is OK. I'll send my TB. Regards, Simon > > If that does not work then could you disable SFDP parsing (but keep > spi-rx-bus-width = 4) and see if that works. This should narrow down > whether SFDP parsing is broken or if driver has an issue. > > Regards > Vignesh > > > It works as expected when not parsing SFDP or setting the bus-width to 1. > > So the change itself probably works, but SFDP parsing is broken? > > > > I did the tests with this applied first: > > https://patchwork.ozlabs.org/project/uboot/list/?series=135505 > > > > Regards, > > Simon > > > > > >> > >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > >> --- > >> drivers/spi/cadence_qspi.c | 136 +++++++++++++-------------------- > >> drivers/spi/cadence_qspi.h | 9 +-- > >> drivers/spi/cadence_qspi_apb.c | 124 ++++++++---------------------- > >> 3 files changed, 91 insertions(+), 178 deletions(-) > >> > >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > >> index e2e54cd27723..673a2e9a6c4c 100644 > >> --- a/drivers/spi/cadence_qspi.c > >> +++ b/drivers/spi/cadence_qspi.c > >> @@ -10,6 +10,7 @@ > >> #include <malloc.h> > >> #include <reset.h> > >> #include <spi.h> > >> +#include <spi-mem.h> > >> #include <linux/errno.h> > >> #include "cadence_qspi.h" > >> > >> @@ -34,12 +35,21 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz) > >> return 0; > >> } > >> > >> +static int cadence_spi_read_id(void *reg_base, u8 len, u8 *idcode) > >> +{ > >> + struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0x9F, 1), > >> + SPI_MEM_OP_NO_ADDR, > >> + SPI_MEM_OP_NO_DUMMY, > >> + SPI_MEM_OP_DATA_IN(len, idcode, 1)); > >> + > >> + return cadence_qspi_apb_command_read(reg_base, &op); > >> +} > >> + > >> /* Calibration sequence to determine the read data capture delay register */ > >> static int spi_calibration(struct udevice *bus, uint hz) > >> { > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> void *base = priv->regbase; > >> - u8 opcode_rdid = 0x9F; > >> unsigned int idcode = 0, temp = 0; > >> int err = 0, i, range_lo = -1, range_hi = -1; > >> > >> @@ -53,8 +63,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > >> cadence_qspi_apb_controller_enable(base); > >> > >> /* read the ID which will be our golden value */ > >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > >> - 3, (u8 *)&idcode); > >> + err = cadence_spi_read_id(base, 3, (u8 *)&idcode); > >> if (err) { > >> puts("SF: Calibration failed (read)\n"); > >> return err; > >> @@ -73,8 +82,7 @@ static int spi_calibration(struct udevice *bus, uint hz) > >> cadence_qspi_apb_controller_enable(base); > >> > >> /* issue a RDID to get the ID value */ > >> - err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid, > >> - 3, (u8 *)&temp); > >> + err = cadence_spi_read_id(base, 3, (u8 *)&temp); > >> if (err) { > >> puts("SF: Calibration failed (read)\n"); > >> return err; > >> @@ -195,96 +203,56 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >> return 0; > >> } > >> > >> -static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> - const void *dout, void *din, unsigned long flags) > >> +static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >> + const struct spi_mem_op *op) > >> { > >> - struct udevice *bus = dev->parent; > >> + struct udevice *bus = spi->dev->parent; > >> struct cadence_spi_platdata *plat = bus->platdata; > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> - struct dm_spi_slave_platdata *dm_plat = dev_get_parent_platdata(dev); > >> void *base = priv->regbase; > >> - u8 *cmd_buf = priv->cmd_buf; > >> - size_t data_bytes; > >> int err = 0; > >> - u32 mode = CQSPI_STIG_WRITE; > >> - > >> - if (flags & SPI_XFER_BEGIN) { > >> - /* copy command to local buffer */ > >> - priv->cmd_len = bitlen / 8; > >> - memcpy(cmd_buf, dout, priv->cmd_len); > >> - } > >> - > >> - if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) { > >> - /* if start and end bit are set, the data bytes is 0. */ > >> - data_bytes = 0; > >> - } else { > >> - data_bytes = bitlen / 8; > >> - } > >> - debug("%s: len=%zu [bytes]\n", __func__, data_bytes); > >> + u32 mode; > >> > >> /* Set Chip select */ > >> - cadence_qspi_apb_chipselect(base, spi_chip_select(dev), > >> + cadence_qspi_apb_chipselect(base, spi_chip_select(spi->dev), > >> plat->is_decoded_cs); > >> > >> - if ((flags & SPI_XFER_END) || (flags == 0)) { > >> - if (priv->cmd_len == 0) { > >> - printf("QSPI: Error, command is empty.\n"); > >> - return -1; > >> - } > >> - > >> - if (din && data_bytes) { > >> - /* read */ > >> - /* Use STIG if no address. */ > >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) > >> - mode = CQSPI_STIG_READ; > >> - else > >> - mode = CQSPI_INDIRECT_READ; > >> - } else if (dout && !(flags & SPI_XFER_BEGIN)) { > >> - /* write */ > >> - if (!CQSPI_IS_ADDR(priv->cmd_len)) > >> - mode = CQSPI_STIG_WRITE; > >> - else > >> - mode = CQSPI_INDIRECT_WRITE; > >> - } > >> - > >> - switch (mode) { > >> - case CQSPI_STIG_READ: > >> - err = cadence_qspi_apb_command_read( > >> - base, priv->cmd_len, cmd_buf, > >> - data_bytes, din); > >> + if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) { > >> + if (!op->addr.nbytes) > >> + mode = CQSPI_STIG_READ; > >> + else > >> + mode = CQSPI_INDIRECT_READ; > >> + } else { > >> + if (!op->addr.nbytes || !op->data.buf.out) > >> + mode = CQSPI_STIG_WRITE; > >> + else > >> + mode = CQSPI_INDIRECT_WRITE; > >> + } > >> > >> + switch (mode) { > >> + case CQSPI_STIG_READ: > >> + err = cadence_qspi_apb_command_read(base, op); > >> break; > >> - case CQSPI_STIG_WRITE: > >> - err = cadence_qspi_apb_command_write(base, > >> - priv->cmd_len, cmd_buf, > >> - data_bytes, dout); > >> + case CQSPI_STIG_WRITE: > >> + err = cadence_qspi_apb_command_write(base, op); > >> break; > >> - case CQSPI_INDIRECT_READ: > >> - err = cadence_qspi_apb_indirect_read_setup(plat, > >> - priv->cmd_len, dm_plat->mode, cmd_buf); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_read_execute > >> - (plat, data_bytes, din); > >> - } > >> - break; > >> - case CQSPI_INDIRECT_WRITE: > >> - err = cadence_qspi_apb_indirect_write_setup > >> - (plat, priv->cmd_len, dm_plat->mode, cmd_buf); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_write_execute > >> - (plat, data_bytes, dout); > >> - } > >> - break; > >> - default: > >> - err = -1; > >> - break; > >> + case CQSPI_INDIRECT_READ: > >> + err = cadence_qspi_apb_indirect_read_setup(plat, op); > >> + if (!err) { > >> + err = cadence_qspi_apb_indirect_read_execute > >> + (plat, op->data.nbytes, op->data.buf.in); > >> } > >> - > >> - if (flags & SPI_XFER_END) { > >> - /* clear command buffer */ > >> - memset(cmd_buf, 0, sizeof(priv->cmd_buf)); > >> - priv->cmd_len = 0; > >> + break; > >> + case CQSPI_INDIRECT_WRITE: > >> + err = cadence_qspi_apb_indirect_write_setup(plat, op); > >> + if (!err) { > >> + err = cadence_qspi_apb_indirect_write_execute > >> + (plat, op->data.nbytes, op->data.buf.out); > >> } > >> + break; > >> + default: > >> + err = -1; > >> + break; > >> } > >> > >> return err; > >> @@ -332,10 +300,14 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > >> return 0; > >> } > >> > >> +static const struct spi_controller_mem_ops cadence_spi_mem_ops = { > >> + .exec_op = cadence_spi_mem_exec_op, > >> +}; > >> + > >> static const struct dm_spi_ops cadence_spi_ops = { > >> - .xfer = cadence_spi_xfer, > >> .set_speed = cadence_spi_set_speed, > >> .set_mode = cadence_spi_set_mode, > >> + .mem_ops = &cadence_spi_mem_ops, > >> /* > >> * cs_info is not needed, since we require all chip selects to be > >> * in the device tree explicitly > >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > >> index 20cceca239f8..e655b027d788 100644 > >> --- a/drivers/spi/cadence_qspi.h > >> +++ b/drivers/spi/cadence_qspi.h > >> @@ -54,17 +54,16 @@ void cadence_qspi_apb_controller_enable(void *reg_base_addr); > >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > >> > >> int cadence_qspi_apb_command_read(void *reg_base_addr, > >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8 *rxbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_command_write(void *reg_base_addr, > >> - unsigned int cmdlen, const u8 *cmdbuf, > >> - unsigned int txlen, const u8 *txbuf); > >> + const struct spi_mem_op *op); > >> > >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >> unsigned int rxlen, u8 *rxbuf); > >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf); > >> + const struct spi_mem_op *op); > >> int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >> unsigned int txlen, const u8 *txbuf); > >> > >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >> index 55a7501913a8..8dd0495dfcf4 100644 > >> --- a/drivers/spi/cadence_qspi_apb.c > >> +++ b/drivers/spi/cadence_qspi_apb.c > >> @@ -30,6 +30,7 @@ > >> #include <linux/errno.h> > >> #include <wait_bit.h> > >> #include <spi.h> > >> +#include <spi-mem.h> > >> #include <malloc.h> > >> #include "cadence_qspi.h" > >> > >> @@ -172,19 +173,6 @@ > >> (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > >> CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK) > >> > >> -static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, > >> - unsigned int addr_width) > >> -{ > >> - unsigned int addr; > >> - > >> - addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2]; > >> - > >> - if (addr_width == 4) > >> - addr = (addr << 8) | addr_buf[3]; > >> - > >> - return addr; > >> -} > >> - > >> void cadence_qspi_apb_controller_enable(void *reg_base) > >> { > >> unsigned int reg; > >> @@ -433,21 +421,20 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base, > >> } > >> > >> /* For command RDID, RDSR. */ > >> -int cadence_qspi_apb_command_read(void *reg_base, > >> - unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, > >> - u8 *rxbuf) > >> +int cadence_qspi_apb_command_read(void *reg_base, const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> unsigned int read_len; > >> int status; > >> + unsigned int rxlen = op->data.nbytes; > >> + void *rxbuf = op->data.buf.in; > >> > >> - if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) { > >> - printf("QSPI: Invalid input arguments cmdlen %d rxlen %d\n", > >> - cmdlen, rxlen); > >> + if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { > >> + printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); > >> return -EINVAL; > >> } > >> > >> - reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> + reg = op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> > >> reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB); > >> > >> @@ -475,34 +462,30 @@ int cadence_qspi_apb_command_read(void *reg_base, > >> } > >> > >> /* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */ > >> -int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > >> - const u8 *cmdbuf, unsigned int txlen, const u8 *txbuf) > >> +int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > >> { > >> unsigned int reg = 0; > >> - unsigned int addr_value; > >> unsigned int wr_data; > >> unsigned int wr_len; > >> + unsigned int txlen = op->data.nbytes; > >> + const void *txbuf = op->data.buf.out; > >> + u32 addr; > >> + > >> + /* Reorder address to SPI bus order if only transferring address */ > >> + if (!txlen) { > >> + addr = cpu_to_be32(op->addr.val); > >> + if (op->addr.nbytes == 3) > >> + addr >>= 8; > >> + txbuf = &addr; > >> + txlen = op->addr.nbytes; > >> + } > >> > >> - if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) { > >> - printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n", > >> - cmdlen, txlen); > >> + if (txlen > CQSPI_STIG_DATA_LEN_MAX) { > >> + printf("QSPI: Invalid input arguments txlen %u\n", txlen); > >> return -EINVAL; > >> } > >> > >> - reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> - > >> - if (cmdlen == 4 || cmdlen == 5) { > >> - /* Command with address */ > >> - reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); > >> - /* Number of bytes to write. */ > >> - reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) > >> - << CQSPI_REG_CMDCTRL_ADD_BYTES_LSB; > >> - /* Get address */ > >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], > >> - cmdlen >= 5 ? 4 : 3); > >> - > >> - writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS); > >> - } > >> + reg |= op->cmd.opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > >> > >> if (txlen) { > >> /* writing data = yes */ > >> @@ -530,61 +513,32 @@ int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen, > >> > >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > >> int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int rx_width, const u8 *cmdbuf) > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> unsigned int rd_reg; > >> - unsigned int addr_value; > >> unsigned int dummy_clk; > >> - unsigned int dummy_bytes; > >> - unsigned int addr_bytes; > >> - > >> - /* > >> - * Identify addr_byte. All NOR flash device drivers are using fast read > >> - * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte. > >> - * With that, the length is in value of 5 or 6. Only FRAM chip from > >> - * ramtron using normal read (which won't need dummy byte). > >> - * Unlikely NOR flash using normal read due to performance issue. > >> - */ > >> - if (cmdlen >= 5) > >> - /* to cater fast read where cmd + addr + dummy */ > >> - addr_bytes = cmdlen - 2; > >> - else > >> - /* for normal read (only ramtron as of now) */ > >> - addr_bytes = cmdlen - 1; > >> + unsigned int dummy_bytes = op->dummy.nbytes; > >> > >> /* Setup the indirect trigger address */ > >> writel(plat->trigger_address, > >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > >> > >> /* Configure the opcode */ > >> - rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; > >> + rd_reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; > >> > >> - if (rx_width & SPI_RX_QUAD) > >> + if (op->data.buswidth == 4) > >> /* Instruction and address at DQ0, data at DQ0-3. */ > >> rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > >> > >> - /* Get address */ > >> - addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > >> - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); > >> > >> - /* The remaining lenght is dummy bytes. */ > >> - dummy_bytes = cmdlen - addr_bytes - 1; > >> if (dummy_bytes) { > >> if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX) > >> dummy_bytes = CQSPI_DUMMY_BYTES_MAX; > >> > >> - rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); > >> -#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD) > >> - writel(0x0, plat->regbase + CQSPI_REG_MODE_BIT); > >> -#else > >> - writel(0xFF, plat->regbase + CQSPI_REG_MODE_BIT); > >> -#endif > >> - > >> /* Convert to clock cycles. */ > >> dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE; > >> - /* Need to minus the mode byte (8 clocks). */ > >> - dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE; > >> > >> if (dummy_clk) > >> rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) > >> @@ -596,7 +550,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> /* set device size */ > >> reg = readl(plat->regbase + CQSPI_REG_SIZE); > >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > >> - reg |= (addr_bytes - 1); > >> + reg |= (op->addr.nbytes - 1); > >> writel(reg, plat->regbase + CQSPI_REG_SIZE); > >> return 0; > >> } > >> @@ -687,35 +641,23 @@ failrd: > >> > >> /* Opcode + Address (3/4 bytes) */ > >> int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - unsigned int cmdlen, unsigned int tx_width, const u8 *cmdbuf) > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> - unsigned int addr_bytes = cmdlen > 4 ? 4 : 3; > >> > >> - if (cmdlen < 4 || cmdbuf == NULL) { > >> - printf("QSPI: Invalid input argument, len %d cmdbuf %p\n", > >> - cmdlen, cmdbuf); > >> - return -EINVAL; > >> - } > >> /* Setup the indirect trigger address */ > >> writel(plat->trigger_address, > >> plat->regbase + CQSPI_REG_INDIRECTTRIGGER); > >> > >> /* Configure the opcode */ > >> - reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; > >> - > >> - if (tx_width & SPI_TX_QUAD) > >> - reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; > >> - > >> + reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB; > >> writel(reg, plat->regbase + CQSPI_REG_WR_INSTR); > >> > >> - /* Setup write address. */ > >> - reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); > >> - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > >> + writel(op->addr.val, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); > >> > >> reg = readl(plat->regbase + CQSPI_REG_SIZE); > >> reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > >> - reg |= (addr_bytes - 1); > >> + reg |= (op->addr.nbytes - 1); > >> writel(reg, plat->regbase + CQSPI_REG_SIZE); > >> return 0; > >> } > >> -- > >> 2.23.0 > >> > > -- > Regards > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-17 11:20 ` Simon Goldschmidt 2019-10-17 12:31 ` Vignesh Raghavendra @ 2019-10-17 13:48 ` Tudor.Ambarus at microchip.com 2020-01-17 15:36 ` Simon Goldschmidt 1 sibling, 1 reply; 18+ messages in thread From: Tudor.Ambarus at microchip.com @ 2019-10-17 13:48 UTC (permalink / raw) To: u-boot Hi, Simon, Vignesh, On 10/17/2019 02:20 PM, Simon Goldschmidt wrote: > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> Current Cadence QSPI driver has few limitations. It assumes all read >> operations to be in Quad mode and thus does not support SFDP parsing. >> Also, adding support for new mode such as Octal mode would not be >> possible with current configuration. Therefore move the driver over to spi-mem >> framework. This has added advantage that driver can be used to support >> SPI NAND memories too. >> Hence, move driver over to new spi-mem APIs. >> >> Please note that this gets rid of mode bit setting done when >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to >> that config option. > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > produces invalid results (I haven't tested what's wrong there). > > It works as expected when not parsing SFDP or setting the bus-width to 1. > So the change itself probably works, but SFDP parsing is broken? This can happen if the quad enable method is not correctly set/called. Would you try the patch form below? I don't see much benefit in having those guards, especially that we have SPI_FLASH_SFDP_SUPPORT defined - it trims most of the SFDP logic. More, these #ifdef guards are not scalable and with the addition of flashes that support SFDP the #ifdefs will look uglier and uglier. Cheers, ta diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e5b9899c64b2..3002f97a7342 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -188,7 +188,6 @@ static int read_fsr(struct spi_nor *nor) * location. Return the configuration register value. * Returns negative if error occurred. */ -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) static int read_cr(struct spi_nor *nor) { int ret; @@ -202,7 +201,6 @@ static int read_cr(struct spi_nor *nor) return val; } -#endif /* * Write status register 1 byte @@ -591,7 +589,6 @@ erase_err: return ret; } -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) { @@ -877,7 +874,6 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) return stm_is_locked_sr(nor, ofs, len, status); } -#endif /* CONFIG_SPI_FLASH_STMICRO */ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor) { @@ -1116,7 +1112,6 @@ write_err: return ret; } -#ifdef CONFIG_SPI_FLASH_MACRONIX /** * macronix_quad_enable() - set QE bit in Status Register. * @nor: pointer to a 'struct spi_nor' @@ -1153,9 +1148,7 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } -#endif -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) /* * Write status Register and configuration register with 2 bytes * The first byte will be written to the status register, while the @@ -1269,7 +1262,6 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor) } #endif /* CONFIG_SPI_FLASH_SFDP_SUPPORT */ -#endif /* CONFIG_SPI_FLASH_SPANSION */ struct spi_nor_read_command { u8 num_mode_clocks; @@ -1787,22 +1779,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, case BFPT_DWORD15_QER_NONE: params->quad_enable = NULL; break; -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: case BFPT_DWORD15_QER_SR2_BIT1_NO_RD: params->quad_enable = spansion_no_read_cr_quad_enable; break; -#endif -#ifdef CONFIG_SPI_FLASH_MACRONIX case BFPT_DWORD15_QER_SR1_BIT6: params->quad_enable = macronix_quad_enable; break; -#endif -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) case BFPT_DWORD15_QER_SR2_BIT1: params->quad_enable = spansion_read_cr_quad_enable; break; -#endif default: return -EINVAL; } @@ -2013,20 +1999,16 @@ static int spi_nor_init_params(struct spi_nor *nor, if (params->hwcaps.mask & (SNOR_HWCAPS_READ_QUAD | SNOR_HWCAPS_PP_QUAD)) { switch (JEDEC_MFR(info)) { -#ifdef CONFIG_SPI_FLASH_MACRONIX case SNOR_MFR_MACRONIX: params->quad_enable = macronix_quad_enable; break; -#endif case SNOR_MFR_ST: case SNOR_MFR_MICRON: break; default: -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) /* Kept only for backward compatibility purpose. */ params->quad_enable = spansion_read_cr_quad_enable; -#endif break; } } @@ -2337,7 +2319,6 @@ int spi_nor_scan(struct spi_nor *nor) mtd->_erase = spi_nor_erase; mtd->_read = spi_nor_read; -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* NOR protection support for STmicro/Micron chips and similar */ if (JEDEC_MFR(info) == SNOR_MFR_ST || JEDEC_MFR(info) == SNOR_MFR_MICRON || @@ -2347,7 +2328,6 @@ int spi_nor_scan(struct spi_nor *nor) nor->flash_unlock = stm_unlock; nor->flash_is_locked = stm_is_locked; } -#endif #ifdef CONFIG_SPI_FLASH_SST /* sst nor chips use AAI word program */ ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework 2019-10-17 13:48 ` [U-Boot] " Tudor.Ambarus at microchip.com @ 2020-01-17 15:36 ` Simon Goldschmidt 0 siblings, 0 replies; 18+ messages in thread From: Simon Goldschmidt @ 2020-01-17 15:36 UTC (permalink / raw) To: u-boot On Thu, Oct 17, 2019 at 3:48 PM <Tudor.Ambarus@microchip.com> wrote: > > Hi, Simon, Vignesh, > > On 10/17/2019 02:20 PM, Simon Goldschmidt wrote: > > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> Current Cadence QSPI driver has few limitations. It assumes all read > >> operations to be in Quad mode and thus does not support SFDP parsing. > >> Also, adding support for new mode such as Octal mode would not be > >> possible with current configuration. Therefore move the driver over to spi-mem > >> framework. This has added advantage that driver can be used to support > >> SPI NAND memories too. > >> Hence, move driver over to new spi-mem APIs. > >> > >> Please note that this gets rid of mode bit setting done when > >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to > >> that config option. > > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > > produces invalid results (I haven't tested what's wrong there). > > > > It works as expected when not parsing SFDP or setting the bus-width to 1. > > So the change itself probably works, but SFDP parsing is broken? > > This can happen if the quad enable method is not correctly set/called. Would you > try the patch form below? I tried, but of course that didn't help. And I fail to see why it would, unless you do something wrong in the flash device manufacturer selection. Doing the patch like below would only make sense if enabling SFDP parsing would at the same time completely remove the manufacturer selection (e.g. manufacturer selection in Kconfig would depend on SFDP not being set). Regards, Simon > > I don't see much benefit in having those guards, especially that we have > SPI_FLASH_SFDP_SUPPORT defined - it trims most of the SFDP logic. > > More, these #ifdef guards are not scalable and with the addition of flashes that > support SFDP the #ifdefs will look uglier and uglier. > > Cheers, > ta > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index e5b9899c64b2..3002f97a7342 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -188,7 +188,6 @@ static int read_fsr(struct spi_nor *nor) > * location. Return the configuration register value. > * Returns negative if error occurred. > */ > -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) > static int read_cr(struct spi_nor *nor) > { > int ret; > @@ -202,7 +201,6 @@ static int read_cr(struct spi_nor *nor) > > return val; > } > -#endif > > /* > * Write status register 1 byte > @@ -591,7 +589,6 @@ erase_err: > return ret; > } > > -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) > /* Write status register and ensure bits in mask match written values */ > static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) > { > @@ -877,7 +874,6 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, > uint64_t len) > > return stm_is_locked_sr(nor, ofs, len, status); > } > -#endif /* CONFIG_SPI_FLASH_STMICRO */ > > static const struct flash_info *spi_nor_read_id(struct spi_nor *nor) > { > @@ -1116,7 +1112,6 @@ write_err: > return ret; > } > > -#ifdef CONFIG_SPI_FLASH_MACRONIX > /** > * macronix_quad_enable() - set QE bit in Status Register. > * @nor: pointer to a 'struct spi_nor' > @@ -1153,9 +1148,7 @@ static int macronix_quad_enable(struct spi_nor *nor) > > return 0; > } > -#endif > > -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) > /* > * Write status Register and configuration register with 2 bytes > * The first byte will be written to the status register, while the > @@ -1269,7 +1262,6 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor > *nor) > } > > #endif /* CONFIG_SPI_FLASH_SFDP_SUPPORT */ > -#endif /* CONFIG_SPI_FLASH_SPANSION */ > > struct spi_nor_read_command { > u8 num_mode_clocks; > @@ -1787,22 +1779,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > case BFPT_DWORD15_QER_NONE: > params->quad_enable = NULL; > break; > -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) > case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: > case BFPT_DWORD15_QER_SR2_BIT1_NO_RD: > params->quad_enable = spansion_no_read_cr_quad_enable; > break; > -#endif > -#ifdef CONFIG_SPI_FLASH_MACRONIX > case BFPT_DWORD15_QER_SR1_BIT6: > params->quad_enable = macronix_quad_enable; > break; > -#endif > -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) > case BFPT_DWORD15_QER_SR2_BIT1: > params->quad_enable = spansion_read_cr_quad_enable; > break; > -#endif > default: > return -EINVAL; > } > @@ -2013,20 +1999,16 @@ static int spi_nor_init_params(struct spi_nor *nor, > if (params->hwcaps.mask & (SNOR_HWCAPS_READ_QUAD | > SNOR_HWCAPS_PP_QUAD)) { > switch (JEDEC_MFR(info)) { > -#ifdef CONFIG_SPI_FLASH_MACRONIX > case SNOR_MFR_MACRONIX: > params->quad_enable = macronix_quad_enable; > break; > -#endif > case SNOR_MFR_ST: > case SNOR_MFR_MICRON: > break; > > default: > -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) > /* Kept only for backward compatibility purpose. */ > params->quad_enable = spansion_read_cr_quad_enable; > -#endif > break; > } > } > @@ -2337,7 +2319,6 @@ int spi_nor_scan(struct spi_nor *nor) > mtd->_erase = spi_nor_erase; > mtd->_read = spi_nor_read; > > -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) > /* NOR protection support for STmicro/Micron chips and similar */ > if (JEDEC_MFR(info) == SNOR_MFR_ST || > JEDEC_MFR(info) == SNOR_MFR_MICRON || > @@ -2347,7 +2328,6 @@ int spi_nor_scan(struct spi_nor *nor) > nor->flash_unlock = stm_unlock; > nor->flash_is_locked = stm_is_locked; > } > -#endif > > #ifdef CONFIG_SPI_FLASH_SST > /* sst nor chips use AAI word program */ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-14 13:27 [U-Boot] [PATCH 0/2] spi: cadence-qspi: Move to spi-mem APIs Vignesh Raghavendra 2019-10-14 13:27 ` [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework Vignesh Raghavendra @ 2019-10-14 13:27 ` Vignesh Raghavendra 2019-10-17 11:39 ` Simon Goldschmidt 1 sibling, 1 reply; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-14 13:27 UTC (permalink / raw) To: u-boot Add support for Direct Access Controller mode of Cadence QSPI. This allows MMIO access to SPI NOR flash providing better read performance. Signed-off-by: Vignesh R <vigneshr@ti.com> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> --- drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- drivers/spi/cadence_qspi.h | 19 ++++++----- drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 33 deletions(-) diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 673a2e9a6c4c..6c98cbf39ae4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -12,12 +12,13 @@ #include <spi.h> #include <spi-mem.h> #include <linux/errno.h> +#include <linux/sizes.h> #include "cadence_qspi.h" #define CQSPI_STIG_READ 0 #define CQSPI_STIG_WRITE 1 -#define CQSPI_INDIRECT_READ 2 -#define CQSPI_INDIRECT_WRITE 3 +#define CQSPI_READ 2 +#define CQSPI_WRITE 3 static int cadence_spi_write_speed(struct udevice *bus, uint hz) { @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) static int cadence_spi_set_mode(struct udevice *bus, uint mode) { + struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus); /* Disable QSPI */ @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) /* Set SPI mode */ cadence_qspi_apb_set_clk_mode(priv->regbase, mode); + /* Enable Direct Access Controller */ + if (plat->use_dac_mode) + cadence_qspi_apb_dac_mode_enable(priv->regbase); + /* Enable QSPI */ cadence_qspi_apb_controller_enable(priv->regbase); @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, if (!op->addr.nbytes) mode = CQSPI_STIG_READ; else - mode = CQSPI_INDIRECT_READ; + mode = CQSPI_READ; } else { if (!op->addr.nbytes || !op->data.buf.out) mode = CQSPI_STIG_WRITE; else - mode = CQSPI_INDIRECT_WRITE; + mode = CQSPI_WRITE; } switch (mode) { @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, case CQSPI_STIG_WRITE: err = cadence_qspi_apb_command_write(base, op); break; - case CQSPI_INDIRECT_READ: - err = cadence_qspi_apb_indirect_read_setup(plat, op); - if (!err) { - err = cadence_qspi_apb_indirect_read_execute - (plat, op->data.nbytes, op->data.buf.in); - } + case CQSPI_READ: + err = cadence_qspi_apb_read_setup(plat, op); + if (!err) + err = cadence_qspi_apb_read_execute(plat, op); break; - case CQSPI_INDIRECT_WRITE: - err = cadence_qspi_apb_indirect_write_setup(plat, op); - if (!err) { - err = cadence_qspi_apb_indirect_write_execute - (plat, op->data.nbytes, op->data.buf.out); - } + case CQSPI_WRITE: + err = cadence_qspi_apb_write_setup(plat, op); + if (!err) + err = cadence_qspi_apb_write_execute(plat, op); break; default: err = -1; @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) ofnode subnode; plat->regbase = (void *)devfdt_get_addr_index(bus, 0); - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, + &plat->ahbsize); plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); plat->trigger_address = dev_read_u32_default(bus, "cdns,trigger-address", 0); + /* Use DAC mode only when MMIO window is at least 8M wide */ + if (plat->ahbsize >= SZ_8M) + plat->use_dac_mode = true; /* All other paramters are embedded in the child node */ subnode = dev_read_first_subnode(bus); diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index e655b027d788..619f0bed8efd 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -23,6 +23,8 @@ struct cadence_spi_platdata { u32 fifo_depth; u32 fifo_width; u32 trigger_address; + fdt_addr_t ahbsize; + bool use_dac_mode; /* Flash parameters */ u32 page_size; @@ -52,20 +54,21 @@ struct cadence_spi_priv { void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); void cadence_qspi_apb_controller_enable(void *reg_base_addr); void cadence_qspi_apb_controller_disable(void *reg_base_addr); +void cadence_qspi_apb_dac_mode_enable(void *reg_base); int cadence_qspi_apb_command_read(void *reg_base_addr, const struct spi_mem_op *op); int cadence_qspi_apb_command_write(void *reg_base_addr, const struct spi_mem_op *op); -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, - const struct spi_mem_op *op); -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, - unsigned int rxlen, u8 *rxbuf); -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, - const struct spi_mem_op *op); -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, - unsigned int txlen, const u8 *txbuf); +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op); +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op); +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op); +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op); void cadence_qspi_apb_chipselect(void *reg_base, unsigned int chip_select, unsigned int decoder_enable); diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 8dd0495dfcf4..fd491f2c8104 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) writel(reg, reg_base + CQSPI_REG_CONFIG); } +void cadence_qspi_apb_dac_mode_enable(void *reg_base) +{ + unsigned int reg; + + reg = readl(reg_base + CQSPI_REG_CONFIG); + reg |= CQSPI_REG_CONFIG_DIRECT; + writel(reg, reg_base + CQSPI_REG_CONFIG); +} + /* Return 1 if idle, otherwise return 0 (busy). */ static unsigned int cadence_qspi_wait_idle(void *reg_base) { @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) } /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, - const struct spi_mem_op *op) +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op) { unsigned int reg; unsigned int rd_reg; @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) return -ETIMEDOUT; } -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, - unsigned int n_rx, u8 *rxbuf) +static int +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, + unsigned int n_rx, u8 *rxbuf) { unsigned int remaining = n_rx; unsigned int bytes_to_read = 0; @@ -639,9 +649,26 @@ failrd: return ret; } +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op) +{ + u32 from = op->addr.val; + void *buf = op->data.buf.in; + size_t len = op->data.nbytes; + + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { + memcpy_fromio(buf, plat->ahbbase + from, len); + if (!cadence_qspi_wait_idle(plat->regbase)) + return -EIO; + return 0; + } + + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); +} + /* Opcode + Address (3/4 bytes) */ -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, - const struct spi_mem_op *op) +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op) { unsigned int reg; @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, return 0; } -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, - unsigned int n_tx, const u8 *txbuf) +static int +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, + unsigned int n_tx, const u8 *txbuf) { unsigned int page_size = plat->page_size; unsigned int remaining = n_tx; @@ -735,6 +763,23 @@ failwr: return ret; } +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, + const struct spi_mem_op *op) +{ + u32 to = op->addr.val; + const void *buf = op->data.buf.out; + size_t len = op->data.nbytes; + + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { + memcpy_toio(plat->ahbbase + to, buf, len); + if (!cadence_qspi_wait_idle(plat->regbase)) + return -EIO; + return 0; + } + + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); +} + void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) { unsigned int reg; -- 2.23.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-14 13:27 ` [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support Vignesh Raghavendra @ 2019-10-17 11:39 ` Simon Goldschmidt 2019-10-17 12:44 ` Vignesh Raghavendra 0 siblings, 1 reply; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-17 11:39 UTC (permalink / raw) To: u-boot On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Add support for Direct Access Controller mode of Cadence QSPI. This > allows MMIO access to SPI NOR flash providing better read performance. > > Signed-off-by: Vignesh R <vigneshr@ti.com> > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at 50MHz) and it seems to work fine. However, I had the impression it was a bit slower, not faster, although I haven't measured, and running at 50MHz with 4 data lines, reading the whole flash takes about 1.5 seconds only, so without actually measureing it, it's hard to tell if this performance is really worth the 440 bytes of extra code it adds? Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes for me :-) Regards, Simon > --- > drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- > drivers/spi/cadence_qspi.h | 19 ++++++----- > drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- > 3 files changed, 87 insertions(+), 33 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 673a2e9a6c4c..6c98cbf39ae4 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -12,12 +12,13 @@ > #include <spi.h> > #include <spi-mem.h> > #include <linux/errno.h> > +#include <linux/sizes.h> > #include "cadence_qspi.h" > > #define CQSPI_STIG_READ 0 > #define CQSPI_STIG_WRITE 1 > -#define CQSPI_INDIRECT_READ 2 > -#define CQSPI_INDIRECT_WRITE 3 > +#define CQSPI_READ 2 > +#define CQSPI_WRITE 3 > > static int cadence_spi_write_speed(struct udevice *bus, uint hz) > { > @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) > > static int cadence_spi_set_mode(struct udevice *bus, uint mode) > { > + struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > > /* Disable QSPI */ > @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > /* Set SPI mode */ > cadence_qspi_apb_set_clk_mode(priv->regbase, mode); > > + /* Enable Direct Access Controller */ > + if (plat->use_dac_mode) > + cadence_qspi_apb_dac_mode_enable(priv->regbase); > + > /* Enable QSPI */ > cadence_qspi_apb_controller_enable(priv->regbase); > > @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > if (!op->addr.nbytes) > mode = CQSPI_STIG_READ; > else > - mode = CQSPI_INDIRECT_READ; > + mode = CQSPI_READ; > } else { > if (!op->addr.nbytes || !op->data.buf.out) > mode = CQSPI_STIG_WRITE; > else > - mode = CQSPI_INDIRECT_WRITE; > + mode = CQSPI_WRITE; > } > > switch (mode) { > @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > case CQSPI_STIG_WRITE: > err = cadence_qspi_apb_command_write(base, op); > break; > - case CQSPI_INDIRECT_READ: > - err = cadence_qspi_apb_indirect_read_setup(plat, op); > - if (!err) { > - err = cadence_qspi_apb_indirect_read_execute > - (plat, op->data.nbytes, op->data.buf.in); > - } > + case CQSPI_READ: > + err = cadence_qspi_apb_read_setup(plat, op); > + if (!err) > + err = cadence_qspi_apb_read_execute(plat, op); > break; > - case CQSPI_INDIRECT_WRITE: > - err = cadence_qspi_apb_indirect_write_setup(plat, op); > - if (!err) { > - err = cadence_qspi_apb_indirect_write_execute > - (plat, op->data.nbytes, op->data.buf.out); > - } > + case CQSPI_WRITE: > + err = cadence_qspi_apb_write_setup(plat, op); > + if (!err) > + err = cadence_qspi_apb_write_execute(plat, op); > break; > default: > err = -1; > @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > ofnode subnode; > > plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); > + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > + &plat->ahbsize); > plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); > plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); > plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); > plat->trigger_address = dev_read_u32_default(bus, > "cdns,trigger-address", > 0); > + /* Use DAC mode only when MMIO window is at least 8M wide */ > + if (plat->ahbsize >= SZ_8M) > + plat->use_dac_mode = true; > > /* All other paramters are embedded in the child node */ > subnode = dev_read_first_subnode(bus); > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > index e655b027d788..619f0bed8efd 100644 > --- a/drivers/spi/cadence_qspi.h > +++ b/drivers/spi/cadence_qspi.h > @@ -23,6 +23,8 @@ struct cadence_spi_platdata { > u32 fifo_depth; > u32 fifo_width; > u32 trigger_address; > + fdt_addr_t ahbsize; > + bool use_dac_mode; > > /* Flash parameters */ > u32 page_size; > @@ -52,20 +54,21 @@ struct cadence_spi_priv { > void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); > void cadence_qspi_apb_controller_enable(void *reg_base_addr); > void cadence_qspi_apb_controller_disable(void *reg_base_addr); > +void cadence_qspi_apb_dac_mode_enable(void *reg_base); > > int cadence_qspi_apb_command_read(void *reg_base_addr, > const struct spi_mem_op *op); > int cadence_qspi_apb_command_write(void *reg_base_addr, > const struct spi_mem_op *op); > > -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > - const struct spi_mem_op *op); > -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > - unsigned int rxlen, u8 *rxbuf); > -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > - const struct spi_mem_op *op); > -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > - unsigned int txlen, const u8 *txbuf); > +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op); > +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op); > +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op); > +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op); > > void cadence_qspi_apb_chipselect(void *reg_base, > unsigned int chip_select, unsigned int decoder_enable); > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > index 8dd0495dfcf4..fd491f2c8104 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) > writel(reg, reg_base + CQSPI_REG_CONFIG); > } > > +void cadence_qspi_apb_dac_mode_enable(void *reg_base) > +{ > + unsigned int reg; > + > + reg = readl(reg_base + CQSPI_REG_CONFIG); > + reg |= CQSPI_REG_CONFIG_DIRECT; > + writel(reg, reg_base + CQSPI_REG_CONFIG); > +} > + > /* Return 1 if idle, otherwise return 0 (busy). */ > static unsigned int cadence_qspi_wait_idle(void *reg_base) > { > @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > } > > /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > - const struct spi_mem_op *op) > +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op) > { > unsigned int reg; > unsigned int rd_reg; > @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) > return -ETIMEDOUT; > } > > -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > - unsigned int n_rx, u8 *rxbuf) > +static int > +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > + unsigned int n_rx, u8 *rxbuf) > { > unsigned int remaining = n_rx; > unsigned int bytes_to_read = 0; > @@ -639,9 +649,26 @@ failrd: > return ret; > } > > +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op) > +{ > + u32 from = op->addr.val; > + void *buf = op->data.buf.in; > + size_t len = op->data.nbytes; > + > + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { > + memcpy_fromio(buf, plat->ahbbase + from, len); > + if (!cadence_qspi_wait_idle(plat->regbase)) > + return -EIO; > + return 0; > + } > + > + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); > +} > + > /* Opcode + Address (3/4 bytes) */ > -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > - const struct spi_mem_op *op) > +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op) > { > unsigned int reg; > > @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > return 0; > } > > -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > - unsigned int n_tx, const u8 *txbuf) > +static int > +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > + unsigned int n_tx, const u8 *txbuf) > { > unsigned int page_size = plat->page_size; > unsigned int remaining = n_tx; > @@ -735,6 +763,23 @@ failwr: > return ret; > } > > +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > + const struct spi_mem_op *op) > +{ > + u32 to = op->addr.val; > + const void *buf = op->data.buf.out; > + size_t len = op->data.nbytes; > + > + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { > + memcpy_toio(plat->ahbbase + to, buf, len); > + if (!cadence_qspi_wait_idle(plat->regbase)) > + return -EIO; > + return 0; > + } > + > + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); > +} > + > void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) > { > unsigned int reg; > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-17 11:39 ` Simon Goldschmidt @ 2019-10-17 12:44 ` Vignesh Raghavendra 2019-10-17 12:55 ` Simon Goldschmidt 0 siblings, 1 reply; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-17 12:44 UTC (permalink / raw) To: u-boot Hi, On 17/10/19 5:09 PM, Simon Goldschmidt wrote: > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> Add support for Direct Access Controller mode of Cadence QSPI. This >> allows MMIO access to SPI NOR flash providing better read performance. >> >> Signed-off-by: Vignesh R <vigneshr@ti.com> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > > I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at > 50MHz) and it seems to work fine. > > However, I had the impression it was a bit slower, not faster, although I > haven't measured, and running at 50MHz with 4 data lines, reading the whole > flash takes about 1.5 seconds only, so without actually measureing it, it's > hard to tell if this performance is really worth the 440 bytes of extra code > it adds? > I should have noted in the commit msg that direct mode is used only when AHB window is at least 8MB. Working with smaller window sizes would mean code would fall back to indirect mode most of the time. I see that socfgpa don't seem to have large AHB windows and therefore would not use direct access mode. On TI platforms its possible to use DMA to read data from memory mapped region using mem to mem DMA channels which improves throughput by 5 times. > Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes > do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes > for me :-) > Hmm, I read this as patch 1 brings down memory consumption by 320 bytes and patch 2 adds 440 bytes (delta being +120 bytes for entire series). I can see if size can be optimized, but would like to avoid #ifdef'ery within the driver if possible. Regards Vignesh > Regards, > Simon > > >> --- >> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- >> drivers/spi/cadence_qspi.h | 19 ++++++----- >> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- >> 3 files changed, 87 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c >> index 673a2e9a6c4c..6c98cbf39ae4 100644 >> --- a/drivers/spi/cadence_qspi.c >> +++ b/drivers/spi/cadence_qspi.c >> @@ -12,12 +12,13 @@ >> #include <spi.h> >> #include <spi-mem.h> >> #include <linux/errno.h> >> +#include <linux/sizes.h> >> #include "cadence_qspi.h" >> >> #define CQSPI_STIG_READ 0 >> #define CQSPI_STIG_WRITE 1 >> -#define CQSPI_INDIRECT_READ 2 >> -#define CQSPI_INDIRECT_WRITE 3 >> +#define CQSPI_READ 2 >> +#define CQSPI_WRITE 3 >> >> static int cadence_spi_write_speed(struct udevice *bus, uint hz) >> { >> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) >> >> static int cadence_spi_set_mode(struct udevice *bus, uint mode) >> { >> + struct cadence_spi_platdata *plat = bus->platdata; >> struct cadence_spi_priv *priv = dev_get_priv(bus); >> >> /* Disable QSPI */ >> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) >> /* Set SPI mode */ >> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); >> >> + /* Enable Direct Access Controller */ >> + if (plat->use_dac_mode) >> + cadence_qspi_apb_dac_mode_enable(priv->regbase); >> + >> /* Enable QSPI */ >> cadence_qspi_apb_controller_enable(priv->regbase); >> >> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >> if (!op->addr.nbytes) >> mode = CQSPI_STIG_READ; >> else >> - mode = CQSPI_INDIRECT_READ; >> + mode = CQSPI_READ; >> } else { >> if (!op->addr.nbytes || !op->data.buf.out) >> mode = CQSPI_STIG_WRITE; >> else >> - mode = CQSPI_INDIRECT_WRITE; >> + mode = CQSPI_WRITE; >> } >> >> switch (mode) { >> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >> case CQSPI_STIG_WRITE: >> err = cadence_qspi_apb_command_write(base, op); >> break; >> - case CQSPI_INDIRECT_READ: >> - err = cadence_qspi_apb_indirect_read_setup(plat, op); >> - if (!err) { >> - err = cadence_qspi_apb_indirect_read_execute >> - (plat, op->data.nbytes, op->data.buf.in); >> - } >> + case CQSPI_READ: >> + err = cadence_qspi_apb_read_setup(plat, op); >> + if (!err) >> + err = cadence_qspi_apb_read_execute(plat, op); >> break; >> - case CQSPI_INDIRECT_WRITE: >> - err = cadence_qspi_apb_indirect_write_setup(plat, op); >> - if (!err) { >> - err = cadence_qspi_apb_indirect_write_execute >> - (plat, op->data.nbytes, op->data.buf.out); >> - } >> + case CQSPI_WRITE: >> + err = cadence_qspi_apb_write_setup(plat, op); >> + if (!err) >> + err = cadence_qspi_apb_write_execute(plat, op); >> break; >> default: >> err = -1; >> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) >> ofnode subnode; >> >> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); >> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); >> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, >> + &plat->ahbsize); >> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); >> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); >> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); >> plat->trigger_address = dev_read_u32_default(bus, >> "cdns,trigger-address", >> 0); >> + /* Use DAC mode only when MMIO window is at least 8M wide */ >> + if (plat->ahbsize >= SZ_8M) >> + plat->use_dac_mode = true; >> >> /* All other paramters are embedded in the child node */ >> subnode = dev_read_first_subnode(bus); >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >> index e655b027d788..619f0bed8efd 100644 >> --- a/drivers/spi/cadence_qspi.h >> +++ b/drivers/spi/cadence_qspi.h >> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { >> u32 fifo_depth; >> u32 fifo_width; >> u32 trigger_address; >> + fdt_addr_t ahbsize; >> + bool use_dac_mode; >> >> /* Flash parameters */ >> u32 page_size; >> @@ -52,20 +54,21 @@ struct cadence_spi_priv { >> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); >> void cadence_qspi_apb_controller_enable(void *reg_base_addr); >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); >> >> int cadence_qspi_apb_command_read(void *reg_base_addr, >> const struct spi_mem_op *op); >> int cadence_qspi_apb_command_write(void *reg_base_addr, >> const struct spi_mem_op *op); >> >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> - const struct spi_mem_op *op); >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >> - unsigned int rxlen, u8 *rxbuf); >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >> - const struct spi_mem_op *op); >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >> - unsigned int txlen, const u8 *txbuf); >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op); >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op); >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op); >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op); >> >> void cadence_qspi_apb_chipselect(void *reg_base, >> unsigned int chip_select, unsigned int decoder_enable); >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >> index 8dd0495dfcf4..fd491f2c8104 100644 >> --- a/drivers/spi/cadence_qspi_apb.c >> +++ b/drivers/spi/cadence_qspi_apb.c >> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) >> writel(reg, reg_base + CQSPI_REG_CONFIG); >> } >> >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) >> +{ >> + unsigned int reg; >> + >> + reg = readl(reg_base + CQSPI_REG_CONFIG); >> + reg |= CQSPI_REG_CONFIG_DIRECT; >> + writel(reg, reg_base + CQSPI_REG_CONFIG); >> +} >> + >> /* Return 1 if idle, otherwise return 0 (busy). */ >> static unsigned int cadence_qspi_wait_idle(void *reg_base) >> { >> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) >> } >> >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> - const struct spi_mem_op *op) >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op) >> { >> unsigned int reg; >> unsigned int rd_reg; >> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) >> return -ETIMEDOUT; >> } >> >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >> - unsigned int n_rx, u8 *rxbuf) >> +static int >> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >> + unsigned int n_rx, u8 *rxbuf) >> { >> unsigned int remaining = n_rx; >> unsigned int bytes_to_read = 0; >> @@ -639,9 +649,26 @@ failrd: >> return ret; >> } >> >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op) >> +{ >> + u32 from = op->addr.val; >> + void *buf = op->data.buf.in; >> + size_t len = op->data.nbytes; >> + >> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { >> + memcpy_fromio(buf, plat->ahbbase + from, len); >> + if (!cadence_qspi_wait_idle(plat->regbase)) >> + return -EIO; >> + return 0; >> + } >> + >> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); >> +} >> + >> /* Opcode + Address (3/4 bytes) */ >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >> - const struct spi_mem_op *op) >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op) >> { >> unsigned int reg; >> >> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >> return 0; >> } >> >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >> - unsigned int n_tx, const u8 *txbuf) >> +static int >> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >> + unsigned int n_tx, const u8 *txbuf) >> { >> unsigned int page_size = plat->page_size; >> unsigned int remaining = n_tx; >> @@ -735,6 +763,23 @@ failwr: >> return ret; >> } >> >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >> + const struct spi_mem_op *op) >> +{ >> + u32 to = op->addr.val; >> + const void *buf = op->data.buf.out; >> + size_t len = op->data.nbytes; >> + >> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { >> + memcpy_toio(plat->ahbbase + to, buf, len); >> + if (!cadence_qspi_wait_idle(plat->regbase)) >> + return -EIO; >> + return 0; >> + } >> + >> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); >> +} >> + >> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) >> { >> unsigned int reg; >> -- >> 2.23.0 >> -- Regards Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-17 12:44 ` Vignesh Raghavendra @ 2019-10-17 12:55 ` Simon Goldschmidt 2019-10-18 9:04 ` Simon Goldschmidt 0 siblings, 1 reply; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-17 12:55 UTC (permalink / raw) To: u-boot On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi, > > On 17/10/19 5:09 PM, Simon Goldschmidt wrote: > > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> Add support for Direct Access Controller mode of Cadence QSPI. This > >> allows MMIO access to SPI NOR flash providing better read performance. > >> > >> Signed-off-by: Vignesh R <vigneshr@ti.com> > >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > > > > I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at > > 50MHz) and it seems to work fine. > > > > However, I had the impression it was a bit slower, not faster, although I > > haven't measured, and running at 50MHz with 4 data lines, reading the whole > > flash takes about 1.5 seconds only, so without actually measureing it, it's > > hard to tell if this performance is really worth the 440 bytes of extra code > > it adds? > > > > I should have noted in the commit msg that direct mode is used only when > AHB window is at least 8MB. Working with smaller window sizes would mean > code would fall back to indirect mode most of the time. > I see that socfgpa don't seem to have large AHB windows and therefore > would not use direct access mode. Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't changed. > > On TI platforms its possible to use DMA to read data from memory mapped > region using mem to mem DMA channels which improves throughput by 5 times. Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 seconds. The maximum speed of that setup would be 25 MByte/s without any overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement by 5 on my platform! > > > Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes > > do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes > > for me :-) > > > > Hmm, I read this as patch 1 brings down memory consumption by 320 bytes > and patch 2 adds 440 bytes (delta being +120 bytes for entire series). > I can see if size can be optimized, but would like to avoid #ifdef'ery > within the driver if possible. Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled at the same time in socfpga_socrates (moving more code to DM). And even if it sounds like not much, 440 bytes *are* much for me at this point. I still would prefer having a Kconfig option for this that can be disabled for socfpga. Regards, Simon > > Regards > Vignesh > > > Regards, > > Simon > > > > > >> --- > >> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- > >> drivers/spi/cadence_qspi.h | 19 ++++++----- > >> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- > >> 3 files changed, 87 insertions(+), 33 deletions(-) > >> > >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > >> index 673a2e9a6c4c..6c98cbf39ae4 100644 > >> --- a/drivers/spi/cadence_qspi.c > >> +++ b/drivers/spi/cadence_qspi.c > >> @@ -12,12 +12,13 @@ > >> #include <spi.h> > >> #include <spi-mem.h> > >> #include <linux/errno.h> > >> +#include <linux/sizes.h> > >> #include "cadence_qspi.h" > >> > >> #define CQSPI_STIG_READ 0 > >> #define CQSPI_STIG_WRITE 1 > >> -#define CQSPI_INDIRECT_READ 2 > >> -#define CQSPI_INDIRECT_WRITE 3 > >> +#define CQSPI_READ 2 > >> +#define CQSPI_WRITE 3 > >> > >> static int cadence_spi_write_speed(struct udevice *bus, uint hz) > >> { > >> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) > >> > >> static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >> { > >> + struct cadence_spi_platdata *plat = bus->platdata; > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > >> > >> /* Disable QSPI */ > >> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >> /* Set SPI mode */ > >> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); > >> > >> + /* Enable Direct Access Controller */ > >> + if (plat->use_dac_mode) > >> + cadence_qspi_apb_dac_mode_enable(priv->regbase); > >> + > >> /* Enable QSPI */ > >> cadence_qspi_apb_controller_enable(priv->regbase); > >> > >> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >> if (!op->addr.nbytes) > >> mode = CQSPI_STIG_READ; > >> else > >> - mode = CQSPI_INDIRECT_READ; > >> + mode = CQSPI_READ; > >> } else { > >> if (!op->addr.nbytes || !op->data.buf.out) > >> mode = CQSPI_STIG_WRITE; > >> else > >> - mode = CQSPI_INDIRECT_WRITE; > >> + mode = CQSPI_WRITE; > >> } > >> > >> switch (mode) { > >> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >> case CQSPI_STIG_WRITE: > >> err = cadence_qspi_apb_command_write(base, op); > >> break; > >> - case CQSPI_INDIRECT_READ: > >> - err = cadence_qspi_apb_indirect_read_setup(plat, op); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_read_execute > >> - (plat, op->data.nbytes, op->data.buf.in); > >> - } > >> + case CQSPI_READ: > >> + err = cadence_qspi_apb_read_setup(plat, op); > >> + if (!err) > >> + err = cadence_qspi_apb_read_execute(plat, op); > >> break; > >> - case CQSPI_INDIRECT_WRITE: > >> - err = cadence_qspi_apb_indirect_write_setup(plat, op); > >> - if (!err) { > >> - err = cadence_qspi_apb_indirect_write_execute > >> - (plat, op->data.nbytes, op->data.buf.out); > >> - } > >> + case CQSPI_WRITE: > >> + err = cadence_qspi_apb_write_setup(plat, op); > >> + if (!err) > >> + err = cadence_qspi_apb_write_execute(plat, op); > >> break; > >> default: > >> err = -1; > >> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > >> ofnode subnode; > >> > >> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > >> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); > >> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > >> + &plat->ahbsize); > >> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); > >> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); > >> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); > >> plat->trigger_address = dev_read_u32_default(bus, > >> "cdns,trigger-address", > >> 0); > >> + /* Use DAC mode only when MMIO window is at least 8M wide */ > >> + if (plat->ahbsize >= SZ_8M) > >> + plat->use_dac_mode = true; > >> > >> /* All other paramters are embedded in the child node */ > >> subnode = dev_read_first_subnode(bus); > >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > >> index e655b027d788..619f0bed8efd 100644 > >> --- a/drivers/spi/cadence_qspi.h > >> +++ b/drivers/spi/cadence_qspi.h > >> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { > >> u32 fifo_depth; > >> u32 fifo_width; > >> u32 trigger_address; > >> + fdt_addr_t ahbsize; > >> + bool use_dac_mode; > >> > >> /* Flash parameters */ > >> u32 page_size; > >> @@ -52,20 +54,21 @@ struct cadence_spi_priv { > >> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); > >> void cadence_qspi_apb_controller_enable(void *reg_base_addr); > >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); > >> > >> int cadence_qspi_apb_command_read(void *reg_base_addr, > >> const struct spi_mem_op *op); > >> int cadence_qspi_apb_command_write(void *reg_base_addr, > >> const struct spi_mem_op *op); > >> > >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - const struct spi_mem_op *op); > >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >> - unsigned int rxlen, u8 *rxbuf); > >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - const struct spi_mem_op *op); > >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >> - unsigned int txlen, const u8 *txbuf); > >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op); > >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op); > >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op); > >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op); > >> > >> void cadence_qspi_apb_chipselect(void *reg_base, > >> unsigned int chip_select, unsigned int decoder_enable); > >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >> index 8dd0495dfcf4..fd491f2c8104 100644 > >> --- a/drivers/spi/cadence_qspi_apb.c > >> +++ b/drivers/spi/cadence_qspi_apb.c > >> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) > >> writel(reg, reg_base + CQSPI_REG_CONFIG); > >> } > >> > >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) > >> +{ > >> + unsigned int reg; > >> + > >> + reg = readl(reg_base + CQSPI_REG_CONFIG); > >> + reg |= CQSPI_REG_CONFIG_DIRECT; > >> + writel(reg, reg_base + CQSPI_REG_CONFIG); > >> +} > >> + > >> /* Return 1 if idle, otherwise return 0 (busy). */ > >> static unsigned int cadence_qspi_wait_idle(void *reg_base) > >> { > >> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > >> } > >> > >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> - const struct spi_mem_op *op) > >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> unsigned int rd_reg; > >> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) > >> return -ETIMEDOUT; > >> } > >> > >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >> - unsigned int n_rx, u8 *rxbuf) > >> +static int > >> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >> + unsigned int n_rx, u8 *rxbuf) > >> { > >> unsigned int remaining = n_rx; > >> unsigned int bytes_to_read = 0; > >> @@ -639,9 +649,26 @@ failrd: > >> return ret; > >> } > >> > >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op) > >> +{ > >> + u32 from = op->addr.val; > >> + void *buf = op->data.buf.in; > >> + size_t len = op->data.nbytes; > >> + > >> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { > >> + memcpy_fromio(buf, plat->ahbbase + from, len); > >> + if (!cadence_qspi_wait_idle(plat->regbase)) > >> + return -EIO; > >> + return 0; > >> + } > >> + > >> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); > >> +} > >> + > >> /* Opcode + Address (3/4 bytes) */ > >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> - const struct spi_mem_op *op) > >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op) > >> { > >> unsigned int reg; > >> > >> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >> return 0; > >> } > >> > >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >> - unsigned int n_tx, const u8 *txbuf) > >> +static int > >> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >> + unsigned int n_tx, const u8 *txbuf) > >> { > >> unsigned int page_size = plat->page_size; > >> unsigned int remaining = n_tx; > >> @@ -735,6 +763,23 @@ failwr: > >> return ret; > >> } > >> > >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >> + const struct spi_mem_op *op) > >> +{ > >> + u32 to = op->addr.val; > >> + const void *buf = op->data.buf.out; > >> + size_t len = op->data.nbytes; > >> + > >> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { > >> + memcpy_toio(plat->ahbbase + to, buf, len); > >> + if (!cadence_qspi_wait_idle(plat->regbase)) > >> + return -EIO; > >> + return 0; > >> + } > >> + > >> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); > >> +} > >> + > >> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) > >> { > >> unsigned int reg; > >> -- > >> 2.23.0 > >> > > -- > Regards > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-17 12:55 ` Simon Goldschmidt @ 2019-10-18 9:04 ` Simon Goldschmidt 2019-10-18 12:40 ` Vignesh Raghavendra 0 siblings, 1 reply; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-18 9:04 UTC (permalink / raw) To: u-boot On Thu, Oct 17, 2019 at 2:55 PM Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > Hi, > > > > On 17/10/19 5:09 PM, Simon Goldschmidt wrote: > > > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > >> > > >> Add support for Direct Access Controller mode of Cadence QSPI. This > > >> allows MMIO access to SPI NOR flash providing better read performance. > > >> > > >> Signed-off-by: Vignesh R <vigneshr@ti.com> > > >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > > > > > > I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at > > > 50MHz) and it seems to work fine. > > > > > > However, I had the impression it was a bit slower, not faster, although I > > > haven't measured, and running at 50MHz with 4 data lines, reading the whole > > > flash takes about 1.5 seconds only, so without actually measureing it, it's > > > hard to tell if this performance is really worth the 440 bytes of extra code > > > it adds? > > > > > > > I should have noted in the commit msg that direct mode is used only when > > AHB window is at least 8MB. Working with smaller window sizes would mean > > code would fall back to indirect mode most of the time. > > I see that socfgpa don't seem to have large AHB windows and therefore > > would not use direct access mode. > > Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't > changed. > > > > > On TI platforms its possible to use DMA to read data from memory mapped > > region using mem to mem DMA channels which improves throughput by 5 times. > > Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 > seconds. The maximum speed of that setup would be 25 MByte/s without any > overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement > by 5 on my platform! > > > > > > Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes > > > do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes > > > for me :-) > > > > > > > Hmm, I read this as patch 1 brings down memory consumption by 320 bytes > > and patch 2 adds 440 bytes (delta being +120 bytes for entire series). > > I can see if size can be optimized, but would like to avoid #ifdef'ery > > within the driver if possible. > > Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled > at the same time in socfpga_socrates (moving more code to DM). And even if it > sounds like not much, 440 bytes *are* much for me at this point. > > I still would prefer having a Kconfig option for this that can be disabled > for socfpga. Oh well, maybe just go ahead adding this and I'll try how it fits. We can alway add reduced functionality later by checking for CONFIG_SPL_SPI_FLASH_TINY, whicht might be a better config option to guard this than something cadence-qspi specific. Regards, Simon > > Regards, > Simon > > > > > Regards > > Vignesh > > > > > Regards, > > > Simon > > > > > > > > >> --- > > >> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- > > >> drivers/spi/cadence_qspi.h | 19 ++++++----- > > >> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- > > >> 3 files changed, 87 insertions(+), 33 deletions(-) > > >> > > >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > >> index 673a2e9a6c4c..6c98cbf39ae4 100644 > > >> --- a/drivers/spi/cadence_qspi.c > > >> +++ b/drivers/spi/cadence_qspi.c > > >> @@ -12,12 +12,13 @@ > > >> #include <spi.h> > > >> #include <spi-mem.h> > > >> #include <linux/errno.h> > > >> +#include <linux/sizes.h> > > >> #include "cadence_qspi.h" > > >> > > >> #define CQSPI_STIG_READ 0 > > >> #define CQSPI_STIG_WRITE 1 > > >> -#define CQSPI_INDIRECT_READ 2 > > >> -#define CQSPI_INDIRECT_WRITE 3 > > >> +#define CQSPI_READ 2 > > >> +#define CQSPI_WRITE 3 > > >> > > >> static int cadence_spi_write_speed(struct udevice *bus, uint hz) > > >> { > > >> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) > > >> > > >> static int cadence_spi_set_mode(struct udevice *bus, uint mode) > > >> { > > >> + struct cadence_spi_platdata *plat = bus->platdata; > > >> struct cadence_spi_priv *priv = dev_get_priv(bus); > > >> > > >> /* Disable QSPI */ > > >> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > > >> /* Set SPI mode */ > > >> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); > > >> > > >> + /* Enable Direct Access Controller */ > > >> + if (plat->use_dac_mode) > > >> + cadence_qspi_apb_dac_mode_enable(priv->regbase); > > >> + > > >> /* Enable QSPI */ > > >> cadence_qspi_apb_controller_enable(priv->regbase); > > >> > > >> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > > >> if (!op->addr.nbytes) > > >> mode = CQSPI_STIG_READ; > > >> else > > >> - mode = CQSPI_INDIRECT_READ; > > >> + mode = CQSPI_READ; > > >> } else { > > >> if (!op->addr.nbytes || !op->data.buf.out) > > >> mode = CQSPI_STIG_WRITE; > > >> else > > >> - mode = CQSPI_INDIRECT_WRITE; > > >> + mode = CQSPI_WRITE; > > >> } > > >> > > >> switch (mode) { > > >> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > > >> case CQSPI_STIG_WRITE: > > >> err = cadence_qspi_apb_command_write(base, op); > > >> break; > > >> - case CQSPI_INDIRECT_READ: > > >> - err = cadence_qspi_apb_indirect_read_setup(plat, op); > > >> - if (!err) { > > >> - err = cadence_qspi_apb_indirect_read_execute > > >> - (plat, op->data.nbytes, op->data.buf.in); > > >> - } > > >> + case CQSPI_READ: > > >> + err = cadence_qspi_apb_read_setup(plat, op); > > >> + if (!err) > > >> + err = cadence_qspi_apb_read_execute(plat, op); > > >> break; > > >> - case CQSPI_INDIRECT_WRITE: > > >> - err = cadence_qspi_apb_indirect_write_setup(plat, op); > > >> - if (!err) { > > >> - err = cadence_qspi_apb_indirect_write_execute > > >> - (plat, op->data.nbytes, op->data.buf.out); > > >> - } > > >> + case CQSPI_WRITE: > > >> + err = cadence_qspi_apb_write_setup(plat, op); > > >> + if (!err) > > >> + err = cadence_qspi_apb_write_execute(plat, op); > > >> break; > > >> default: > > >> err = -1; > > >> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > > >> ofnode subnode; > > >> > > >> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > > >> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); > > >> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > > >> + &plat->ahbsize); > > >> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); > > >> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); > > >> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); > > >> plat->trigger_address = dev_read_u32_default(bus, > > >> "cdns,trigger-address", > > >> 0); > > >> + /* Use DAC mode only when MMIO window is at least 8M wide */ > > >> + if (plat->ahbsize >= SZ_8M) > > >> + plat->use_dac_mode = true; > > >> > > >> /* All other paramters are embedded in the child node */ > > >> subnode = dev_read_first_subnode(bus); > > >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > > >> index e655b027d788..619f0bed8efd 100644 > > >> --- a/drivers/spi/cadence_qspi.h > > >> +++ b/drivers/spi/cadence_qspi.h > > >> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { > > >> u32 fifo_depth; > > >> u32 fifo_width; > > >> u32 trigger_address; > > >> + fdt_addr_t ahbsize; > > >> + bool use_dac_mode; > > >> > > >> /* Flash parameters */ > > >> u32 page_size; > > >> @@ -52,20 +54,21 @@ struct cadence_spi_priv { > > >> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); > > >> void cadence_qspi_apb_controller_enable(void *reg_base_addr); > > >> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > > >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); > > >> > > >> int cadence_qspi_apb_command_read(void *reg_base_addr, > > >> const struct spi_mem_op *op); > > >> int cadence_qspi_apb_command_write(void *reg_base_addr, > > >> const struct spi_mem_op *op); > > >> > > >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > > >> - const struct spi_mem_op *op); > > >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > > >> - unsigned int rxlen, u8 *rxbuf); > > >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > > >> - const struct spi_mem_op *op); > > >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > > >> - unsigned int txlen, const u8 *txbuf); > > >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op); > > >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op); > > >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op); > > >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op); > > >> > > >> void cadence_qspi_apb_chipselect(void *reg_base, > > >> unsigned int chip_select, unsigned int decoder_enable); > > >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > > >> index 8dd0495dfcf4..fd491f2c8104 100644 > > >> --- a/drivers/spi/cadence_qspi_apb.c > > >> +++ b/drivers/spi/cadence_qspi_apb.c > > >> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) > > >> writel(reg, reg_base + CQSPI_REG_CONFIG); > > >> } > > >> > > >> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) > > >> +{ > > >> + unsigned int reg; > > >> + > > >> + reg = readl(reg_base + CQSPI_REG_CONFIG); > > >> + reg |= CQSPI_REG_CONFIG_DIRECT; > > >> + writel(reg, reg_base + CQSPI_REG_CONFIG); > > >> +} > > >> + > > >> /* Return 1 if idle, otherwise return 0 (busy). */ > > >> static unsigned int cadence_qspi_wait_idle(void *reg_base) > > >> { > > >> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > > >> } > > >> > > >> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > > >> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > > >> - const struct spi_mem_op *op) > > >> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op) > > >> { > > >> unsigned int reg; > > >> unsigned int rd_reg; > > >> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) > > >> return -ETIMEDOUT; > > >> } > > >> > > >> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > > >> - unsigned int n_rx, u8 *rxbuf) > > >> +static int > > >> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > > >> + unsigned int n_rx, u8 *rxbuf) > > >> { > > >> unsigned int remaining = n_rx; > > >> unsigned int bytes_to_read = 0; > > >> @@ -639,9 +649,26 @@ failrd: > > >> return ret; > > >> } > > >> > > >> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op) > > >> +{ > > >> + u32 from = op->addr.val; > > >> + void *buf = op->data.buf.in; > > >> + size_t len = op->data.nbytes; > > >> + > > >> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { > > >> + memcpy_fromio(buf, plat->ahbbase + from, len); > > >> + if (!cadence_qspi_wait_idle(plat->regbase)) > > >> + return -EIO; > > >> + return 0; > > >> + } > > >> + > > >> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); > > >> +} > > >> + > > >> /* Opcode + Address (3/4 bytes) */ > > >> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > > >> - const struct spi_mem_op *op) > > >> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op) > > >> { > > >> unsigned int reg; > > >> > > >> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > > >> return 0; > > >> } > > >> > > >> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > > >> - unsigned int n_tx, const u8 *txbuf) > > >> +static int > > >> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > > >> + unsigned int n_tx, const u8 *txbuf) > > >> { > > >> unsigned int page_size = plat->page_size; > > >> unsigned int remaining = n_tx; > > >> @@ -735,6 +763,23 @@ failwr: > > >> return ret; > > >> } > > >> > > >> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > > >> + const struct spi_mem_op *op) > > >> +{ > > >> + u32 to = op->addr.val; > > >> + const void *buf = op->data.buf.out; > > >> + size_t len = op->data.nbytes; > > >> + > > >> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { > > >> + memcpy_toio(plat->ahbbase + to, buf, len); > > >> + if (!cadence_qspi_wait_idle(plat->regbase)) > > >> + return -EIO; > > >> + return 0; > > >> + } > > >> + > > >> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); > > >> +} > > >> + > > >> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) > > >> { > > >> unsigned int reg; > > >> -- > > >> 2.23.0 > > >> > > > > -- > > Regards > > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-18 9:04 ` Simon Goldschmidt @ 2019-10-18 12:40 ` Vignesh Raghavendra 2019-10-18 12:42 ` Simon Goldschmidt 0 siblings, 1 reply; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-18 12:40 UTC (permalink / raw) To: u-boot Hi, On 18/10/19 2:34 PM, Simon Goldschmidt wrote: > On Thu, Oct 17, 2019 at 2:55 PM Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com> wrote: >> >> On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>> >>> Hi, >>> >>> On 17/10/19 5:09 PM, Simon Goldschmidt wrote: >>>> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>>>> >>>>> Add support for Direct Access Controller mode of Cadence QSPI. This >>>>> allows MMIO access to SPI NOR flash providing better read performance. >>>>> >>>>> Signed-off-by: Vignesh R <vigneshr@ti.com> >>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>>> >>>> I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at >>>> 50MHz) and it seems to work fine. >>>> >>>> However, I had the impression it was a bit slower, not faster, although I >>>> haven't measured, and running at 50MHz with 4 data lines, reading the whole >>>> flash takes about 1.5 seconds only, so without actually measureing it, it's >>>> hard to tell if this performance is really worth the 440 bytes of extra code >>>> it adds? >>>> >>> >>> I should have noted in the commit msg that direct mode is used only when >>> AHB window is at least 8MB. Working with smaller window sizes would mean >>> code would fall back to indirect mode most of the time. >>> I see that socfgpa don't seem to have large AHB windows and therefore >>> would not use direct access mode. >> >> Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't >> changed. >> >>> >>> On TI platforms its possible to use DMA to read data from memory mapped >>> region using mem to mem DMA channels which improves throughput by 5 times. >> >> Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 >> seconds. The maximum speed of that setup would be 25 MByte/s without any >> overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement >> by 5 on my platform! >> >>> >>>> Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes >>>> do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes >>>> for me :-) >>>> >>> >>> Hmm, I read this as patch 1 brings down memory consumption by 320 bytes >>> and patch 2 adds 440 bytes (delta being +120 bytes for entire series). >>> I can see if size can be optimized, but would like to avoid #ifdef'ery >>> within the driver if possible. >> >> Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled >> at the same time in socfpga_socrates (moving more code to DM). And even if it >> sounds like not much, 440 bytes *are* much for me at this point. >> >> I still would prefer having a Kconfig option for this that can be disabled >> for socfpga. > > Oh well, maybe just go ahead adding this and I'll try how it fits. We can alway > add reduced functionality later by checking for CONFIG_SPL_SPI_FLASH_TINY, > whicht might be a better config option to guard this than something cadence-qspi > specific. > Okay, thanks! Guarding with CONFIG_SPL_SPI_FLASH_TINY as and where required seems fair idea. BTW, it would be great if you could confirm Quad read works fine when spi-tx-bus-width = 1 and spi-rx-bus-width = 4 as I suggested in other thread? Regards Vignesh > Regards, > Simon > >> >> Regards, >> Simon >> >>> >>> Regards >>> Vignesh >>> >>>> Regards, >>>> Simon >>>> >>>> >>>>> --- >>>>> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- >>>>> drivers/spi/cadence_qspi.h | 19 ++++++----- >>>>> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- >>>>> 3 files changed, 87 insertions(+), 33 deletions(-) >>>>> >>>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c >>>>> index 673a2e9a6c4c..6c98cbf39ae4 100644 >>>>> --- a/drivers/spi/cadence_qspi.c >>>>> +++ b/drivers/spi/cadence_qspi.c >>>>> @@ -12,12 +12,13 @@ >>>>> #include <spi.h> >>>>> #include <spi-mem.h> >>>>> #include <linux/errno.h> >>>>> +#include <linux/sizes.h> >>>>> #include "cadence_qspi.h" >>>>> >>>>> #define CQSPI_STIG_READ 0 >>>>> #define CQSPI_STIG_WRITE 1 >>>>> -#define CQSPI_INDIRECT_READ 2 >>>>> -#define CQSPI_INDIRECT_WRITE 3 >>>>> +#define CQSPI_READ 2 >>>>> +#define CQSPI_WRITE 3 >>>>> >>>>> static int cadence_spi_write_speed(struct udevice *bus, uint hz) >>>>> { >>>>> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) >>>>> >>>>> static int cadence_spi_set_mode(struct udevice *bus, uint mode) >>>>> { >>>>> + struct cadence_spi_platdata *plat = bus->platdata; >>>>> struct cadence_spi_priv *priv = dev_get_priv(bus); >>>>> >>>>> /* Disable QSPI */ >>>>> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) >>>>> /* Set SPI mode */ >>>>> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); >>>>> >>>>> + /* Enable Direct Access Controller */ >>>>> + if (plat->use_dac_mode) >>>>> + cadence_qspi_apb_dac_mode_enable(priv->regbase); >>>>> + >>>>> /* Enable QSPI */ >>>>> cadence_qspi_apb_controller_enable(priv->regbase); >>>>> >>>>> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >>>>> if (!op->addr.nbytes) >>>>> mode = CQSPI_STIG_READ; >>>>> else >>>>> - mode = CQSPI_INDIRECT_READ; >>>>> + mode = CQSPI_READ; >>>>> } else { >>>>> if (!op->addr.nbytes || !op->data.buf.out) >>>>> mode = CQSPI_STIG_WRITE; >>>>> else >>>>> - mode = CQSPI_INDIRECT_WRITE; >>>>> + mode = CQSPI_WRITE; >>>>> } >>>>> >>>>> switch (mode) { >>>>> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >>>>> case CQSPI_STIG_WRITE: >>>>> err = cadence_qspi_apb_command_write(base, op); >>>>> break; >>>>> - case CQSPI_INDIRECT_READ: >>>>> - err = cadence_qspi_apb_indirect_read_setup(plat, op); >>>>> - if (!err) { >>>>> - err = cadence_qspi_apb_indirect_read_execute >>>>> - (plat, op->data.nbytes, op->data.buf.in); >>>>> - } >>>>> + case CQSPI_READ: >>>>> + err = cadence_qspi_apb_read_setup(plat, op); >>>>> + if (!err) >>>>> + err = cadence_qspi_apb_read_execute(plat, op); >>>>> break; >>>>> - case CQSPI_INDIRECT_WRITE: >>>>> - err = cadence_qspi_apb_indirect_write_setup(plat, op); >>>>> - if (!err) { >>>>> - err = cadence_qspi_apb_indirect_write_execute >>>>> - (plat, op->data.nbytes, op->data.buf.out); >>>>> - } >>>>> + case CQSPI_WRITE: >>>>> + err = cadence_qspi_apb_write_setup(plat, op); >>>>> + if (!err) >>>>> + err = cadence_qspi_apb_write_execute(plat, op); >>>>> break; >>>>> default: >>>>> err = -1; >>>>> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) >>>>> ofnode subnode; >>>>> >>>>> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); >>>>> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); >>>>> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, >>>>> + &plat->ahbsize); >>>>> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); >>>>> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); >>>>> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); >>>>> plat->trigger_address = dev_read_u32_default(bus, >>>>> "cdns,trigger-address", >>>>> 0); >>>>> + /* Use DAC mode only when MMIO window is at least 8M wide */ >>>>> + if (plat->ahbsize >= SZ_8M) >>>>> + plat->use_dac_mode = true; >>>>> >>>>> /* All other paramters are embedded in the child node */ >>>>> subnode = dev_read_first_subnode(bus); >>>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >>>>> index e655b027d788..619f0bed8efd 100644 >>>>> --- a/drivers/spi/cadence_qspi.h >>>>> +++ b/drivers/spi/cadence_qspi.h >>>>> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { >>>>> u32 fifo_depth; >>>>> u32 fifo_width; >>>>> u32 trigger_address; >>>>> + fdt_addr_t ahbsize; >>>>> + bool use_dac_mode; >>>>> >>>>> /* Flash parameters */ >>>>> u32 page_size; >>>>> @@ -52,20 +54,21 @@ struct cadence_spi_priv { >>>>> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); >>>>> void cadence_qspi_apb_controller_enable(void *reg_base_addr); >>>>> void cadence_qspi_apb_controller_disable(void *reg_base_addr); >>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); >>>>> >>>>> int cadence_qspi_apb_command_read(void *reg_base_addr, >>>>> const struct spi_mem_op *op); >>>>> int cadence_qspi_apb_command_write(void *reg_base_addr, >>>>> const struct spi_mem_op *op); >>>>> >>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>>> - const struct spi_mem_op *op); >>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>> - unsigned int rxlen, u8 *rxbuf); >>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>> - const struct spi_mem_op *op); >>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>> - unsigned int txlen, const u8 *txbuf); >>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op); >>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op); >>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op); >>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op); >>>>> >>>>> void cadence_qspi_apb_chipselect(void *reg_base, >>>>> unsigned int chip_select, unsigned int decoder_enable); >>>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >>>>> index 8dd0495dfcf4..fd491f2c8104 100644 >>>>> --- a/drivers/spi/cadence_qspi_apb.c >>>>> +++ b/drivers/spi/cadence_qspi_apb.c >>>>> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) >>>>> writel(reg, reg_base + CQSPI_REG_CONFIG); >>>>> } >>>>> >>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) >>>>> +{ >>>>> + unsigned int reg; >>>>> + >>>>> + reg = readl(reg_base + CQSPI_REG_CONFIG); >>>>> + reg |= CQSPI_REG_CONFIG_DIRECT; >>>>> + writel(reg, reg_base + CQSPI_REG_CONFIG); >>>>> +} >>>>> + >>>>> /* Return 1 if idle, otherwise return 0 (busy). */ >>>>> static unsigned int cadence_qspi_wait_idle(void *reg_base) >>>>> { >>>>> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) >>>>> } >>>>> >>>>> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ >>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>>> - const struct spi_mem_op *op) >>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op) >>>>> { >>>>> unsigned int reg; >>>>> unsigned int rd_reg; >>>>> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) >>>>> return -ETIMEDOUT; >>>>> } >>>>> >>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>> - unsigned int n_rx, u8 *rxbuf) >>>>> +static int >>>>> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>> + unsigned int n_rx, u8 *rxbuf) >>>>> { >>>>> unsigned int remaining = n_rx; >>>>> unsigned int bytes_to_read = 0; >>>>> @@ -639,9 +649,26 @@ failrd: >>>>> return ret; >>>>> } >>>>> >>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op) >>>>> +{ >>>>> + u32 from = op->addr.val; >>>>> + void *buf = op->data.buf.in; >>>>> + size_t len = op->data.nbytes; >>>>> + >>>>> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { >>>>> + memcpy_fromio(buf, plat->ahbbase + from, len); >>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) >>>>> + return -EIO; >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); >>>>> +} >>>>> + >>>>> /* Opcode + Address (3/4 bytes) */ >>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>> - const struct spi_mem_op *op) >>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op) >>>>> { >>>>> unsigned int reg; >>>>> >>>>> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>> return 0; >>>>> } >>>>> >>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>> - unsigned int n_tx, const u8 *txbuf) >>>>> +static int >>>>> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>> + unsigned int n_tx, const u8 *txbuf) >>>>> { >>>>> unsigned int page_size = plat->page_size; >>>>> unsigned int remaining = n_tx; >>>>> @@ -735,6 +763,23 @@ failwr: >>>>> return ret; >>>>> } >>>>> >>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >>>>> + const struct spi_mem_op *op) >>>>> +{ >>>>> + u32 to = op->addr.val; >>>>> + const void *buf = op->data.buf.out; >>>>> + size_t len = op->data.nbytes; >>>>> + >>>>> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { >>>>> + memcpy_toio(plat->ahbbase + to, buf, len); >>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) >>>>> + return -EIO; >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); >>>>> +} >>>>> + >>>>> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) >>>>> { >>>>> unsigned int reg; >>>>> -- >>>>> 2.23.0 >>>>> >>> >>> -- >>> Regards >>> Vignesh -- Regards Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-18 12:40 ` Vignesh Raghavendra @ 2019-10-18 12:42 ` Simon Goldschmidt 2019-10-23 10:01 ` Vignesh Raghavendra 0 siblings, 1 reply; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-18 12:42 UTC (permalink / raw) To: u-boot On Fri, Oct 18, 2019 at 2:40 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi, > > On 18/10/19 2:34 PM, Simon Goldschmidt wrote: > > On Thu, Oct 17, 2019 at 2:55 PM Simon Goldschmidt > > <simon.k.r.goldschmidt@gmail.com> wrote: > >> > >> On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >>> > >>> Hi, > >>> > >>> On 17/10/19 5:09 PM, Simon Goldschmidt wrote: > >>>> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >>>>> > >>>>> Add support for Direct Access Controller mode of Cadence QSPI. This > >>>>> allows MMIO access to SPI NOR flash providing better read performance. > >>>>> > >>>>> Signed-off-by: Vignesh R <vigneshr@ti.com> > >>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > >>>> > >>>> I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at > >>>> 50MHz) and it seems to work fine. > >>>> > >>>> However, I had the impression it was a bit slower, not faster, although I > >>>> haven't measured, and running at 50MHz with 4 data lines, reading the whole > >>>> flash takes about 1.5 seconds only, so without actually measureing it, it's > >>>> hard to tell if this performance is really worth the 440 bytes of extra code > >>>> it adds? > >>>> > >>> > >>> I should have noted in the commit msg that direct mode is used only when > >>> AHB window is at least 8MB. Working with smaller window sizes would mean > >>> code would fall back to indirect mode most of the time. > >>> I see that socfgpa don't seem to have large AHB windows and therefore > >>> would not use direct access mode. > >> > >> Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't > >> changed. > >> > >>> > >>> On TI platforms its possible to use DMA to read data from memory mapped > >>> region using mem to mem DMA channels which improves throughput by 5 times. > >> > >> Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 > >> seconds. The maximum speed of that setup would be 25 MByte/s without any > >> overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement > >> by 5 on my platform! > >> > >>> > >>>> Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes > >>>> do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes > >>>> for me :-) > >>>> > >>> > >>> Hmm, I read this as patch 1 brings down memory consumption by 320 bytes > >>> and patch 2 adds 440 bytes (delta being +120 bytes for entire series). > >>> I can see if size can be optimized, but would like to avoid #ifdef'ery > >>> within the driver if possible. > >> > >> Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled > >> at the same time in socfpga_socrates (moving more code to DM). And even if it > >> sounds like not much, 440 bytes *are* much for me at this point. > >> > >> I still would prefer having a Kconfig option for this that can be disabled > >> for socfpga. > > > > Oh well, maybe just go ahead adding this and I'll try how it fits. We can alway > > add reduced functionality later by checking for CONFIG_SPL_SPI_FLASH_TINY, > > whicht might be a better config option to guard this than something cadence-qspi > > specific. > > > > Okay, thanks! Guarding with CONFIG_SPL_SPI_FLASH_TINY as and where > required seems fair idea. BTW, it would be great if you could confirm > Quad read works fine when spi-tx-bus-width = 1 and spi-rx-bus-width = 4 > as I suggested in other thread? Yes, I'll do that when I find the time. Unfortunately, I can only do that on the devices at work, not at home. I also would like to check the memory-mapped code works on socfpga by removing the 8MB size check, does that make sense? Regards, Simon > > Regards > Vignesh > > > Regards, > > Simon > > > >> > >> Regards, > >> Simon > >> > >>> > >>> Regards > >>> Vignesh > >>> > >>>> Regards, > >>>> Simon > >>>> > >>>> > >>>>> --- > >>>>> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- > >>>>> drivers/spi/cadence_qspi.h | 19 ++++++----- > >>>>> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- > >>>>> 3 files changed, 87 insertions(+), 33 deletions(-) > >>>>> > >>>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > >>>>> index 673a2e9a6c4c..6c98cbf39ae4 100644 > >>>>> --- a/drivers/spi/cadence_qspi.c > >>>>> +++ b/drivers/spi/cadence_qspi.c > >>>>> @@ -12,12 +12,13 @@ > >>>>> #include <spi.h> > >>>>> #include <spi-mem.h> > >>>>> #include <linux/errno.h> > >>>>> +#include <linux/sizes.h> > >>>>> #include "cadence_qspi.h" > >>>>> > >>>>> #define CQSPI_STIG_READ 0 > >>>>> #define CQSPI_STIG_WRITE 1 > >>>>> -#define CQSPI_INDIRECT_READ 2 > >>>>> -#define CQSPI_INDIRECT_WRITE 3 > >>>>> +#define CQSPI_READ 2 > >>>>> +#define CQSPI_WRITE 3 > >>>>> > >>>>> static int cadence_spi_write_speed(struct udevice *bus, uint hz) > >>>>> { > >>>>> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) > >>>>> > >>>>> static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >>>>> { > >>>>> + struct cadence_spi_platdata *plat = bus->platdata; > >>>>> struct cadence_spi_priv *priv = dev_get_priv(bus); > >>>>> > >>>>> /* Disable QSPI */ > >>>>> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >>>>> /* Set SPI mode */ > >>>>> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); > >>>>> > >>>>> + /* Enable Direct Access Controller */ > >>>>> + if (plat->use_dac_mode) > >>>>> + cadence_qspi_apb_dac_mode_enable(priv->regbase); > >>>>> + > >>>>> /* Enable QSPI */ > >>>>> cadence_qspi_apb_controller_enable(priv->regbase); > >>>>> > >>>>> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >>>>> if (!op->addr.nbytes) > >>>>> mode = CQSPI_STIG_READ; > >>>>> else > >>>>> - mode = CQSPI_INDIRECT_READ; > >>>>> + mode = CQSPI_READ; > >>>>> } else { > >>>>> if (!op->addr.nbytes || !op->data.buf.out) > >>>>> mode = CQSPI_STIG_WRITE; > >>>>> else > >>>>> - mode = CQSPI_INDIRECT_WRITE; > >>>>> + mode = CQSPI_WRITE; > >>>>> } > >>>>> > >>>>> switch (mode) { > >>>>> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >>>>> case CQSPI_STIG_WRITE: > >>>>> err = cadence_qspi_apb_command_write(base, op); > >>>>> break; > >>>>> - case CQSPI_INDIRECT_READ: > >>>>> - err = cadence_qspi_apb_indirect_read_setup(plat, op); > >>>>> - if (!err) { > >>>>> - err = cadence_qspi_apb_indirect_read_execute > >>>>> - (plat, op->data.nbytes, op->data.buf.in); > >>>>> - } > >>>>> + case CQSPI_READ: > >>>>> + err = cadence_qspi_apb_read_setup(plat, op); > >>>>> + if (!err) > >>>>> + err = cadence_qspi_apb_read_execute(plat, op); > >>>>> break; > >>>>> - case CQSPI_INDIRECT_WRITE: > >>>>> - err = cadence_qspi_apb_indirect_write_setup(plat, op); > >>>>> - if (!err) { > >>>>> - err = cadence_qspi_apb_indirect_write_execute > >>>>> - (plat, op->data.nbytes, op->data.buf.out); > >>>>> - } > >>>>> + case CQSPI_WRITE: > >>>>> + err = cadence_qspi_apb_write_setup(plat, op); > >>>>> + if (!err) > >>>>> + err = cadence_qspi_apb_write_execute(plat, op); > >>>>> break; > >>>>> default: > >>>>> err = -1; > >>>>> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > >>>>> ofnode subnode; > >>>>> > >>>>> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > >>>>> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); > >>>>> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > >>>>> + &plat->ahbsize); > >>>>> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); > >>>>> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); > >>>>> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); > >>>>> plat->trigger_address = dev_read_u32_default(bus, > >>>>> "cdns,trigger-address", > >>>>> 0); > >>>>> + /* Use DAC mode only when MMIO window is at least 8M wide */ > >>>>> + if (plat->ahbsize >= SZ_8M) > >>>>> + plat->use_dac_mode = true; > >>>>> > >>>>> /* All other paramters are embedded in the child node */ > >>>>> subnode = dev_read_first_subnode(bus); > >>>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > >>>>> index e655b027d788..619f0bed8efd 100644 > >>>>> --- a/drivers/spi/cadence_qspi.h > >>>>> +++ b/drivers/spi/cadence_qspi.h > >>>>> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { > >>>>> u32 fifo_depth; > >>>>> u32 fifo_width; > >>>>> u32 trigger_address; > >>>>> + fdt_addr_t ahbsize; > >>>>> + bool use_dac_mode; > >>>>> > >>>>> /* Flash parameters */ > >>>>> u32 page_size; > >>>>> @@ -52,20 +54,21 @@ struct cadence_spi_priv { > >>>>> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); > >>>>> void cadence_qspi_apb_controller_enable(void *reg_base_addr); > >>>>> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > >>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); > >>>>> > >>>>> int cadence_qspi_apb_command_read(void *reg_base_addr, > >>>>> const struct spi_mem_op *op); > >>>>> int cadence_qspi_apb_command_write(void *reg_base_addr, > >>>>> const struct spi_mem_op *op); > >>>>> > >>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >>>>> - const struct spi_mem_op *op); > >>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>> - unsigned int rxlen, u8 *rxbuf); > >>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>> - const struct spi_mem_op *op); > >>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>> - unsigned int txlen, const u8 *txbuf); > >>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op); > >>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op); > >>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op); > >>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op); > >>>>> > >>>>> void cadence_qspi_apb_chipselect(void *reg_base, > >>>>> unsigned int chip_select, unsigned int decoder_enable); > >>>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >>>>> index 8dd0495dfcf4..fd491f2c8104 100644 > >>>>> --- a/drivers/spi/cadence_qspi_apb.c > >>>>> +++ b/drivers/spi/cadence_qspi_apb.c > >>>>> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) > >>>>> writel(reg, reg_base + CQSPI_REG_CONFIG); > >>>>> } > >>>>> > >>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) > >>>>> +{ > >>>>> + unsigned int reg; > >>>>> + > >>>>> + reg = readl(reg_base + CQSPI_REG_CONFIG); > >>>>> + reg |= CQSPI_REG_CONFIG_DIRECT; > >>>>> + writel(reg, reg_base + CQSPI_REG_CONFIG); > >>>>> +} > >>>>> + > >>>>> /* Return 1 if idle, otherwise return 0 (busy). */ > >>>>> static unsigned int cadence_qspi_wait_idle(void *reg_base) > >>>>> { > >>>>> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > >>>>> } > >>>>> > >>>>> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > >>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >>>>> - const struct spi_mem_op *op) > >>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op) > >>>>> { > >>>>> unsigned int reg; > >>>>> unsigned int rd_reg; > >>>>> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) > >>>>> return -ETIMEDOUT; > >>>>> } > >>>>> > >>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>> - unsigned int n_rx, u8 *rxbuf) > >>>>> +static int > >>>>> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>> + unsigned int n_rx, u8 *rxbuf) > >>>>> { > >>>>> unsigned int remaining = n_rx; > >>>>> unsigned int bytes_to_read = 0; > >>>>> @@ -639,9 +649,26 @@ failrd: > >>>>> return ret; > >>>>> } > >>>>> > >>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op) > >>>>> +{ > >>>>> + u32 from = op->addr.val; > >>>>> + void *buf = op->data.buf.in; > >>>>> + size_t len = op->data.nbytes; > >>>>> + > >>>>> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { > >>>>> + memcpy_fromio(buf, plat->ahbbase + from, len); > >>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) > >>>>> + return -EIO; > >>>>> + return 0; > >>>>> + } > >>>>> + > >>>>> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); > >>>>> +} > >>>>> + > >>>>> /* Opcode + Address (3/4 bytes) */ > >>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>> - const struct spi_mem_op *op) > >>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op) > >>>>> { > >>>>> unsigned int reg; > >>>>> > >>>>> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>> return 0; > >>>>> } > >>>>> > >>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>> - unsigned int n_tx, const u8 *txbuf) > >>>>> +static int > >>>>> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>> + unsigned int n_tx, const u8 *txbuf) > >>>>> { > >>>>> unsigned int page_size = plat->page_size; > >>>>> unsigned int remaining = n_tx; > >>>>> @@ -735,6 +763,23 @@ failwr: > >>>>> return ret; > >>>>> } > >>>>> > >>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >>>>> + const struct spi_mem_op *op) > >>>>> +{ > >>>>> + u32 to = op->addr.val; > >>>>> + const void *buf = op->data.buf.out; > >>>>> + size_t len = op->data.nbytes; > >>>>> + > >>>>> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { > >>>>> + memcpy_toio(plat->ahbbase + to, buf, len); > >>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) > >>>>> + return -EIO; > >>>>> + return 0; > >>>>> + } > >>>>> + > >>>>> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); > >>>>> +} > >>>>> + > >>>>> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) > >>>>> { > >>>>> unsigned int reg; > >>>>> -- > >>>>> 2.23.0 > >>>>> > >>> > >>> -- > >>> Regards > >>> Vignesh > > -- > Regards > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-18 12:42 ` Simon Goldschmidt @ 2019-10-23 10:01 ` Vignesh Raghavendra 2019-10-23 10:06 ` Simon Goldschmidt 0 siblings, 1 reply; 18+ messages in thread From: Vignesh Raghavendra @ 2019-10-23 10:01 UTC (permalink / raw) To: u-boot On 18/10/19 6:12 PM, Simon Goldschmidt wrote: > On Fri, Oct 18, 2019 at 2:40 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> Hi, >> >> On 18/10/19 2:34 PM, Simon Goldschmidt wrote: >>> On Thu, Oct 17, 2019 at 2:55 PM Simon Goldschmidt >>> <simon.k.r.goldschmidt@gmail.com> wrote: >>>> >>>> On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>>>> >>>>> Hi, >>>>> >>>>> On 17/10/19 5:09 PM, Simon Goldschmidt wrote: >>>>>> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>>>>>> >>>>>>> Add support for Direct Access Controller mode of Cadence QSPI. This >>>>>>> allows MMIO access to SPI NOR flash providing better read performance. >>>>>>> >>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com> >>>>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>>>>> >>>>>> I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at >>>>>> 50MHz) and it seems to work fine. >>>>>> >>>>>> However, I had the impression it was a bit slower, not faster, although I >>>>>> haven't measured, and running at 50MHz with 4 data lines, reading the whole >>>>>> flash takes about 1.5 seconds only, so without actually measureing it, it's >>>>>> hard to tell if this performance is really worth the 440 bytes of extra code >>>>>> it adds? >>>>>> >>>>> >>>>> I should have noted in the commit msg that direct mode is used only when >>>>> AHB window is at least 8MB. Working with smaller window sizes would mean >>>>> code would fall back to indirect mode most of the time. >>>>> I see that socfgpa don't seem to have large AHB windows and therefore >>>>> would not use direct access mode. >>>> >>>> Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't >>>> changed. >>>> >>>>> >>>>> On TI platforms its possible to use DMA to read data from memory mapped >>>>> region using mem to mem DMA channels which improves throughput by 5 times. >>>> >>>> Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 >>>> seconds. The maximum speed of that setup would be 25 MByte/s without any >>>> overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement >>>> by 5 on my platform! >>>> >>>>> >>>>>> Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes >>>>>> do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes >>>>>> for me :-) >>>>>> >>>>> >>>>> Hmm, I read this as patch 1 brings down memory consumption by 320 bytes >>>>> and patch 2 adds 440 bytes (delta being +120 bytes for entire series). >>>>> I can see if size can be optimized, but would like to avoid #ifdef'ery >>>>> within the driver if possible. >>>> >>>> Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled >>>> at the same time in socfpga_socrates (moving more code to DM). And even if it >>>> sounds like not much, 440 bytes *are* much for me at this point. >>>> >>>> I still would prefer having a Kconfig option for this that can be disabled >>>> for socfpga. >>> >>> Oh well, maybe just go ahead adding this and I'll try how it fits. We can alway >>> add reduced functionality later by checking for CONFIG_SPL_SPI_FLASH_TINY, >>> whicht might be a better config option to guard this than something cadence-qspi >>> specific. >>> >> >> Okay, thanks! Guarding with CONFIG_SPL_SPI_FLASH_TINY as and where >> required seems fair idea. BTW, it would be great if you could confirm >> Quad read works fine when spi-tx-bus-width = 1 and spi-rx-bus-width = 4 >> as I suggested in other thread? > > Yes, I'll do that when I find the time. Unfortunately, I can only do > that on the devices > at work, not at home. Alright, thanks! > I also would like to check the memory-mapped code works on > socfpga by removing the 8MB size check, does that make sense? > Well, in that case memory-mapped mode will be used for accessing data within first 1MB of flash. Rest of the data would be accessed via indirect mode. I think if AHB window is small, then probably SoC designers did not intend, controller to be used in DAC or memory-mapped mode. Regards Vignesh > Regards, > Simon > >> >> Regards >> Vignesh >> >>> Regards, >>> Simon >>> >>>> >>>> Regards, >>>> Simon >>>> >>>>> >>>>> Regards >>>>> Vignesh >>>>> >>>>>> Regards, >>>>>> Simon >>>>>> >>>>>> >>>>>>> --- >>>>>>> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- >>>>>>> drivers/spi/cadence_qspi.h | 19 ++++++----- >>>>>>> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- >>>>>>> 3 files changed, 87 insertions(+), 33 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c >>>>>>> index 673a2e9a6c4c..6c98cbf39ae4 100644 >>>>>>> --- a/drivers/spi/cadence_qspi.c >>>>>>> +++ b/drivers/spi/cadence_qspi.c >>>>>>> @@ -12,12 +12,13 @@ >>>>>>> #include <spi.h> >>>>>>> #include <spi-mem.h> >>>>>>> #include <linux/errno.h> >>>>>>> +#include <linux/sizes.h> >>>>>>> #include "cadence_qspi.h" >>>>>>> >>>>>>> #define CQSPI_STIG_READ 0 >>>>>>> #define CQSPI_STIG_WRITE 1 >>>>>>> -#define CQSPI_INDIRECT_READ 2 >>>>>>> -#define CQSPI_INDIRECT_WRITE 3 >>>>>>> +#define CQSPI_READ 2 >>>>>>> +#define CQSPI_WRITE 3 >>>>>>> >>>>>>> static int cadence_spi_write_speed(struct udevice *bus, uint hz) >>>>>>> { >>>>>>> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) >>>>>>> >>>>>>> static int cadence_spi_set_mode(struct udevice *bus, uint mode) >>>>>>> { >>>>>>> + struct cadence_spi_platdata *plat = bus->platdata; >>>>>>> struct cadence_spi_priv *priv = dev_get_priv(bus); >>>>>>> >>>>>>> /* Disable QSPI */ >>>>>>> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) >>>>>>> /* Set SPI mode */ >>>>>>> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); >>>>>>> >>>>>>> + /* Enable Direct Access Controller */ >>>>>>> + if (plat->use_dac_mode) >>>>>>> + cadence_qspi_apb_dac_mode_enable(priv->regbase); >>>>>>> + >>>>>>> /* Enable QSPI */ >>>>>>> cadence_qspi_apb_controller_enable(priv->regbase); >>>>>>> >>>>>>> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >>>>>>> if (!op->addr.nbytes) >>>>>>> mode = CQSPI_STIG_READ; >>>>>>> else >>>>>>> - mode = CQSPI_INDIRECT_READ; >>>>>>> + mode = CQSPI_READ; >>>>>>> } else { >>>>>>> if (!op->addr.nbytes || !op->data.buf.out) >>>>>>> mode = CQSPI_STIG_WRITE; >>>>>>> else >>>>>>> - mode = CQSPI_INDIRECT_WRITE; >>>>>>> + mode = CQSPI_WRITE; >>>>>>> } >>>>>>> >>>>>>> switch (mode) { >>>>>>> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, >>>>>>> case CQSPI_STIG_WRITE: >>>>>>> err = cadence_qspi_apb_command_write(base, op); >>>>>>> break; >>>>>>> - case CQSPI_INDIRECT_READ: >>>>>>> - err = cadence_qspi_apb_indirect_read_setup(plat, op); >>>>>>> - if (!err) { >>>>>>> - err = cadence_qspi_apb_indirect_read_execute >>>>>>> - (plat, op->data.nbytes, op->data.buf.in); >>>>>>> - } >>>>>>> + case CQSPI_READ: >>>>>>> + err = cadence_qspi_apb_read_setup(plat, op); >>>>>>> + if (!err) >>>>>>> + err = cadence_qspi_apb_read_execute(plat, op); >>>>>>> break; >>>>>>> - case CQSPI_INDIRECT_WRITE: >>>>>>> - err = cadence_qspi_apb_indirect_write_setup(plat, op); >>>>>>> - if (!err) { >>>>>>> - err = cadence_qspi_apb_indirect_write_execute >>>>>>> - (plat, op->data.nbytes, op->data.buf.out); >>>>>>> - } >>>>>>> + case CQSPI_WRITE: >>>>>>> + err = cadence_qspi_apb_write_setup(plat, op); >>>>>>> + if (!err) >>>>>>> + err = cadence_qspi_apb_write_execute(plat, op); >>>>>>> break; >>>>>>> default: >>>>>>> err = -1; >>>>>>> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) >>>>>>> ofnode subnode; >>>>>>> >>>>>>> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); >>>>>>> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); >>>>>>> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, >>>>>>> + &plat->ahbsize); >>>>>>> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); >>>>>>> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); >>>>>>> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); >>>>>>> plat->trigger_address = dev_read_u32_default(bus, >>>>>>> "cdns,trigger-address", >>>>>>> 0); >>>>>>> + /* Use DAC mode only when MMIO window is at least 8M wide */ >>>>>>> + if (plat->ahbsize >= SZ_8M) >>>>>>> + plat->use_dac_mode = true; >>>>>>> >>>>>>> /* All other paramters are embedded in the child node */ >>>>>>> subnode = dev_read_first_subnode(bus); >>>>>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >>>>>>> index e655b027d788..619f0bed8efd 100644 >>>>>>> --- a/drivers/spi/cadence_qspi.h >>>>>>> +++ b/drivers/spi/cadence_qspi.h >>>>>>> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { >>>>>>> u32 fifo_depth; >>>>>>> u32 fifo_width; >>>>>>> u32 trigger_address; >>>>>>> + fdt_addr_t ahbsize; >>>>>>> + bool use_dac_mode; >>>>>>> >>>>>>> /* Flash parameters */ >>>>>>> u32 page_size; >>>>>>> @@ -52,20 +54,21 @@ struct cadence_spi_priv { >>>>>>> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); >>>>>>> void cadence_qspi_apb_controller_enable(void *reg_base_addr); >>>>>>> void cadence_qspi_apb_controller_disable(void *reg_base_addr); >>>>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); >>>>>>> >>>>>>> int cadence_qspi_apb_command_read(void *reg_base_addr, >>>>>>> const struct spi_mem_op *op); >>>>>>> int cadence_qspi_apb_command_write(void *reg_base_addr, >>>>>>> const struct spi_mem_op *op); >>>>>>> >>>>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>>>>> - const struct spi_mem_op *op); >>>>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>>>> - unsigned int rxlen, u8 *rxbuf); >>>>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>>>> - const struct spi_mem_op *op); >>>>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>>>> - unsigned int txlen, const u8 *txbuf); >>>>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op); >>>>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op); >>>>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op); >>>>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op); >>>>>>> >>>>>>> void cadence_qspi_apb_chipselect(void *reg_base, >>>>>>> unsigned int chip_select, unsigned int decoder_enable); >>>>>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >>>>>>> index 8dd0495dfcf4..fd491f2c8104 100644 >>>>>>> --- a/drivers/spi/cadence_qspi_apb.c >>>>>>> +++ b/drivers/spi/cadence_qspi_apb.c >>>>>>> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) >>>>>>> writel(reg, reg_base + CQSPI_REG_CONFIG); >>>>>>> } >>>>>>> >>>>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) >>>>>>> +{ >>>>>>> + unsigned int reg; >>>>>>> + >>>>>>> + reg = readl(reg_base + CQSPI_REG_CONFIG); >>>>>>> + reg |= CQSPI_REG_CONFIG_DIRECT; >>>>>>> + writel(reg, reg_base + CQSPI_REG_CONFIG); >>>>>>> +} >>>>>>> + >>>>>>> /* Return 1 if idle, otherwise return 0 (busy). */ >>>>>>> static unsigned int cadence_qspi_wait_idle(void *reg_base) >>>>>>> { >>>>>>> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) >>>>>>> } >>>>>>> >>>>>>> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ >>>>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >>>>>>> - const struct spi_mem_op *op) >>>>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op) >>>>>>> { >>>>>>> unsigned int reg; >>>>>>> unsigned int rd_reg; >>>>>>> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) >>>>>>> return -ETIMEDOUT; >>>>>>> } >>>>>>> >>>>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>>>> - unsigned int n_rx, u8 *rxbuf) >>>>>>> +static int >>>>>>> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, >>>>>>> + unsigned int n_rx, u8 *rxbuf) >>>>>>> { >>>>>>> unsigned int remaining = n_rx; >>>>>>> unsigned int bytes_to_read = 0; >>>>>>> @@ -639,9 +649,26 @@ failrd: >>>>>>> return ret; >>>>>>> } >>>>>>> >>>>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op) >>>>>>> +{ >>>>>>> + u32 from = op->addr.val; >>>>>>> + void *buf = op->data.buf.in; >>>>>>> + size_t len = op->data.nbytes; >>>>>>> + >>>>>>> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { >>>>>>> + memcpy_fromio(buf, plat->ahbbase + from, len); >>>>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) >>>>>>> + return -EIO; >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); >>>>>>> +} >>>>>>> + >>>>>>> /* Opcode + Address (3/4 bytes) */ >>>>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>>>> - const struct spi_mem_op *op) >>>>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op) >>>>>>> { >>>>>>> unsigned int reg; >>>>>>> >>>>>>> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>>>> - unsigned int n_tx, const u8 *txbuf) >>>>>>> +static int >>>>>>> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, >>>>>>> + unsigned int n_tx, const u8 *txbuf) >>>>>>> { >>>>>>> unsigned int page_size = plat->page_size; >>>>>>> unsigned int remaining = n_tx; >>>>>>> @@ -735,6 +763,23 @@ failwr: >>>>>>> return ret; >>>>>>> } >>>>>>> >>>>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, >>>>>>> + const struct spi_mem_op *op) >>>>>>> +{ >>>>>>> + u32 to = op->addr.val; >>>>>>> + const void *buf = op->data.buf.out; >>>>>>> + size_t len = op->data.nbytes; >>>>>>> + >>>>>>> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { >>>>>>> + memcpy_toio(plat->ahbbase + to, buf, len); >>>>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) >>>>>>> + return -EIO; >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); >>>>>>> +} >>>>>>> + >>>>>>> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) >>>>>>> { >>>>>>> unsigned int reg; >>>>>>> -- >>>>>>> 2.23.0 >>>>>>> >>>>> >>>>> -- >>>>> Regards >>>>> Vignesh >> >> -- >> Regards >> Vignesh -- Regards Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support 2019-10-23 10:01 ` Vignesh Raghavendra @ 2019-10-23 10:06 ` Simon Goldschmidt 0 siblings, 0 replies; 18+ messages in thread From: Simon Goldschmidt @ 2019-10-23 10:06 UTC (permalink / raw) To: u-boot On Wed, Oct 23, 2019 at 12:00 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > On 18/10/19 6:12 PM, Simon Goldschmidt wrote: > > On Fri, Oct 18, 2019 at 2:40 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> Hi, > >> > >> On 18/10/19 2:34 PM, Simon Goldschmidt wrote: > >>> On Thu, Oct 17, 2019 at 2:55 PM Simon Goldschmidt > >>> <simon.k.r.goldschmidt@gmail.com> wrote: > >>>> > >>>> On Thu, Oct 17, 2019 at 2:44 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >>>>> > >>>>> Hi, > >>>>> > >>>>> On 17/10/19 5:09 PM, Simon Goldschmidt wrote: > >>>>>> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >>>>>>> > >>>>>>> Add support for Direct Access Controller mode of Cadence QSPI. This > >>>>>>> allows MMIO access to SPI NOR flash providing better read performance. > >>>>>>> > >>>>>>> Signed-off-by: Vignesh R <vigneshr@ti.com> > >>>>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > >>>>>> > >>>>>> I've tested this on my socfpga cyclone5 board with an mt25ql256a (running at > >>>>>> 50MHz) and it seems to work fine. > >>>>>> > >>>>>> However, I had the impression it was a bit slower, not faster, although I > >>>>>> haven't measured, and running at 50MHz with 4 data lines, reading the whole > >>>>>> flash takes about 1.5 seconds only, so without actually measureing it, it's > >>>>>> hard to tell if this performance is really worth the 440 bytes of extra code > >>>>>> it adds? > >>>>>> > >>>>> > >>>>> I should have noted in the commit msg that direct mode is used only when > >>>>> AHB window is at least 8MB. Working with smaller window sizes would mean > >>>>> code would fall back to indirect mode most of the time. > >>>>> I see that socfgpa don't seem to have large AHB windows and therefore > >>>>> would not use direct access mode. > >>>> > >>>> Aha. Yes, socfpga gen5 has a 1MB window only, I think. So the speed hasn't > >>>> changed. > >>>> > >>>>> > >>>>> On TI platforms its possible to use DMA to read data from memory mapped > >>>>> region using mem to mem DMA channels which improves throughput by 5 times. > >>>> > >>>> Hmm, I'm using a QSPI at 50 MHz and transferring 31 MByte takes about 1.5 > >>>> seconds. The maximum speed of that setup would be 25 MByte/s without any > >>>> overhead (1.24 seconds for 31 MB). There's no way I could achieve an improvement > >>>> by 5 on my platform! > >>>> > >>>>> > >>>>>> Note that I'm already short on RAM in SPL (socfpga gen5), so these 440 bytes > >>>>>> do hurt. Note that patch 1/2 of this series reduced SPL code size by 320 bytes > >>>>>> for me :-) > >>>>>> > >>>>> > >>>>> Hmm, I read this as patch 1 brings down memory consumption by 320 bytes > >>>>> and patch 2 adds 440 bytes (delta being +120 bytes for entire series). > >>>>> I can see if size can be optimized, but would like to avoid #ifdef'ery > >>>>> within the driver if possible. > >>>> > >>>> Well, I can only say I'm currently struggling to keep SPI-NOR and MMC enabled > >>>> at the same time in socfpga_socrates (moving more code to DM). And even if it > >>>> sounds like not much, 440 bytes *are* much for me at this point. > >>>> > >>>> I still would prefer having a Kconfig option for this that can be disabled > >>>> for socfpga. > >>> > >>> Oh well, maybe just go ahead adding this and I'll try how it fits. We can alway > >>> add reduced functionality later by checking for CONFIG_SPL_SPI_FLASH_TINY, > >>> whicht might be a better config option to guard this than something cadence-qspi > >>> specific. > >>> > >> > >> Okay, thanks! Guarding with CONFIG_SPL_SPI_FLASH_TINY as and where > >> required seems fair idea. BTW, it would be great if you could confirm > >> Quad read works fine when spi-tx-bus-width = 1 and spi-rx-bus-width = 4 > >> as I suggested in other thread? > > > > Yes, I'll do that when I find the time. Unfortunately, I can only do > > that on the devices > > at work, not at home. > > Alright, thanks! > > > I also would like to check the memory-mapped code works on > > socfpga by removing the 8MB size check, does that make sense? > > > > Well, in that case memory-mapped mode will be used for accessing data > within first 1MB of flash. Rest of the data would be accessed via > indirect mode. > I think if AHB window is small, then probably SoC designers did not > intend, controller to be used in DAC or memory-mapped mode. Oh, well I thought you would then switch the window (register named 'remapaddr' for cyclone5 qspi)? I admit, I haven't looked at that code... Would it make sense to implement that? Sequential reads should still benefit even if switching the window every X MB. Regards, Simon > > Regards > Vignesh > > > Regards, > > Simon > > > >> > >> Regards > >> Vignesh > >> > >>> Regards, > >>> Simon > >>> > >>>> > >>>> Regards, > >>>> Simon > >>>> > >>>>> > >>>>> Regards > >>>>> Vignesh > >>>>> > >>>>>> Regards, > >>>>>> Simon > >>>>>> > >>>>>> > >>>>>>> --- > >>>>>>> drivers/spi/cadence_qspi.c | 40 ++++++++++++---------- > >>>>>>> drivers/spi/cadence_qspi.h | 19 ++++++----- > >>>>>>> drivers/spi/cadence_qspi_apb.c | 61 +++++++++++++++++++++++++++++----- > >>>>>>> 3 files changed, 87 insertions(+), 33 deletions(-) > >>>>>>> > >>>>>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > >>>>>>> index 673a2e9a6c4c..6c98cbf39ae4 100644 > >>>>>>> --- a/drivers/spi/cadence_qspi.c > >>>>>>> +++ b/drivers/spi/cadence_qspi.c > >>>>>>> @@ -12,12 +12,13 @@ > >>>>>>> #include <spi.h> > >>>>>>> #include <spi-mem.h> > >>>>>>> #include <linux/errno.h> > >>>>>>> +#include <linux/sizes.h> > >>>>>>> #include "cadence_qspi.h" > >>>>>>> > >>>>>>> #define CQSPI_STIG_READ 0 > >>>>>>> #define CQSPI_STIG_WRITE 1 > >>>>>>> -#define CQSPI_INDIRECT_READ 2 > >>>>>>> -#define CQSPI_INDIRECT_WRITE 3 > >>>>>>> +#define CQSPI_READ 2 > >>>>>>> +#define CQSPI_WRITE 3 > >>>>>>> > >>>>>>> static int cadence_spi_write_speed(struct udevice *bus, uint hz) > >>>>>>> { > >>>>>>> @@ -189,6 +190,7 @@ static int cadence_spi_remove(struct udevice *dev) > >>>>>>> > >>>>>>> static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >>>>>>> { > >>>>>>> + struct cadence_spi_platdata *plat = bus->platdata; > >>>>>>> struct cadence_spi_priv *priv = dev_get_priv(bus); > >>>>>>> > >>>>>>> /* Disable QSPI */ > >>>>>>> @@ -197,6 +199,10 @@ static int cadence_spi_set_mode(struct udevice *bus, uint mode) > >>>>>>> /* Set SPI mode */ > >>>>>>> cadence_qspi_apb_set_clk_mode(priv->regbase, mode); > >>>>>>> > >>>>>>> + /* Enable Direct Access Controller */ > >>>>>>> + if (plat->use_dac_mode) > >>>>>>> + cadence_qspi_apb_dac_mode_enable(priv->regbase); > >>>>>>> + > >>>>>>> /* Enable QSPI */ > >>>>>>> cadence_qspi_apb_controller_enable(priv->regbase); > >>>>>>> > >>>>>>> @@ -221,12 +227,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >>>>>>> if (!op->addr.nbytes) > >>>>>>> mode = CQSPI_STIG_READ; > >>>>>>> else > >>>>>>> - mode = CQSPI_INDIRECT_READ; > >>>>>>> + mode = CQSPI_READ; > >>>>>>> } else { > >>>>>>> if (!op->addr.nbytes || !op->data.buf.out) > >>>>>>> mode = CQSPI_STIG_WRITE; > >>>>>>> else > >>>>>>> - mode = CQSPI_INDIRECT_WRITE; > >>>>>>> + mode = CQSPI_WRITE; > >>>>>>> } > >>>>>>> > >>>>>>> switch (mode) { > >>>>>>> @@ -236,19 +242,15 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > >>>>>>> case CQSPI_STIG_WRITE: > >>>>>>> err = cadence_qspi_apb_command_write(base, op); > >>>>>>> break; > >>>>>>> - case CQSPI_INDIRECT_READ: > >>>>>>> - err = cadence_qspi_apb_indirect_read_setup(plat, op); > >>>>>>> - if (!err) { > >>>>>>> - err = cadence_qspi_apb_indirect_read_execute > >>>>>>> - (plat, op->data.nbytes, op->data.buf.in); > >>>>>>> - } > >>>>>>> + case CQSPI_READ: > >>>>>>> + err = cadence_qspi_apb_read_setup(plat, op); > >>>>>>> + if (!err) > >>>>>>> + err = cadence_qspi_apb_read_execute(plat, op); > >>>>>>> break; > >>>>>>> - case CQSPI_INDIRECT_WRITE: > >>>>>>> - err = cadence_qspi_apb_indirect_write_setup(plat, op); > >>>>>>> - if (!err) { > >>>>>>> - err = cadence_qspi_apb_indirect_write_execute > >>>>>>> - (plat, op->data.nbytes, op->data.buf.out); > >>>>>>> - } > >>>>>>> + case CQSPI_WRITE: > >>>>>>> + err = cadence_qspi_apb_write_setup(plat, op); > >>>>>>> + if (!err) > >>>>>>> + err = cadence_qspi_apb_write_execute(plat, op); > >>>>>>> break; > >>>>>>> default: > >>>>>>> err = -1; > >>>>>>> @@ -264,13 +266,17 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) > >>>>>>> ofnode subnode; > >>>>>>> > >>>>>>> plat->regbase = (void *)devfdt_get_addr_index(bus, 0); > >>>>>>> - plat->ahbbase = (void *)devfdt_get_addr_index(bus, 1); > >>>>>>> + plat->ahbbase = (void *)devfdt_get_addr_size_index(bus, 1, > >>>>>>> + &plat->ahbsize); > >>>>>>> plat->is_decoded_cs = dev_read_bool(bus, "cdns,is-decoded-cs"); > >>>>>>> plat->fifo_depth = dev_read_u32_default(bus, "cdns,fifo-depth", 128); > >>>>>>> plat->fifo_width = dev_read_u32_default(bus, "cdns,fifo-width", 4); > >>>>>>> plat->trigger_address = dev_read_u32_default(bus, > >>>>>>> "cdns,trigger-address", > >>>>>>> 0); > >>>>>>> + /* Use DAC mode only when MMIO window is at least 8M wide */ > >>>>>>> + if (plat->ahbsize >= SZ_8M) > >>>>>>> + plat->use_dac_mode = true; > >>>>>>> > >>>>>>> /* All other paramters are embedded in the child node */ > >>>>>>> subnode = dev_read_first_subnode(bus); > >>>>>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > >>>>>>> index e655b027d788..619f0bed8efd 100644 > >>>>>>> --- a/drivers/spi/cadence_qspi.h > >>>>>>> +++ b/drivers/spi/cadence_qspi.h > >>>>>>> @@ -23,6 +23,8 @@ struct cadence_spi_platdata { > >>>>>>> u32 fifo_depth; > >>>>>>> u32 fifo_width; > >>>>>>> u32 trigger_address; > >>>>>>> + fdt_addr_t ahbsize; > >>>>>>> + bool use_dac_mode; > >>>>>>> > >>>>>>> /* Flash parameters */ > >>>>>>> u32 page_size; > >>>>>>> @@ -52,20 +54,21 @@ struct cadence_spi_priv { > >>>>>>> void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat); > >>>>>>> void cadence_qspi_apb_controller_enable(void *reg_base_addr); > >>>>>>> void cadence_qspi_apb_controller_disable(void *reg_base_addr); > >>>>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base); > >>>>>>> > >>>>>>> int cadence_qspi_apb_command_read(void *reg_base_addr, > >>>>>>> const struct spi_mem_op *op); > >>>>>>> int cadence_qspi_apb_command_write(void *reg_base_addr, > >>>>>>> const struct spi_mem_op *op); > >>>>>>> > >>>>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >>>>>>> - const struct spi_mem_op *op); > >>>>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>>>> - unsigned int rxlen, u8 *rxbuf); > >>>>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>>>> - const struct spi_mem_op *op); > >>>>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>>>> - unsigned int txlen, const u8 *txbuf); > >>>>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op); > >>>>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op); > >>>>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op); > >>>>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op); > >>>>>>> > >>>>>>> void cadence_qspi_apb_chipselect(void *reg_base, > >>>>>>> unsigned int chip_select, unsigned int decoder_enable); > >>>>>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >>>>>>> index 8dd0495dfcf4..fd491f2c8104 100644 > >>>>>>> --- a/drivers/spi/cadence_qspi_apb.c > >>>>>>> +++ b/drivers/spi/cadence_qspi_apb.c > >>>>>>> @@ -189,6 +189,15 @@ void cadence_qspi_apb_controller_disable(void *reg_base) > >>>>>>> writel(reg, reg_base + CQSPI_REG_CONFIG); > >>>>>>> } > >>>>>>> > >>>>>>> +void cadence_qspi_apb_dac_mode_enable(void *reg_base) > >>>>>>> +{ > >>>>>>> + unsigned int reg; > >>>>>>> + > >>>>>>> + reg = readl(reg_base + CQSPI_REG_CONFIG); > >>>>>>> + reg |= CQSPI_REG_CONFIG_DIRECT; > >>>>>>> + writel(reg, reg_base + CQSPI_REG_CONFIG); > >>>>>>> +} > >>>>>>> + > >>>>>>> /* Return 1 if idle, otherwise return 0 (busy). */ > >>>>>>> static unsigned int cadence_qspi_wait_idle(void *reg_base) > >>>>>>> { > >>>>>>> @@ -512,8 +521,8 @@ int cadence_qspi_apb_command_write(void *reg_base, const struct spi_mem_op *op) > >>>>>>> } > >>>>>>> > >>>>>>> /* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */ > >>>>>>> -int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >>>>>>> - const struct spi_mem_op *op) > >>>>>>> +int cadence_qspi_apb_read_setup(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op) > >>>>>>> { > >>>>>>> unsigned int reg; > >>>>>>> unsigned int rd_reg; > >>>>>>> @@ -577,8 +586,9 @@ static int cadence_qspi_wait_for_data(struct cadence_spi_platdata *plat) > >>>>>>> return -ETIMEDOUT; > >>>>>>> } > >>>>>>> > >>>>>>> -int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>>>> - unsigned int n_rx, u8 *rxbuf) > >>>>>>> +static int > >>>>>>> +cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, > >>>>>>> + unsigned int n_rx, u8 *rxbuf) > >>>>>>> { > >>>>>>> unsigned int remaining = n_rx; > >>>>>>> unsigned int bytes_to_read = 0; > >>>>>>> @@ -639,9 +649,26 @@ failrd: > >>>>>>> return ret; > >>>>>>> } > >>>>>>> > >>>>>>> +int cadence_qspi_apb_read_execute(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op) > >>>>>>> +{ > >>>>>>> + u32 from = op->addr.val; > >>>>>>> + void *buf = op->data.buf.in; > >>>>>>> + size_t len = op->data.nbytes; > >>>>>>> + > >>>>>>> + if (plat->use_dac_mode && (from + len < plat->ahbsize)) { > >>>>>>> + memcpy_fromio(buf, plat->ahbbase + from, len); > >>>>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) > >>>>>>> + return -EIO; > >>>>>>> + return 0; > >>>>>>> + } > >>>>>>> + > >>>>>>> + return cadence_qspi_apb_indirect_read_execute(plat, len, buf); > >>>>>>> +} > >>>>>>> + > >>>>>>> /* Opcode + Address (3/4 bytes) */ > >>>>>>> -int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>>>> - const struct spi_mem_op *op) > >>>>>>> +int cadence_qspi_apb_write_setup(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op) > >>>>>>> { > >>>>>>> unsigned int reg; > >>>>>>> > >>>>>>> @@ -662,8 +689,9 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, > >>>>>>> return 0; > >>>>>>> } > >>>>>>> > >>>>>>> -int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>>>> - unsigned int n_tx, const u8 *txbuf) > >>>>>>> +static int > >>>>>>> +cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, > >>>>>>> + unsigned int n_tx, const u8 *txbuf) > >>>>>>> { > >>>>>>> unsigned int page_size = plat->page_size; > >>>>>>> unsigned int remaining = n_tx; > >>>>>>> @@ -735,6 +763,23 @@ failwr: > >>>>>>> return ret; > >>>>>>> } > >>>>>>> > >>>>>>> +int cadence_qspi_apb_write_execute(struct cadence_spi_platdata *plat, > >>>>>>> + const struct spi_mem_op *op) > >>>>>>> +{ > >>>>>>> + u32 to = op->addr.val; > >>>>>>> + const void *buf = op->data.buf.out; > >>>>>>> + size_t len = op->data.nbytes; > >>>>>>> + > >>>>>>> + if (plat->use_dac_mode && (to + len < plat->ahbsize)) { > >>>>>>> + memcpy_toio(plat->ahbbase + to, buf, len); > >>>>>>> + if (!cadence_qspi_wait_idle(plat->regbase)) > >>>>>>> + return -EIO; > >>>>>>> + return 0; > >>>>>>> + } > >>>>>>> + > >>>>>>> + return cadence_qspi_apb_indirect_write_execute(plat, len, buf); > >>>>>>> +} > >>>>>>> + > >>>>>>> void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy) > >>>>>>> { > >>>>>>> unsigned int reg; > >>>>>>> -- > >>>>>>> 2.23.0 > >>>>>>> > >>>>> > >>>>> -- > >>>>> Regards > >>>>> Vignesh > >> > >> -- > >> Regards > >> Vignesh > > -- > Regards > Vignesh ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-01-17 15:36 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-14 13:27 [U-Boot] [PATCH 0/2] spi: cadence-qspi: Move to spi-mem APIs Vignesh Raghavendra 2019-10-14 13:27 ` [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework Vignesh Raghavendra 2019-10-17 11:20 ` Simon Goldschmidt 2019-10-17 12:31 ` Vignesh Raghavendra 2019-11-06 19:55 ` Simon Goldschmidt 2019-11-07 6:09 ` Vignesh Raghavendra 2020-01-17 15:33 ` Simon Goldschmidt 2019-10-17 13:48 ` [U-Boot] " Tudor.Ambarus at microchip.com 2020-01-17 15:36 ` Simon Goldschmidt 2019-10-14 13:27 ` [U-Boot] [PATCH 2/2] spi: cadence-qspi: Add direct mode support Vignesh Raghavendra 2019-10-17 11:39 ` Simon Goldschmidt 2019-10-17 12:44 ` Vignesh Raghavendra 2019-10-17 12:55 ` Simon Goldschmidt 2019-10-18 9:04 ` Simon Goldschmidt 2019-10-18 12:40 ` Vignesh Raghavendra 2019-10-18 12:42 ` Simon Goldschmidt 2019-10-23 10:01 ` Vignesh Raghavendra 2019-10-23 10:06 ` Simon Goldschmidt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.