Re: [PATCH 18/23] hw/sd/sdcard: Add sd_invalid_mode_for_cmd to report invalid mode switch

[Cédric Le Goater]

On 6/21/24 10:05 AM, Philippe Mathieu-Daudé wrote:
> Having the mode switch displayed help to track incomplete
> command implementations.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


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

Thanks,

C.


> ---
>   hw/sd/sd.c | 75 +++++++++++++++++++++++++++++-------------------------
>   1 file changed, 41 insertions(+), 34 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1df16ce6a2..8d63a39a54 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -180,6 +180,17 @@ static const char *sd_version_str(enum 
> SDPhySpecificationVersion version)
>       return sdphy_version[version];
>   }
>   
> +static const char *sd_mode_name(enum SDCardModes mode)
> +{
> +    static const char *mode_name[] = {
> +        [sd_inactive]                   = "inactive",
> +        [sd_card_identification_mode]   = "identification",
> +        [sd_data_transfer_mode]         = "transfer",
> +    };
> +    assert(mode < ARRAY_SIZE(mode_name));
> +    return mode_name[mode];
> +}
> +
>   static const char *sd_state_name(enum SDCardStates state)
>   {
>       static const char *state_name[] = {
> @@ -1015,6 +1026,15 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState 
> *sd, SDRequest req)
>       return sd_illegal;
>   }
>   
> +static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
> +                  sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
> +                  sd_version_str(sd->spec_version));
> +
> +    return sd_illegal;
> +}
> +
>   static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
>   {
>       qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
> @@ -1154,18 +1174,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
> SDRequest req)
>           break;
>   
>       case 6:  /* CMD6:   SWITCH_FUNCTION */
> -        switch (sd->mode) {
> -        case sd_data_transfer_mode:
> -            sd_function_switch(sd, req.arg);
> -            sd->state = sd_sendingdata_state;
> -            sd->data_start = 0;
> -            sd->data_offset = 0;
> -            return sd_r1;
> -
> -        default:
> -            break;
> +        if (sd->mode != sd_data_transfer_mode) {
> +            return sd_invalid_mode_for_cmd(sd, req);
>           }
> -        break;
> +        sd_function_switch(sd, req.arg);
> +        sd->state = sd_sendingdata_state;
> +        sd->data_start = 0;
> +        sd->data_offset = 0;
> +        return sd_r1;
>   
>       case 7:  /* CMD7:   SELECT/DESELECT_CARD */
>           rca = sd_req_get_rca(sd, req);
> @@ -1289,33 +1305,24 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
> SDRequest req)
>   
>       case 13:  /* CMD13:  SEND_STATUS */
>           rca = sd_req_get_rca(sd, req);
> -        switch (sd->mode) {
> -        case sd_data_transfer_mode:
> -            if (!sd_is_spi(sd) && sd->rca != rca) {
> -                return sd_r0;
> -            }
> -
> -            return sd_r1;
> -
> -        default:
> -            break;
> +        if (sd->mode != sd_data_transfer_mode) {
> +            return sd_invalid_mode_for_cmd(sd, req);
>           }
> -        break;
> +        if (!sd_is_spi(sd) && sd->rca != rca) {
> +            return sd_r0;
> +        }
> +
> +        return sd_r1;
>   
>       case 15:  /* CMD15:  GO_INACTIVE_STATE */
> -        rca = sd_req_get_rca(sd, req);
> -        switch (sd->mode) {
> -        case sd_data_transfer_mode:
> -            if (sd->rca != rca)
> -                return sd_r0;
> -
> -            sd->state = sd_inactive_state;
> -            return sd_r0;
> -
> -        default:
> -            break;
> +        if (sd->mode != sd_data_transfer_mode) {
> +            return sd_invalid_mode_for_cmd(sd, req);
>           }
> -        break;
> +        rca = sd_req_get_rca(sd, req);
> +        if (sd->rca == rca) {
> +            sd->state = sd_inactive_state;
> +        }
> +        return sd_r0;
>   
>       /* Block read commands (Class 2) */
>       case 16:  /* CMD16:  SET_BLOCKLEN */