[Patch] SDHCI: fixing extend sdhci controller ops usage

1. ops bug fixing
For IMX and S3C sdhci control the ops are given in inst_init
but it will be overwritten by class_init which calls the class_realize
fix it by checking whether ops has value of not.

2. CLKCON updates
According to usdhc RM for imx, the lower byte is reserved
when the card is present, we need set the CLKCON CLK related bit
and when write to the CLKCON register the lower byte shall be masked

Co-authored-by: Jiamin Ma 423337961@qq.com<mailto:423337961@qq.com>
Signed-off-by: Hake Huang hake.huang@oss.nxp.com<mailto:hake.huang@oss.nxp.com>
---
hw/sd/sdhci.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6811f0f1a8..df45f08da4 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -259,6 +259,10 @@ static void sdhci_set_inserted(DeviceState *dev, bool level)
             if (s->norintstsen & SDHC_NISEN_INSERT) {
                 s->norintsts |= SDHC_NIS_INSERT;
             }
+            /*always ensure the clock is ready */
+            s->clkcon |= SDHC_CLOCK_SDCLK_EN;
+            s->clkcon |= SDHC_CLOCK_INT_STABLE;
+            s->clkcon |= SDHC_CLOCK_INT_EN;
         } else {
             s->prnsts = 0x1fa0000;
             s->pwrcon &= ~SDHC_POWER_ON;
@@ -1397,16 +1401,18 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
{
     ERRP_GUARD();
-    switch (s->endianness) {
-    case DEVICE_LITTLE_ENDIAN:
-        s->io_ops = &sdhci_mmio_le_ops;
-        break;
-    case DEVICE_BIG_ENDIAN:
-        s->io_ops = &sdhci_mmio_be_ops;
-        break;
-    default:
-        error_setg(errp, "Incorrect endianness");
-        return;
+    if (s->io_ops == NULL) {
+        switch (s->endianness) {
+        case DEVICE_LITTLE_ENDIAN:
+            s->io_ops = &sdhci_mmio_le_ops;
+            break;
+        case DEVICE_BIG_ENDIAN:
+            s->io_ops = &sdhci_mmio_be_ops;
+            break;
+        default:
+            error_setg(errp, "Incorrect endianness");
+            return;
+        }
     }
     sdhci_init_readonly_registers(s, errp);
@@ -1669,10 +1675,12 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
     case USDHC_TUNE_CTRL_STATUS:
     case USDHC_UNDOCUMENTED_REG27:
     case USDHC_TUNING_CTRL:
-    case USDHC_MIX_CTRL:
     case USDHC_WTMK_LVL:
         ret = 0;
         break;
+    case USDHC_MIX_CTRL:
+        ret = s->trnmod;
+        break;
     }
     return ret;
@@ -1822,6 +1830,10 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
          */
         sdhci_write(opaque, offset, val | s->trnmod, size);
         break;
+    case SDHC_CLKCON:
+        /* imx usdhc low 7 bits of CLKCON is always 1  */
+        sdhci_write(opaque, offset, val | 0xf, size);
+        break;
     case SDHC_BLKSIZE:
         /*
          * ESDHCI does not implement "Host SDMA Buffer Boundary", and
--
2.34.1

Regards,
Hake

Hi Phil, Meng,

Would you please kindly review and comments on below patch, this fixes an issue when imx/s3c sdhci controller is used. We plan to have one new NXP platform supported in QEMU following this. Thanks again.

Regards,
Hake

From: Hake Huang (OSS) <hake.huang@oss.nxp.com>
Sent: 2023年4月12日 16:42
To: philmd@linaro.org; Meng, Bin <Bin.Meng@windriver.com>; qemu-block@nongnu.org; qemu-devel@nongnu.org
Subject: [Patch] SDHCI: fixing extend sdhci controller ops usage

[Patch] SDHCI: fixing extend sdhci controller ops usage

1. ops bug fixing
For IMX and S3C sdhci control the ops are given in inst_init
but it will be overwritten by class_init which calls the class_realize
fix it by checking whether ops has value of not.

2. CLKCON updates
According to usdhc RM for imx, the lower byte is reserved
when the card is present, we need set the CLKCON CLK related bit
and when write to the CLKCON register the lower byte shall be masked

Co-authored-by: Jiamin Ma 423337961@qq.com<mailto:423337961@qq.com>
Signed-off-by: Hake Huang hake.huang@oss.nxp.com<mailto:hake.huang@oss.nxp.com>
---
hw/sd/sdhci.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6811f0f1a8..df45f08da4 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -259,6 +259,10 @@ static void sdhci_set_inserted(DeviceState *dev, bool level)
             if (s->norintstsen & SDHC_NISEN_INSERT) {
                 s->norintsts |= SDHC_NIS_INSERT;
             }
+            /*always ensure the clock is ready */
+            s->clkcon |= SDHC_CLOCK_SDCLK_EN;
+            s->clkcon |= SDHC_CLOCK_INT_STABLE;
+            s->clkcon |= SDHC_CLOCK_INT_EN;
         } else {
             s->prnsts = 0x1fa0000;
             s->pwrcon &= ~SDHC_POWER_ON;
@@ -1397,16 +1401,18 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
{
     ERRP_GUARD();

-    switch (s->endianness) {
-    case DEVICE_LITTLE_ENDIAN:
-        s->io_ops = &sdhci_mmio_le_ops;
-        break;
-    case DEVICE_BIG_ENDIAN:
-        s->io_ops = &sdhci_mmio_be_ops;
-        break;
-    default:
-        error_setg(errp, "Incorrect endianness");
-        return;
+    if (s->io_ops == NULL) {
+        switch (s->endianness) {
+        case DEVICE_LITTLE_ENDIAN:
+            s->io_ops = &sdhci_mmio_le_ops;
+            break;
+        case DEVICE_BIG_ENDIAN:
+            s->io_ops = &sdhci_mmio_be_ops;
+            break;
+        default:
+            error_setg(errp, "Incorrect endianness");
+            return;
+        }
     }

     sdhci_init_readonly_registers(s, errp);
@@ -1669,10 +1675,12 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
     case USDHC_TUNE_CTRL_STATUS:
     case USDHC_UNDOCUMENTED_REG27:
     case USDHC_TUNING_CTRL:
-    case USDHC_MIX_CTRL:
     case USDHC_WTMK_LVL:
         ret = 0;
         break;
+    case USDHC_MIX_CTRL:
+        ret = s->trnmod;
+        break;
     }

     return ret;
@@ -1822,6 +1830,10 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
          */
         sdhci_write(opaque, offset, val | s->trnmod, size);
         break;
+    case SDHC_CLKCON:
+        /* imx usdhc low 7 bits of CLKCON is always 1  */
+        sdhci_write(opaque, offset, val | 0xf, size);
+        break;
     case SDHC_BLKSIZE:
         /*
          * ESDHCI does not implement "Host SDMA Buffer Boundary", and
--
2.34.1

Regards,
Hake