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

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 01/06/2026 at 14:57:20 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:

> 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@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@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

Maybe this is not totally generic and cannot be used as a solid ground:
it highly depends on the base frequency. If the base frequency is rather
high (typically > 85MHz)n then 8D non-PHY might be faster. But maybe
this is not the typical case and is rare enough to ignore.

> 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.

Very interesting.

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

I'm eager to see the result!

Thanks,
Miquèl