RE: [PATCH v2 4/4] spi: cadence-quadspi: use STIG mode for small reads

[Yoshitaka Ikeda <ikeda@xxxxxxxxxxxx>]

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