Ben Dooks <ben.do...@codethink.co.uk> writes:

(Add maintainers to CC)

You should get your patch workflow to use scripts/get_maintainer.pl so
they get CC'd and reduces the chance of it being missed in the fire-hose
of qemu-devel.

> If we get "ssi_sd: error: Unexpected response to cmd" then having
> the bad s->arglen would be useful debug and does not add any complexity
> to the code.

Generally we should be removing the old-style DPRINTF debug and
replacing them with tracepoints where they are warranted. The main
problem with the old style DPRINTF's is the format strings tend to
bitrot because they are not enabled by default.

>
> Signed-off-by: Ben Dooks <ben.do...@codethink.co.uk>
> ---
>  hw/sd/ssi-sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 6c90a86ab4..f1441d2c97 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -183,7 +183,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
> uint32_t val)
>                  s->response[0] = 1;
>                  memcpy(&s->response[1], longresp, 4);
>              } else if (s->arglen != 4) {
> -                BADF("Unexpected response to cmd %d\n", s->cmd);
> +                BADF("Unexpected response to cmd %d, arglen=%d\n", s->cmd, 
> s->arglen);

That said BADF is defined in both cases (although the exit(1) for the
debug leg is a bit aggressive). Is this an error of the guest
miss-programming the device with invalid data?

There could be an argument for using:

  qemu_log_mask(LOG_GUEST_ERROR, "Unexpected response to cmd %d, arglen=%d\n", 
s->cmd, s->arglen);

instead.

Phillipe WDYT?

>                  /* Illegal command is about as near as we can get.  */
>                  s->arglen = 1;
>                  s->response[0] = 4;

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to