diff mbox series

[21/32] hw/sd: Add mmc switch function support

Message ID 20230703132509.2474225-22-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series hw/sd: eMMC support | expand

Commit Message

Cédric Le Goater July 3, 2023, 1:24 p.m. UTC 
From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

switch operation in mmc cards, updated the ext_csd register to
request changes in card operations. Here we implement similar
sequence but requests are mostly dummy and make no change.

Implement SWITCH_ERROR if the write operation offset goes beyond length
of ext_csd.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[ clg: - ported on SDProto framework ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Philippe Mathieu-Daudé June 12, 2024, 10:49 p.m. UTC | #1 
On 3/7/23 15:24, Cédric Le Goater wrote:
> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> 
> switch operation in mmc cards, updated the ext_csd register to
> request changes in card operations. Here we implement similar
> sequence but requests are mostly dummy and make no change.
> 
> Implement SWITCH_ERROR if the write operation offset goes beyond length
> of ext_csd.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> [ clg: - ported on SDProto framework ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)


> +static void mmc_function_switch(SDState *sd, uint32_t arg)
> +{
> +    uint32_t access = extract32(arg, 24, 2);
> +    uint32_t index = extract32(arg, 16, 8);
> +    uint32_t value = extract32(arg, 8, 8);
> +    uint8_t b = sd->ext_csd[index];

This field is added in the next patch :)

../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct 
SDState'
     uint8_t b = sd->ext_csd[index];
                 ~~  ^
../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct SDState'
     sd->ext_csd[index] = b;
     ~~  ^

No need to respin, as I'm integrating your work.

> +    switch (access) {
> +    case MMC_CMD6_ACCESS_COMMAND_SET:
> +        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
> +        return;
> +    case MMC_CMD6_ACCESS_SET_BITS:
> +        b |= value;
> +        break;
> +    case MMC_CMD6_ACCESS_CLEAR_BITS:
> +        b &= ~value;
> +        break;
> +    case MMC_CMD6_ACCESS_WRITE_BYTE:
> +        b = value;
> +        break;
> +    }
> +
> +    if (index >= 192) {
> +        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
> +        return;
> +    }
> +
> +    sd->ext_csd[index] = b;
> +}
Cédric Le Goater June 13, 2024, 7:44 a.m. UTC | #2 
On 6/13/24 12:49 AM, Philippe Mathieu-Daudé wrote:
> On 3/7/23 15:24, Cédric Le Goater wrote:
>> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>
>> switch operation in mmc cards, updated the ext_csd register to
>> request changes in card operations. Here we implement similar
>> sequence but requests are mostly dummy and make no change.
>>
>> Implement SWITCH_ERROR if the write operation offset goes beyond length
>> of ext_csd.
>>
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> [ clg: - ported on SDProto framework ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 52 insertions(+)
> 
> 
>> +static void mmc_function_switch(SDState *sd, uint32_t arg)
>> +{
>> +    uint32_t access = extract32(arg, 24, 2);
>> +    uint32_t index = extract32(arg, 16, 8);
>> +    uint32_t value = extract32(arg, 8, 8);
>> +    uint8_t b = sd->ext_csd[index];
> 
> This field is added in the next patch :)
> 
> ../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct SDState'
>      uint8_t b = sd->ext_csd[index];
>                  ~~  ^
> ../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct SDState'
>      sd->ext_csd[index] = b;
>      ~~  ^
> 
> No need to respin, as I'm integrating your work.


Ah good !

There are 3 main parts :

   * Base eMMC support:
     hw/sd: Basis for eMMC support
     hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
     hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
     hw/sd: Add emmc_cmd_SET_RELATIVE_ADDR() handler
     hw/sd: Add emmc_cmd_APP_CMD() handler
     hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
     hw/sd: Add CMD21 tuning sequence
     hw/sd: Add mmc switch function support
     hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler

   * Boot area support
     hw/sd: Support boot area in emmc image
     hw/sd: Subtract bootarea size from blk
     hw/sd: Add boot config support
     hw/sd: Fix SET_BLOCK_COUNT command argument
     hw/sd: Update CMD1 definition for MMC

   * Aspeed eMMC support :
     hw/arm/aspeed: Add eMMC device
     hw/arm/aspeed: Load eMMC first boot area as a boot rom
     hw/arm/aspeed: Set boot device to emmc
     aspeed: Set bootconfig
     aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

and I can rework the aspeed part if needed.

Here is an image you can try boot on :

   https://www.kaod.org/qemu/aspeed/rainier/mmc-p10bmc.qcow2

Run with :

   qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev user,id=net0 -drive file=./mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd2,index=2 -nographic -serial mon:stdio

Thanks,

C.
  

>> +    switch (access) {
>> +    case MMC_CMD6_ACCESS_COMMAND_SET:
>> +        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
>> +        return;
>> +    case MMC_CMD6_ACCESS_SET_BITS:
>> +        b |= value;
>> +        break;
>> +    case MMC_CMD6_ACCESS_CLEAR_BITS:
>> +        b &= ~value;
>> +        break;
>> +    case MMC_CMD6_ACCESS_WRITE_BYTE:
>> +        b = value;
>> +        break;
>> +    }
>> +
>> +    if (index >= 192) {
>> +        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
>> +        return;
>> +    }
>> +
>> +    sd->ext_csd[index] = b;
>> +}
>
Philippe Mathieu-Daudé June 13, 2024, 8:41 a.m. UTC | #3 
On 13/6/24 09:44, Cédric Le Goater wrote:
> On 6/13/24 12:49 AM, Philippe Mathieu-Daudé wrote:
>> On 3/7/23 15:24, Cédric Le Goater wrote:
>>> From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>
>>> switch operation in mmc cards, updated the ext_csd register to
>>> request changes in card operations. Here we implement similar
>>> sequence but requests are mostly dummy and make no change.
>>>
>>> Implement SWITCH_ERROR if the write operation offset goes beyond length
>>> of ext_csd.
>>>
>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>>> [ clg: - ported on SDProto framework ]
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 52 insertions(+)
>>
>>
>>> +static void mmc_function_switch(SDState *sd, uint32_t arg)
>>> +{
>>> +    uint32_t access = extract32(arg, 24, 2);
>>> +    uint32_t index = extract32(arg, 16, 8);
>>> +    uint32_t value = extract32(arg, 8, 8);
>>> +    uint8_t b = sd->ext_csd[index];
>>
>> This field is added in the next patch :)
>>
>> ../../hw/sd/sd.c:927:21: error: no member named 'ext_csd' in 'struct 
>> SDState'
>>      uint8_t b = sd->ext_csd[index];
>>                  ~~  ^
>> ../../hw/sd/sd.c:949:9: error: no member named 'ext_csd' in 'struct 
>> SDState'
>>      sd->ext_csd[index] = b;
>>      ~~  ^
>>
>> No need to respin, as I'm integrating your work.
> 
> 
> Ah good !
> 
> There are 3 main parts :
> 
>    * Base eMMC support:
>      hw/sd: Basis for eMMC support
>      hw/sd: Add emmc_cmd_SEND_OP_CMD() handler
>      hw/sd: Add emmc_cmd_ALL_SEND_CID() handler
>      hw/sd: Add emmc_cmd_SET_RELATIVE_ADDR() handler
>      hw/sd: Add emmc_cmd_APP_CMD() handler
>      hw/sd: add emmc_cmd_SEND_TUNING_BLOCK() handler
>      hw/sd: Add CMD21 tuning sequence
>      hw/sd: Add mmc switch function support
>      hw/sd: Add emmc_cmd_SEND_EXT_CSD() handler
> 
 >      hw/sd: Fix SET_BLOCK_COUNT command argument
 >      hw/sd: Update CMD1 definition for MMC

I am focusing on these for now and should include them
in the next SD pull request, but I'll repost them with
more patches to be reviewed first.

>    * Boot area support
>      hw/sd: Support boot area in emmc image
>      hw/sd: Subtract bootarea size from blk
>      hw/sd: Add boot config support

I haven't studied them yet and plan to look at them
after mentioned repost. I'll try to also include them
in the PR.

>    * Aspeed eMMC support :
>      hw/arm/aspeed: Add eMMC device
>      hw/arm/aspeed: Load eMMC first boot area as a boot rom
>      hw/arm/aspeed: Set boot device to emmc
>      aspeed: Set bootconfig
>      aspeed: Introduce a 'boot-emmc' property for AST2600 based machines

Once my PR posted I'll review them and let you merge them.
Then I'll invite you a beer at the next KVM forum :)

> and I can rework the aspeed part if needed.
> 
> Here is an image you can try boot on :
> 
>    https://www.kaod.org/qemu/aspeed/rainier/mmc-p10bmc.qcow2
> 
> Run with :
> 
>    qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev 
> user,id=net0 -drive 
> file=./mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd2,index=2 -nographic 
> -serial mon:stdio
> 
> Thanks,
> 
> C.
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7332f7a18435..51e2254728a6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -482,6 +482,7 @@  static void sd_set_rca(SDState *sd)
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, SWITCH_ERROR,                7,  1)
 FIELD(CSR, READY_FOR_DATA,              8,  1)
 FIELD(CSR, CURRENT_STATE,               9,  4)
 FIELD(CSR, ERASE_RESET,                13,  1)
@@ -878,6 +879,43 @@  static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+enum {
+    MMC_CMD6_ACCESS_COMMAND_SET = 0,
+    MMC_CMD6_ACCESS_SET_BITS,
+    MMC_CMD6_ACCESS_CLEAR_BITS,
+    MMC_CMD6_ACCESS_WRITE_BYTE,
+};
+
+static void mmc_function_switch(SDState *sd, uint32_t arg)
+{
+    uint32_t access = extract32(arg, 24, 2);
+    uint32_t index = extract32(arg, 16, 8);
+    uint32_t value = extract32(arg, 8, 8);
+    uint8_t b = sd->ext_csd[index];
+
+    switch (access) {
+    case MMC_CMD6_ACCESS_COMMAND_SET:
+        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
+        return;
+    case MMC_CMD6_ACCESS_SET_BITS:
+        b |= value;
+        break;
+    case MMC_CMD6_ACCESS_CLEAR_BITS:
+        b &= ~value;
+        break;
+    case MMC_CMD6_ACCESS_WRITE_BYTE:
+        b = value;
+        break;
+    }
+
+    if (index >= 192) {
+        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
+        return;
+    }
+
+    sd->ext_csd[index] = b;
+}
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
     int i, mode, new_func;
@@ -2263,6 +2301,19 @@  static sd_rsp_type_t emmc_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+static sd_rsp_type_t emmc_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_transfer_state:
+        sd->state = sd_programming_state;
+        mmc_function_switch(sd, req.arg);
+        sd->state = sd_transfer_state;
+        return sd_r1b;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2271,6 +2322,7 @@  static const SDProto sd_proto_emmc = {
         [2]         = emmc_cmd_ALL_SEND_CID,
         [3]         = emmc_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
+        [6]         = emmc_cmd_SWITCH_FUNCTION,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [23]        = sd_cmd_SET_BLOCK_COUNT,
         [21]        = emmc_cmd_SEND_TUNING_BLOCK,