On 7/24/23 00:18, Bernhard Beschow wrote:
Am 16. Juli 2023 19:53:37 UTC schrieb Bernhard Beschow <shen...@gmail.com>:
Am 10. Juli 2023 16:01:46 UTC schrieb Bernhard Beschow <shen...@gmail.com>:
Am 10. Juli 2023 10:16:35 UTC schrieb "Philippe Mathieu-Daudé"
<phi...@linaro.org>:
On 9/7/23 10:09, Bernhard Beschow wrote:
Since commit c0a55a0c9da2 "hw/sd/sdhci: Support big endian SD host controller
interfaces" sdhci_common_realize() forces all SD card controllers to use either
sdhci_mmio_le_ops or sdhci_mmio_be_ops, depending on the "endianness" property.
However, there are device models which use different MMIO ops: TYPE_IMX_USDHC
uses usdhc_mmio_ops and TYPE_S3C_SDHCI uses sdhci_s3c_mmio_ops.
Forcing sdhci_mmio_le_ops breaks SD card handling on the "sabrelite" board, for
example. Fix this by defaulting the io_ops to little endian and switch to big
endian in sdhci_common_realize() only if there is a matchig big endian variant
available.
Fixes: c0a55a0c9da2 ("hw/sd/sdhci: Support big endian SD host controller
interfaces")
Signed-off-by: Bernhard Beschow <shen...@gmail.com>
---
hw/sd/sdhci.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6811f0f1a8..362c2c86aa 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1382,6 +1382,8 @@ void sdhci_initfn(SDHCIState *s)
s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
sdhci_raise_insertion_irq, s);
s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
sdhci_data_transfer, s);
+
+ s->io_ops = &sdhci_mmio_le_ops;
}
void sdhci_uninitfn(SDHCIState *s)
@@ -1399,9 +1401,13 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
What about simply keeping the same code guarded with 'if (!s->io_ops)'?
I chose below approach since it provides an error message when one attempts to
set one of the other device models to BE rather than just silently ignoring it.
Also, I didn't want to make the assumption that `s->io_ops == NULL` implied
that sdhci_mmio_*_ops is needed. That's similar material the bug fixed is made of,
so I wanted to prevent that in the first place by being more explicit.
In combination with the new error message the limitations of the current code
become hopefully very apparent now, and at the same time should provide enough
hints for adding BE support to the other device models if ever needed.
Best regards,
Bernhard
Ping
Ping^2
I would like to have the bug fixed in 8.1.
+1
Not that I care too much - I build qemu myself anyway and carry the patch
locally -
but this really should get fixed.
Guenter