Re: [PATCH v7 12/17] hw/sd/sdcard: Simplify cmd_valid_while_locked()

[Alistair Francis]

On Tue, Jun 30, 2020 at 6:44 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> cmd_valid_while_locked() only needs to read SDRequest->cmd,
> pass it directly and make it const.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/sd/sd.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 723e66bbf2..2946fe3040 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1678,7 +1678,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>      return sd_illegal;
>  }
>
> -static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
> +static int cmd_valid_while_locked(SDState *sd, const uint8_t cmd)
>  {
>      /* Valid commands in locked state:
>       * basic class (0)
> @@ -1689,13 +1689,12 @@ static int cmd_valid_while_locked(SDState *sd, 
> SDRequest *req)
>       * Anything else provokes an "illegal command" response.
>       */
>      if (sd->expecting_acmd) {
> -        return req->cmd == 41 || req->cmd == 42;
> +        return cmd == 41 || cmd == 42;
>      }
> -    if (req->cmd == 16 || req->cmd == 55) {
> +    if (cmd == 16 || cmd == 55) {
>          return 1;
>      }
> -    return sd_cmd_class[req->cmd] == 0
> -            || sd_cmd_class[req->cmd] == 7;
> +    return sd_cmd_class[cmd] == 0 || sd_cmd_class[cmd] == 7;
>  }
>
>  int sd_do_command(SDState *sd, SDRequest *req,
> @@ -1721,7 +1720,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
>      }
>
>      if (sd->card_status & CARD_IS_LOCKED) {
> -        if (!cmd_valid_while_locked(sd, req)) {
> +        if (!cmd_valid_while_locked(sd, req->cmd)) {
>              sd->card_status |= ILLEGAL_COMMAND;
>              sd->expecting_acmd = false;
>              qemu_log_mask(LOG_GUEST_ERROR, "SD: Card is locked\n");
> --
> 2.21.3
>
>