[PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[Bin Meng]

This series is spun off from the following series as it is hw/sd
centric, so that it can be picked up separately by Philippe.

http://patchwork.ozlabs.org/project/qemu-devel/list/?series=195648

This series fixed 2 SD card issues, and added a new model for
Cadence SDHCI controller.

Patch "[09/18] hw/sd: sdhci: Make sdhci_poweron_reset() internal visible"
in this series per the review comments.

Changes in v2:
- remove the pointless zero initialization
- fix SDSC size check in sd_set_csd() too
- use 's' for the model state
- call device_cold_reset() in cadence_sdhci_reset()
- add .impl in cadence_sdhci_ops
- move Cadence specific register defines to cadence_sdhci.c
- use 'sdhci' instead of 'slot' to represent SDHCIState
- use sysbus_mmio_get_region() to access SDHCI model's memory region
- initialize TYPE_SYSBUS_SDHCI in the instance_init() so that users
  of Cadence SDHCI do not have to do that themselves
- propergate irq and 'sd-bus' from generic-sdhci

Bin Meng (3):
  hw/sd: sd: Fix incorrect populated function switch status data
    structure
  hw/sd: sd: Correct the maximum size of a Standard Capacity SD Memory
    Card
  hw/sd: Add Cadence SDHCI emulation

 hw/sd/Kconfig                 |   4 +
 hw/sd/Makefile.objs           |   1 +
 hw/sd/cadence_sdhci.c         | 200 ++++++++++++++++++++++++++++++++++++++++++
 hw/sd/sd.c                    |   9 +-
 include/hw/sd/cadence_sdhci.h |  46 ++++++++++
 5 files changed, 257 insertions(+), 3 deletions(-)
 create mode 100644 hw/sd/cadence_sdhci.c
 create mode 100644 include/hw/sd/cadence_sdhci.h

-- 
2.7.4

[PATCH v2 1/3] hw/sd: sd: Fix incorrect populated function switch status data structure

[Bin Meng]

At present the function switch status data structure bit [399:376]
are wrongly pupulated. These 3 bytes encode function switch status
for the 6 function groups, with 4 bits per group, starting from
function group 6 at bit 399, then followed by function group 5 at
bit 395, and so on.

However the codes mistakenly fills in the function group 1 status
at bit 399. This fixes the code logic.

Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

---

Changes in v2:
- remove the pointless zero initialization

 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 fad9cf1..3226404 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -806,11 +806,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
     sd->data[11] = 0x43;
     sd->data[12] = 0x80;	/* Supported group 1 functions */
     sd->data[13] = 0x03;
+
     for (i = 0; i < 6; i ++) {
         new_func = (arg >> (i * 4)) & 0x0f;
         if (mode && new_func != 0x0f)
             sd->function_group[i] = new_func;
-        sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);
+        sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);
     }
     memset(&sd->data[17], 0, 47);
     stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));
-- 
2.7.4

[PATCH v2 2/3] hw/sd: sd: Correct the maximum size of a Standard Capacity SD Memory Card

[Bin Meng]

Per the SD spec, Standard Capacity SD Memory Card (SDSC) supports
capacity up to and including 2 GiB.

Fixes: 2d7adea4fe ("hw/sd: Support SDHC size cards")
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- fix SDSC size check in sd_set_csd() too

 hw/sd/sd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3226404..254d713 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -50,6 +50,8 @@
 
 //#define DEBUG_SD 1
 
+#define SDSC_MAX_CAPACITY   (2 * GiB)
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -313,7 +315,7 @@ static void sd_ocr_powerup(void *opaque)
     /* card power-up OK */
     sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
 
-    if (sd->size > 1 * GiB) {
+    if (sd->size > SDSC_MAX_CAPACITY) {
         sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
     }
 }
@@ -385,7 +387,7 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
     uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
 
-    if (size <= 1 * GiB) { /* Standard Capacity SD */
+    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
         sd->csd[0] = 0x00;	/* CSD structure */
         sd->csd[1] = 0x26;	/* Data read access-time-1 */
         sd->csd[2] = 0x00;	/* Data read access-time-2 */
-- 
2.7.4

[PATCH v2 2/3] hw/sd: sd: Correct the maximum size of a Standard Capacity SD Memory Card

[Bin Meng]

Per the SD spec, Standard Capacity SD Memory Card (SDSC) supports
capacity up to and including 2 GiB.

Fixes: 2d7adea4fe ("hw/sd: Support SDHC size cards")
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- fix SDSC size check in sd_set_csd() too

 hw/sd/sd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3226404..254d713 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -50,6 +50,8 @@
 
 //#define DEBUG_SD 1
 
+#define SDSC_MAX_CAPACITY   (2 * GiB)
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -313,7 +315,7 @@ static void sd_ocr_powerup(void *opaque)
     /* card power-up OK */
     sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
 
-    if (sd->size > 1 * GiB) {
+    if (sd->size > SDSC_MAX_CAPACITY) {
         sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
     }
 }
@@ -385,7 +387,7 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
     uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
 
-    if (size <= 1 * GiB) { /* Standard Capacity SD */
+    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
         sd->csd[0] = 0x00;	/* CSD structure */
         sd->csd[1] = 0x26;	/* Data read access-time-1 */
         sd->csd[2] = 0x00;	/* Data read access-time-2 */
-- 
2.7.4

Re: [PATCH v2 2/3] hw/sd: sd: Correct the maximum size of a Standard Capacity SD Memory Card

[Philippe Mathieu-Daudé]

On 8/17/20 12:03 PM, Bin Meng wrote:
> Per the SD spec, Standard Capacity SD Memory Card (SDSC) supports
> capacity up to and including 2 GiB.
> 
> Fixes: 2d7adea4fe ("hw/sd: Support SDHC size cards")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> 
> Changes in v2:
> - fix SDSC size check in sd_set_csd() too
> 
>  hw/sd/sd.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 3226404..254d713 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -50,6 +50,8 @@
>  
>  //#define DEBUG_SD 1
>  
> +#define SDSC_MAX_CAPACITY   (2 * GiB)
> +
>  typedef enum {
>      sd_r0 = 0,    /* no response */
>      sd_r1,        /* normal response command */
> @@ -313,7 +315,7 @@ static void sd_ocr_powerup(void *opaque)
>      /* card power-up OK */
>      sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
>  
> -    if (sd->size > 1 * GiB) {
> +    if (sd->size > SDSC_MAX_CAPACITY) {
>          sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
>      }
>  }
> @@ -385,7 +387,7 @@ static void sd_set_csd(SDState *sd, uint64_t size)
>      uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
>      uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
>  
> -    if (size <= 1 * GiB) { /* Standard Capacity SD */
> +    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
>          sd->csd[0] = 0x00;	/* CSD structure */
>          sd->csd[1] = 0x26;	/* Data read access-time-1 */
>          sd->csd[2] = 0x00;	/* Data read access-time-2 */
> 

Re: [PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[Philippe Mathieu-Daudé]

Hi Sai Pavan, you said you were interested to test the first 2
patches. FYI I plan to queue them and send the pull request tomorrow
or Saturday the latest.

On 8/17/20 12:03 PM, Bin Meng wrote:
> This series is spun off from the following series as it is hw/sd
> centric, so that it can be picked up separately by Philippe.
> 
> http://patchwork.ozlabs.org/project/qemu-devel/list/?series=195648
> 
> This series fixed 2 SD card issues, and added a new model for
> Cadence SDHCI controller.
> 
> Patch "[09/18] hw/sd: sdhci: Make sdhci_poweron_reset() internal visible"
> in this series per the review comments.
> 
> Changes in v2:
> - remove the pointless zero initialization
> - fix SDSC size check in sd_set_csd() too
> - use 's' for the model state
> - call device_cold_reset() in cadence_sdhci_reset()
> - add .impl in cadence_sdhci_ops
> - move Cadence specific register defines to cadence_sdhci.c
> - use 'sdhci' instead of 'slot' to represent SDHCIState
> - use sysbus_mmio_get_region() to access SDHCI model's memory region
> - initialize TYPE_SYSBUS_SDHCI in the instance_init() so that users
>   of Cadence SDHCI do not have to do that themselves
> - propergate irq and 'sd-bus' from generic-sdhci
> 
> Bin Meng (3):
>   hw/sd: sd: Fix incorrect populated function switch status data
>     structure
>   hw/sd: sd: Correct the maximum size of a Standard Capacity SD Memory
>     Card
>   hw/sd: Add Cadence SDHCI emulation
> 
>  hw/sd/Kconfig                 |   4 +
>  hw/sd/Makefile.objs           |   1 +
>  hw/sd/cadence_sdhci.c         | 200 ++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sd.c                    |   9 +-
>  include/hw/sd/cadence_sdhci.h |  46 ++++++++++
>  5 files changed, 257 insertions(+), 3 deletions(-)
>  create mode 100644 hw/sd/cadence_sdhci.c
>  create mode 100644 include/hw/sd/cadence_sdhci.h
> 

Re: [PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[Bin Meng]

Hi Philippe,

On Fri, Aug 21, 2020 at 2:04 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Sai Pavan, you said you were interested to test the first 2
> patches. FYI I plan to queue them and send the pull request tomorrow
> or Saturday the latest.

Have you got a chance to review the v2 of 3rd patch?

"hw/sd: Add Cadence SDHCI emulation"

Regards,
Bin

Re: [PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[Philippe Mathieu-Daudé]

Hi Bin,

On 8/21/20 2:54 AM, Bin Meng wrote:
> Hi Philippe,
> 
> On Fri, Aug 21, 2020 at 2:04 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Hi Sai Pavan, you said you were interested to test the first 2
>> patches. FYI I plan to queue them and send the pull request tomorrow
>> or Saturday the latest.
> 
> Have you got a chance to review the v2 of 3rd patch?
> 
> "hw/sd: Add Cadence SDHCI emulation"

I'll have a look at it, but it makes sense to merge it via the
tree using it (so the RISCV tree).

Meanwhile I'm queueing patches 1 and 2 to my sd-next tree,
adding the Tested-by from Sai Pavan from:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg732027.html

Thanks,

Phil.

> 
> Regards,
> Bin
> 

Re: [PATCH v2 0/3] hw/sd: Add Cadence SDHCI emulation

[Bin Meng]

Hi Philippe,

On Fri, Aug 21, 2020 at 10:21 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Bin,
>
> On 8/21/20 2:54 AM, Bin Meng wrote:
> > Hi Philippe,
> >
> > On Fri, Aug 21, 2020 at 2:04 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> Hi Sai Pavan, you said you were interested to test the first 2
> >> patches. FYI I plan to queue them and send the pull request tomorrow
> >> or Saturday the latest.
> >
> > Have you got a chance to review the v2 of 3rd patch?
> >
> > "hw/sd: Add Cadence SDHCI emulation"
>
> I'll have a look at it, but it makes sense to merge it via the
> tree using it (so the RISCV tree).

Thank you. Sure I will include the Cadence SDHCI patch in the
PolarFire SoC support series in the next version.

>
> Meanwhile I'm queueing patches 1 and 2 to my sd-next tree,
> adding the Tested-by from Sai Pavan from:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg732027.html
>

I just noticed that the v2 patch has the wrong author email address,
so I plan to send v3 of patch 1 and 2 to correct it, with Sai Pavan's
Tested-by tag. Sorry!

Regards,
Bin