[PATCH] hw/sd: Correct the CURRENT_STATE bits in SPI-mode response

frank.chang@sifive.com posted 1 patch 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220118023509.14496-1-frank.chang@sifive.com
Maintainers: Bin Meng <bin.meng@windriver.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
hw/sd/sd.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
[PATCH] hw/sd: Correct the CURRENT_STATE bits in SPI-mode response
Posted by frank.chang@sifive.com 2 years, 2 months ago
From: Frank Chang <frank.chang@sifive.com>

In SPI-mode, type B ("cleared on valid command") clear condition is not
supported, and as the "In idle state" bit in SPI-mode has type A
("according to current state") clear condition, the CURRENT_STATE bits
in an SPI-mode response should be the SD card's state after the command
is executed, instead of the state when it received the preceding
command.

Also, we don't need to clear the type B ("clear on valid command")
status bits after the response is updated in SPI-mode.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 hw/sd/sd.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cd67a7bac8..9736b8912d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1757,12 +1757,20 @@ int sd_do_command(SDState *sd, SDRequest *req,
     if (rtype == sd_illegal) {
         sd->card_status |= ILLEGAL_COMMAND;
     } else {
-        /* Valid command, we can update the 'state before command' bits.
-         * (Do this now so they appear in r1 responses.)
-         */
         sd->current_cmd = req->cmd;
         sd->card_status &= ~CURRENT_STATE;
-        sd->card_status |= (last_state << 9);
+
+        if (!sd->spi) {
+            /* Valid command, we can update the 'state before command' bits.
+             * (Do this now so they appear in r1 responses.)
+             */
+            sd->card_status |= (last_state << 9);
+        } else {
+            /* Type B ("clear on valid command") is not supported
+             * in SPI-mode.
+             */
+            sd->card_status |= (sd->state << 9);
+        }
     }
 
 send_response:
@@ -1808,10 +1816,12 @@ send_response:
     trace_sdcard_response(sd_response_name(rtype), rsplen);
 
     if (rtype != sd_illegal) {
-        /* Clear the "clear on valid command" status bits now we've
-         * sent any response
-         */
-        sd->card_status &= ~CARD_STATUS_B;
+        if (!sd->spi) {
+            /* Clear the "clear on valid command" status bits now we've
+             * sent any response
+             */
+            sd->card_status &= ~CARD_STATUS_B;
+        }
     }
 
 #ifdef DEBUG_SD
-- 
2.31.1


Re: [PATCH] hw/sd: Correct the CURRENT_STATE bits in SPI-mode response
Posted by Frank Chang 2 years, 2 months ago
On Tue, Jan 18, 2022 at 10:35 AM <frank.chang@sifive.com> wrote:

> From: Frank Chang <frank.chang@sifive.com>
>
> In SPI-mode, type B ("cleared on valid command") clear condition is not
> supported, and as the "In idle state" bit in SPI-mode has type A
> ("according to current state") clear condition, the CURRENT_STATE bits
> in an SPI-mode response should be the SD card's state after the command
> is executed, instead of the state when it received the preceding
> command.
>
> Also, we don't need to clear the type B ("clear on valid command")
> status bits after the response is updated in SPI-mode.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  hw/sd/sd.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index cd67a7bac8..9736b8912d 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1757,12 +1757,20 @@ int sd_do_command(SDState *sd, SDRequest *req,
>      if (rtype == sd_illegal) {
>          sd->card_status |= ILLEGAL_COMMAND;
>      } else {
> -        /* Valid command, we can update the 'state before command' bits.
> -         * (Do this now so they appear in r1 responses.)
> -         */
>          sd->current_cmd = req->cmd;
>          sd->card_status &= ~CURRENT_STATE;
> -        sd->card_status |= (last_state << 9);
> +
> +        if (!sd->spi) {
> +            /* Valid command, we can update the 'state before command'
> bits.
> +             * (Do this now so they appear in r1 responses.)
> +             */
> +            sd->card_status |= (last_state << 9);
> +        } else {
> +            /* Type B ("clear on valid command") is not supported
> +             * in SPI-mode.
> +             */
> +            sd->card_status |= (sd->state << 9);
> +        }
>      }
>
>  send_response:
> @@ -1808,10 +1816,12 @@ send_response:
>      trace_sdcard_response(sd_response_name(rtype), rsplen);
>
>      if (rtype != sd_illegal) {
> -        /* Clear the "clear on valid command" status bits now we've
> -         * sent any response
> -         */
> -        sd->card_status &= ~CARD_STATUS_B;
> +        if (!sd->spi) {
> +            /* Clear the "clear on valid command" status bits now we've
> +             * sent any response
> +             */
> +            sd->card_status &= ~CARD_STATUS_B;
> +        }
>      }
>
>  #ifdef DEBUG_SD
> --
> 2.31.1
>
>
This patch is replaced with more proper SPI clear conditions fix patch:
https://patchew.org/QEMU/20220124060449.22498-1-frank.chang@sifive.com/
Please ignore this one, sorry for the confusion.

Regards,
Frank Chang