RE: [PATCH v2 4/4] spi: cadence-quadspi: use STIG mode for small reads
From: Yoshitaka Ikeda
Date: Mon May 29 2023 - 22:56:12 EST
Hi Dhruva,
The patch described below was able to avoid this problem.
However, we could not find any material to determine if it was correct.
Dear all,
Is there anyone else in a similar environment?
```
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 8f36e1306e16..4a9535e6c86a 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1365,7 +1365,9 @@ static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
* reads, prefer STIG mode for such small reads.
*/
if (!op->addr.nbytes ||
- op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX)
+ (op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX &&
+ !of_device_is_compatible(cqspi->pdev->dev.of_node,
+ "intel,socfpga-qspi")))
return cqspi_command_read(f_pdata, op);
return cqspi_read(f_pdata, op);
```
Thanks and Regards,
Yoshitaka Ikeda
> -----Original Message-----
> From: Yoshitaka Ikeda <ikeda@xxxxxxxxxxxx>
> Sent: Monday, May 8, 2023 4:44 PM
> To: Dhruva Gole <d-gole@xxxxxx>
> Cc: linux-spi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Vignesh
> Raghavendra <vigneshr@xxxxxx>; Vaishnav Achath <vaishnav.a@xxxxxx>;
> linux-mtd@xxxxxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> Takahiro.Kuwano@xxxxxxxxxxxx; Pratyush Yadav <ptyadav@xxxxxxxxx>;
> Mark Brown <broonie@xxxxxxxxxx>
> Subject: RE: [PATCH v2 4/4] spi: cadence-quadspi: use STIG mode for small
> reads
>
> Hi Dhruva,
>
> > > - Error at startup
> > > - Kernel log
> > > [ 0.980598] **********spi_mem_op dump**************
> > > [ 0.980613] addr: nbytes:0x0 , buswidth 0x0, dtr 0x0, val 0x0
> > > [ 0.984223] cmd: nbytes:0x1 , buswidth 0x1, dtr 0x0, opcode 0x9F
> > > [ 0.988656] data: nbytes:0x6 , buswidth 0x1, dtr 0x0, data dir 0x1
> > > [ 0.993362] ***************************************
> > > [ 0.998329] spi-nor spi0.0: found mt25ql512a, expected n25q512a
> > > [ 1.006574] **********spi_mem_op dump**************
> > > [ 1.006583] addr: nbytes:0x3 , buswidth 0x1, dtr 0x0, val 0x0
> > > [ 1.010150] cmd: nbytes:0x1 , buswidth 0x1, dtr 0x0, opcode 0x5A
> > > [ 1.014596] data: nbytes:0x10 , buswidth 0x1, dtr 0x0, data dir 0x1
> > > [ 1.019285] ***************************************
> > > [ 1.524271] cadence-qspi ff705000.flash: Flash command execution
> > timed out.
> >
> > This print message is from cqspi_exec_flash_cmd. This function should
> > only be called from cqspi_command_read/write .
> >
> > However, from spi_mem_op dump that you have provided above, where
> > addr.nbytes is 3 and data.nbytes is 0x10 (which is > 8) it should
> > never have entered the cqspi_command_read function.
>
> The location of the log output is after the call to cqspi_mem_process(), so I
> assume it is called with the following data.
>
> [ 1.533483] **********spi_mem_op dump************** [ 1.533489] addr:
> nbytes:0x3 , buswidth 0x1, dtr 0x0, val 0x10 [ 1.537055] cmd: nbytes:0x1 ,
> buswidth 0x1, dtr 0x0, opcode 0x5A [ 1.541579] data: nbytes:0x8 , buswidth 0x1,
> dtr 0x0, data dir 0x1 [ 1.546266] ***************************************
>
> Thus, the condition is met and cqspi_command_read() is called.
> We have also applied the following patch and confirmed that it is called.
>
> - patch
> ```
> diff --git a/drivers/spi/spi-cadence-quadspi.c
> b/drivers/spi/spi-cadence-quadspi.c
> index 64b6a460d739..8d3681e1f35c 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1365,15 +1365,21 @@ static int cqspi_mem_process(struct spi_mem
> *mem, const struct spi_mem_op *op)
> * reads, prefer STIG mode for such small reads.
> */
> if (!op->addr.nbytes ||
> - op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX)
> + op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX) {
> + printk("----- %d:%s() (1) -----\n", __LINE__,
> __func__);
> return cqspi_command_read(f_pdata, op);
> + }
>
> + printk("----- %d:%s() (2) -----\n", __LINE__, __func__);
> return cqspi_read(f_pdata, op);
> }
>
> - if (!op->addr.nbytes || !op->data.buf.out)
> + if (!op->addr.nbytes || !op->data.buf.out) {
> + printk("----- %d:%s() (3) -----\n", __LINE__, __func__);
> return cqspi_command_write(f_pdata, op);
> + }
>
> + printk("----- %d:%s() (4) -----\n", __LINE__, __func__);
> return cqspi_write(f_pdata, op);
> }
>
> ```
>
> - log
> ```
> [ 0.786600] ----- 1369:cqspi_mem_process() (1) -----
> [ 0.790312] **********spi_mem_op dump**************
> [ 0.790319] addr: nbytes:0x0 , buswidth 0x0, dtr 0x0, val 0x0
> [ 0.793885] cmd: nbytes:0x1 , buswidth 0x1, dtr 0x0, opcode 0x9F
> [ 0.798325] data: nbytes:0x6 , buswidth 0x1, dtr 0x0, data dir 0x1
> [ 0.803013] ***************************************
> [ 0.807892] spi-nor spi0.0: found mt25ql512a, expected n25q512a
> [ 0.816062] ----- 1373:cqspi_mem_process() (2) -----
> [ 0.819775] **********spi_mem_op dump**************
> [ 0.819782] addr: nbytes:0x3 , buswidth 0x1, dtr 0x0, val 0x0
> [ 0.823348] cmd: nbytes:0x1 , buswidth 0x1, dtr 0x0, opcode 0x5A
> [ 0.827792] data: nbytes:0x10 , buswidth 0x1, dtr 0x0, data dir 0x1
> [ 0.832480] ***************************************
> [ 0.837442] ----- 1369:cqspi_mem_process() (1) -----
> [ 1.344671] cadence-qspi ff705000.flash: Flash command execution timed
> out.
> [ 1.350328] **********spi_mem_op dump**************
> [ 1.350334] addr: nbytes:0x3 , buswidth 0x1, dtr 0x0, val 0x10
> [ 1.353901] cmd: nbytes:0x1 , buswidth 0x1, dtr 0x0, opcode 0x5A
> [ 1.358427] data: nbytes:0x8 , buswidth 0x1, dtr 0x0, data dir 0x1
> [ 1.363114] ***************************************
> [ 1.367981] spi-nor spi0.0: operation failed with -110
> [ 1.375376] spi-nor spi0.0: mt25ql512a (65536 Kbytes)
> ```
>
> > Please can you share the exact output of uname -a where you observe
> > this error?
>
> My environment has shortened the output of uname -a, so it may not be useful.
> Instead, I will give you information about the kernel I am using.
> It is the following (v6.3):
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id
> =457391b0380335d5e9a5babdec90ac53928b23b4
>
> > Are you carrying any sort of local patches? Can you make sure that the
> > CQSPI_STIG_DATA_LEN_MAX is 8 in your case too?
>
> There is no local patch and CQSPI_STIG_DATA_LEN_MAX remains 8.
>
> --
> Thanks and Regards,
> Yoshitaka Ikeda