diff mbox series

[v2,01/13] hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate

Message ID 20250111183711.2338-2-shentey@gmail.com
State New
Headers show
Series i.MX and SDHCI improvements | expand

Commit Message

Bernhard Beschow Jan. 11, 2025, 6:36 p.m. UTC 
In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
"DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
currently misses to set the DMA bit which causes the driver to loop forever. Fix
that by setting the DMA bit if enabled when doing DMA block transfers.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Michael Tokarev Jan. 15, 2025, 12:55 p.m. UTC | #1 
11.01.2025 21:36, Bernhard Beschow wrote:
> In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
> "DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
> currently misses to set the DMA bit which causes the driver to loop forever. Fix
> that by setting the DMA bit if enabled when doing DMA block transfers.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Is this a qemu-stable material?

Thanks,

/mjt
Bernhard Beschow Jan. 16, 2025, 11:39 p.m. UTC | #2 
Am 15. Januar 2025 12:55:29 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:
>11.01.2025 21:36, Bernhard Beschow wrote:
>> In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
>> "DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
>> currently misses to set the DMA bit which causes the driver to loop forever. Fix
>> that by setting the DMA bit if enabled when doing DMA block transfers.
>> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
>Is this a qemu-stable material?

Good question. Given that this part of the code has some further issues [1] I'd rather not alter stable behavior because we might just trade one bug for another.

Best regards,
Bernhard

[1] <https://lore.kernel.org/qemu-devel/4b846383-83bf-4252-a172-95604f2f585b@linaro.org/>


>
>Thanks,
>
>/mjt
Michael Tokarev Jan. 17, 2025, 7:11 a.m. UTC | #3 
17.01.2025 02:39, Bernhard Beschow wrote:
> Am 15. Januar 2025 12:55:29 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:

>> Is this a qemu-stable material?
> 
> Good question. Given that this part of the code has some further issues [1] I'd rather not alter stable behavior because we might just trade one bug for another.

And it's a good answer!  Thank you for sharing your thoughts.
I agree, I dropped this (simple) change from the stable series.

/mjt
diff mbox series

Patch

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 299cd4bc1b..a958c11497 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -665,12 +665,13 @@  static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
         }
     }
 
+    if (s->norintstsen & SDHC_NISEN_DMA) {
+        s->norintsts |= SDHC_NIS_DMA;
+    }
+
     if (s->blkcnt == 0) {
         sdhci_end_transfer(s);
     } else {
-        if (s->norintstsen & SDHC_NISEN_DMA) {
-            s->norintsts |= SDHC_NIS_DMA;
-        }
         sdhci_update_irq(s);
     }
 }
@@ -691,6 +692,10 @@  static void sdhci_sdma_transfer_single_block(SDHCIState *s)
     }
     s->blkcnt--;
 
+    if (s->norintstsen & SDHC_NISEN_DMA) {
+        s->norintsts |= SDHC_NIS_DMA;
+    }
+
     sdhci_end_transfer(s);
 }