[PATCH V3 2/2] spi: cadence_qspi: use STIG mode for small reads

[Dhruva Gole]

Hey Pratyush,
Thanks for reviewing.

On 13/12/22 05:05, Pratyush Yadav wrote:
> On 25/11/22 11:29AM, 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
> 
> I know where to find the useful information in this 12 thousand page
> long document since I used to work at TI. But other people might not
> find it so easy. It would be good to point to specific section numbers

I will try to be more specific about section numbers in future, however
since this fix applies across any SoC's out there I thought not to go
into too much detail just for AM625. I feared it may send a wrong
message that this fix is ONLY and ONLY relevant to AM62x which might not
necessarily be the case. Also, I have mentioned further below about it
being "part of the Flash Subsystem (FSS)". Hence in my humble opinion
should be enough to point which section to look into, but if someone
really wanted to deep dive and see for themselves they still could
go through the FSS part of the TRM.

> here to make this easier to understand and review. Also, this document
> does not contain the register map for the CQSPI controller. You should
> link that too.

I believe that it does have the reg map for OSPI. Kindly refer
to FSS_OSPI_0_config_reg and onwards,
(Addresses 0FC4 0000h onwards)

It's under 12.4.6 Memory Interfaces Registers: FSS_OSPI_0 Registers

I am on revision: SPRUIV7A – MAY 2022 – REVISED NOVEMBER 2022

> 
>> 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. This operates by
>> default in a 32 bit mode causing it to always align all data to 4 bytes
> 
> Is this new in AM62x? I do not remember seeing this on J7 class devices.
> I remember being able to read all registers from Cypress S28 flash
> without having to do anything special.

Taking a look at FSS0_0_SYSCONFIG Register (Offset = 4h) [reset = 0h ]

The Table 12-2293. SYSCONFIG Register Field Descriptions tells us,

OSPI_32B_DISABLE_MODE: Reset value = 0
and this means, 0: OSPI 32bit mode enabled by default,

So rather than complicating things to enable/ disable 32 bit mode all
around, this fix should avoid major restructuring.

> 
[...]
>> +		if (!op->addr.nbytes ||
>> +		    op->data.nbytes < CQSPI_STIG_DATA_LEN_MAX)
> 
>                                      <= instead?
This would be fine too, just wanted to ensure we enter this condition
for small reads only. Registers are generally only 32 bit long (4Bytes) 
and CQSPI_STIG_DATA_LEN_MAX is 8Bytes so it's anyway more than a
register read would really need.
> 
>>   			mode = CQSPI_STIG_READ;
>>   		else
>>   			mode = CQSPI_READ;
>> -- 
>> 2.25.1
>>
> 


-- 
Thanks and Regards,
Dhruva Gole