diff mbox series

[v1,1/2] spi: stm32_qspi: Always check SR_TCF flags in stm32_qspi_wait_cmd()

Message ID 20220512071738.741406-2-patrice.chotard@foss.st.com
State Accepted
Commit a6d7eeb66db720e77aba587fce6cc88a2003e8ab
Delegated to: Patrick Delaunay
Headers show
Series spi: stm32_qspi: flags management fixes | expand

Commit Message

Patrice CHOTARD May 12, 2022, 7:17 a.m. UTC
Currently, SR_TCF flag is checked in case there is data, this criteria
is not correct.

SR_TCF flags is set when programmed number of bytes have been transferred
to the memory device ("bytes" comprised command and data send to the
SPI device).
So even if there is no data, we must check SR_TCF flag.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---

 drivers/spi/stm32_qspi.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Comments

Patrick Delaunay May 17, 2022, 8:23 a.m. UTC | #1
Hi Patrice,

On 5/12/22 09:17, Patrice Chotard wrote:
> Currently, SR_TCF flag is checked in case there is data, this criteria
> is not correct.
>
> SR_TCF flags is set when programmed number of bytes have been transferred
> to the memory device ("bytes" comprised command and data send to the
> SPI device).
> So even if there is no data, we must check SR_TCF flag.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
> ---
>
>   drivers/spi/stm32_qspi.c | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/spi/stm32_qspi.c b/drivers/spi/stm32_qspi.c
> index 8f4aabc3d1..3c8faecb54 100644
> --- a/drivers/spi/stm32_qspi.c
> +++ b/drivers/spi/stm32_qspi.c
> @@ -150,20 +150,19 @@ static int _stm32_qspi_wait_cmd(struct stm32_qspi_priv *priv,
>   	u32 sr;
>   	int ret = 0;
>   
> -	if (op->data.nbytes) {
> -		ret = readl_poll_timeout(&priv->regs->sr, sr,
> -					 sr & STM32_QSPI_SR_TCF,
> -					 STM32_QSPI_CMD_TIMEOUT_US);
> -		if (ret) {
> -			log_err("cmd timeout (stat:%#x)\n", sr);
> -		} else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
> -			log_err("transfer error (stat:%#x)\n", sr);
> -			ret = -EIO;
> -		}
> -		/* clear flags */
> -		writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, &priv->regs->fcr);
> +	ret = readl_poll_timeout(&priv->regs->sr, sr,
> +				 sr & STM32_QSPI_SR_TCF,
> +				 STM32_QSPI_CMD_TIMEOUT_US);
> +	if (ret) {
> +		log_err("cmd timeout (stat:%#x)\n", sr);
> +	} else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
> +		log_err("transfer error (stat:%#x)\n", sr);
> +		ret = -EIO;
>   	}
>   
> +	/* clear flags */
> +	writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, &priv->regs->fcr);
> +
>   	if (!ret)
>   		ret = _stm32_qspi_wait_for_not_busy(priv);
>   


Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Thanks
Patrick
Patrick Delaunay May 20, 2022, 7:21 a.m. UTC | #2
Hi,

On 5/17/22 10:23, Patrick DELAUNAY wrote:
> Hi Patrice,
>
> On 5/12/22 09:17, Patrice Chotard wrote:
>> Currently, SR_TCF flag is checked in case there is data, this criteria
>> is not correct.
>>
>> SR_TCF flags is set when programmed number of bytes have been 
>> transferred
>> to the memory device ("bytes" comprised command and data send to the
>> SPI device).
>> So even if there is no data, we must check SR_TCF flag.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
>> ---
>>
>>   drivers/spi/stm32_qspi.c | 23 +++++++++++------------
>>   1 file changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/spi/stm32_qspi.c b/drivers/spi/stm32_qspi.c
>> index 8f4aabc3d1..3c8faecb54 100644
>> --- a/drivers/spi/stm32_qspi.c
>> +++ b/drivers/spi/stm32_qspi.c
>> @@ -150,20 +150,19 @@ static int _stm32_qspi_wait_cmd(struct 
>> stm32_qspi_priv *priv,
>>       u32 sr;
>>       int ret = 0;
>>   -    if (op->data.nbytes) {
>> -        ret = readl_poll_timeout(&priv->regs->sr, sr,
>> -                     sr & STM32_QSPI_SR_TCF,
>> -                     STM32_QSPI_CMD_TIMEOUT_US);
>> -        if (ret) {
>> -            log_err("cmd timeout (stat:%#x)\n", sr);
>> -        } else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
>> -            log_err("transfer error (stat:%#x)\n", sr);
>> -            ret = -EIO;
>> -        }
>> -        /* clear flags */
>> -        writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, 
>> &priv->regs->fcr);
>> +    ret = readl_poll_timeout(&priv->regs->sr, sr,
>> +                 sr & STM32_QSPI_SR_TCF,
>> +                 STM32_QSPI_CMD_TIMEOUT_US);
>> +    if (ret) {
>> +        log_err("cmd timeout (stat:%#x)\n", sr);
>> +    } else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
>> +        log_err("transfer error (stat:%#x)\n", sr);
>> +        ret = -EIO;
>>       }
>>   +    /* clear flags */
>> +    writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, 
>> &priv->regs->fcr);
>> +
>>       if (!ret)
>>           ret = _stm32_qspi_wait_for_not_busy(priv);
>
>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>
> Thanks
> Patrick
>
> _______________________________________________
> Uboot-stm32 mailing list
> Uboot-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32



Applied to u-boot-stm/master, thanks!

Regards
Patrick
diff mbox series

Patch

diff --git a/drivers/spi/stm32_qspi.c b/drivers/spi/stm32_qspi.c
index 8f4aabc3d1..3c8faecb54 100644
--- a/drivers/spi/stm32_qspi.c
+++ b/drivers/spi/stm32_qspi.c
@@ -150,20 +150,19 @@  static int _stm32_qspi_wait_cmd(struct stm32_qspi_priv *priv,
 	u32 sr;
 	int ret = 0;
 
-	if (op->data.nbytes) {
-		ret = readl_poll_timeout(&priv->regs->sr, sr,
-					 sr & STM32_QSPI_SR_TCF,
-					 STM32_QSPI_CMD_TIMEOUT_US);
-		if (ret) {
-			log_err("cmd timeout (stat:%#x)\n", sr);
-		} else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
-			log_err("transfer error (stat:%#x)\n", sr);
-			ret = -EIO;
-		}
-		/* clear flags */
-		writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, &priv->regs->fcr);
+	ret = readl_poll_timeout(&priv->regs->sr, sr,
+				 sr & STM32_QSPI_SR_TCF,
+				 STM32_QSPI_CMD_TIMEOUT_US);
+	if (ret) {
+		log_err("cmd timeout (stat:%#x)\n", sr);
+	} else if (readl(&priv->regs->sr) & STM32_QSPI_SR_TEF) {
+		log_err("transfer error (stat:%#x)\n", sr);
+		ret = -EIO;
 	}
 
+	/* clear flags */
+	writel(STM32_QSPI_FCR_CTCF | STM32_QSPI_FCR_CTEF, &priv->regs->fcr);
+
 	if (!ret)
 		ret = _stm32_qspi_wait_for_not_busy(priv);