linux-kernel - Re: [PATCH 2/2] spi: cadence-quadspi: use STIG mode for small reads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3881d0e8-c2ec-1de7-a7d9-4eeb1565b0c1@ti.com>
Date:   Tue, 10 Jan 2023 10:40:32 +0530
From:   Vaishnav Achath <vaishnav.a@...com>
To:     Dhruva Gole <d-gole@...com>, <broonie@...nel.org>
CC:     <linux-spi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Vignesh <vigneshr@...com>, Pratyush Yadav <pratyush@...nel.org>
Subject: Re: [PATCH 2/2] spi: cadence-quadspi: use STIG mode for small reads

Hi Dhruva,

On 04/01/23 11:56, Dhruva Gole wrote:
> Fix the issue where some flash chips like cypress S25HS256T return the
> value of the same register over and over in DAC mode.
> 
> For example in the TI K3-AM62x Processors refer [0] Technical Reference
> Manual there is a layer of digital logic in front of the QSPI/OSPI
> Drive when used in DAC mode. This is part of the Flash Subsystem (FSS)
> which provides access to external Flash devices.
> 
> The FSS0_0_SYSCONFIG Register (Offset = 4h) has a BIT Field for
> OSPI_32B_DISABLE_MODE which has a Reset value = 0. This means, OSPI 32bit
> mode enabled by default.
> 
> Thus, by default controller operates in 32 bit mode causing it to always
> align all data to 4 bytes from a 4byte aligned address. In some flash
> chips like cypress for example if we try to read some regs in DAC mode
> then it keeps sending the value of the first register that was requested
> and inorder to read the next reg, we have to stop and re-initiate a new
> transaction.
> 
> This causes wrong register values to be read than what is desired when
> registers are read in DAC mode. Hence if the data.nbytes is very less
> then prefer STIG mode for such small reads.
> 
> [0] https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
> 
> Signed-off-by: Dhruva Gole <d-gole@...com>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 8c7938776cfc..f5188dc52db6 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1344,7 +1344,13 @@ static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
>  	cqspi_configure(f_pdata, mem->spi->max_speed_hz);
>  
>  	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
> -		if (!op->addr.nbytes)
> +	/*
> +	 * Performing reads in DAC mode forces to read minimum 4 bytes
> +	 * which is unsupported on some flash devices during register
> +	 * reads, prefer STIG mode for such small reads.
> +	 */
> +		if (!op->addr.nbytes ||
> +		    op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX)
>  			return cqspi_command_read(f_pdata, op);
>  

I am seeing issues while testing after applying this series,

On J7200 EVM,
[    2.164655] spi-nor spi7.0: Failed to parse optional parameter table: ff81
[    2.171644] spi-nor spi7.0: s28hs512t (65536 Kbytes)

On J721E EVM,
[    5.565961] spi-nor spi7.0: mt35xu512aba (0 Kbytes)
[    5.732084] spi-nor spi8.0: mt25qu512a (81753 Kbytes)

In all the three cases above, the behavior is normal without these patches.

>  		return cqspi_read(f_pdata, op);

-- 
Regards,
Vaishnav