* [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2)
@ 2018-01-23 2:08 Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 01/16] sdhci: use error_propagate(local_err) in realize() Philippe Mathieu-Daudé
` (15 more replies)
0 siblings, 16 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum, Krzysztof Kozlowski
Since v8:
- we keep the 'capareg' property, this simplify a lot the series
- the Zynq 7000 uses the datasheet CAPAREG
- reset R-b/A-b
Since v7:
- use error_propagate()
Since v6:
- rebased on upstream to use DEFINE_SDHCI_COMMON_PROPERTIES
- included qtests back
- add PCI qtests
- series was getting big, splitting again; next one is "implement Spec v3"
- following Alistair advice, new properties default to current
SDHC_CAPAB_REG_DEFAULT (probably 'default' for historical reason, being
the first capabilities set). This leads to weird properties imho, i.e.
defaulting 'max-frequency' to 52 MHz, the Exynos SoC has to set this
property to 0.
- addressed Alistair reviews
- confirmed the Exynos4210 specs after chatting on IRC with Krzysztof
Kozlowski who kindly verified in the datasheet.
Since v5:
- addressed Alistair reviews
- dropped "abstract generic-sdhci"
- dropped Linux Device Tree names
- split qtests in another series
- change the bcm2835 minimum blocksize to 1KB (Andrew Baumann)
- added Alistair R-b
- based on Alistair work:
- add SD tunning sequence via Host Control 2 to use UHS-I cards
- add CMD/DAT[] fields in the Present State (used in next series
to switch card voltage)
based on Alistair work:
- add SD tunning sequence via Host Control 2 to use UHS-I cards
- add CMD/DAT[] fields in the Present State (used in next series
to switch card voltage)
Since v4 ("SDHCI: add qtests and fix few issues"):
- spec_version default to v2 (current behaviour)
- addressed Alistair review (no v1, tell user about valid version)
Since v3:
- no change, but split back in 2 series, 1st part is "SDHCI: housekeeping v5",
Since v2:
- more detailed 'capabilities', all boards converted to use these properties
- since all qtests pass, removed the previous 'capareg' property
- added Stefan/Alistair R-b
- corrected 'access' LED behavior (Alistair's review)
- more uses of the registerfields API
- remove some dead code
- cosmetix:
- added more comments
- renamed a pair of registers
- reordered few struct members
Since v1:
- addressed Alistair Francis review comments, added some R-b
- only move register defines to "sd-internal.h"
- fixed deposit64() arguments
- dropped unuseful s->fifo_buffer = NULL
- use a qemu_irq for the LED, restrict the logging to ON/OFF
- fixed a trace format string error
- included Andrey Smirnov ACMD12ERRSTS write patch
- dropped few unuseful patches, and separate the Python polemical ones for later
>From the "SDHCI housekeeping" series:
- 1: we restrict part of "sd/sd.h" into local "sd-internal.h",
- 2,3: we somehow beautiful the code, no logical changes,
- 4-7: we refactor the common sysbus/pci qdev code,
- 8-10: we add plenty of trace events which will result useful later,
- 11: we finally expose a "dma-memory" property.
>From the "SDHCI: add a qtest and fix few issues" series:
- 12,13: fix registers
- 14,15: boards can specify which SDHCI Spec to use (v2 and v3 so far)
- 15-20: HCI qtest
Regards,
Phil.
$ git backport-diff with v8
001/16:[----] [--] 'sdhci: use error_propagate(local_err) in realize()'
002/16:[0004] [FC] 'sdhci: add qtest to check the SD capabilities register'
003/16:[----] [--] 'sdhci: add check_capab_readonly() qtest'
004/16:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
005/16:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
006/16:[----] [--] 'sdhci: add qtest to check the SD Spec version'
007/16:[0022] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
008/16:[down] 'sdhci: use a numeric value for the default CAPAB register'
009/16:[down] 'sdhci: simplify sdhci_get_fifolen()'
010/16:[down] 'sdhci: check the Spec v1 capabilities correctness'
011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
012/16:[----] [--] 'sdhci: Fix 64-bit ADMA2'
013/16:[down] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
014/16:[down] 'hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()'
015/16:[down] 'hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)'
016/16:[down] 'hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet'
Philippe Mathieu-Daudé (15):
sdhci: use error_propagate(local_err) in realize()
sdhci: add qtest to check the SD capabilities register
sdhci: add check_capab_readonly() qtest
sdhci: add a check_capab_baseclock() qtest
sdhci: add a check_capab_sdma() qtest
sdhci: add qtest to check the SD Spec version
sdhci: add a 'spec_version property' (default to v2)
sdhci: use a numeric value for the default CAPAB register
sdhci: simplify sdhci_get_fifolen()
sdhci: check the Spec v1 capabilities correctness
sdhci: add BLOCK_SIZE_MASK for DMA
sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
Sai Pavan Boddu (1):
sdhci: Fix 64-bit ADMA2
include/hw/sd/sdhci.h | 2 +
hw/sd/sdhci-internal.h | 43 ++++++---
hw/arm/exynos4210.c | 14 ++-
hw/arm/xilinx_zynq.c | 53 +++++-----
hw/sd/sdhci.c | 257 ++++++++++++++++++++++++++++++++-----------------
tests/sdhci-test.c | 216 +++++++++++++++++++++++++++++++++++++++++
hw/sd/trace-events | 1 +
tests/Makefile.include | 3 +
8 files changed, 464 insertions(+), 125 deletions(-)
create mode 100644 tests/sdhci-test.c
--
2.15.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 01/16] sdhci: use error_propagate(local_err) in realize()
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register Philippe Mathieu-Daudé
` (14 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
avoid the "errp && *errp" pattern (not recommended in "qapi/error.h" comments).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/sd/sdhci.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index f9264d3be5..8c9c0fbc2a 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1300,10 +1300,12 @@ static Property sdhci_pci_properties[] = {
static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
{
SDHCIState *s = PCI_SDHCI(dev);
+ Error *local_err = NULL;
sdhci_initfn(s);
sdhci_common_realize(s, errp);
- if (errp && *errp) {
+ if (local_err) {
+ error_propagate(errp, local_err);
return;
}
@@ -1381,9 +1383,11 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
{
SDHCIState *s = SYSBUS_SDHCI(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ Error *local_err = NULL;
sdhci_common_realize(s, errp);
- if (errp && *errp) {
+ if (local_err) {
+ error_propagate(errp, local_err);
return;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 01/16] sdhci: use error_propagate(local_err) in realize() Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-02-07 17:10 ` Paolo Bonzini
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 03/16] sdhci: add check_capab_readonly() qtest Philippe Mathieu-Daudé
` (13 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
The PCI model is tested with the pc/x86_64 machine,
the SysBus model with the smdkc210/arm machine.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdhci-test.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/Makefile.include | 3 ++
2 files changed, 137 insertions(+)
create mode 100644 tests/sdhci-test.c
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
new file mode 100644
index 0000000000..517e2ed5a2
--- /dev/null
+++ b/tests/sdhci-test.c
@@ -0,0 +1,134 @@
+/*
+ * QTest testcase for SDHCI controllers
+ *
+ * Written by Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "hw/registerfields.h"
+#include "libqtest.h"
+#include "libqos/pci-pc.h"
+#include "hw/pci/pci.h"
+
+#define SDHC_CAPAB 0x40
+#define SDHC_HCVER 0xFE
+
+static const struct sdhci_t {
+ const char *arch, *machine;
+ struct {
+ uintptr_t addr;
+ uint8_t version;
+ uint8_t baseclock;
+ struct {
+ bool sdma;
+ uint64_t reg;
+ } capab;
+ } sdhci;
+ struct {
+ uint16_t vendor_id, device_id;
+ } pci;
+} models[] = {
+ /* PC via PCI */
+ { "x86_64", "pc",
+ {-1, 2, 0, {1, 0x057834b4} },
+ .pci = { PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_SDHCI } },
+
+ /* Exynos4210 */
+ { "arm", "smdkc210",
+ {0x12510000, 2, 0, {1, 0x5e80080} } },
+};
+
+static struct {
+ QPCIBus *pcibus;
+ QPCIDevice *dev;
+ QPCIBar mem_bar;
+} g = { };
+
+static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
+{
+ if (g.dev) {
+ uint64_t value;
+
+ qpci_memread(g.dev, g.mem_bar, reg_addr, &value, sizeof(value));
+
+ return value;
+ } else {
+ QTestState *qtest = global_qtest;
+
+ return qtest_readq(qtest, base + reg_addr);
+ }
+}
+
+static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
+{
+ uint64_t capab;
+
+ capab = sdhci_readq(addr, SDHC_CAPAB);
+ g_assert_cmphex(capab, ==, expected_capab);
+}
+
+static void machine_start(const struct sdhci_t *test)
+{
+ if (test->pci.vendor_id) {
+ /* PCI */
+ uint16_t vendor_id, device_id;
+ uint64_t barsize;
+
+ global_qtest = qtest_startf("-machine %s -d unimp -device sdhci-pci",
+ test->machine);
+
+ g.pcibus = qpci_init_pc(NULL);
+
+ /* Find PCI device and verify it's the right one */
+ g.dev = qpci_device_find(g.pcibus, QPCI_DEVFN(4, 0));
+ g_assert_nonnull(g.dev);
+ vendor_id = qpci_config_readw(g.dev, PCI_VENDOR_ID);
+ device_id = qpci_config_readw(g.dev, PCI_DEVICE_ID);
+ g_assert(vendor_id == test->pci.vendor_id);
+ g_assert(device_id == test->pci.device_id);
+ g.mem_bar = qpci_iomap(g.dev, 0, &barsize);
+ qpci_device_enable(g.dev);
+ } else {
+ /* SysBus */
+ global_qtest = qtest_startf("-machine %s -d unimp", test->machine);
+ }
+}
+
+static void machine_stop(void)
+{
+ g_free(g.dev);
+ qtest_quit(global_qtest);
+}
+
+static void test_machine(const void *data)
+{
+ const struct sdhci_t *test = data;
+
+ machine_start(test);
+
+ check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
+
+ machine_stop();
+}
+
+int main(int argc, char *argv[])
+{
+ const char *arch = qtest_get_arch();
+ char *name;
+ int i;
+
+ g_test_init(&argc, &argv, NULL);
+ for (i = 0; i < ARRAY_SIZE(models); i++) {
+ if (strcmp(arch, models[i].arch)) {
+ continue;
+ }
+ name = g_strdup_printf("sdhci/%s", models[i].machine);
+ qtest_add_data_func(name, &models[i], test_machine);
+ g_free(name);
+ }
+
+ return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8883274ae1..756725b0f9 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -293,6 +293,7 @@ check-qtest-i386-y += tests/migration-test$(EXESUF)
check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
check-qtest-i386-y += tests/numa-test$(EXESUF)
check-qtest-x86_64-y += $(check-qtest-i386-y)
+check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -363,6 +364,7 @@ gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
gcov-files-arm-y += hw/timer/arm_mptimer.c
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
+check-qtest-arm-y += tests/sdhci-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
@@ -816,6 +818,7 @@ tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
tests/numa-test$(EXESUF): tests/numa-test.o
tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
+tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 03/16] sdhci: add check_capab_readonly() qtest
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 01/16] sdhci: use error_propagate(local_err) in realize() Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 04/16] sdhci: add a check_capab_baseclock() qtest Philippe Mathieu-Daudé
` (12 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/sdhci-test.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 517e2ed5a2..4833dd4204 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -62,6 +62,17 @@ static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
}
}
+static void sdhci_writeq(uintptr_t base, uint32_t reg_addr, uint64_t value)
+{
+ if (g.dev) {
+ qpci_memwrite(g.dev, g.mem_bar, reg_addr, &value, sizeof(value));
+ } else {
+ QTestState *qtest = global_qtest;
+
+ qtest_writeq(qtest, base + reg_addr, value);
+ }
+}
+
static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
{
uint64_t capab;
@@ -70,6 +81,20 @@ static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
g_assert_cmphex(capab, ==, expected_capab);
}
+static void check_capab_readonly(uintptr_t addr)
+{
+ const uint64_t vrand = 0x123456789abcdef;
+ uint64_t capab0, capab1;
+
+ capab0 = sdhci_readq(addr, SDHC_CAPAB);
+ g_assert_cmpuint(capab0, !=, vrand);
+
+ sdhci_writeq(addr, SDHC_CAPAB, vrand);
+ capab1 = sdhci_readq(addr, SDHC_CAPAB);
+ g_assert_cmpuint(capab1, !=, vrand);
+ g_assert_cmpuint(capab1, ==, capab0);
+}
+
static void machine_start(const struct sdhci_t *test)
{
if (test->pci.vendor_id) {
@@ -110,6 +135,7 @@ static void test_machine(const void *data)
machine_start(test);
check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
+ check_capab_readonly(test->sdhci.addr);
machine_stop();
}
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 04/16] sdhci: add a check_capab_baseclock() qtest
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 03/16] sdhci: add check_capab_readonly() qtest Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 05/16] sdhci: add a check_capab_sdma() qtest Philippe Mathieu-Daudé
` (11 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/sdhci-test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 4833dd4204..5f036c8a75 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -14,6 +14,7 @@
#include "hw/pci/pci.h"
#define SDHC_CAPAB 0x40
+FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8); /* since v2 */
#define SDHC_HCVER 0xFE
static const struct sdhci_t {
@@ -95,6 +96,18 @@ static void check_capab_readonly(uintptr_t addr)
g_assert_cmpuint(capab1, ==, capab0);
}
+static void check_capab_baseclock(uintptr_t addr, uint8_t expected_freq)
+{
+ uint64_t capab, capab_freq;
+
+ if (!expected_freq) {
+ return;
+ }
+ capab = sdhci_readq(addr, SDHC_CAPAB);
+ capab_freq = FIELD_EX64(capab, SDHC_CAPAB, BASECLKFREQ);
+ g_assert_cmpuint(capab_freq, ==, expected_freq);
+}
+
static void machine_start(const struct sdhci_t *test)
{
if (test->pci.vendor_id) {
@@ -136,6 +149,7 @@ static void test_machine(const void *data)
check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
check_capab_readonly(test->sdhci.addr);
+ check_capab_baseclock(test->sdhci.addr, test->sdhci.baseclock);
machine_stop();
}
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 05/16] sdhci: add a check_capab_sdma() qtest
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 04/16] sdhci: add a check_capab_baseclock() qtest Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 06/16] sdhci: add qtest to check the SD Spec version Philippe Mathieu-Daudé
` (10 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/sdhci-test.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 5f036c8a75..f4c43b8120 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -15,6 +15,7 @@
#define SDHC_CAPAB 0x40
FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8); /* since v2 */
+FIELD(SDHC_CAPAB, SDMA, 22, 1);
#define SDHC_HCVER 0xFE
static const struct sdhci_t {
@@ -108,6 +109,15 @@ static void check_capab_baseclock(uintptr_t addr, uint8_t expected_freq)
g_assert_cmpuint(capab_freq, ==, expected_freq);
}
+static void check_capab_sdma(uintptr_t addr, bool supported)
+{
+ uint64_t capab, capab_sdma;
+
+ capab = sdhci_readq(addr, SDHC_CAPAB);
+ capab_sdma = FIELD_EX64(capab, SDHC_CAPAB, SDMA);
+ g_assert_cmpuint(capab_sdma, ==, supported);
+}
+
static void machine_start(const struct sdhci_t *test)
{
if (test->pci.vendor_id) {
@@ -149,6 +159,7 @@ static void test_machine(const void *data)
check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
check_capab_readonly(test->sdhci.addr);
+ check_capab_sdma(test->sdhci.addr, test->sdhci.capab.sdma);
check_capab_baseclock(test->sdhci.addr, test->sdhci.baseclock);
machine_stop();
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 06/16] sdhci: add qtest to check the SD Spec version
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 05/16] sdhci: add a check_capab_sdma() qtest Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 07/16] sdhci: add a 'spec_version property' (default to v2) Philippe Mathieu-Daudé
` (9 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/sdhci-test.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index f4c43b8120..094e0570e1 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -49,6 +49,21 @@ static struct {
QPCIBar mem_bar;
} g = { };
+static uint32_t sdhci_readl(uintptr_t base, uint32_t reg_addr)
+{
+ if (g.dev) {
+ uint32_t value;
+
+ qpci_memread(g.dev, g.mem_bar, reg_addr, &value, sizeof(value));
+
+ return value;
+ } else {
+ QTestState *qtest = global_qtest;
+
+ return qtest_readl(qtest, base + reg_addr);
+ }
+}
+
static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
{
if (g.dev) {
@@ -75,6 +90,16 @@ static void sdhci_writeq(uintptr_t base, uint32_t reg_addr, uint64_t value)
}
}
+static void check_specs_version(uintptr_t addr, uint8_t version)
+{
+ uint32_t v;
+
+ v = sdhci_readl(addr, SDHC_HCVER);
+ v &= 0xff;
+ v += 1;
+ g_assert_cmpuint(v, ==, version);
+}
+
static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
{
uint64_t capab;
@@ -157,6 +182,7 @@ static void test_machine(const void *data)
machine_start(test);
+ check_specs_version(test->sdhci.addr, test->sdhci.version);
check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
check_capab_readonly(test->sdhci.addr);
check_capab_sdma(test->sdhci.addr, test->sdhci.capab.sdma);
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 07/16] sdhci: add a 'spec_version property' (default to v2)
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 06/16] sdhci: add qtest to check the SD Spec version Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register Philippe Mathieu-Daudé
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/sd/sdhci-internal.h | 4 ++--
include/hw/sd/sdhci.h | 2 ++
hw/sd/sdhci.c | 27 +++++++++++++++++++++++----
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index fc807f08f3..b7751c815f 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -210,9 +210,9 @@
/* Slot interrupt status */
#define SDHC_SLOT_INT_STATUS 0xFC
-/* HWInit Host Controller Version Register 0x0401 */
+/* HWInit Host Controller Version Register */
#define SDHC_HCVER 0xFE
-#define SD_HOST_SPECv2_VERS 0x2401
+#define SDHC_HCVER_VENDOR 0x24
#define SDHC_REGISTERS_MAP_SIZE 0x100
#define SDHC_INSERTION_DELAY (NANOSECONDS_PER_SECOND)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index cb37182536..96e07de2a2 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -76,6 +76,7 @@ typedef struct SDHCIState {
/* Read-only registers */
uint64_t capareg; /* Capabilities Register */
uint64_t maxcurr; /* Maximum Current Capabilities Register */
+ uint16_t version; /* Host Controller Version Register */
uint8_t *fifo_buffer; /* SD host i/o FIFO buffer */
uint32_t buf_maxsz;
@@ -90,6 +91,7 @@ typedef struct SDHCIState {
/* Configurable properties */
bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
+ uint8_t sd_spec_version;
} SDHCIState;
#define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8c9c0fbc2a..1331062306 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -174,7 +174,8 @@ static void sdhci_reset(SDHCIState *s)
timer_del(s->insert_timer);
timer_del(s->transfer_timer);
- /* Set all registers to 0. Capabilities registers are not cleared
+
+ /* Set all registers to 0. Capabilities/Version registers are not cleared
* and assumed to always preserve their value, given to them during
* initialization */
memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
@@ -918,7 +919,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
ret = (uint32_t)(s->admasysaddr >> 32);
break;
case SDHC_SLOT_INT_STATUS:
- ret = (SD_HOST_SPECv2_VERS << 16) | sdhci_slotint(s);
+ ret = (s->version << 16) | sdhci_slotint(s);
break;
default:
qemu_log_mask(LOG_UNIMP, "SDHC rd_%ub @0x%02" HWADDR_PRIx " "
@@ -1174,11 +1175,22 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
}
}
+static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
+{
+ if (s->sd_spec_version != 2) {
+ error_setg(errp, "Only Spec v2 is supported");
+ return;
+ }
+ s->version = (SDHC_HCVER_VENDOR << 8) | (s->sd_spec_version - 1);
+}
+
/* --- qdev common --- */
#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
- /* Capabilities registers provide information on supported features
- * of this specific host controller implementation */ \
+ DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
+ \
+ /* Capabilities registers provide information on supported
+ * features of this specific host controller implementation */ \
DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
@@ -1204,6 +1216,13 @@ static void sdhci_uninitfn(SDHCIState *s)
static void sdhci_common_realize(SDHCIState *s, Error **errp)
{
+ Error *local_err = NULL;
+
+ sdhci_init_readonly_registers(s, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
s->buf_maxsz = sdhci_get_fifolen(s);
s->fifo_buffer = g_malloc0(s->buf_maxsz);
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 07/16] sdhci: add a 'spec_version property' (default to v2) Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-24 18:27 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen() Philippe Mathieu-Daudé
` (7 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
using many #defines is not portable when scaling to different HCI.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci.c | 74 +++++++++++++----------------------------------------------
1 file changed, 16 insertions(+), 58 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1331062306..1c781c4ba5 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -39,67 +39,25 @@
#define TYPE_SDHCI_BUS "sdhci-bus"
#define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)
+#define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val))
+
/* Default SD/MMC host controller features information, which will be
* presented in CAPABILITIES register of generic SD host controller at reset.
- * If not stated otherwise:
- * 0 - not supported, 1 - supported, other - prohibited.
+ *
+ * support:
+ * - 3.3v and 1.8v voltages
+ * - SDMA/ADMA1/ADMA2
+ * - high-speed
+ * max host controller R/W buffers size: 512B
+ * max clock frequency for SDclock: 52 MHz
+ * timeout clock frequency: 52 MHz
+ *
+ * does not support:
+ * - 3.0v voltage
+ * - 64-bit system bus
+ * - suspend/resume
*/
-#define SDHC_CAPAB_64BITBUS 0ul /* 64-bit System Bus Support */
-#define SDHC_CAPAB_18V 1ul /* Voltage support 1.8v */
-#define SDHC_CAPAB_30V 0ul /* Voltage support 3.0v */
-#define SDHC_CAPAB_33V 1ul /* Voltage support 3.3v */
-#define SDHC_CAPAB_SUSPRESUME 0ul /* Suspend/resume support */
-#define SDHC_CAPAB_SDMA 1ul /* SDMA support */
-#define SDHC_CAPAB_HIGHSPEED 1ul /* High speed support */
-#define SDHC_CAPAB_ADMA1 1ul /* ADMA1 support */
-#define SDHC_CAPAB_ADMA2 1ul /* ADMA2 support */
-/* Maximum host controller R/W buffers size
- * Possible values: 512, 1024, 2048 bytes */
-#define SDHC_CAPAB_MAXBLOCKLENGTH 512ul
-/* Maximum clock frequency for SDclock in MHz
- * value in range 10-63 MHz, 0 - not defined */
-#define SDHC_CAPAB_BASECLKFREQ 52ul
-#define SDHC_CAPAB_TOUNIT 1ul /* Timeout clock unit 0 - kHz, 1 - MHz */
-/* Timeout clock frequency 1-63, 0 - not defined */
-#define SDHC_CAPAB_TOCLKFREQ 52ul
-
-/* Now check all parameters and calculate CAPABILITIES REGISTER value */
-#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_18V > 1 || SDHC_CAPAB_30V > 1 || \
- SDHC_CAPAB_33V > 1 || SDHC_CAPAB_SUSPRESUME > 1 || SDHC_CAPAB_SDMA > 1 || \
- SDHC_CAPAB_HIGHSPEED > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 ||\
- SDHC_CAPAB_TOUNIT > 1
-#error Capabilities features can have value 0 or 1 only!
-#endif
-
-#if SDHC_CAPAB_MAXBLOCKLENGTH == 512
-#define MAX_BLOCK_LENGTH 0ul
-#elif SDHC_CAPAB_MAXBLOCKLENGTH == 1024
-#define MAX_BLOCK_LENGTH 1ul
-#elif SDHC_CAPAB_MAXBLOCKLENGTH == 2048
-#define MAX_BLOCK_LENGTH 2ul
-#else
-#error Max host controller block size can have value 512, 1024 or 2048 only!
-#endif
-
-#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \
- SDHC_CAPAB_BASECLKFREQ > 63
-#error SDclock frequency can have value in range 0, 10-63 only!
-#endif
-
-#if SDHC_CAPAB_TOCLKFREQ > 63
-#error Timeout clock frequency can have value in range 0-63 only!
-#endif
-
-#define SDHC_CAPAB_REG_DEFAULT \
- ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_18V << 26) | \
- (SDHC_CAPAB_30V << 25) | (SDHC_CAPAB_33V << 24) | \
- (SDHC_CAPAB_SUSPRESUME << 23) | (SDHC_CAPAB_SDMA << 22) | \
- (SDHC_CAPAB_HIGHSPEED << 21) | (SDHC_CAPAB_ADMA1 << 20) | \
- (SDHC_CAPAB_ADMA2 << 19) | (MAX_BLOCK_LENGTH << 16) | \
- (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \
- (SDHC_CAPAB_TOCLKFREQ))
-
-#define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val))
+#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
static uint8_t sdhci_slotint(SDHCIState *s)
{
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen()
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-24 0:38 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness Philippe Mathieu-Daudé
` (6 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci-internal.h | 4 +++-
hw/sd/sdhci.c | 20 +++++---------------
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index b7751c815f..577ca9da54 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -24,6 +24,8 @@
#ifndef SDHCI_INTERNAL_H
#define SDHCI_INTERNAL_H
+#include "hw/registerfields.h"
+
/* R/W SDMA System Address register 0x0 */
#define SDHC_SYSAD 0x00
@@ -179,7 +181,7 @@
#define SDHC_CAN_DO_ADMA2 0x00080000
#define SDHC_CAN_DO_ADMA1 0x00100000
#define SDHC_64_BIT_BUS_SUPPORT (1 << 28)
-#define SDHC_CAPAB_BLOCKSIZE(x) (((x) >> 16) & 0x3)
+FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
/* HWInit Maximum Current Capabilities Register 0x0 */
#define SDHC_MAXCURR 0x48
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1c781c4ba5..dce1a49af1 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -59,6 +59,11 @@
*/
#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
+static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
+{
+ return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
+}
+
static uint8_t sdhci_slotint(SDHCIState *s)
{
return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
@@ -1118,21 +1123,6 @@ static const MemoryRegionOps sdhci_mmio_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
-{
- switch (SDHC_CAPAB_BLOCKSIZE(s->capareg)) {
- case 0:
- return 512;
- case 1:
- return 1024;
- case 2:
- return 2048;
- default:
- hw_error("SDHC: unsupported value for maximum block size\n");
- return 0;
- }
-}
-
static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
{
if (s->sd_spec_version != 2) {
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen() Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-24 0:26 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 12/16] sdhci: Fix 64-bit ADMA2 Philippe Mathieu-Daudé
` (5 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Incorrect value will throw an error.
Note than Spec v2 is supported by default.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci-internal.h | 21 ++++++++++-
hw/sd/sdhci.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-
hw/sd/trace-events | 1 +
3 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 577ca9da54..c5e26bf8f3 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -86,6 +86,9 @@
/* R/W Host control Register 0x0 */
#define SDHC_HOSTCTL 0x28
+FIELD(SDHC_HOSTCTL, LED_CTRL, 0, 1);
+FIELD(SDHC_HOSTCTL, DATATRANSFERWIDTH, 1, 1); /* SD mode only */
+FIELD(SDHC_HOSTCTL, HIGH_SPEED, 2, 1);
#define SDHC_CTRL_DMA_CHECK_MASK 0x18
#define SDHC_CTRL_SDMA 0x00
#define SDHC_CTRL_ADMA1_32 0x08
@@ -96,6 +99,7 @@
/* R/W Power Control Register 0x0 */
#define SDHC_PWRCON 0x29
#define SDHC_POWER_ON (1 << 0)
+FIELD(SDHC_PWRCON, BUS_VOLTAGE, 1, 3);
/* R/W Block Gap Control Register 0x0 */
#define SDHC_BLKGAP 0x2A
@@ -118,6 +122,7 @@
/* R/W Timeout Control Register 0x0 */
#define SDHC_TIMEOUTCON 0x2E
+FIELD(SDHC_TIMEOUTCON, COUNTER, 0, 4);
/* R/W Software Reset Register 0x0 */
#define SDHC_SWRST 0x2F
@@ -174,17 +179,31 @@
/* ROC Auto CMD12 error status register 0x0 */
#define SDHC_ACMD12ERRSTS 0x3C
+FIELD(SDHC_ACMD12ERRSTS, TIMEOUT_ERR, 1, 1);
+FIELD(SDHC_ACMD12ERRSTS, CRC_ERR, 2, 1);
+FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR, 4, 1);
/* HWInit Capabilities Register 0x05E80080 */
#define SDHC_CAPAB 0x40
-#define SDHC_CAN_DO_DMA 0x00400000
#define SDHC_CAN_DO_ADMA2 0x00080000
#define SDHC_CAN_DO_ADMA1 0x00100000
#define SDHC_64_BIT_BUS_SUPPORT (1 << 28)
+FIELD(SDHC_CAPAB, TOCLKFREQ, 0, 6);
+FIELD(SDHC_CAPAB, TOUNIT, 7, 1);
+FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8);
FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
+FIELD(SDHC_CAPAB, HIGHSPEED, 21, 1);
+FIELD(SDHC_CAPAB, SDMA, 22, 1);
+FIELD(SDHC_CAPAB, SUSPRESUME, 23, 1);
+FIELD(SDHC_CAPAB, V33, 24, 1);
+FIELD(SDHC_CAPAB, V30, 25, 1);
+FIELD(SDHC_CAPAB, V18, 26, 1);
/* HWInit Maximum Current Capabilities Register 0x0 */
#define SDHC_MAXCURR 0x48
+FIELD(SDHC_MAXCURR, V33_VDD1, 0, 8);
+FIELD(SDHC_MAXCURR, V30_VDD1, 8, 8);
+FIELD(SDHC_MAXCURR, V18_VDD1, 16, 8);
/* W Force Event Auto CMD12 Error Interrupt Register 0x0000 */
#define SDHC_FEAER 0x50
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index dce1a49af1..91dfd684d8 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qapi/error.h"
#include "hw/hw.h"
#include "sysemu/block-backend.h"
@@ -64,6 +65,92 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
}
+/* return true on error */
+static bool sdhci_check_capab_freq_range(SDHCIState *s, const char *desc,
+ uint8_t freq, Error **errp)
+{
+ switch (freq) {
+ case 0:
+ case 10 ... 63:
+ break;
+ default:
+ error_setg(errp, "SD %s clock frequency can have value"
+ "in range 0-63 only", desc);
+ return true;
+ }
+ return false;
+}
+
+static void sdhci_check_capareg(SDHCIState *s, Error **errp)
+{
+ uint64_t msk = s->capareg;
+ uint32_t val;
+ bool y;
+
+ switch (s->sd_spec_version) {
+ case 2: /* default version */
+
+ /* fallback */
+ case 1:
+ y = FIELD_EX64(s->capareg, SDHC_CAPAB, TOUNIT);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, TOUNIT, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, TOCLKFREQ);
+ trace_sdhci_capareg(y ? "timeout (MHz)" : "Timeout (KHz)", val);
+ if (sdhci_check_capab_freq_range(s, "timeout", val, errp)) {
+ return;
+ }
+ msk = FIELD_DP64(msk, SDHC_CAPAB, TOCLKFREQ, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, BASECLKFREQ);
+ trace_sdhci_capareg(y ? "base (MHz)" : "Base (KHz)", val);
+ if (sdhci_check_capab_freq_range(s, "base", val, errp)) {
+ return;
+ }
+ msk = FIELD_DP64(msk, SDHC_CAPAB, BASECLKFREQ, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH);
+ if (val >= 0b11) {
+ error_setg(errp, "block size can be 512, 1024 or 2048 only");
+ return;
+ }
+ trace_sdhci_capareg("max block length", sdhci_get_fifolen(s));
+ msk = FIELD_DP64(msk, SDHC_CAPAB, MAXBLOCKLENGTH, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, HIGHSPEED);
+ trace_sdhci_capareg("high speed", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, HIGHSPEED, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, SDMA);
+ trace_sdhci_capareg("SDMA", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, SDMA, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, SUSPRESUME);
+ trace_sdhci_capareg("suspend/resume", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, SUSPRESUME, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, V33);
+ trace_sdhci_capareg("3.3v", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, V33, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, V30);
+ trace_sdhci_capareg("3.0v", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, V30, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, V18);
+ trace_sdhci_capareg("1.8v", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, V18, 0);
+ break;
+
+ default:
+ error_setg(errp, "Unsupported spec version: %u", s->sd_spec_version);
+ }
+ if (msk) {
+ qemu_log_mask(LOG_UNIMP,
+ "SDHCI: unknown CAPAB mask: 0x%016" PRIx64 "\n", msk);
+ }
+}
+
static uint8_t sdhci_slotint(SDHCIState *s)
{
return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
@@ -990,7 +1077,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
case SDHC_TRNMOD:
/* DMA can be enabled only if it is supported as indicated by
* capabilities register */
- if (!(s->capareg & SDHC_CAN_DO_DMA)) {
+ if (!(s->capareg & R_SDHC_CAPAB_SDMA_MASK)) {
value &= ~SDHC_TRNS_DMA;
}
MASKED_WRITE(s->trnmod, mask, value & SDHC_TRNMOD_MASK);
@@ -1125,11 +1212,19 @@ static const MemoryRegionOps sdhci_mmio_ops = {
static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
{
+ Error *local_err = NULL;
+
if (s->sd_spec_version != 2) {
error_setg(errp, "Only Spec v2 is supported");
return;
}
s->version = (SDHC_HCVER_VENDOR << 8) | (s->sd_spec_version - 1);
+
+ sdhci_check_capareg(s, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
}
/* --- qdev common --- */
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 0a121156a3..78d8707669 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -13,6 +13,7 @@ sdhci_adma_transfer_completed(void) ""
sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s%u: addr[0x%04" PRIx64 "] %s 0x%08" PRIx64 " (%" PRIu64 ")"
sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer"
sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"
+sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
# hw/sd/milkymist-memcard.c
milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 12/16] sdhci: Fix 64-bit ADMA2
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 13/16] sdhci: check Spec v2 capabilities (DMA and 64-bit bus) Philippe Mathieu-Daudé
` (4 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Sai Pavan Boddu, qemu-devel, Kevin O'Connor,
Edgar E . Iglesias, Andrey Smirnov, Marcel Apfelbaum
From: Sai Pavan Boddu <saipava@xilinx.com>
The 64-bit ADMA address is not converted to the cpu endianes correctly.
This patch fixes the issue and uses a valid mask for the attribute data.
Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com>
[AF: Re-write commit message]
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/sd/sdhci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 9f9d6835c0..a6bba1939f 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -667,8 +667,8 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
dscr->length = le16_to_cpu(dscr->length);
dma_memory_read(s->dma_as, entry_addr + 4,
(uint8_t *)(&dscr->addr), 8);
- dscr->attr = le64_to_cpu(dscr->attr);
- dscr->attr &= 0xfffffff8;
+ dscr->addr = le64_to_cpu(dscr->addr);
+ dscr->attr &= (uint8_t) ~0xC0;
dscr->incr = 12;
break;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 13/16] sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 12/16] sdhci: Fix 64-bit ADMA2 Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64() Philippe Mathieu-Daudé
` (3 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum
Incorrect value will throw an error.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci-internal.h | 14 +++++++-------
hw/sd/sdhci.c | 19 +++++++++++++++----
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index c5e26bf8f3..4ed9727ec3 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -89,12 +89,12 @@
FIELD(SDHC_HOSTCTL, LED_CTRL, 0, 1);
FIELD(SDHC_HOSTCTL, DATATRANSFERWIDTH, 1, 1); /* SD mode only */
FIELD(SDHC_HOSTCTL, HIGH_SPEED, 2, 1);
-#define SDHC_CTRL_DMA_CHECK_MASK 0x18
+FIELD(SDHC_HOSTCTL, DMA, 3, 2);
#define SDHC_CTRL_SDMA 0x00
-#define SDHC_CTRL_ADMA1_32 0x08
+#define SDHC_CTRL_ADMA1_32 0x08 /* NOT ALLOWED since v2 */
#define SDHC_CTRL_ADMA2_32 0x10
-#define SDHC_CTRL_ADMA2_64 0x18
-#define SDHC_DMA_TYPE(x) ((x) & SDHC_CTRL_DMA_CHECK_MASK)
+#define SDHC_CTRL_ADMA2_64 0x18 /* only v1 & v2 (v3 optional) */
+#define SDHC_DMA_TYPE(x) ((x) & R_SDHC_HOSTCTL_DMA_MASK)
/* R/W Power Control Register 0x0 */
#define SDHC_PWRCON 0x29
@@ -185,19 +185,19 @@ FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR, 4, 1);
/* HWInit Capabilities Register 0x05E80080 */
#define SDHC_CAPAB 0x40
-#define SDHC_CAN_DO_ADMA2 0x00080000
-#define SDHC_CAN_DO_ADMA1 0x00100000
-#define SDHC_64_BIT_BUS_SUPPORT (1 << 28)
FIELD(SDHC_CAPAB, TOCLKFREQ, 0, 6);
FIELD(SDHC_CAPAB, TOUNIT, 7, 1);
FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8);
FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
+FIELD(SDHC_CAPAB, ADMA2, 19, 1); /* since v2 */
+FIELD(SDHC_CAPAB, ADMA1, 20, 1); /* v1 only? */
FIELD(SDHC_CAPAB, HIGHSPEED, 21, 1);
FIELD(SDHC_CAPAB, SDMA, 22, 1);
FIELD(SDHC_CAPAB, SUSPRESUME, 23, 1);
FIELD(SDHC_CAPAB, V33, 24, 1);
FIELD(SDHC_CAPAB, V30, 25, 1);
FIELD(SDHC_CAPAB, V18, 26, 1);
+FIELD(SDHC_CAPAB, BUS64BIT, 28, 1); /* since v2 */
/* HWInit Maximum Current Capabilities Register 0x0 */
#define SDHC_MAXCURR 0x48
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index a6bba1939f..bc3a4f6e16 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -90,6 +90,17 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp)
switch (s->sd_spec_version) {
case 2: /* default version */
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA2);
+ trace_sdhci_capareg("ADMA2", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA2, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA1);
+ trace_sdhci_capareg("ADMA1", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA1, 0);
+
+ val = FIELD_EX64(s->capareg, SDHC_CAPAB, BUS64BIT);
+ trace_sdhci_capareg("64-bit system bus", val);
+ msk = FIELD_DP64(msk, SDHC_CAPAB, BUS64BIT, 0);
/* fallback */
case 1:
@@ -832,7 +843,7 @@ static void sdhci_data_transfer(void *opaque)
break;
case SDHC_CTRL_ADMA1_32:
- if (!(s->capareg & SDHC_CAN_DO_ADMA1)) {
+ if (!(s->capareg & R_SDHC_CAPAB_ADMA1_MASK)) {
trace_sdhci_error("ADMA1 not supported");
break;
}
@@ -840,7 +851,7 @@ static void sdhci_data_transfer(void *opaque)
sdhci_do_adma(s);
break;
case SDHC_CTRL_ADMA2_32:
- if (!(s->capareg & SDHC_CAN_DO_ADMA2)) {
+ if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK)) {
trace_sdhci_error("ADMA2 not supported");
break;
}
@@ -848,8 +859,8 @@ static void sdhci_data_transfer(void *opaque)
sdhci_do_adma(s);
break;
case SDHC_CTRL_ADMA2_64:
- if (!(s->capareg & SDHC_CAN_DO_ADMA2) ||
- !(s->capareg & SDHC_64_BIT_BUS_SUPPORT)) {
+ if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK) ||
+ !(s->capareg & R_SDHC_CAPAB_BUS64BIT_MASK)) {
trace_sdhci_error("64 bit ADMA not supported");
break;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 13/16] sdhci: check Spec v2 capabilities (DMA and 64-bit bus) Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-24 0:37 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2) Philippe Mathieu-Daudé
` (2 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum, Krzysztof Kozlowski,
Igor Mitsyanko, open list:Exynos
We only set a 32-bit value, but this is a good practice in case this
code is used as reference.
(missed in 5efc9016e52)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/exynos4210.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index e8e1d81e62..d89322c7ea 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -378,7 +378,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
DriveInfo *di;
dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
- qdev_prop_set_uint32(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
+ qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
qdev_init_nofail(dev);
busdev = SYS_BUS_DEVICE(dev);
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64() Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-31 16:22 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet Philippe Mathieu-Daudé
2018-01-31 14:09 ` [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum, Krzysztof Kozlowski,
Igor Mitsyanko, open list:Exynos
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/exynos4210.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index d89322c7ea..06f9d1ffa4 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -377,6 +377,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
BlockBackend *blk;
DriveInfo *di;
+ /* Compatible with:
+ * - SD Host Controller Specification Version 2.0
+ * - SDIO Specification Version 2.0
+ * - MMC Specification Version 4.3
+ * - SDMA
+ * - ADMA2
+ *
+ * As this part of the Exynos4210 is not publically available,
+ * we used the "HS-MMC Controller S3C2416X RISC Microprocessor"
+ * public datasheet which is very similar (implementing
+ * MMC Specification Version 4.0 being the only difference noted)
+ */
dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
qdev_init_nofail(dev);
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2) Philippe Mathieu-Daudé
@ 2018-01-23 2:08 ` Philippe Mathieu-Daudé
2018-01-24 0:36 ` Alistair Francis
2018-01-31 14:09 ` [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-23 2:08 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Philippe Mathieu-Daudé,
qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum, Edgar E. Iglesias,
open list:Xilinx Zynq
checking Xilinx datasheet "UG585" (v1.12.1)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/xilinx_zynq.c | 53 ++++++++++++++++++++++++++++------------------------
tests/sdhci-test.c | 5 +++++
2 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1836a4ed45..0f76333770 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -61,6 +61,8 @@ static const int dma_irqs[8] = {
#define SLCR_XILINX_UNLOCK_KEY 0xdf0d
#define SLCR_XILINX_LOCK_KEY 0x767b
+#define ZYNQ_SDHCI_CAPABILITIES 0x69ec0080 /* Datasheet: UG585 (v1.12.1) */
+
#define ARMV7_IMM16(x) (extract32((x), 0, 12) | \
extract32((x), 12, 4) << 16)
@@ -165,10 +167,8 @@ static void zynq_init(MachineState *machine)
MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
- DeviceState *dev, *carddev;
+ DeviceState *dev;
SysBusDevice *busdev;
- DriveInfo *di;
- BlockBackend *blk;
qemu_irq pic[64];
int n;
@@ -247,27 +247,32 @@ static void zynq_init(MachineState *machine)
gem_init(&nd_table[0], 0xE000B000, pic[54-IRQ_OFFSET]);
gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
- dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
- qdev_init_nofail(dev);
- sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
- sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
-
- di = drive_get_next(IF_SD);
- blk = di ? blk_by_legacy_dinfo(di) : NULL;
- carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
- object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
-
- dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
- qdev_init_nofail(dev);
- sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
- sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
-
- di = drive_get_next(IF_SD);
- blk = di ? blk_by_legacy_dinfo(di) : NULL;
- carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
- object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
+ for (n = 0; n < 2; n++) {
+ int hci_irq = n ? 79 : 56;
+ hwaddr hci_addr = n ? 0xE0101000 : 0xE0100000;
+ DriveInfo *di;
+ BlockBackend *blk;
+ DeviceState *carddev;
+
+ /* Compatible with:
+ * - SD Host Controller Specification Version 2.0 Part A2
+ * - SDIO Specification Version 2.0
+ * - MMC Specification Version 3.31
+ */
+ dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
+ qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+ qdev_prop_set_uint64(dev, "capareg", ZYNQ_SDHCI_CAPABILITIES);
+ qdev_init_nofail(dev);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, hci_addr);
+ sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[hci_irq - IRQ_OFFSET]);
+
+ di = drive_get_next(IF_SD);
+ blk = di ? blk_by_legacy_dinfo(di) : NULL;
+ carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+ qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ object_property_set_bool(OBJECT(carddev), true, "realized",
+ &error_fatal);
+ }
dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
qdev_init_nofail(dev);
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 094e0570e1..aae2cfc1b5 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -41,6 +41,11 @@ static const struct sdhci_t {
/* Exynos4210 */
{ "arm", "smdkc210",
{0x12510000, 2, 0, {1, 0x5e80080} } },
+
+ /* Zynq-7000 */
+ { "arm", "xilinx-zynq-a9", /* Datasheet: UG585 (v1.12.1) */
+ {0xe0100000, 2, 0, {1, 0x69ec0080} } },
+
};
static struct {
--
2.15.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness Philippe Mathieu-Daudé
@ 2018-01-24 0:26 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-24 0:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Kevin O'Connor,
Marcel Apfelbaum
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Incorrect value will throw an error.
>
> Note than Spec v2 is supported by default.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/sd/sdhci-internal.h | 21 ++++++++++-
> hw/sd/sdhci.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-
> hw/sd/trace-events | 1 +
> 3 files changed, 117 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index 577ca9da54..c5e26bf8f3 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -86,6 +86,9 @@
>
> /* R/W Host control Register 0x0 */
> #define SDHC_HOSTCTL 0x28
> +FIELD(SDHC_HOSTCTL, LED_CTRL, 0, 1);
> +FIELD(SDHC_HOSTCTL, DATATRANSFERWIDTH, 1, 1); /* SD mode only */
> +FIELD(SDHC_HOSTCTL, HIGH_SPEED, 2, 1);
> #define SDHC_CTRL_DMA_CHECK_MASK 0x18
> #define SDHC_CTRL_SDMA 0x00
> #define SDHC_CTRL_ADMA1_32 0x08
> @@ -96,6 +99,7 @@
> /* R/W Power Control Register 0x0 */
> #define SDHC_PWRCON 0x29
> #define SDHC_POWER_ON (1 << 0)
> +FIELD(SDHC_PWRCON, BUS_VOLTAGE, 1, 3);
>
> /* R/W Block Gap Control Register 0x0 */
> #define SDHC_BLKGAP 0x2A
> @@ -118,6 +122,7 @@
>
> /* R/W Timeout Control Register 0x0 */
> #define SDHC_TIMEOUTCON 0x2E
> +FIELD(SDHC_TIMEOUTCON, COUNTER, 0, 4);
>
> /* R/W Software Reset Register 0x0 */
> #define SDHC_SWRST 0x2F
> @@ -174,17 +179,31 @@
>
> /* ROC Auto CMD12 error status register 0x0 */
> #define SDHC_ACMD12ERRSTS 0x3C
> +FIELD(SDHC_ACMD12ERRSTS, TIMEOUT_ERR, 1, 1);
> +FIELD(SDHC_ACMD12ERRSTS, CRC_ERR, 2, 1);
> +FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR, 4, 1);
>
> /* HWInit Capabilities Register 0x05E80080 */
> #define SDHC_CAPAB 0x40
> -#define SDHC_CAN_DO_DMA 0x00400000
> #define SDHC_CAN_DO_ADMA2 0x00080000
> #define SDHC_CAN_DO_ADMA1 0x00100000
> #define SDHC_64_BIT_BUS_SUPPORT (1 << 28)
> +FIELD(SDHC_CAPAB, TOCLKFREQ, 0, 6);
> +FIELD(SDHC_CAPAB, TOUNIT, 7, 1);
> +FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8);
> FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
> +FIELD(SDHC_CAPAB, HIGHSPEED, 21, 1);
> +FIELD(SDHC_CAPAB, SDMA, 22, 1);
> +FIELD(SDHC_CAPAB, SUSPRESUME, 23, 1);
> +FIELD(SDHC_CAPAB, V33, 24, 1);
> +FIELD(SDHC_CAPAB, V30, 25, 1);
> +FIELD(SDHC_CAPAB, V18, 26, 1);
>
> /* HWInit Maximum Current Capabilities Register 0x0 */
> #define SDHC_MAXCURR 0x48
> +FIELD(SDHC_MAXCURR, V33_VDD1, 0, 8);
> +FIELD(SDHC_MAXCURR, V30_VDD1, 8, 8);
> +FIELD(SDHC_MAXCURR, V18_VDD1, 16, 8);
>
> /* W Force Event Auto CMD12 Error Interrupt Register 0x0000 */
> #define SDHC_FEAER 0x50
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index dce1a49af1..91dfd684d8 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -23,6 +23,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "hw/hw.h"
> #include "sysemu/block-backend.h"
> @@ -64,6 +65,92 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
> return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
> }
>
> +/* return true on error */
> +static bool sdhci_check_capab_freq_range(SDHCIState *s, const char *desc,
> + uint8_t freq, Error **errp)
> +{
> + switch (freq) {
> + case 0:
> + case 10 ... 63:
> + break;
> + default:
> + error_setg(errp, "SD %s clock frequency can have value"
> + "in range 0-63 only", desc);
> + return true;
> + }
> + return false;
> +}
> +
> +static void sdhci_check_capareg(SDHCIState *s, Error **errp)
> +{
> + uint64_t msk = s->capareg;
> + uint32_t val;
> + bool y;
> +
> + switch (s->sd_spec_version) {
> + case 2: /* default version */
> +
> + /* fallback */
> + case 1:
> + y = FIELD_EX64(s->capareg, SDHC_CAPAB, TOUNIT);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, TOUNIT, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, TOCLKFREQ);
> + trace_sdhci_capareg(y ? "timeout (MHz)" : "Timeout (KHz)", val);
> + if (sdhci_check_capab_freq_range(s, "timeout", val, errp)) {
> + return;
> + }
> + msk = FIELD_DP64(msk, SDHC_CAPAB, TOCLKFREQ, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, BASECLKFREQ);
> + trace_sdhci_capareg(y ? "base (MHz)" : "Base (KHz)", val);
> + if (sdhci_check_capab_freq_range(s, "base", val, errp)) {
> + return;
> + }
> + msk = FIELD_DP64(msk, SDHC_CAPAB, BASECLKFREQ, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH);
> + if (val >= 0b11) {
> + error_setg(errp, "block size can be 512, 1024 or 2048 only");
> + return;
> + }
> + trace_sdhci_capareg("max block length", sdhci_get_fifolen(s));
> + msk = FIELD_DP64(msk, SDHC_CAPAB, MAXBLOCKLENGTH, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, HIGHSPEED);
> + trace_sdhci_capareg("high speed", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, HIGHSPEED, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, SDMA);
> + trace_sdhci_capareg("SDMA", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, SDMA, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, SUSPRESUME);
> + trace_sdhci_capareg("suspend/resume", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, SUSPRESUME, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, V33);
> + trace_sdhci_capareg("3.3v", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, V33, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, V30);
> + trace_sdhci_capareg("3.0v", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, V30, 0);
> +
> + val = FIELD_EX64(s->capareg, SDHC_CAPAB, V18);
> + trace_sdhci_capareg("1.8v", val);
> + msk = FIELD_DP64(msk, SDHC_CAPAB, V18, 0);
> + break;
> +
> + default:
> + error_setg(errp, "Unsupported spec version: %u", s->sd_spec_version);
> + }
> + if (msk) {
> + qemu_log_mask(LOG_UNIMP,
> + "SDHCI: unknown CAPAB mask: 0x%016" PRIx64 "\n", msk);
> + }
> +}
> +
> static uint8_t sdhci_slotint(SDHCIState *s)
> {
> return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
> @@ -990,7 +1077,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
> case SDHC_TRNMOD:
> /* DMA can be enabled only if it is supported as indicated by
> * capabilities register */
> - if (!(s->capareg & SDHC_CAN_DO_DMA)) {
> + if (!(s->capareg & R_SDHC_CAPAB_SDMA_MASK)) {
> value &= ~SDHC_TRNS_DMA;
> }
> MASKED_WRITE(s->trnmod, mask, value & SDHC_TRNMOD_MASK);
> @@ -1125,11 +1212,19 @@ static const MemoryRegionOps sdhci_mmio_ops = {
>
> static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
> {
> + Error *local_err = NULL;
> +
> if (s->sd_spec_version != 2) {
> error_setg(errp, "Only Spec v2 is supported");
> return;
> }
> s->version = (SDHC_HCVER_VENDOR << 8) | (s->sd_spec_version - 1);
> +
> + sdhci_check_capareg(s, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
>
> /* --- qdev common --- */
> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
> index 0a121156a3..78d8707669 100644
> --- a/hw/sd/trace-events
> +++ b/hw/sd/trace-events
> @@ -13,6 +13,7 @@ sdhci_adma_transfer_completed(void) ""
> sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s%u: addr[0x%04" PRIx64 "] %s 0x%08" PRIx64 " (%" PRIu64 ")"
> sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer"
> sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"
> +sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
>
> # hw/sd/milkymist-memcard.c
> milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet Philippe Mathieu-Daudé
@ 2018-01-24 0:36 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-24 0:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Kevin O'Connor,
open list:Xilinx Zynq, Marcel Apfelbaum, Edgar E. Iglesias
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> checking Xilinx datasheet "UG585" (v1.12.1)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/arm/xilinx_zynq.c | 53 ++++++++++++++++++++++++++++------------------------
> tests/sdhci-test.c | 5 +++++
> 2 files changed, 34 insertions(+), 24 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 1836a4ed45..0f76333770 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -61,6 +61,8 @@ static const int dma_irqs[8] = {
> #define SLCR_XILINX_UNLOCK_KEY 0xdf0d
> #define SLCR_XILINX_LOCK_KEY 0x767b
>
> +#define ZYNQ_SDHCI_CAPABILITIES 0x69ec0080 /* Datasheet: UG585 (v1.12.1) */
> +
> #define ARMV7_IMM16(x) (extract32((x), 0, 12) | \
> extract32((x), 12, 4) << 16)
>
> @@ -165,10 +167,8 @@ static void zynq_init(MachineState *machine)
> MemoryRegion *address_space_mem = get_system_memory();
> MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
> MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
> - DeviceState *dev, *carddev;
> + DeviceState *dev;
> SysBusDevice *busdev;
> - DriveInfo *di;
> - BlockBackend *blk;
> qemu_irq pic[64];
> int n;
>
> @@ -247,27 +247,32 @@ static void zynq_init(MachineState *machine)
> gem_init(&nd_table[0], 0xE000B000, pic[54-IRQ_OFFSET]);
> gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
>
> - dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> - qdev_init_nofail(dev);
> - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
> - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
> -
> - di = drive_get_next(IF_SD);
> - blk = di ? blk_by_legacy_dinfo(di) : NULL;
> - carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> - qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> - object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
> -
> - dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> - qdev_init_nofail(dev);
> - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
> - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
> -
> - di = drive_get_next(IF_SD);
> - blk = di ? blk_by_legacy_dinfo(di) : NULL;
> - carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> - qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> - object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
> + for (n = 0; n < 2; n++) {
> + int hci_irq = n ? 79 : 56;
> + hwaddr hci_addr = n ? 0xE0101000 : 0xE0100000;
> + DriveInfo *di;
> + BlockBackend *blk;
> + DeviceState *carddev;
> +
> + /* Compatible with:
> + * - SD Host Controller Specification Version 2.0 Part A2
> + * - SDIO Specification Version 2.0
> + * - MMC Specification Version 3.31
> + */
> + dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> + qdev_prop_set_uint8(dev, "sd-spec-version", 2);
> + qdev_prop_set_uint64(dev, "capareg", ZYNQ_SDHCI_CAPABILITIES);
> + qdev_init_nofail(dev);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, hci_addr);
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[hci_irq - IRQ_OFFSET]);
> +
> + di = drive_get_next(IF_SD);
> + blk = di ? blk_by_legacy_dinfo(di) : NULL;
> + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> + object_property_set_bool(OBJECT(carddev), true, "realized",
> + &error_fatal);
> + }
>
> dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
> qdev_init_nofail(dev);
> diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
> index 094e0570e1..aae2cfc1b5 100644
> --- a/tests/sdhci-test.c
> +++ b/tests/sdhci-test.c
> @@ -41,6 +41,11 @@ static const struct sdhci_t {
> /* Exynos4210 */
> { "arm", "smdkc210",
> {0x12510000, 2, 0, {1, 0x5e80080} } },
> +
> + /* Zynq-7000 */
> + { "arm", "xilinx-zynq-a9", /* Datasheet: UG585 (v1.12.1) */
> + {0xe0100000, 2, 0, {1, 0x69ec0080} } },
> +
> };
>
> static struct {
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64() Philippe Mathieu-Daudé
@ 2018-01-24 0:37 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-24 0:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov, Krzysztof Kozlowski,
qemu-devel@nongnu.org Developers, Igor Mitsyanko,
Kevin O'Connor, open list:Exynos, Marcel Apfelbaum
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> We only set a 32-bit value, but this is a good practice in case this
> code is used as reference.
>
> (missed in 5efc9016e52)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/arm/exynos4210.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index e8e1d81e62..d89322c7ea 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -378,7 +378,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
> DriveInfo *di;
>
> dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> - qdev_prop_set_uint32(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
> + qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
> qdev_init_nofail(dev);
>
> busdev = SYS_BUS_DEVICE(dev);
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen()
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen() Philippe Mathieu-Daudé
@ 2018-01-24 0:38 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-24 0:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Kevin O'Connor,
Marcel Apfelbaum
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/sd/sdhci-internal.h | 4 +++-
> hw/sd/sdhci.c | 20 +++++---------------
> 2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index b7751c815f..577ca9da54 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -24,6 +24,8 @@
> #ifndef SDHCI_INTERNAL_H
> #define SDHCI_INTERNAL_H
>
> +#include "hw/registerfields.h"
> +
> /* R/W SDMA System Address register 0x0 */
> #define SDHC_SYSAD 0x00
>
> @@ -179,7 +181,7 @@
> #define SDHC_CAN_DO_ADMA2 0x00080000
> #define SDHC_CAN_DO_ADMA1 0x00100000
> #define SDHC_64_BIT_BUS_SUPPORT (1 << 28)
> -#define SDHC_CAPAB_BLOCKSIZE(x) (((x) >> 16) & 0x3)
> +FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
>
> /* HWInit Maximum Current Capabilities Register 0x0 */
> #define SDHC_MAXCURR 0x48
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 1c781c4ba5..dce1a49af1 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -59,6 +59,11 @@
> */
> #define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>
> +static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
> +{
> + return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
> +}
> +
> static uint8_t sdhci_slotint(SDHCIState *s)
> {
> return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
> @@ -1118,21 +1123,6 @@ static const MemoryRegionOps sdhci_mmio_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> -static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
> -{
> - switch (SDHC_CAPAB_BLOCKSIZE(s->capareg)) {
> - case 0:
> - return 512;
> - case 1:
> - return 1024;
> - case 2:
> - return 2048;
> - default:
> - hw_error("SDHC: unsupported value for maximum block size\n");
> - return 0;
> - }
> -}
> -
> static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
> {
> if (s->sd_spec_version != 2) {
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register Philippe Mathieu-Daudé
@ 2018-01-24 18:27 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-24 18:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Kevin O'Connor,
Marcel Apfelbaum
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> using many #defines is not portable when scaling to different HCI.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/sd/sdhci.c | 74 +++++++++++++----------------------------------------------
> 1 file changed, 16 insertions(+), 58 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 1331062306..1c781c4ba5 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -39,67 +39,25 @@
> #define TYPE_SDHCI_BUS "sdhci-bus"
> #define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)
>
> +#define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val))
> +
> /* Default SD/MMC host controller features information, which will be
> * presented in CAPABILITIES register of generic SD host controller at reset.
> - * If not stated otherwise:
> - * 0 - not supported, 1 - supported, other - prohibited.
> + *
> + * support:
> + * - 3.3v and 1.8v voltages
> + * - SDMA/ADMA1/ADMA2
> + * - high-speed
> + * max host controller R/W buffers size: 512B
> + * max clock frequency for SDclock: 52 MHz
> + * timeout clock frequency: 52 MHz
> + *
> + * does not support:
> + * - 3.0v voltage
> + * - 64-bit system bus
> + * - suspend/resume
> */
> -#define SDHC_CAPAB_64BITBUS 0ul /* 64-bit System Bus Support */
> -#define SDHC_CAPAB_18V 1ul /* Voltage support 1.8v */
> -#define SDHC_CAPAB_30V 0ul /* Voltage support 3.0v */
> -#define SDHC_CAPAB_33V 1ul /* Voltage support 3.3v */
> -#define SDHC_CAPAB_SUSPRESUME 0ul /* Suspend/resume support */
> -#define SDHC_CAPAB_SDMA 1ul /* SDMA support */
> -#define SDHC_CAPAB_HIGHSPEED 1ul /* High speed support */
> -#define SDHC_CAPAB_ADMA1 1ul /* ADMA1 support */
> -#define SDHC_CAPAB_ADMA2 1ul /* ADMA2 support */
> -/* Maximum host controller R/W buffers size
> - * Possible values: 512, 1024, 2048 bytes */
> -#define SDHC_CAPAB_MAXBLOCKLENGTH 512ul
> -/* Maximum clock frequency for SDclock in MHz
> - * value in range 10-63 MHz, 0 - not defined */
> -#define SDHC_CAPAB_BASECLKFREQ 52ul
> -#define SDHC_CAPAB_TOUNIT 1ul /* Timeout clock unit 0 - kHz, 1 - MHz */
> -/* Timeout clock frequency 1-63, 0 - not defined */
> -#define SDHC_CAPAB_TOCLKFREQ 52ul
> -
> -/* Now check all parameters and calculate CAPABILITIES REGISTER value */
> -#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_18V > 1 || SDHC_CAPAB_30V > 1 || \
> - SDHC_CAPAB_33V > 1 || SDHC_CAPAB_SUSPRESUME > 1 || SDHC_CAPAB_SDMA > 1 || \
> - SDHC_CAPAB_HIGHSPEED > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 ||\
> - SDHC_CAPAB_TOUNIT > 1
> -#error Capabilities features can have value 0 or 1 only!
> -#endif
> -
> -#if SDHC_CAPAB_MAXBLOCKLENGTH == 512
> -#define MAX_BLOCK_LENGTH 0ul
> -#elif SDHC_CAPAB_MAXBLOCKLENGTH == 1024
> -#define MAX_BLOCK_LENGTH 1ul
> -#elif SDHC_CAPAB_MAXBLOCKLENGTH == 2048
> -#define MAX_BLOCK_LENGTH 2ul
> -#else
> -#error Max host controller block size can have value 512, 1024 or 2048 only!
> -#endif
> -
> -#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \
> - SDHC_CAPAB_BASECLKFREQ > 63
> -#error SDclock frequency can have value in range 0, 10-63 only!
> -#endif
> -
> -#if SDHC_CAPAB_TOCLKFREQ > 63
> -#error Timeout clock frequency can have value in range 0-63 only!
> -#endif
> -
> -#define SDHC_CAPAB_REG_DEFAULT \
> - ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_18V << 26) | \
> - (SDHC_CAPAB_30V << 25) | (SDHC_CAPAB_33V << 24) | \
> - (SDHC_CAPAB_SUSPRESUME << 23) | (SDHC_CAPAB_SDMA << 22) | \
> - (SDHC_CAPAB_HIGHSPEED << 21) | (SDHC_CAPAB_ADMA1 << 20) | \
> - (SDHC_CAPAB_ADMA2 << 19) | (MAX_BLOCK_LENGTH << 16) | \
> - (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \
> - (SDHC_CAPAB_TOCLKFREQ))
> -
> -#define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val))
> +#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>
> static uint8_t sdhci_slotint(SDHCIState *s)
> {
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2)
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
` (14 preceding siblings ...)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet Philippe Mathieu-Daudé
@ 2018-01-31 14:09 ` Philippe Mathieu-Daudé
2018-01-31 16:26 ` Alistair Francis
15 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-31 14:09 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: qemu-devel, Kevin O'Connor, Edgar E . Iglesias,
Andrey Smirnov, Marcel Apfelbaum, Krzysztof Kozlowski
ping?
Patches missing review: 2 (PCI qtesting),11,13,15
On 01/22/2018 11:08 PM, Philippe Mathieu-Daudé wrote:
> Since v8:
> - we keep the 'capareg' property, this simplify a lot the series
> - the Zynq 7000 uses the datasheet CAPAREG
> - reset R-b/A-b
>
> Since v7:
> - use error_propagate()
>
> Since v6:
> - rebased on upstream to use DEFINE_SDHCI_COMMON_PROPERTIES
> - included qtests back
> - add PCI qtests
> - series was getting big, splitting again; next one is "implement Spec v3"
> - following Alistair advice, new properties default to current
> SDHC_CAPAB_REG_DEFAULT (probably 'default' for historical reason, being
> the first capabilities set). This leads to weird properties imho, i.e.
> defaulting 'max-frequency' to 52 MHz, the Exynos SoC has to set this
> property to 0.
> - addressed Alistair reviews
> - confirmed the Exynos4210 specs after chatting on IRC with Krzysztof
> Kozlowski who kindly verified in the datasheet.
>
> Since v5:
> - addressed Alistair reviews
> - dropped "abstract generic-sdhci"
> - dropped Linux Device Tree names
> - split qtests in another series
> - change the bcm2835 minimum blocksize to 1KB (Andrew Baumann)
> - added Alistair R-b
> - based on Alistair work:
> - add SD tunning sequence via Host Control 2 to use UHS-I cards
> - add CMD/DAT[] fields in the Present State (used in next series
> to switch card voltage)
>
> based on Alistair work:
> - add SD tunning sequence via Host Control 2 to use UHS-I cards
> - add CMD/DAT[] fields in the Present State (used in next series
> to switch card voltage)
>
> Since v4 ("SDHCI: add qtests and fix few issues"):
> - spec_version default to v2 (current behaviour)
> - addressed Alistair review (no v1, tell user about valid version)
>
> Since v3:
> - no change, but split back in 2 series, 1st part is "SDHCI: housekeeping v5",
>
> Since v2:
> - more detailed 'capabilities', all boards converted to use these properties
> - since all qtests pass, removed the previous 'capareg' property
> - added Stefan/Alistair R-b
> - corrected 'access' LED behavior (Alistair's review)
> - more uses of the registerfields API
> - remove some dead code
> - cosmetix:
> - added more comments
> - renamed a pair of registers
> - reordered few struct members
>
> Since v1:
> - addressed Alistair Francis review comments, added some R-b
> - only move register defines to "sd-internal.h"
> - fixed deposit64() arguments
> - dropped unuseful s->fifo_buffer = NULL
> - use a qemu_irq for the LED, restrict the logging to ON/OFF
> - fixed a trace format string error
> - included Andrey Smirnov ACMD12ERRSTS write patch
> - dropped few unuseful patches, and separate the Python polemical ones for later
>
> From the "SDHCI housekeeping" series:
> - 1: we restrict part of "sd/sd.h" into local "sd-internal.h",
> - 2,3: we somehow beautiful the code, no logical changes,
> - 4-7: we refactor the common sysbus/pci qdev code,
> - 8-10: we add plenty of trace events which will result useful later,
> - 11: we finally expose a "dma-memory" property.
> From the "SDHCI: add a qtest and fix few issues" series:
> - 12,13: fix registers
> - 14,15: boards can specify which SDHCI Spec to use (v2 and v3 so far)
> - 15-20: HCI qtest
>
> Regards,
>
> Phil.
>
> $ git backport-diff with v8
> 001/16:[----] [--] 'sdhci: use error_propagate(local_err) in realize()'
> 002/16:[0004] [FC] 'sdhci: add qtest to check the SD capabilities register'
> 003/16:[----] [--] 'sdhci: add check_capab_readonly() qtest'
> 004/16:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
> 005/16:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
> 006/16:[----] [--] 'sdhci: add qtest to check the SD Spec version'
> 007/16:[0022] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
> 008/16:[down] 'sdhci: use a numeric value for the default CAPAB register'
> 009/16:[down] 'sdhci: simplify sdhci_get_fifolen()'
> 010/16:[down] 'sdhci: check the Spec v1 capabilities correctness'
> 011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
> 012/16:[----] [--] 'sdhci: Fix 64-bit ADMA2'
> 013/16:[down] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
> 014/16:[down] 'hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()'
> 015/16:[down] 'hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)'
> 016/16:[down] 'hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet'
>
> Philippe Mathieu-Daudé (15):
> sdhci: use error_propagate(local_err) in realize()
> sdhci: add qtest to check the SD capabilities register
> sdhci: add check_capab_readonly() qtest
> sdhci: add a check_capab_baseclock() qtest
> sdhci: add a check_capab_sdma() qtest
> sdhci: add qtest to check the SD Spec version
> sdhci: add a 'spec_version property' (default to v2)
> sdhci: use a numeric value for the default CAPAB register
> sdhci: simplify sdhci_get_fifolen()
> sdhci: check the Spec v1 capabilities correctness
> sdhci: add BLOCK_SIZE_MASK for DMA
> sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
> hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
> hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
> hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
>
> Sai Pavan Boddu (1):
> sdhci: Fix 64-bit ADMA2
>
> include/hw/sd/sdhci.h | 2 +
> hw/sd/sdhci-internal.h | 43 ++++++---
> hw/arm/exynos4210.c | 14 ++-
> hw/arm/xilinx_zynq.c | 53 +++++-----
> hw/sd/sdhci.c | 257 ++++++++++++++++++++++++++++++++-----------------
> tests/sdhci-test.c | 216 +++++++++++++++++++++++++++++++++++++++++
> hw/sd/trace-events | 1 +
> tests/Makefile.include | 3 +
> 8 files changed, 464 insertions(+), 125 deletions(-)
> create mode 100644 tests/sdhci-test.c
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2) Philippe Mathieu-Daudé
@ 2018-01-31 16:22 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2018-01-31 16:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov, Krzysztof Kozlowski,
qemu-devel@nongnu.org Developers, Igor Mitsyanko,
Kevin O'Connor, open list:Exynos, Marcel Apfelbaum
On Mon, Jan 22, 2018 at 6:08 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> hw/arm/exynos4210.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index d89322c7ea..06f9d1ffa4 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -377,6 +377,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
> BlockBackend *blk;
> DriveInfo *di;
>
> + /* Compatible with:
> + * - SD Host Controller Specification Version 2.0
> + * - SDIO Specification Version 2.0
> + * - MMC Specification Version 4.3
> + * - SDMA
> + * - ADMA2
> + *
> + * As this part of the Exynos4210 is not publically available,
> + * we used the "HS-MMC Controller S3C2416X RISC Microprocessor"
> + * public datasheet which is very similar (implementing
> + * MMC Specification Version 4.0 being the only difference noted)
> + */
> dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
> qdev_init_nofail(dev);
> --
> 2.15.1
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2)
2018-01-31 14:09 ` [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
@ 2018-01-31 16:26 ` Alistair Francis
2018-02-06 12:22 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 27+ messages in thread
From: Alistair Francis @ 2018-01-31 16:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi,
Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Krzysztof Kozlowski,
Kevin O'Connor, Marcel Apfelbaum
On Wed, Jan 31, 2018 at 6:09 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> ping?
>
> Patches missing review: 2 (PCI qtesting),11,13,15
Sorry about the slowness. Besides 2 they should be all reviewed now,
let me know if anything else needs to be done.
As for 2 I don't have much experience with Qtest, I'll try to go over
it, but ideally someone who knows Qtest should look at it.
Thanks,
Alistair
>
> On 01/22/2018 11:08 PM, Philippe Mathieu-Daudé wrote:
>> Since v8:
>> - we keep the 'capareg' property, this simplify a lot the series
>> - the Zynq 7000 uses the datasheet CAPAREG
>> - reset R-b/A-b
>>
>> Since v7:
>> - use error_propagate()
>>
>> Since v6:
>> - rebased on upstream to use DEFINE_SDHCI_COMMON_PROPERTIES
>> - included qtests back
>> - add PCI qtests
>> - series was getting big, splitting again; next one is "implement Spec v3"
>> - following Alistair advice, new properties default to current
>> SDHC_CAPAB_REG_DEFAULT (probably 'default' for historical reason, being
>> the first capabilities set). This leads to weird properties imho, i.e.
>> defaulting 'max-frequency' to 52 MHz, the Exynos SoC has to set this
>> property to 0.
>> - addressed Alistair reviews
>> - confirmed the Exynos4210 specs after chatting on IRC with Krzysztof
>> Kozlowski who kindly verified in the datasheet.
>>
>> Since v5:
>> - addressed Alistair reviews
>> - dropped "abstract generic-sdhci"
>> - dropped Linux Device Tree names
>> - split qtests in another series
>> - change the bcm2835 minimum blocksize to 1KB (Andrew Baumann)
>> - added Alistair R-b
>> - based on Alistair work:
>> - add SD tunning sequence via Host Control 2 to use UHS-I cards
>> - add CMD/DAT[] fields in the Present State (used in next series
>> to switch card voltage)
>>
>> based on Alistair work:
>> - add SD tunning sequence via Host Control 2 to use UHS-I cards
>> - add CMD/DAT[] fields in the Present State (used in next series
>> to switch card voltage)
>>
>> Since v4 ("SDHCI: add qtests and fix few issues"):
>> - spec_version default to v2 (current behaviour)
>> - addressed Alistair review (no v1, tell user about valid version)
>>
>> Since v3:
>> - no change, but split back in 2 series, 1st part is "SDHCI: housekeeping v5",
>>
>> Since v2:
>> - more detailed 'capabilities', all boards converted to use these properties
>> - since all qtests pass, removed the previous 'capareg' property
>> - added Stefan/Alistair R-b
>> - corrected 'access' LED behavior (Alistair's review)
>> - more uses of the registerfields API
>> - remove some dead code
>> - cosmetix:
>> - added more comments
>> - renamed a pair of registers
>> - reordered few struct members
>>
>> Since v1:
>> - addressed Alistair Francis review comments, added some R-b
>> - only move register defines to "sd-internal.h"
>> - fixed deposit64() arguments
>> - dropped unuseful s->fifo_buffer = NULL
>> - use a qemu_irq for the LED, restrict the logging to ON/OFF
>> - fixed a trace format string error
>> - included Andrey Smirnov ACMD12ERRSTS write patch
>> - dropped few unuseful patches, and separate the Python polemical ones for later
>>
>> From the "SDHCI housekeeping" series:
>> - 1: we restrict part of "sd/sd.h" into local "sd-internal.h",
>> - 2,3: we somehow beautiful the code, no logical changes,
>> - 4-7: we refactor the common sysbus/pci qdev code,
>> - 8-10: we add plenty of trace events which will result useful later,
>> - 11: we finally expose a "dma-memory" property.
>> From the "SDHCI: add a qtest and fix few issues" series:
>> - 12,13: fix registers
>> - 14,15: boards can specify which SDHCI Spec to use (v2 and v3 so far)
>> - 15-20: HCI qtest
>>
>> Regards,
>>
>> Phil.
>>
>> $ git backport-diff with v8
>> 001/16:[----] [--] 'sdhci: use error_propagate(local_err) in realize()'
>> 002/16:[0004] [FC] 'sdhci: add qtest to check the SD capabilities register'
>> 003/16:[----] [--] 'sdhci: add check_capab_readonly() qtest'
>> 004/16:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
>> 005/16:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
>> 006/16:[----] [--] 'sdhci: add qtest to check the SD Spec version'
>> 007/16:[0022] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
>> 008/16:[down] 'sdhci: use a numeric value for the default CAPAB register'
>> 009/16:[down] 'sdhci: simplify sdhci_get_fifolen()'
>> 010/16:[down] 'sdhci: check the Spec v1 capabilities correctness'
>> 011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
>> 012/16:[----] [--] 'sdhci: Fix 64-bit ADMA2'
>> 013/16:[down] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
>> 014/16:[down] 'hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()'
>> 015/16:[down] 'hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)'
>> 016/16:[down] 'hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet'
>>
>> Philippe Mathieu-Daudé (15):
>> sdhci: use error_propagate(local_err) in realize()
>> sdhci: add qtest to check the SD capabilities register
>> sdhci: add check_capab_readonly() qtest
>> sdhci: add a check_capab_baseclock() qtest
>> sdhci: add a check_capab_sdma() qtest
>> sdhci: add qtest to check the SD Spec version
>> sdhci: add a 'spec_version property' (default to v2)
>> sdhci: use a numeric value for the default CAPAB register
>> sdhci: simplify sdhci_get_fifolen()
>> sdhci: check the Spec v1 capabilities correctness
>> sdhci: add BLOCK_SIZE_MASK for DMA
>> sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
>> hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
>> hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
>> hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
>>
>> Sai Pavan Boddu (1):
>> sdhci: Fix 64-bit ADMA2
>>
>> include/hw/sd/sdhci.h | 2 +
>> hw/sd/sdhci-internal.h | 43 ++++++---
>> hw/arm/exynos4210.c | 14 ++-
>> hw/arm/xilinx_zynq.c | 53 +++++-----
>> hw/sd/sdhci.c | 257 ++++++++++++++++++++++++++++++++-----------------
>> tests/sdhci-test.c | 216 +++++++++++++++++++++++++++++++++++++++++
>> hw/sd/trace-events | 1 +
>> tests/Makefile.include | 3 +
>> 8 files changed, 464 insertions(+), 125 deletions(-)
>> create mode 100644 tests/sdhci-test.c
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2)
2018-01-31 16:26 ` Alistair Francis
@ 2018-02-06 12:22 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-06 12:22 UTC (permalink / raw)
To: Alistair Francis, Paolo Bonzini, Stefan Hajnoczi
Cc: Peter Maydell, Edgar E . Iglesias, Andrey Smirnov,
qemu-devel@nongnu.org Developers, Krzysztof Kozlowski,
Kevin O'Connor, Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 6980 bytes --]
On 01/31/2018 01:26 PM, Alistair Francis wrote:
> On Wed, Jan 31, 2018 at 6:09 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> ping?
>>
>> Patches missing review: 2 (PCI qtesting),11,13,15
>
> Sorry about the slowness. Besides 2 they should be all reviewed now,
> let me know if anything else needs to be done.
Thanks for reviewing the whole series!
> As for 2 I don't have much experience with Qtest, I'll try to go over
> it, but ideally someone who knows Qtest should look at it.
Yes. This patch is provided for completeness but is not
required/blocking. I can split it away for later and respin.
Paolo/Stefan do you mind reviewing patch #2?
It uses the qpci API to access the SDHCI via PCI, as suggested
2 months ago when I started SDHCI qtesting.
Thanks,
Phil.
>> On 01/22/2018 11:08 PM, Philippe Mathieu-Daudé wrote:
>>> Since v8:
>>> - we keep the 'capareg' property, this simplify a lot the series
>>> - the Zynq 7000 uses the datasheet CAPAREG
>>> - reset R-b/A-b
>>>
>>> Since v7:
>>> - use error_propagate()
>>>
>>> Since v6:
>>> - rebased on upstream to use DEFINE_SDHCI_COMMON_PROPERTIES
>>> - included qtests back
>>> - add PCI qtests
>>> - series was getting big, splitting again; next one is "implement Spec v3"
>>> - following Alistair advice, new properties default to current
>>> SDHC_CAPAB_REG_DEFAULT (probably 'default' for historical reason, being
>>> the first capabilities set). This leads to weird properties imho, i.e.
>>> defaulting 'max-frequency' to 52 MHz, the Exynos SoC has to set this
>>> property to 0.
>>> - addressed Alistair reviews
>>> - confirmed the Exynos4210 specs after chatting on IRC with Krzysztof
>>> Kozlowski who kindly verified in the datasheet.
>>>
>>> Since v5:
>>> - addressed Alistair reviews
>>> - dropped "abstract generic-sdhci"
>>> - dropped Linux Device Tree names
>>> - split qtests in another series
>>> - change the bcm2835 minimum blocksize to 1KB (Andrew Baumann)
>>> - added Alistair R-b
>>> - based on Alistair work:
>>> - add SD tunning sequence via Host Control 2 to use UHS-I cards
>>> - add CMD/DAT[] fields in the Present State (used in next series
>>> to switch card voltage)
>>>
>>> based on Alistair work:
>>> - add SD tunning sequence via Host Control 2 to use UHS-I cards
>>> - add CMD/DAT[] fields in the Present State (used in next series
>>> to switch card voltage)
>>>
>>> Since v4 ("SDHCI: add qtests and fix few issues"):
>>> - spec_version default to v2 (current behaviour)
>>> - addressed Alistair review (no v1, tell user about valid version)
>>>
>>> Since v3:
>>> - no change, but split back in 2 series, 1st part is "SDHCI: housekeeping v5",
>>>
>>> Since v2:
>>> - more detailed 'capabilities', all boards converted to use these properties
>>> - since all qtests pass, removed the previous 'capareg' property
>>> - added Stefan/Alistair R-b
>>> - corrected 'access' LED behavior (Alistair's review)
>>> - more uses of the registerfields API
>>> - remove some dead code
>>> - cosmetix:
>>> - added more comments
>>> - renamed a pair of registers
>>> - reordered few struct members
>>>
>>> Since v1:
>>> - addressed Alistair Francis review comments, added some R-b
>>> - only move register defines to "sd-internal.h"
>>> - fixed deposit64() arguments
>>> - dropped unuseful s->fifo_buffer = NULL
>>> - use a qemu_irq for the LED, restrict the logging to ON/OFF
>>> - fixed a trace format string error
>>> - included Andrey Smirnov ACMD12ERRSTS write patch
>>> - dropped few unuseful patches, and separate the Python polemical ones for later
>>>
>>> From the "SDHCI housekeeping" series:
>>> - 1: we restrict part of "sd/sd.h" into local "sd-internal.h",
>>> - 2,3: we somehow beautiful the code, no logical changes,
>>> - 4-7: we refactor the common sysbus/pci qdev code,
>>> - 8-10: we add plenty of trace events which will result useful later,
>>> - 11: we finally expose a "dma-memory" property.
>>> From the "SDHCI: add a qtest and fix few issues" series:
>>> - 12,13: fix registers
>>> - 14,15: boards can specify which SDHCI Spec to use (v2 and v3 so far)
>>> - 15-20: HCI qtest
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>> $ git backport-diff with v8
>>> 001/16:[----] [--] 'sdhci: use error_propagate(local_err) in realize()'
>>> 002/16:[0004] [FC] 'sdhci: add qtest to check the SD capabilities register'
>>> 003/16:[----] [--] 'sdhci: add check_capab_readonly() qtest'
>>> 004/16:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
>>> 005/16:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
>>> 006/16:[----] [--] 'sdhci: add qtest to check the SD Spec version'
>>> 007/16:[0022] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
>>> 008/16:[down] 'sdhci: use a numeric value for the default CAPAB register'
>>> 009/16:[down] 'sdhci: simplify sdhci_get_fifolen()'
>>> 010/16:[down] 'sdhci: check the Spec v1 capabilities correctness'
>>> 011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
>>> 012/16:[----] [--] 'sdhci: Fix 64-bit ADMA2'
>>> 013/16:[down] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
>>> 014/16:[down] 'hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()'
>>> 015/16:[down] 'hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)'
>>> 016/16:[down] 'hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet'
>>>
>>> Philippe Mathieu-Daudé (15):
>>> sdhci: use error_propagate(local_err) in realize()
>>> sdhci: add qtest to check the SD capabilities register
>>> sdhci: add check_capab_readonly() qtest
>>> sdhci: add a check_capab_baseclock() qtest
>>> sdhci: add a check_capab_sdma() qtest
>>> sdhci: add qtest to check the SD Spec version
>>> sdhci: add a 'spec_version property' (default to v2)
>>> sdhci: use a numeric value for the default CAPAB register
>>> sdhci: simplify sdhci_get_fifolen()
>>> sdhci: check the Spec v1 capabilities correctness
>>> sdhci: add BLOCK_SIZE_MASK for DMA
>>> sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
>>> hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
>>> hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
>>> hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
>>>
>>> Sai Pavan Boddu (1):
>>> sdhci: Fix 64-bit ADMA2
>>>
>>> include/hw/sd/sdhci.h | 2 +
>>> hw/sd/sdhci-internal.h | 43 ++++++---
>>> hw/arm/exynos4210.c | 14 ++-
>>> hw/arm/xilinx_zynq.c | 53 +++++-----
>>> hw/sd/sdhci.c | 257 ++++++++++++++++++++++++++++++++-----------------
>>> tests/sdhci-test.c | 216 +++++++++++++++++++++++++++++++++++++++++
>>> hw/sd/trace-events | 1 +
>>> tests/Makefile.include | 3 +
>>> 8 files changed, 464 insertions(+), 125 deletions(-)
>>> create mode 100644 tests/sdhci-test.c
>>>
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register Philippe Mathieu-Daudé
@ 2018-02-07 17:10 ` Paolo Bonzini
2018-02-07 17:59 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2018-02-07 17:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Edgar E . Iglesias, Andrey Smirnov, qemu-devel,
Kevin O'Connor, Marcel Apfelbaum
On 23/01/2018 03:08, Philippe Mathieu-Daudé wrote:
> The PCI model is tested with the pc/x86_64 machine,
> the SysBus model with the smdkc210/arm machine.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/sdhci-test.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
> tests/Makefile.include | 3 ++
> 2 files changed, 137 insertions(+)
> create mode 100644 tests/sdhci-test.c
>
> diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
> new file mode 100644
> index 0000000000..517e2ed5a2
> --- /dev/null
> +++ b/tests/sdhci-test.c
> @@ -0,0 +1,134 @@
> +/*
> + * QTest testcase for SDHCI controllers
> + *
> + * Written by Philippe Mathieu-Daudé <f4bug@amsat.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include "qemu/osdep.h"
> +#include "hw/registerfields.h"
> +#include "libqtest.h"
> +#include "libqos/pci-pc.h"
> +#include "hw/pci/pci.h"
> +
> +#define SDHC_CAPAB 0x40
> +#define SDHC_HCVER 0xFE
> +
> +static const struct sdhci_t {
> + const char *arch, *machine;
> + struct {
> + uintptr_t addr;
> + uint8_t version;
> + uint8_t baseclock;
> + struct {
> + bool sdma;
> + uint64_t reg;
> + } capab;
> + } sdhci;
> + struct {
> + uint16_t vendor_id, device_id;
> + } pci;
> +} models[] = {
> + /* PC via PCI */
> + { "x86_64", "pc",
> + {-1, 2, 0, {1, 0x057834b4} },
> + .pci = { PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_SDHCI } },
> +
> + /* Exynos4210 */
> + { "arm", "smdkc210",
> + {0x12510000, 2, 0, {1, 0x5e80080} } },
> +};
> +
> +static struct {
> + QPCIBus *pcibus;
> + QPCIDevice *dev;
> + QPCIBar mem_bar;
> +} g = { };
> +
> +static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
> +{
> + if (g.dev) {
> + uint64_t value;
> +
> + qpci_memread(g.dev, g.mem_bar, reg_addr, &value, sizeof(value));
> +
> + return value;
> + } else {
> + QTestState *qtest = global_qtest;
> +
> + return qtest_readq(qtest, base + reg_addr);
> + }
> +}
Maybe the struct about could be a "QSDHCI*" that is returned by
machine_start and accepted by sdhci_readq and check_capab_capareg?
Later the same would work for sdhci_readl, check_specs_version, etc.
That's my only remark though. Thanks!
Paolo
> +static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
> +{
> + uint64_t capab;
> +
> + capab = sdhci_readq(addr, SDHC_CAPAB);
> + g_assert_cmphex(capab, ==, expected_capab);
> +}
> +
> +static void machine_start(const struct sdhci_t *test)
> +{
> + if (test->pci.vendor_id) {
> + /* PCI */
> + uint16_t vendor_id, device_id;
> + uint64_t barsize;
> +
> + global_qtest = qtest_startf("-machine %s -d unimp -device sdhci-pci",
> + test->machine);
> +
> + g.pcibus = qpci_init_pc(NULL);
> +
> + /* Find PCI device and verify it's the right one */
> + g.dev = qpci_device_find(g.pcibus, QPCI_DEVFN(4, 0));
> + g_assert_nonnull(g.dev);
> + vendor_id = qpci_config_readw(g.dev, PCI_VENDOR_ID);
> + device_id = qpci_config_readw(g.dev, PCI_DEVICE_ID);
> + g_assert(vendor_id == test->pci.vendor_id);
> + g_assert(device_id == test->pci.device_id);
> + g.mem_bar = qpci_iomap(g.dev, 0, &barsize);
> + qpci_device_enable(g.dev);
> + } else {
> + /* SysBus */
> + global_qtest = qtest_startf("-machine %s -d unimp", test->machine);
> + }
> +}
> +
> +static void machine_stop(void)
> +{
> + g_free(g.dev);
> + qtest_quit(global_qtest);
> +}
> +
> +static void test_machine(const void *data)
> +{
> + const struct sdhci_t *test = data;
> +
> + machine_start(test);
> +
> + check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
> +
> + machine_stop();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + const char *arch = qtest_get_arch();
> + char *name;
> + int i;
> +
> + g_test_init(&argc, &argv, NULL);
> + for (i = 0; i < ARRAY_SIZE(models); i++) {
> + if (strcmp(arch, models[i].arch)) {
> + continue;
> + }
> + name = g_strdup_printf("sdhci/%s", models[i].machine);
> + qtest_add_data_func(name, &models[i], test_machine);
> + g_free(name);
> + }
> +
> + return g_test_run();
> +}
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 8883274ae1..756725b0f9 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -293,6 +293,7 @@ check-qtest-i386-y += tests/migration-test$(EXESUF)
> check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
> check-qtest-i386-y += tests/numa-test$(EXESUF)
> check-qtest-x86_64-y += $(check-qtest-i386-y)
> +check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
> gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
> gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
>
> @@ -363,6 +364,7 @@ gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
> check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
> gcov-files-arm-y += hw/timer/arm_mptimer.c
> check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>
> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>
> @@ -816,6 +818,7 @@ tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
> tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
> tests/numa-test$(EXESUF): tests/numa-test.o
> tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
> +tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
>
> tests/migration/stress$(EXESUF): tests/migration/stress.o
> $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register
2018-02-07 17:10 ` Paolo Bonzini
@ 2018-02-07 17:59 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-07 17:59 UTC (permalink / raw)
To: Paolo Bonzini, Alistair Francis, Peter Maydell, Stefan Hajnoczi
Cc: Edgar E . Iglesias, Andrey Smirnov, qemu-devel,
Kevin O'Connor, Marcel Apfelbaum
On 02/07/2018 02:10 PM, Paolo Bonzini wrote:
> On 23/01/2018 03:08, Philippe Mathieu-Daudé wrote:
>> The PCI model is tested with the pc/x86_64 machine,
>> the SysBus model with the smdkc210/arm machine.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> tests/sdhci-test.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/Makefile.include | 3 ++
>> 2 files changed, 137 insertions(+)
>> create mode 100644 tests/sdhci-test.c
>>
>> diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
>> new file mode 100644
>> index 0000000000..517e2ed5a2
>> --- /dev/null
>> +++ b/tests/sdhci-test.c
>> @@ -0,0 +1,134 @@
>> +/*
>> + * QTest testcase for SDHCI controllers
>> + *
>> + * Written by Philippe Mathieu-Daudé <f4bug@amsat.org>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#include "qemu/osdep.h"
>> +#include "hw/registerfields.h"
>> +#include "libqtest.h"
>> +#include "libqos/pci-pc.h"
>> +#include "hw/pci/pci.h"
>> +
>> +#define SDHC_CAPAB 0x40
>> +#define SDHC_HCVER 0xFE
>> +
>> +static const struct sdhci_t {
>> + const char *arch, *machine;
>> + struct {
>> + uintptr_t addr;
>> + uint8_t version;
>> + uint8_t baseclock;
>> + struct {
>> + bool sdma;
>> + uint64_t reg;
>> + } capab;
>> + } sdhci;
>> + struct {
>> + uint16_t vendor_id, device_id;
>> + } pci;
>> +} models[] = {
>> + /* PC via PCI */
>> + { "x86_64", "pc",
>> + {-1, 2, 0, {1, 0x057834b4} },
>> + .pci = { PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_SDHCI } },
>> +
>> + /* Exynos4210 */
>> + { "arm", "smdkc210",
>> + {0x12510000, 2, 0, {1, 0x5e80080} } },
>> +};
>> +
>> +static struct {
>> + QPCIBus *pcibus;
>> + QPCIDevice *dev;
>> + QPCIBar mem_bar;
>> +} g = { };
>> +
>> +static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
>> +{
>> + if (g.dev) {
>> + uint64_t value;
>> +
>> + qpci_memread(g.dev, g.mem_bar, reg_addr, &value, sizeof(value));
>> +
>> + return value;
>> + } else {
>> + QTestState *qtest = global_qtest;
>> +
>> + return qtest_readq(qtest, base + reg_addr);
>> + }
>> +}
>
> Maybe the struct about could be a "QSDHCI*" that is returned by
> machine_start and accepted by sdhci_readq and check_capab_capareg?
> Later the same would work for sdhci_readl, check_specs_version, etc.
Good idea, I didn't like the global 'g' but wanted to have PCI/qtesting
feedback before improving this.
>
> That's my only remark though. Thanks!
Thanks :)
>
> Paolo
>
>> +static void check_capab_capareg(uintptr_t addr, uint64_t expected_capab)
>> +{
>> + uint64_t capab;
>> +
>> + capab = sdhci_readq(addr, SDHC_CAPAB);
>> + g_assert_cmphex(capab, ==, expected_capab);
>> +}
>> +
>> +static void machine_start(const struct sdhci_t *test)
>> +{
>> + if (test->pci.vendor_id) {
>> + /* PCI */
>> + uint16_t vendor_id, device_id;
>> + uint64_t barsize;
>> +
>> + global_qtest = qtest_startf("-machine %s -d unimp -device sdhci-pci",
>> + test->machine);
>> +
>> + g.pcibus = qpci_init_pc(NULL);
>> +
>> + /* Find PCI device and verify it's the right one */
>> + g.dev = qpci_device_find(g.pcibus, QPCI_DEVFN(4, 0));
>> + g_assert_nonnull(g.dev);
>> + vendor_id = qpci_config_readw(g.dev, PCI_VENDOR_ID);
>> + device_id = qpci_config_readw(g.dev, PCI_DEVICE_ID);
>> + g_assert(vendor_id == test->pci.vendor_id);
>> + g_assert(device_id == test->pci.device_id);
>> + g.mem_bar = qpci_iomap(g.dev, 0, &barsize);
>> + qpci_device_enable(g.dev);
>> + } else {
>> + /* SysBus */
>> + global_qtest = qtest_startf("-machine %s -d unimp", test->machine);
>> + }
>> +}
>> +
>> +static void machine_stop(void)
>> +{
>> + g_free(g.dev);
>> + qtest_quit(global_qtest);
>> +}
>> +
>> +static void test_machine(const void *data)
>> +{
>> + const struct sdhci_t *test = data;
>> +
>> + machine_start(test);
>> +
>> + check_capab_capareg(test->sdhci.addr, test->sdhci.capab.reg);
>> +
>> + machine_stop();
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + const char *arch = qtest_get_arch();
>> + char *name;
>> + int i;
>> +
>> + g_test_init(&argc, &argv, NULL);
>> + for (i = 0; i < ARRAY_SIZE(models); i++) {
>> + if (strcmp(arch, models[i].arch)) {
>> + continue;
>> + }
>> + name = g_strdup_printf("sdhci/%s", models[i].machine);
>> + qtest_add_data_func(name, &models[i], test_machine);
>> + g_free(name);
>> + }
>> +
>> + return g_test_run();
>> +}
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 8883274ae1..756725b0f9 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -293,6 +293,7 @@ check-qtest-i386-y += tests/migration-test$(EXESUF)
>> check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
>> check-qtest-i386-y += tests/numa-test$(EXESUF)
>> check-qtest-x86_64-y += $(check-qtest-i386-y)
>> +check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
>> gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
>> gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
>>
>> @@ -363,6 +364,7 @@ gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
>> check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
>> gcov-files-arm-y += hw/timer/arm_mptimer.c
>> check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>>
>> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>>
>> @@ -816,6 +818,7 @@ tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
>> tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
>> tests/numa-test$(EXESUF): tests/numa-test.o
>> tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
>> +tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
>>
>> tests/migration/stress$(EXESUF): tests/migration/stress.o
>> $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-02-07 17:59 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-23 2:08 [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 01/16] sdhci: use error_propagate(local_err) in realize() Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 02/16] sdhci: add qtest to check the SD capabilities register Philippe Mathieu-Daudé
2018-02-07 17:10 ` Paolo Bonzini
2018-02-07 17:59 ` Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 03/16] sdhci: add check_capab_readonly() qtest Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 04/16] sdhci: add a check_capab_baseclock() qtest Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 05/16] sdhci: add a check_capab_sdma() qtest Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 06/16] sdhci: add qtest to check the SD Spec version Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 07/16] sdhci: add a 'spec_version property' (default to v2) Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 08/16] sdhci: use a numeric value for the default CAPAB register Philippe Mathieu-Daudé
2018-01-24 18:27 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 09/16] sdhci: simplify sdhci_get_fifolen() Philippe Mathieu-Daudé
2018-01-24 0:38 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 10/16] sdhci: check the Spec v1 capabilities correctness Philippe Mathieu-Daudé
2018-01-24 0:26 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 12/16] sdhci: Fix 64-bit ADMA2 Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 13/16] sdhci: check Spec v2 capabilities (DMA and 64-bit bus) Philippe Mathieu-Daudé
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 14/16] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64() Philippe Mathieu-Daudé
2018-01-24 0:37 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 15/16] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2) Philippe Mathieu-Daudé
2018-01-31 16:22 ` Alistair Francis
2018-01-23 2:08 ` [Qemu-devel] [PATCH v9 16/16] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet Philippe Mathieu-Daudé
2018-01-24 0:36 ` Alistair Francis
2018-01-31 14:09 ` [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2) Philippe Mathieu-Daudé
2018-01-31 16:26 ` Alistair Francis
2018-02-06 12:22 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.