Patchwork [arm-devs,v1,1/5] sd/sd.c: Fix "inquiry" ACMD41

login
register
mail settings
Submitter Igor Mitsyanko
Date May 22, 2013, 1:37 p.m.
Message ID <CA+x0pt76OOvgEjK37L5N_BacpNC=gxbK5nhcVKxzc65BKFatiQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/245621/
State New
Headers show

Comments

Igor Mitsyanko - May 22, 2013, 1:37 p.m.
On 05/21/2013 10:50 AM, peter.crosthwaite@xilinx.com wrote:

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
<peter.crosthwaite@xilinx.com>

the SD command ACMD41 can be used in a read only mode to query device
state without doing the SD card initialisation. This is valid even
which the device is already initialised. Fix the command to be
responsive when in the ready state accordingly.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
<peter.crosthwaite@xilinx.com>
---

 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)



I couldn't find any info in SD specification that would confirm this change
correctness, what about
table "Table 4-29: Card State Transition Table" which states that ACMD41 is
illegal in "ready" state?
Peter Crosthwaite - May 22, 2013, 11:42 p.m.
Hi Igor,

On Wed, May 22, 2013 at 11:37 PM, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
>
> On 05/21/2013 10:50 AM, peter.crosthwaite@xilinx.com wrote:
>
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> the SD command ACMD41 can be used in a read only mode to query device
> state without doing the SD card initialisation. This is valid even
> which the device is already initialised. Fix the command to be
> responsive when in the ready state accordingly.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
>  hw/sd/sd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 2e0ef3e..89bfb7a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1277,6 +1277,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          }
>          switch (sd->state) {
>          case sd_idle_state:
> +        case sd_ready_state:
>              /* We accept any voltage.  10000 V is nothing.  */
>              if (req.arg)
>                  sd->state = sd_ready_state;
>
>
> I couldn't find any info in SD specification that would confirm this change
> correctness, what about
> table "Table 4-29: Card State Transition Table" which states that ACMD41 is
> illegal in "ready" state?
>

By the letter of the spec I think you are right. Although this patch
is needed to make my QEMU consistent with my real hardware. I'll dig
deeper.

Regards,
Peter

> --
> Best wishes,
> Igor Mitsyanko
> email: i.mitsyanko@gmail.com
Igor Mitsyanko - May 23, 2013, 10:31 a.m.
On 05/23/2013 03:42 AM, Peter Crosthwaite wrote:
> Hi Igor,
>
> On Wed, May 22, 2013 at 11:37 PM, Igor Mitsyanko <i.mitsyanko@gmail.com> wrote:
>>
>> On 05/21/2013 10:50 AM, peter.crosthwaite@xilinx.com wrote:
>>
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> the SD command ACMD41 can be used in a read only mode to query device
>> state without doing the SD card initialisation. This is valid even
>> which the device is already initialised. Fix the command to be
>> responsive when in the ready state accordingly.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>
>>   hw/sd/sd.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 2e0ef3e..89bfb7a 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -1277,6 +1277,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>>           }
>>           switch (sd->state) {
>>           case sd_idle_state:
>> +        case sd_ready_state:
>>               /* We accept any voltage.  10000 V is nothing.  */
>>               if (req.arg)
>>                   sd->state = sd_ready_state;
>>
>>
>> I couldn't find any info in SD specification that would confirm this change
>> correctness, what about
>> table "Table 4-29: Card State Transition Table" which states that ACMD41 is
>> illegal in "ready" state?
>>
>
> By the letter of the spec I think you are right. Although this patch
> is needed to make my QEMU consistent with my real hardware. I'll dig
> deeper.
>

Hello, Peter, after thinking some more about this, I assume that table
4-29 might be incorrect. It depends on when idle->ready state transition
occurs, its not clear from specification.

Controller issues first ACMD41 to start card's initialisation. Spec
states that this process could take up to 1sec, and all this time
controller should query card's busy state in a loop with ACMD41. After
response to ACMD41 has busy flag deasserted, card is considered to be
"ready". But this means that card was already in ready state when it
received last ACMD41 command, right? Unless card transitions to ready
state only after a response to last ACMD41 was sent.

If that's how real SD card behaves in your tests, then I think this
patch is OK, but it could benefit from a short comment explaining that
this behaviour is not covered by specification.


Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com>


> Regards,
> Peter
>
>> --
>> Best wishes,
>> Igor Mitsyanko
>> email: i.mitsyanko@gmail.com
>
>


--
Best wishes,
Igor Mitsyanko
email: i.mitsyanko@gmail.com

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2e0ef3e..89bfb7a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1277,6 +1277,7 @@  static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         switch (sd->state) {
         case sd_idle_state:
+        case sd_ready_state:
             /* We accept any voltage.  10000 V is nothing.  */
             if (req.arg)
                 sd->state = sd_ready_state;