On 7/20/25 14:48, Ramin Moussavi wrote:
> Hello Eugen
>
> here is the updated patch
Please do not attach the patch to the email. Just create the patch using
git format-patch -v 2 and then use git send-email to send the patch.
We will review it after you send it like that.
Thanks
>
> Am Mo., 14. Juli 2025 um 22:33 Uhr schrieb Eugen Hristev <
> eugen.hris...@linaro.org>:
>
>> Hello Ramin,
>>
>> Thank you for your patch.
>>
>> Can you please change the subject to have "spi: atmel_qspi: ..."
>>
>> On 7/13/25 19:08, Ramin Moussavi wrote:
>>> In atmel_qspi_transfer(), the status register is polled with:
>>>
>>> imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
>>> return readl_poll_timeout(aq->regs + QSPI_SR, sr,
>>> (sr & imr) == imr,
>>> ATMEL_QSPI_TIMEOUT);
>>>
>>> However, this is racy: QSPI_SR_INSTRE can be set before QSPI_SR_CSR,
>>> and will then be cleared by the read. If that happens, the condition
>>> "(sr & imr) == imr" can never be true, and the function times out.
>>>
>>> This race condition is avoided in at91bootstrap by accumulating the
>>> status bits across reads until both bits have been observed:
>>>
>>> /* Poll INSTruction End and Chip Select Rise flags. */
>>> imr = (QSPI_SR_INSTRE | QSPI_SR_CSR);
>>> sr = 0;
>>> do {
>>> udelay(1);
>>> sr |= qspi_readl(qspi, QSPI_SR) & imr;
>>> } while ((--timeout) && (sr != imr));
>>>
>>> Update U-Boot's atmel_qspi_transfer() to use the same pattern,
>>> ensuring that both flags are observed even if they are not set
>>> simultaneously.
>>>
>>> Signed-off-by: Ramin Seyed-Mousavi <lordras...@gmail.com>
>>> ---
>>> drivers/spi/atmel-quadspi.c | 17 +++++++++++++++--
>>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
>>> index 8aa7a83aef4..fba32876f8f 100644
>>> --- a/drivers/spi/atmel-quadspi.c
>>> +++ b/drivers/spi/atmel-quadspi.c
>>> @@ -616,6 +616,8 @@ static int atmel_qspi_transfer(struct atmel_qspi *aq,
>>> const struct spi_mem_op *op, u32 offset)
>>> {
>>> u32 sr, imr;
>>> + uint32_t val = 0;
>>> + unsigned long timeout;
>>>
>>> /* Skip to the final steps if there is no data */
>>> if (op->data.nbytes) {
>>> @@ -636,8 +638,19 @@ static int atmel_qspi_transfer(struct atmel_qspi
>> *aq,
>>>
>>> /* Poll INSTruction End and Chip Select Rise flags. */
>>> imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
>>> - return readl_poll_timeout(aq->regs + QSPI_SR, sr, (sr & imr) ==
>> imr,
>>> - ATMEL_QSPI_TIMEOUT);
>>> +
>>> + timeout = timer_get_us() + ATMEL_QSPI_TIMEOUT;
>>> + while(1){
>>
>> There is a missing space, can you please run checkpatch tool and fix the
>> warnings given ?
>>
>>> + val |= readl( aq->regs + QSPI_SR ) & imr;
>>> + if( ( val & imr ) == imr ){
>>
>> Remove unnecessary brackets please, also spacing is odd
>>
>>> + return 0;
>>> + }
>>> +
>>> + if ( time_after(timer_get_us(), timeout)) {
>> Same here
>>> + return -ETIMEDOUT;
>>> + }
>>> + }
>>> +
>>> }
>>>
>>> static int atmel_qspi_sama7g5_set_cfg(struct atmel_qspi *aq,
>>
>>
>> Eugen
>>
>
