2020-12-22 18:46:48

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

Hi,

This series adds support for Octal DTR mode now that SPI NOR supports
these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
Patche 5/7 lays some groundwork by implementing the supports_op() hook.
Patch 7/7 adds the Octal DTR mode support.

While the main aim of this series is to support 8D-8D-8D mode, other
modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
they have not been tested.

Tested on J721E with Micron MT35XU512ABA and on J7200 with Cypress
S28HS512T. Tested on J721E with Micron MT25QU512A (1S-1S-4S) for
regressions.

Pratyush Yadav (7):
spi: cadence-quadspi: Set master max_speed_hz
spi: cadence-quadspi: Abort read if dummy cycles required are too many
spi: cadence-quadspi: Set dummy cycles from STIG commands
spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
spi: cadence-quadspi: Implement a simple supports_op hook
spi: cadence-quadspi: Wait at least 500 ms for direct reads
spi: cadence-quadspi: Add DTR support

drivers/spi/spi-cadence-quadspi.c | 364 ++++++++++++++++++++++++++----
1 file changed, 325 insertions(+), 39 deletions(-)

--
2.28.0


2020-12-22 18:46:55

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands

If a command does not have an address phase it goes via the STIG path.
The dummy cycles are not initialized for the STIG commands. As a result,
STIG commands with dummy cycles will not work.

Initialize the dummy cycle field before issuing the STIG command to make
sure it is sent correctly. Move the code to calculate dummy cycle value
to a separate function so it is not repeated twice. DTR support will add
some more logic here to it is worth it to extract it out in a function.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 5efb1f929be0..6a778014ff60 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -188,6 +188,7 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_CMDCTRL 0x90
#define CQSPI_REG_CMDCTRL_EXECUTE_MASK BIT(0)
#define CQSPI_REG_CMDCTRL_INPROGRESS_MASK BIT(1)
+#define CQSPI_REG_CMDCTRL_DUMMY_LSB 7
#define CQSPI_REG_CMDCTRL_WR_BYTES_LSB 12
#define CQSPI_REG_CMDCTRL_WR_EN_LSB 15
#define CQSPI_REG_CMDCTRL_ADD_BYTES_LSB 16
@@ -198,6 +199,7 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_CMDCTRL_WR_BYTES_MASK 0x7
#define CQSPI_REG_CMDCTRL_ADD_BYTES_MASK 0x3
#define CQSPI_REG_CMDCTRL_RD_BYTES_MASK 0x7
+#define CQSPI_REG_CMDCTRL_DUMMY_MASK 0x1F

#define CQSPI_REG_INDIRECTWR 0x70
#define CQSPI_REG_INDIRECTWR_START_MASK BIT(0)
@@ -288,6 +290,15 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
return rdreg;
}

+static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
+{
+ unsigned int dummy_clk;
+
+ dummy_clk = op->dummy.nbytes * 8;
+
+ return dummy_clk;
+}
+
static int cqspi_wait_idle(struct cqspi_st *cqspi)
{
const unsigned int poll_idle_retry = 3;
@@ -355,6 +366,7 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
size_t n_rx = op->data.nbytes;
unsigned int rdreg;
unsigned int reg;
+ unsigned int dummy_clk;
size_t read_len;
int status;

@@ -370,6 +382,14 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
rdreg = cqspi_calc_rdreg(f_pdata);
writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);

+ dummy_clk = cqspi_calc_dummy(op);
+ if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
+ return -EOPNOTSUPP;
+
+ if (dummy_clk)
+ reg |= (dummy_clk & CQSPI_REG_CMDCTRL_DUMMY_MASK)
+ << CQSPI_REG_CMDCTRL_DUMMY_LSB;
+
reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);

/* 0 means 1 byte. */
@@ -459,7 +479,8 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
reg |= cqspi_calc_rdreg(f_pdata);

/* Setup dummy clock cycles */
- dummy_clk = op->dummy.nbytes * 8;
+ dummy_clk = cqspi_calc_dummy(op);
+
if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
return -EOPNOTSUPP;

--
2.28.0

2020-12-22 18:46:59

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 1/7] spi: cadence-quadspi: Set master max_speed_hz

As of commit 9326e4f1e5dd ("spi: Limit the spi device max speed to
controller's max speed"), the SPI device max speed is set to the
controller's max speed if it is larger. The Cadence QSPI controller does
not set the controller's max speed so it is left at its initial value of
0. This means the SPI device max speed is always set to 0.

The SPI device max speed is used to calculate the baud rate divider when
performing an operation. If this speed is 0, the default divider of 32
is used. No matter what speed is specified by the device tree property
'spi-max-frequency', the device will always operate at ref_clk / 32.

Fix this by setting master->max_speed_hz to the ref clock speed so the
correct divider can be calculated.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ba7d40c2922f..ea3890c7d9ff 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1279,6 +1279,7 @@ static int cqspi_probe(struct platform_device *pdev)
reset_control_deassert(rstc_ocp);

cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
+ master->max_speed_hz = cqspi->master_ref_clk_hz;
ddata = of_device_get_match_data(dev);
if (ddata) {
if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
--
2.28.0

2020-12-22 18:47:32

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 5/7] spi: cadence-quadspi: Implement a simple supports_op hook

The default SPI MEM supports_op hook rejects DTR ops by default. Add a
simple supports_op hook that very closely imitates the SPI MEM one. It
will be extended in later commits to allow DTR ops.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 61 +++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 376abef43530..1781d4e94ebd 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1031,6 +1031,66 @@ static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
return ret;
}

+static int cqspi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
+{
+ u32 mode = mem->spi->mode;
+
+ switch (buswidth) {
+ case 1:
+ return 0;
+
+ case 2:
+ if ((tx &&
+ (mode & (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL))) ||
+ (!tx &&
+ (mode & (SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL))))
+ return 0;
+
+ break;
+
+ case 4:
+ if ((tx && (mode & (SPI_TX_QUAD | SPI_TX_OCTAL))) ||
+ (!tx && (mode & (SPI_RX_QUAD | SPI_RX_OCTAL))))
+ return 0;
+
+ break;
+
+ case 8:
+ if ((tx && (mode & SPI_TX_OCTAL)) ||
+ (!tx && (mode & SPI_RX_OCTAL)))
+ return 0;
+
+ break;
+
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static bool cqspi_supports_mem_op(struct spi_mem *mem,
+ const struct spi_mem_op *op)
+{
+ if (cqspi_check_buswidth_req(mem, op->cmd.buswidth, true))
+ return false;
+
+ if (op->addr.nbytes &&
+ cqspi_check_buswidth_req(mem, op->addr.buswidth, true))
+ return false;
+
+ if (op->dummy.nbytes &&
+ cqspi_check_buswidth_req(mem, op->dummy.buswidth, true))
+ return false;
+
+ if (op->data.nbytes &&
+ cqspi_check_buswidth_req(mem, op->data.buswidth,
+ op->data.dir == SPI_MEM_DATA_OUT))
+ return false;
+
+ return true;
+}
+
static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
struct cqspi_flash_pdata *f_pdata,
struct device_node *np)
@@ -1159,6 +1219,7 @@ static const char *cqspi_get_name(struct spi_mem *mem)
static const struct spi_controller_mem_ops cqspi_mem_ops = {
.exec_op = cqspi_exec_mem_op,
.get_name = cqspi_get_name,
+ .supports_op = cqspi_supports_mem_op,
};

static int cqspi_setup_flash(struct cqspi_st *cqspi)
--
2.28.0

2020-12-22 18:47:38

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 7/7] spi: cadence-quadspi: Add DTR support

Double Transfer Rate (DTR) mode transfers data twice per clock cycle.
Add support for parsing DTR ops and set up the registers to allow it.

Most SPI NOR flashes expect 2 byte commands. Parse the 2-byte opcode
from SPI MEM and set it up in the CQSPI_REG_OP_EXT_LOWER register.

Increment the delay needed before issuing indirect writes because larger
delay is needed for DTR mode. With the current delay some writes end up
missing.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 281 +++++++++++++++++++++++++-----
1 file changed, 242 insertions(+), 39 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 90040664e1b9..06a65e9a8a60 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -52,6 +52,7 @@ struct cqspi_flash_pdata {
u8 inst_width;
u8 addr_width;
u8 data_width;
+ bool dtr;
u8 cs;
};

@@ -111,6 +112,8 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_CONFIG_CHIPSELECT_LSB 10
#define CQSPI_REG_CONFIG_DMA_MASK BIT(15)
#define CQSPI_REG_CONFIG_BAUD_LSB 19
+#define CQSPI_REG_CONFIG_DTR_PROTO BIT(24)
+#define CQSPI_REG_CONFIG_DUAL_OPCODE BIT(30)
#define CQSPI_REG_CONFIG_IDLE_LSB 31
#define CQSPI_REG_CONFIG_CHIPSELECT_MASK 0xF
#define CQSPI_REG_CONFIG_BAUD_MASK 0xF
@@ -173,6 +176,9 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_SDRAMLEVEL_RD_MASK 0xFFFF
#define CQSPI_REG_SDRAMLEVEL_WR_MASK 0xFFFF

+#define CQSPI_REG_WR_COMPLETION_CTRL 0x38
+#define CQSPI_REG_WR_DISABLE_AUTO_POLL BIT(14)
+
#define CQSPI_REG_IRQSTATUS 0x40
#define CQSPI_REG_IRQMASK 0x44

@@ -216,6 +222,14 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC

+#define CQSPI_REG_POLLING_STATUS 0xB0
+#define CQSPI_REG_POLLING_STATUS_DUMMY_LSB 16
+
+#define CQSPI_REG_OP_EXT_LOWER 0xE0
+#define CQSPI_REG_OP_EXT_READ_LSB 24
+#define CQSPI_REG_OP_EXT_WRITE_LSB 16
+#define CQSPI_REG_OP_EXT_STIG_LSB 0
+
/* Interrupt status bits */
#define CQSPI_REG_IRQ_MODE_ERR BIT(0)
#define CQSPI_REG_IRQ_UNDERFLOW BIT(1)
@@ -290,15 +304,80 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
return rdreg;
}

-static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
+static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr)
{
unsigned int dummy_clk;

dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
+ if (dtr)
+ dummy_clk /= 2;

return dummy_clk;
}

+static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
+ const struct spi_mem_op *op)
+{
+ f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
+ f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
+ f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
+ f_pdata->dtr = op->data.dtr && op->cmd.dtr && op->addr.dtr;
+
+ switch (op->data.buswidth) {
+ case 0:
+ break;
+ case 1:
+ f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
+ break;
+ case 2:
+ f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
+ break;
+ case 4:
+ f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
+ break;
+ case 8:
+ f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Right now we only support 8-8-8 DTR mode. */
+ if (f_pdata->dtr) {
+ switch (op->cmd.buswidth) {
+ case 0:
+ break;
+ case 8:
+ f_pdata->inst_width = CQSPI_INST_TYPE_OCTAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (op->addr.buswidth) {
+ case 0:
+ break;
+ case 8:
+ f_pdata->addr_width = CQSPI_INST_TYPE_OCTAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (op->data.buswidth) {
+ case 0:
+ break;
+ case 8:
+ f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int cqspi_wait_idle(struct cqspi_st *cqspi)
{
const unsigned int poll_idle_retry = 3;
@@ -356,13 +435,69 @@ static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
return cqspi_wait_idle(cqspi);
}

+static int cqspi_setup_opcode_ext(struct cqspi_flash_pdata *f_pdata,
+ const struct spi_mem_op *op,
+ unsigned int shift)
+{
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+ u8 ext;
+
+ if (op->cmd.nbytes != 2)
+ return -EINVAL;
+
+ /* Opcode extension is the LSB. */
+ ext = op->cmd.opcode & 0xff;
+
+ reg = readl(reg_base + CQSPI_REG_OP_EXT_LOWER);
+ reg &= ~(0xff << shift);
+ reg |= ext << shift;
+ writel(reg, reg_base + CQSPI_REG_OP_EXT_LOWER);
+
+ return 0;
+}
+
+static int cqspi_enable_dtr(struct cqspi_flash_pdata *f_pdata,
+ const struct spi_mem_op *op, unsigned int shift,
+ bool enable)
+{
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+ int ret;
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+
+ /*
+ * We enable dual byte opcode here. The callers have to set up the
+ * extension opcode based on which type of operation it is.
+ */
+ if (enable) {
+ reg |= CQSPI_REG_CONFIG_DTR_PROTO;
+ reg |= CQSPI_REG_CONFIG_DUAL_OPCODE;
+
+ /* Set up command opcode extension. */
+ ret = cqspi_setup_opcode_ext(f_pdata, op, shift);
+ if (ret)
+ return ret;
+ } else {
+ reg &= ~CQSPI_REG_CONFIG_DTR_PROTO;
+ reg &= ~CQSPI_REG_CONFIG_DUAL_OPCODE;
+ }
+
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ return cqspi_wait_idle(cqspi);
+}
+
static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
const struct spi_mem_op *op)
{
struct cqspi_st *cqspi = f_pdata->cqspi;
void __iomem *reg_base = cqspi->iobase;
u8 *rxbuf = op->data.buf.in;
- u8 opcode = op->cmd.opcode;
+ u8 opcode;
size_t n_rx = op->data.nbytes;
unsigned int rdreg;
unsigned int reg;
@@ -370,6 +505,15 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
size_t read_len;
int status;

+ status = cqspi_set_protocol(f_pdata, op);
+ if (status)
+ return status;
+
+ status = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_STIG_LSB,
+ f_pdata->dtr);
+ if (status)
+ return status;
+
if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
dev_err(&cqspi->pdev->dev,
"Invalid input argument, len %zu rxbuf 0x%p\n",
@@ -377,12 +521,17 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
return -EINVAL;
}

+ if (f_pdata->dtr)
+ opcode = op->cmd.opcode >> 8;
+ else
+ opcode = op->cmd.opcode;
+
reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;

rdreg = cqspi_calc_rdreg(f_pdata);
writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);

- dummy_clk = cqspi_calc_dummy(op);
+ dummy_clk = cqspi_calc_dummy(op, f_pdata->dtr);
if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
return -EOPNOTSUPP;

@@ -421,12 +570,22 @@ static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
{
struct cqspi_st *cqspi = f_pdata->cqspi;
void __iomem *reg_base = cqspi->iobase;
- const u8 opcode = op->cmd.opcode;
+ u8 opcode;
const u8 *txbuf = op->data.buf.out;
size_t n_tx = op->data.nbytes;
unsigned int reg;
unsigned int data;
size_t write_len;
+ int ret;
+
+ ret = cqspi_set_protocol(f_pdata, op);
+ if (ret)
+ return ret;
+
+ ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_STIG_LSB,
+ f_pdata->dtr);
+ if (ret)
+ return ret;

if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
dev_err(&cqspi->pdev->dev,
@@ -435,6 +594,14 @@ static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
return -EINVAL;
}

+ reg = cqspi_calc_rdreg(f_pdata);
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+
+ if (f_pdata->dtr)
+ opcode = op->cmd.opcode >> 8;
+ else
+ opcode = op->cmd.opcode;
+
reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;

if (op->addr.nbytes) {
@@ -474,12 +641,24 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
void __iomem *reg_base = cqspi->iobase;
unsigned int dummy_clk = 0;
unsigned int reg;
+ int ret;
+ u8 opcode;

- reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
+ ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_READ_LSB,
+ f_pdata->dtr);
+ if (ret)
+ return ret;
+
+ if (f_pdata->dtr)
+ opcode = op->cmd.opcode >> 8;
+ else
+ opcode = op->cmd.opcode;
+
+ reg = opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
reg |= cqspi_calc_rdreg(f_pdata);

/* Setup dummy clock cycles */
- dummy_clk = cqspi_calc_dummy(op);
+ dummy_clk = cqspi_calc_dummy(op, f_pdata->dtr);

if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
return -EOPNOTSUPP;
@@ -594,15 +773,43 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
const struct spi_mem_op *op)
{
unsigned int reg;
+ int ret;
struct cqspi_st *cqspi = f_pdata->cqspi;
void __iomem *reg_base = cqspi->iobase;
+ u8 opcode;
+
+ ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_WRITE_LSB,
+ f_pdata->dtr);
+ if (ret)
+ return ret;
+
+ if (f_pdata->dtr)
+ opcode = op->cmd.opcode >> 8;
+ else
+ opcode = op->cmd.opcode;

/* Set opcode. */
- reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+ reg = opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+ reg |= f_pdata->data_width << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB;
+ reg |= f_pdata->addr_width << CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB;
writel(reg, reg_base + CQSPI_REG_WR_INSTR);
reg = cqspi_calc_rdreg(f_pdata);
writel(reg, reg_base + CQSPI_REG_RD_INSTR);

+ if (f_pdata->dtr) {
+ /*
+ * Some flashes like the cypress Semper flash expect a 4-byte
+ * dummy address with the Read SR command in DTR mode, but this
+ * controller does not support sending address with the Read SR
+ * command. So, disable write completion polling on the
+ * controller's side. spi-nor will take care of polling the
+ * status register.
+ */
+ reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+ reg |= CQSPI_REG_WR_DISABLE_AUTO_POLL;
+ writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+ }
+
reg = readl(reg_base + CQSPI_REG_SIZE);
reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
reg |= (op->addr.nbytes - 1);
@@ -856,35 +1063,6 @@ static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
cqspi_controller_enable(cqspi, 1);
}

-static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
- const struct spi_mem_op *op)
-{
- f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
- f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
- f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
-
- if (op->data.dir == SPI_MEM_DATA_IN) {
- switch (op->data.buswidth) {
- case 1:
- f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
- break;
- case 2:
- f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
- break;
- case 4:
- f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
- break;
- case 8:
- f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
- break;
- default:
- return -EINVAL;
- }
- }
-
- return 0;
-}
-
static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
const struct spi_mem_op *op)
{
@@ -902,7 +1080,16 @@ static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
if (ret)
return ret;

- if (cqspi->use_direct_mode && ((to + len) <= cqspi->ahb_size)) {
+ /*
+ * Some flashes like the Cypress Semper flash expect a dummy 4-byte
+ * address (all 0s) with the read status register command in DTR mode.
+ * But this controller does not support sending dummy address bytes to
+ * the flash when it is polling the write completion register in DTR
+ * mode. So, we can not use direct mode when in DTR mode for writing
+ * data.
+ */
+ if (!f_pdata->dtr && cqspi->use_direct_mode &&
+ ((to + len) <= cqspi->ahb_size)) {
memcpy_toio(cqspi->ahb_base + to, buf, len);
return cqspi_wait_idle(cqspi);
}
@@ -1072,6 +1259,8 @@ static int cqspi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
static bool cqspi_supports_mem_op(struct spi_mem *mem,
const struct spi_mem_op *op)
{
+ bool all_true, all_false;
+
if (cqspi_check_buswidth_req(mem, op->cmd.buswidth, true))
return false;

@@ -1088,6 +1277,19 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
op->data.dir == SPI_MEM_DATA_OUT))
return false;

+ all_true = op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
+ op->data.dtr;
+ all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
+ !op->data.dtr;
+
+ /* Mixed DTR modes not supported. */
+ if (!(all_true || all_false))
+ return false;
+
+ /* DTR mode opcodes should be 2 bytes. */
+ if (all_true && op->cmd.nbytes != 2)
+ return false;
+
return true;
}

@@ -1365,10 +1567,10 @@ static int cqspi_probe(struct platform_device *pdev)
ddata = of_device_get_match_data(dev);
if (ddata) {
if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
- cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+ cqspi->wr_delay = 50 * DIV_ROUND_UP(NSEC_PER_SEC,
cqspi->master_ref_clk_hz);
if (ddata->hwcaps_mask & CQSPI_SUPPORTS_OCTAL)
- master->mode_bits |= SPI_RX_OCTAL;
+ master->mode_bits |= SPI_RX_OCTAL | SPI_TX_OCTAL;
if (!(ddata->quirks & CQSPI_DISABLE_DAC_MODE))
cqspi->use_direct_mode = true;
}
@@ -1510,3 +1712,4 @@ MODULE_AUTHOR("Ley Foon Tan <[email protected]>");
MODULE_AUTHOR("Graham Moore <[email protected]>");
MODULE_AUTHOR("Vadivel Murugan R <[email protected]>");
MODULE_AUTHOR("Vignesh Raghavendra <[email protected]>");
+MODULE_AUTHOR("Pratyush Yadav <[email protected]>");
--
2.28.0

2020-12-22 18:47:42

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many

The controller can only support up to 31 dummy cycles. If the command
requires more it falls back to using 31. This command is likely to fail
because the correct number of cycles are not waited upon. Rather than
silently issuing an incorrect command, fail loudly so the caller can get
a chance to find out the command can't be supported by the controller.

Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller")
Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ea3890c7d9ff..5efb1f929be0 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -461,7 +461,7 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
/* Setup dummy clock cycles */
dummy_clk = op->dummy.nbytes * 8;
if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
- dummy_clk = CQSPI_DUMMY_CLKS_MAX;
+ return -EOPNOTSUPP;

if (dummy_clk)
reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
--
2.28.0

2020-12-22 18:48:28

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1

SPI MEM deals with dummy bytes but the controller deals with dummy
cycles. Multiplying bytes by 8 is correct if the dummy phase uses 1S
mode since 1 byte will be sent in 8 cycles. But if the dummy phase uses
4S mode then 1 byte will be sent in 2 cycles.

To correctly translate dummy bytes to dummy cycles, the dummy buswidth
also needs to be taken into account. Divide 8 by the buswidth to get the
correct multiplier for getting the number of cycles.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 6a778014ff60..376abef43530 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -294,7 +294,7 @@ static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
{
unsigned int dummy_clk;

- dummy_clk = op->dummy.nbytes * 8;
+ dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);

return dummy_clk;
}
--
2.28.0

2020-12-22 18:48:39

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

When performing a direct read via DMA the timeout for completion is set
equal to the read length. This is fine for larger reads. For a small
read like the Read Status Register command, the timeout would be 1 or 2
milliseconds. This is not enough to cover the overhead needed in setting
up DMA.

Make sure the timeout is at least 500 ms to allow DMA ample time to
finish. For reads larger than 500 bytes, the timeout will continue to be
equal to the read length.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 1781d4e94ebd..90040664e1b9 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -963,7 +963,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,

dma_async_issue_pending(cqspi->rx_chan);
if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
- msecs_to_jiffies(len))) {
+ msecs_to_jiffies(max(len, 500UL)))) {
dmaengine_terminate_sync(cqspi->rx_chan);
dev_err(dev, "DMA wait_for_completion_timeout\n");
ret = -ETIMEDOUT;
--
2.28.0

2020-12-29 03:32:59

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

Hi Pratyush,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: arm-randconfig-r006-20201221 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
msecs_to_jiffies(max(len, 500UL)))) {
^~~~~~~~~~~~~~~
include/linux/minmax.h:58:19: note: expanded from macro 'max'
#define max(x, y) __careful_cmp(x, y, >)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~~~~~~~
include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~~~~~~~
include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
1 warning generated.


vim +966 drivers/spi/spi-cadence-quadspi.c

919
920 static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
921 u_char *buf, loff_t from, size_t len)
922 {
923 struct cqspi_st *cqspi = f_pdata->cqspi;
924 struct device *dev = &cqspi->pdev->dev;
925 enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
926 dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
927 int ret = 0;
928 struct dma_async_tx_descriptor *tx;
929 dma_cookie_t cookie;
930 dma_addr_t dma_dst;
931 struct device *ddev;
932
933 if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
934 memcpy_fromio(buf, cqspi->ahb_base + from, len);
935 return 0;
936 }
937
938 ddev = cqspi->rx_chan->device->dev;
939 dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
940 if (dma_mapping_error(ddev, dma_dst)) {
941 dev_err(dev, "dma mapping failed\n");
942 return -ENOMEM;
943 }
944 tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
945 len, flags);
946 if (!tx) {
947 dev_err(dev, "device_prep_dma_memcpy error\n");
948 ret = -EIO;
949 goto err_unmap;
950 }
951
952 tx->callback = cqspi_rx_dma_callback;
953 tx->callback_param = cqspi;
954 cookie = tx->tx_submit(tx);
955 reinit_completion(&cqspi->rx_dma_complete);
956
957 ret = dma_submit_error(cookie);
958 if (ret) {
959 dev_err(dev, "dma_submit_error %d\n", cookie);
960 ret = -EIO;
961 goto err_unmap;
962 }
963
964 dma_async_issue_pending(cqspi->rx_chan);
965 if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
> 966 msecs_to_jiffies(max(len, 500UL)))) {
967 dmaengine_terminate_sync(cqspi->rx_chan);
968 dev_err(dev, "DMA wait_for_completion_timeout\n");
969 ret = -ETIMEDOUT;
970 goto err_unmap;
971 }
972
973 err_unmap:
974 dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
975
976 return ret;
977 }
978

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (4.90 kB)
.config.gz (31.65 kB)
Download all attachments

2020-12-29 09:20:51

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

On 29/12/20 11:29AM, kernel test robot wrote:
> Hi Pratyush,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on spi/for-next]
> [also build test WARNING on v5.11-rc1 next-20201223]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> config: arm-randconfig-r006-20201221 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install arm cross compiling tool for clang build
> # apt-get install binutils-arm-linux-gnueabi
> # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
> msecs_to_jiffies(max(len, 500UL)))) {
> ^~~~~~~~~~~~~~~
> include/linux/minmax.h:58:19: note: expanded from macro 'max'
> #define max(x, y) __careful_cmp(x, y, >)
> ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
> __builtin_choose_expr(__safe_cmp(x, y), \
> ^~~~~~~~~~~~~~~~
> include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
> (__typecheck(x, y) && __no_side_effects(x, y))
> ^~~~~~~~~~~~~~~~~
> include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
> (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> 1 warning generated.

On arm64 size_t is defined as unsigned long and on arm is it defined as
unsigned int. So using 500U would generate the same warning on 64-bit
platforms. Maybe the fix is to do something like: max(len, (size_t)500).
Any better ideas?

>
>
> vim +966 drivers/spi/spi-cadence-quadspi.c
>
> 919
> 920 static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
> 921 u_char *buf, loff_t from, size_t len)
> 922 {
> 923 struct cqspi_st *cqspi = f_pdata->cqspi;
> 924 struct device *dev = &cqspi->pdev->dev;
> 925 enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> 926 dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
> 927 int ret = 0;
> 928 struct dma_async_tx_descriptor *tx;
> 929 dma_cookie_t cookie;
> 930 dma_addr_t dma_dst;
> 931 struct device *ddev;
> 932
> 933 if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
> 934 memcpy_fromio(buf, cqspi->ahb_base + from, len);
> 935 return 0;
> 936 }
> 937
> 938 ddev = cqspi->rx_chan->device->dev;
> 939 dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
> 940 if (dma_mapping_error(ddev, dma_dst)) {
> 941 dev_err(dev, "dma mapping failed\n");
> 942 return -ENOMEM;
> 943 }
> 944 tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
> 945 len, flags);
> 946 if (!tx) {
> 947 dev_err(dev, "device_prep_dma_memcpy error\n");
> 948 ret = -EIO;
> 949 goto err_unmap;
> 950 }
> 951
> 952 tx->callback = cqspi_rx_dma_callback;
> 953 tx->callback_param = cqspi;
> 954 cookie = tx->tx_submit(tx);
> 955 reinit_completion(&cqspi->rx_dma_complete);
> 956
> 957 ret = dma_submit_error(cookie);
> 958 if (ret) {
> 959 dev_err(dev, "dma_submit_error %d\n", cookie);
> 960 ret = -EIO;
> 961 goto err_unmap;
> 962 }
> 963
> 964 dma_async_issue_pending(cqspi->rx_chan);
> 965 if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
> > 966 msecs_to_jiffies(max(len, 500UL)))) {
> 967 dmaengine_terminate_sync(cqspi->rx_chan);
> 968 dev_err(dev, "DMA wait_for_completion_timeout\n");
> 969 ret = -ETIMEDOUT;
> 970 goto err_unmap;
> 971 }
> 972
> 973 err_unmap:
> 974 dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
> 975
> 976 return ret;
> 977 }
> 978
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]



--
Regards,
Pratyush Yadav
Texas Instruments India

2021-01-06 00:12:51

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

On Tue, Dec 29, 2020 at 1:18 AM 'Pratyush Yadav' via Clang Built Linux
<[email protected]> wrote:
>
> On 29/12/20 11:29AM, kernel test robot wrote:
> > Hi Pratyush,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on spi/for-next]
> > [also build test WARNING on v5.11-rc1 next-20201223]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url: https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> > config: arm-randconfig-r006-20201221 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # install arm cross compiling tool for clang build
> > # apt-get install binutils-arm-linux-gnueabi
> > # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> > git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
> > # save the attached .config to linux build tree
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
> > msecs_to_jiffies(max(len, 500UL)))) {
> > ^~~~~~~~~~~~~~~
> > include/linux/minmax.h:58:19: note: expanded from macro 'max'
> > #define max(x, y) __careful_cmp(x, y, >)
> > ^~~~~~~~~~~~~~~~~~~~~~
> > include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
> > __builtin_choose_expr(__safe_cmp(x, y), \
> > ^~~~~~~~~~~~~~~~
> > include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
> > (__typecheck(x, y) && __no_side_effects(x, y))
> > ^~~~~~~~~~~~~~~~~
> > include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
> > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> > ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> > 1 warning generated.
>
> On arm64 size_t is defined as unsigned long and on arm is it defined as
> unsigned int. So using 500U would generate the same warning on 64-bit
> platforms. Maybe the fix is to do something like: max(len, (size_t)500).
> Any better ideas?

SGTM

--
Thanks,
~Nick Desaulniers

2021-01-06 15:02:31

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

On Wed, 23 Dec 2020 00:14:18 +0530, Pratyush Yadav wrote:
> This series adds support for Octal DTR mode now that SPI NOR supports
> these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
> Patche 5/7 lays some groundwork by implementing the supports_op() hook.
> Patch 7/7 adds the Octal DTR mode support.
>
> While the main aim of this series is to support 8D-8D-8D mode, other
> modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
> they have not been tested.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/7] spi: cadence-quadspi: Set master max_speed_hz
commit: 3a5c09c8d1ed9a7323f0e5c435021531f0865c16
[2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many
commit: ceeda328edeeeeac7579e9dbf0610785a3b83d39
[3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands
commit: 888d517b992532df2b6115fbdc9620673ca7c651
[4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
commit: 7512eaf54190e4cc9247f18439c008d44b15022c
[5/7] spi: cadence-quadspi: Implement a simple supports_op hook
commit: a273596b9b50c76a9cc1f65d3eb7f8ab5c3eb3e3
[6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads
commit: 0920a32cf6f20467aa133a47b776ee782daa889f
[7/7] spi: cadence-quadspi: Add DTR support
commit: f453f293979fb648d2e505c132887811acb6bde6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2021-01-08 16:50:26

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

Hi Mark,

On 06/01/21 02:59PM, Mark Brown wrote:
> On Wed, 23 Dec 2020 00:14:18 +0530, Pratyush Yadav wrote:
> > This series adds support for Octal DTR mode now that SPI NOR supports
> > these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
> > Patche 5/7 lays some groundwork by implementing the supports_op() hook.
> > Patch 7/7 adds the Octal DTR mode support.
> >
> > While the main aim of this series is to support 8D-8D-8D mode, other
> > modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
> > they have not been tested.
> >
> > [...]
>
> Applied to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
>
> Thanks!

:-)

> [1/7] spi: cadence-quadspi: Set master max_speed_hz
> commit: 3a5c09c8d1ed9a7323f0e5c435021531f0865c16
> [2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many
> commit: ceeda328edeeeeac7579e9dbf0610785a3b83d39
> [3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands
> commit: 888d517b992532df2b6115fbdc9620673ca7c651
> [4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
> commit: 7512eaf54190e4cc9247f18439c008d44b15022c
> [5/7] spi: cadence-quadspi: Implement a simple supports_op hook
> commit: a273596b9b50c76a9cc1f65d3eb7f8ab5c3eb3e3
> [6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads
> commit: 0920a32cf6f20467aa133a47b776ee782daa889f

The kernel test robot reported some build warnings on this patch on
32-bit platforms and I was planning on re-rolling it soon. Now that it
is in your tree, I will send a follow-up patch to fix it.

> [7/7] spi: cadence-quadspi: Add DTR support
> commit: f453f293979fb648d2e505c132887811acb6bde6
>
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
>
> Thanks,
> Mark

--
Regards,
Pratyush Yadav
Texas Instruments India