| Message ID | 20240628070216.92609-4-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/sd/sdcard: Add eMMC support | expand |
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 --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); }
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(-)