hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

[PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Bin Meng 3 years, 3 months ago

From: Bin Meng <bin.meng@windriver.com>

Unlike SD mode, when SD card is working in SPI mode, the argument
of CMD13 is stuff bits. Hence we should bypass the RCA check.

Signed-off-by: Bin Meng <bin.meng@windriver.com>

---
Based-on: http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226787

 hw/sd/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8c397d4ad7..4f902d0b72 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1166,8 +1166,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 13:	/* CMD13:  SEND_STATUS */
         switch (sd->mode) {
         case sd_data_transfer_mode:
-            if (sd->rca != rca)
+            if (!sd->spi && sd->rca != rca) {
                 return sd_r0;
+            }
 
             return sd_r1;
 
-- 
2.25.1

Re: [PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Philippe Mathieu-Daudé 3 years, 3 months ago

Hi Bin,

On 1/29/21 9:51 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Unlike SD mode, when SD card is working in SPI mode, the argument
> of CMD13 is stuff bits. Hence we should bypass the RCA check.

Is this for detecting events by polling? From spec v8:

  5.7.5 Event Indication Method
  5.7.5.1 FX_EVENT (Bit06 of Card Status)

    An event indication method is introduced from Version 4.20.
    Card may use Bit06 of R1 (R1b) response of any SD command
    to indicate event generation.

  F.2 Concept of Event Detection Method
  F.2.2 Host Implementation to use Event Detection Method

> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> Based-on: http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226787
> 
>  hw/sd/sd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 8c397d4ad7..4f902d0b72 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1166,8 +1166,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>      case 13:	/* CMD13:  SEND_STATUS */
>          switch (sd->mode) {
>          case sd_data_transfer_mode:
> -            if (sd->rca != rca)
> +            if (!sd->spi && sd->rca != rca) {
>                  return sd_r0;
> +            }
>  
>              return sd_r1;
>  
>

Re: [PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Bin Meng 3 years, 3 months ago

Hi Philippe,

On Fri, Jan 29, 2021 at 10:11 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Bin,
>
> On 1/29/21 9:51 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Unlike SD mode, when SD card is working in SPI mode, the argument
> > of CMD13 is stuff bits. Hence we should bypass the RCA check.
>
> Is this for detecting events by polling? From spec v8:
>
>   5.7.5 Event Indication Method
>   5.7.5.1 FX_EVENT (Bit06 of Card Status)
>
>     An event indication method is introduced from Version 4.20.
>     Card may use Bit06 of R1 (R1b) response of any SD command
>     to indicate event generation.
>
>   F.2 Concept of Event Detection Method
>   F.2.2 Host Implementation to use Event Detection Method
>

I think you looked at the wrong place. See spec v8

chapter 7.3.1.3 Detailed Command Description

"The card shall ignore stuff bits and reserved bits in an argument"

and Table 7-3 Commands and Arguments

CMD13 Argument [31:0] stuff bits

Regards,
Bin

Re: [PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Philippe Mathieu-Daudé 3 years, 2 months ago

On 1/30/21 11:20 AM, Bin Meng wrote:
> Hi Philippe,
> 
> On Fri, Jan 29, 2021 at 10:11 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Hi Bin,
>>
>> On 1/29/21 9:51 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> Unlike SD mode, when SD card is working in SPI mode, the argument
>>> of CMD13 is stuff bits. Hence we should bypass the RCA check.
>>
>> Is this for detecting events by polling? From spec v8:
>>
>>   5.7.5 Event Indication Method
>>   5.7.5.1 FX_EVENT (Bit06 of Card Status)
>>
>>     An event indication method is introduced from Version 4.20.
>>     Card may use Bit06 of R1 (R1b) response of any SD command
>>     to indicate event generation.
>>
>>   F.2 Concept of Event Detection Method
>>   F.2.2 Host Implementation to use Event Detection Method
>>
> 
> I think you looked at the wrong place. See spec v8
> 
> chapter 7.3.1.3 Detailed Command Description
> 
> "The card shall ignore stuff bits and reserved bits in an argument"
> 
> and Table 7-3 Commands and Arguments
> 
> CMD13 Argument [31:0] stuff bits

Indeed. Adding this information in the commit description makes
things obvious ;)

The SPI responses are not well implemented, in this case (CMD13)
it is incorrect, we should return a 2-byte R2. Your patch
correctly skip the RCA check, but we still invalid data.

Re: [PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Bin Meng 3 years, 2 months ago

Hi Philippe,

On Mon, Feb 8, 2021 at 9:08 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/30/21 11:20 AM, Bin Meng wrote:
> > Hi Philippe,
> >
> > On Fri, Jan 29, 2021 at 10:11 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> Hi Bin,
> >>
> >> On 1/29/21 9:51 AM, Bin Meng wrote:
> >>> From: Bin Meng <bin.meng@windriver.com>
> >>>
> >>> Unlike SD mode, when SD card is working in SPI mode, the argument
> >>> of CMD13 is stuff bits. Hence we should bypass the RCA check.
> >>
> >> Is this for detecting events by polling? From spec v8:
> >>
> >>   5.7.5 Event Indication Method
> >>   5.7.5.1 FX_EVENT (Bit06 of Card Status)
> >>
> >>     An event indication method is introduced from Version 4.20.
> >>     Card may use Bit06 of R1 (R1b) response of any SD command
> >>     to indicate event generation.
> >>
> >>   F.2 Concept of Event Detection Method
> >>   F.2.2 Host Implementation to use Event Detection Method
> >>
> >
> > I think you looked at the wrong place. See spec v8
> >
> > chapter 7.3.1.3 Detailed Command Description
> >
> > "The card shall ignore stuff bits and reserved bits in an argument"
> >
> > and Table 7-3 Commands and Arguments
> >
> > CMD13 Argument [31:0] stuff bits
>
> Indeed. Adding this information in the commit description makes
> things obvious ;)

I thought that's already obvious. If you search for "stuff bits" or
"CMD13" in the spec, you will get the information.

>
> The SPI responses are not well implemented, in this case (CMD13)
> it is incorrect, we should return a 2-byte R2. Your patch
> correctly skip the RCA check, but we still invalid data.

This is already handled in ssi-sd.c in SPI mode. As for the SD mode,
that should be a separate patch instead of mixing two fixes into one.

Regards,
Bin

Re: [PATCH] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode

Posted by Bin Meng 3 years, 2 months ago

On Mon, Feb 8, 2021 at 9:55 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Philippe,
>
> On Mon, Feb 8, 2021 at 9:08 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/30/21 11:20 AM, Bin Meng wrote:
> > > Hi Philippe,
> > >
> > > On Fri, Jan 29, 2021 at 10:11 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >>
> > >> Hi Bin,
> > >>
> > >> On 1/29/21 9:51 AM, Bin Meng wrote:
> > >>> From: Bin Meng <bin.meng@windriver.com>
> > >>>
> > >>> Unlike SD mode, when SD card is working in SPI mode, the argument
> > >>> of CMD13 is stuff bits. Hence we should bypass the RCA check.
> > >>
> > >> Is this for detecting events by polling? From spec v8:
> > >>
> > >>   5.7.5 Event Indication Method
> > >>   5.7.5.1 FX_EVENT (Bit06 of Card Status)
> > >>
> > >>     An event indication method is introduced from Version 4.20.
> > >>     Card may use Bit06 of R1 (R1b) response of any SD command
> > >>     to indicate event generation.
> > >>
> > >>   F.2 Concept of Event Detection Method
> > >>   F.2.2 Host Implementation to use Event Detection Method
> > >>
> > >
> > > I think you looked at the wrong place. See spec v8
> > >
> > > chapter 7.3.1.3 Detailed Command Description
> > >
> > > "The card shall ignore stuff bits and reserved bits in an argument"
> > >
> > > and Table 7-3 Commands and Arguments
> > >
> > > CMD13 Argument [31:0] stuff bits
> >
> > Indeed. Adding this information in the commit description makes
> > things obvious ;)
>
> I thought that's already obvious. If you search for "stuff bits" or
> "CMD13" in the spec, you will get the information.
>
> >
> > The SPI responses are not well implemented, in this case (CMD13)
> > it is incorrect, we should return a 2-byte R2. Your patch
> > correctly skip the RCA check, but we still invalid data.
>
> This is already handled in ssi-sd.c in SPI mode. As for the SD mode,
> that should be a separate patch instead of mixing two fixes into one.

Sigh, I should have checked the spec before I replied.

The CMD13 response for SD card is R1. Only SPI mode is R2. As I
mentioned SPI mode R2 is already handled in ssi-sd.c, so there is
nothing wrong here.

Regards,
Bin