Re: [PATCH] hw/sd/sdcard: fix spi_cmd_SEND_CSD/CID state check

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 29/7/25 18:35, Guenter Roeck wrote:
> On Tue, Jul 29, 2025 at 04:06:33PM +0100, Ben Dooks wrote:
>>
>>
>> On 2025-07-29 14:51, Philippe Mathieu-Daudé wrote:
>>> Hi Ben,
>>>
>>> On 24/7/25 12:58, Ben Dooks wrote:
>>>> The addition of specific handlers for mmc-spi for SEND_CSD and
>>>> SEND_CID has broken at least Linux and possibly also u-boot's
>>>> mmc-spi code.
>>>>
>>>> It looks like when adding the code, it is checking for these
>>>> commands to not be in sd_standby_state but the check looks to
>>>> have been accidentally reversed (see below)
>>>>
>>>>        if (sd->state != sd_standby_state) {
>>>>            return sd_invalid_state_for_cmd(sd, req);
>>>>        }
>>>>
>>>> Linux shows the following:
>>>>
>>>> [    0.293983] Waiting for root device /dev/mmcblk0...
>>>> [    1.363071] mmc0: error -38 whilst initialising SD card
>>>> [    2.418566] mmc0: error -38 whilst initialising SD card
>>>>
>>>> Fixes: da954d0e32444f122a4 ("hw/sd/sdcard: Add spi_cmd_SEND_CSD/CID
>>>> handlers (CMD9 & CMD10)")
>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>> ---
>>>>    hw/sd/sd.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
> ...
>>
>> ok, so what is the correct fix? the sd-spi has been broken for some time.

I couldn't figure yet the problem with SEND_CSD and SEND_CID
*not* being in sd_standby_state, but I figured what was missing
with R2 return in SEND_STATUS. I'll post the series ASAP.

About "broken for some time", I was only testing the SPI mode with
the Gumstix machines, but we removed their support in commit a2ccff4d2bc
("hw/arm: Remove 'connex' and 'verdex' machines").
For older SPI testing, see:
https://lore.kernel.org/qemu-devel/2b825be1-bc57-49c2-d1c9-be83577e8ce1@amsat.org/
> FWIW, I use the patch below on top of upstream qemu. I have no idea if it
> is correct, but it fixes the problem for me.
> 
> Guenter
> 
> ---
>  From 87a2004005eb47758c524b54dd3fbc68a00e317f Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Thu, 24 Oct 2024 12:16:44 -0700
> Subject: [PATCH] sd: Fix boot failures seen on sifive_u
> 
> sifive_u fails to boot from SD. This patch fixes the problem.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   hw/sd/sd.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c275fdda2d..f5c44a4a86 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1520,7 +1520,7 @@ static sd_rsp_type_t emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
>   /* CMD9 */
>   static sd_rsp_type_t spi_cmd_SEND_CSD(SDState *sd, SDRequest req)
>   {
> -    if (sd->state != sd_standby_state) {
> +    if (sd->state != sd_transfer_state) {
>           return sd_invalid_state_for_cmd(sd, req);
>       }
>       return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
> @@ -1539,7 +1539,7 @@ static sd_rsp_type_t sd_cmd_SEND_CSD(SDState *sd, SDRequest req)
>   /* CMD10 */
>   static sd_rsp_type_t spi_cmd_SEND_CID(SDState *sd, SDRequest req)
>   {
> -    if (sd->state != sd_standby_state) {
> +    if (sd->state != sd_transfer_state) {
>           return sd_invalid_state_for_cmd(sd, req);
>       }
>       return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
> @@ -1592,7 +1592,7 @@ static sd_rsp_type_t sd_cmd_SEND_STATUS(SDState *sd, SDRequest req)
>       }
>   
>       if (sd_is_spi(sd)) {
> -        return sd_r2_s;
> +        return sd_r1;
>       }
>   
>       return sd_req_rca_same(sd, req) ? sd_r1 : sd_r0;