[PATCH v3 09/13] spi: cadence-quadspi: reject 2-byte-address DDR ops on PHY-tunable hardware

[Santhosh Kumar K]

Hello Miquel,

On 28/05/26 14:31, Miquel Raynal wrote:
> On 27/05/2026 at 23:25:23 +0530, Santhosh Kumar K <s-k6 at ti.com> wrote:
> 
>> Erratum i2383 affects the AM654 OSPI controller: in PHY DDR mode,
>> operations with a 2-byte address cause an internal state machine to
>> mis-compare the transmitted address byte count against 1 instead of 2,
>> locking up the address phase. [0]
>>
>> Add a CQSPI_NO_2BYTE_ADDR_PHY_DDR quirk flag and set it on the am654_ospi
>> platform data. In cqspi_supports_mem_op(), when a controller carries this
>> quirk and has PHY tuning support, reject DDR operations that use 2-byte
>> addressing.
>>
>> [0] https://www.ti.com/lit/er/sprz544c/sprz544c.pdf
>>
>> Signed-off-by: Santhosh Kumar K <s-k6 at ti.com>
>> ---
>>   drivers/spi/spi-cadence-quadspi.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>> index 508bc5bc4ab5..72208d376305 100644
>> --- a/drivers/spi/spi-cadence-quadspi.c
>> +++ b/drivers/spi/spi-cadence-quadspi.c
>> @@ -49,6 +49,7 @@ static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>>   #define CQSPI_DISABLE_RUNTIME_PM	BIT(10)
>>   #define CQSPI_NO_INDIRECT_MODE		BIT(11)
>>   #define CQSPI_HAS_WR_PROTECT		BIT(12)
>> +#define CQSPI_NO_2BYTE_ADDR_PHY_DDR	BIT(13)
>>   
>>   /* Capabilities */
>>   #define CQSPI_SUPPORTS_OCTAL		BIT(0)
>> @@ -1627,6 +1628,18 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
>>   		if (op->data.nbytes && op->data.buswidth != 8)
>>   			return false;
>>   
>> +		/*
>> +		 * Erratum i2383: In PHY DDR mode, 2-byte addressing causes an
>> +		 * internal state machine to mis-compare the transmitted
>> +		 * address byte count against 1 instead of 2, locking up the
>> +		 * address phase. Reject such ops on controllers that need it.
>> +		 */
>> +		if (cqspi->ddata &&
>> +		    (cqspi->ddata->quirks & CQSPI_NO_2BYTE_ADDR_PHY_DDR)) {
>> +			if (op->addr.nbytes == 2 && cqspi->ddata->execute_tuning)
>> +				return false;
>> +		}
> 
> I don't think this is a valid approach. What we want is to prevent
> tuning in octal DTR mode with 2 bytes addressing, instead of preventing
> reads/writes in octal DTR modes after tuning. Have you tried on an AM62A LP
> SK? I bet probe fails..
> 
> The quirk should be handled at the beginning of the tuning procedure, so
> we skip tuning entirely in this case.

I see your point. However, in my testing on AM62Ax LP SK, the controller
and the flash probes and operates correctly in 8S PHY mode.

The reason I handled this in supports_op() is that, for these devices,
simply skipping tuning when a 2-byte DDR operation is selected may not
lead to the best achievable operating point. The performance ordering I
measured is:

8S non-PHY < 8D non-PHY < 8S PHY

So falling back from 8D PHY to 8D non-PHY would leave performance on the
table compared to selecting and tuning an 8S operation.

That said, I agree that encoding this as an operation support
restriction is probably not the right place to handle it. I'll rework
the initialization flow so that operation selection can take
PHY-achievable frequencies and controller-specific restrictions such as
i2383 into account when choosing the final operations.

Let me prototype this approach and will report back with the results.

Thanks,
Santhosh.

> 
> Thanks,
> Miquèl