On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigne...@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 <vigne...@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 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot