hw/sd: eMMC support

[PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

Posted by Cédric Le Goater 2 years, 7 months ago

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5ff132139ea9..95cb46b87519 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
+{
+    return sd_r0;
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
         [23]        = sd_cmd_SET_BLOCK_COUNT,
         [41]        = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
+        [55]        = emmc_cmd_APP_CMD,
         [58]        = sd_cmd_illegal,
         [59]        = sd_cmd_illegal,
     },
-- 
2.41.0

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

Posted by Philippe Mathieu-Daudé 1 year, 7 months ago

Hi Cédric,

On 3/7/23 15:24, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 5ff132139ea9..95cb46b87519 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>       return sd_r2_i;
>   }
>   
> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
> +{
> +    return sd_r0;

Why are you returning R0? This is invalid, only R1 can be
returned by APP_CMD.

> +}
> +
>   static const SDProto sd_proto_emmc = {
>       .name = "eMMC",
>       .cmd = {
> @@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
>           [23]        = sd_cmd_SET_BLOCK_COUNT,
>           [41]        = sd_cmd_illegal,
>           [52 ... 54] = sd_cmd_illegal,
> +        [55]        = emmc_cmd_APP_CMD,
>           [58]        = sd_cmd_illegal,
>           [59]        = sd_cmd_illegal,
>       },

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

Posted by Cédric Le Goater 1 year, 7 months ago

On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 5ff132139ea9..95cb46b87519 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>       return sd_r2_i;
>>   }
>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>> +{
>> +    return sd_r0;
> 
> Why are you returning R0? This is invalid, only R1 can be
> returned by APP_CMD.

Probably a typo. This is old ... 4/5 years at least.

Thanks,

C.




> 
>> +}
>> +
>>   static const SDProto sd_proto_emmc = {
>>       .name = "eMMC",
>>       .cmd = {
>> @@ -2219,6 +2224,7 @@ static const SDProto sd_proto_emmc = {
>>           [23]        = sd_cmd_SET_BLOCK_COUNT,
>>           [41]        = sd_cmd_illegal,
>>           [52 ... 54] = sd_cmd_illegal,
>> +        [55]        = emmc_cmd_APP_CMD,
>>           [58]        = sd_cmd_illegal,
>>           [59]        = sd_cmd_illegal,
>>       },
>

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

Posted by Philippe Mathieu-Daudé 1 year, 7 months ago

On 25/6/24 17:13, Cédric Le Goater wrote:
> On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
>> Hi Cédric,
>>
>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sd.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 5ff132139ea9..95cb46b87519 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t 
>>> emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>>       return sd_r2_i;
>>>   }
>>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>>> +{
>>> +    return sd_r0;
>>
>> Why are you returning R0? This is invalid, only R1 can be
>> returned by APP_CMD.
> 
> Probably a typo. This is old ... 4/5 years at least.

Well, a smart typo, because it hides unimplemented features
(and probably some bugs). Maybe someone clever used R0 on
purpose :)

> Thanks,
> 
> C.

Re: [PATCH 18/32] hw/sd: Add emmc_cmd_APP_CMD() handler

Posted by Cédric Le Goater 1 year, 7 months ago

On 6/25/24 5:32 PM, Philippe Mathieu-Daudé wrote:
> On 25/6/24 17:13, Cédric Le Goater wrote:
>> On 6/25/24 5:04 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Cédric,
>>>
>>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>   hw/sd/sd.c | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>>> index 5ff132139ea9..95cb46b87519 100644
>>>> --- a/hw/sd/sd.c
>>>> +++ b/hw/sd/sd.c
>>>> @@ -2207,6 +2207,11 @@ static sd_rsp_type_t emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
>>>>       return sd_r2_i;
>>>>   }
>>>> +static sd_rsp_type_t emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
>>>> +{
>>>> +    return sd_r0;
>>>
>>> Why are you returning R0? This is invalid, only R1 can be
>>> returned by APP_CMD.
>>
>> Probably a typo. This is old ... 4/5 years at least.
> 
> Well, a smart typo, because it hides unimplemented features
> (and probably some bugs). Maybe someone clever used R0 on
> purpose :)

I can't tell. The initial patch [*] had :

@@ -1115,6 +1219,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
  
      /* Application specific commands (Class 8) */
      case 55:	/* CMD55:  APP_CMD */
+        /* Not supported by MMC */
+        if (sd->emmc) {
+            return sd_r0;
+        }
+

but it's 13 years old now.

Thanks,

C.


[*] https://lore.kernel.org/qemu-devel/1311635951-11047-5-git-send-email-vpalatin@chromium.org/