diff mbox series

[v42,03/98] hw/sd/sdcard: Track last command used to help logging

Message ID 20240628070216.92609-4-philmd@linaro.org
State Superseded
Headers show
Series hw/sd/sdcard: Add eMMC support | expand

Commit Message

Philippe Mathieu-Daudé June 28, 2024, 7 a.m. UTC
The command is selected on the I/O lines, and further
processing might be done on the DAT lines via the
sd_read_byte() and sd_write_byte() handlers. Since
these methods can't distinct between normal and APP
commands, keep the name of the current command in
the SDState and use it in the DAT handlers. This
fixes a bug that all normal commands were displayed
as APP commands.

Fixes: 2ed61fb57b ("sdcard: Display command name when tracing CMD/ACMD")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
---
 hw/sd/sd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Cédric Le Goater June 28, 2024, 7:30 a.m. UTC | #1
On 6/28/24 9:00 AM, Philippe Mathieu-Daudé wrote:
> The command is selected on the I/O lines, and further
> processing might be done on the DAT lines via the
> sd_read_byte() and sd_write_byte() handlers. Since
> these methods can't distinct between normal and APP
> commands, keep the name of the current command in
> the SDState and use it in the DAT handlers. This
> fixes a bug that all normal commands were displayed
> as APP commands.
> 
> Fixes: 2ed61fb57b ("sdcard: Display command name when tracing CMD/ACMD")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Cédric Le Goater <clg@redhat.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/sd/sd.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index d0a1d5db18..bc87807793 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -133,6 +133,7 @@ struct SDState {
>       uint32_t pwd_len;
>       uint8_t function_group[6];
>       uint8_t current_cmd;
> +    const char *last_cmd_name;
>       /* True if we will handle the next command as an ACMD. Note that this does
>        * *not* track the APP_CMD status bit!
>        */
> @@ -1154,12 +1155,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>       uint16_t rca;
>       uint64_t addr;
>   
> +    sd->last_cmd_name = sd_cmd_name(req.cmd);
>       /* CMD55 precedes an ACMD, so we are not interested in tracing it.
>        * However there is no ACMD55, so we want to trace this particular case.
>        */
>       if (req.cmd != 55 || sd->expecting_acmd) {
>           trace_sdcard_normal_command(sd_proto(sd)->name,
> -                                    sd_cmd_name(req.cmd), req.cmd,
> +                                    sd->last_cmd_name, req.cmd,
>                                       req.arg, sd_state_name(sd->state));
>       }
>   
> @@ -1620,7 +1622,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>   static sd_rsp_type_t sd_app_command(SDState *sd,
>                                       SDRequest req)
>   {
> -    trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
> +    sd->last_cmd_name = sd_acmd_name(req.cmd);
> +    trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
>                                req.cmd, req.arg, sd_state_name(sd->state));
>       sd->card_status |= APP_CMD;
>   
> @@ -1913,7 +1916,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>           return;
>   
>       trace_sdcard_write_data(sd_proto(sd)->name,
> -                            sd_acmd_name(sd->current_cmd),
> +                            sd->last_cmd_name,
>                               sd->current_cmd, value);
>       switch (sd->current_cmd) {
>       case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
> @@ -2069,7 +2072,7 @@ uint8_t sd_read_byte(SDState *sd)
>       io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
>   
>       trace_sdcard_read_data(sd_proto(sd)->name,
> -                           sd_acmd_name(sd->current_cmd),
> +                           sd->last_cmd_name,
>                              sd->current_cmd, io_len);
>       switch (sd->current_cmd) {
>       case 6:  /* CMD6:   SWITCH_FUNCTION */
> @@ -2214,6 +2217,7 @@ static void sd_instance_init(Object *obj)
>   {
>       SDState *sd = SD_CARD(obj);
>   
> +    sd->last_cmd_name = "UNSET";
>       sd->enable = true;
>       sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
>   }
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d0a1d5db18..bc87807793 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -133,6 +133,7 @@  struct SDState {
     uint32_t pwd_len;
     uint8_t function_group[6];
     uint8_t current_cmd;
+    const char *last_cmd_name;
     /* True if we will handle the next command as an ACMD. Note that this does
      * *not* track the APP_CMD status bit!
      */
@@ -1154,12 +1155,13 @@  static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     uint16_t rca;
     uint64_t addr;
 
+    sd->last_cmd_name = sd_cmd_name(req.cmd);
     /* CMD55 precedes an ACMD, so we are not interested in tracing it.
      * However there is no ACMD55, so we want to trace this particular case.
      */
     if (req.cmd != 55 || sd->expecting_acmd) {
         trace_sdcard_normal_command(sd_proto(sd)->name,
-                                    sd_cmd_name(req.cmd), req.cmd,
+                                    sd->last_cmd_name, req.cmd,
                                     req.arg, sd_state_name(sd->state));
     }
 
@@ -1620,7 +1622,8 @@  static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
+    sd->last_cmd_name = sd_acmd_name(req.cmd);
+    trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
 
@@ -1913,7 +1916,7 @@  void sd_write_byte(SDState *sd, uint8_t value)
         return;
 
     trace_sdcard_write_data(sd_proto(sd)->name,
-                            sd_acmd_name(sd->current_cmd),
+                            sd->last_cmd_name,
                             sd->current_cmd, value);
     switch (sd->current_cmd) {
     case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
@@ -2069,7 +2072,7 @@  uint8_t sd_read_byte(SDState *sd)
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
     trace_sdcard_read_data(sd_proto(sd)->name,
-                           sd_acmd_name(sd->current_cmd),
+                           sd->last_cmd_name,
                            sd->current_cmd, io_len);
     switch (sd->current_cmd) {
     case 6:  /* CMD6:   SWITCH_FUNCTION */
@@ -2214,6 +2217,7 @@  static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
 
+    sd->last_cmd_name = "UNSET";
     sd->enable = true;
     sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
 }