RE: [Patch] SDHCI: fixing extend sdhci controller ops usage

[Hake Huang (OSS)]

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

Signed-off-by: Hake Huang 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