[Qemu-devel] [PATCH v8 00/17] SDHCI: clean v1/v2 Specs (part 2)

[Qemu-devel] [PATCH v8 00/17] SDHCI: clean v1/v2 Specs (part 2)

[Philippe Mathieu-Daudé]

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 v7
001/17:[down] 'use error_propagate(local_err) in realize()'
002/17:[----] [--] 'sdhci: add qtest to check the SD capabilities register'
003/17:[----] [--] 'sdhci: add check_capab_readonly() qtest'
004/17:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
005/17:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
006/17:[----] [--] 'sdhci: add qtest to check the SD Spec version'
007/17:[0007] [FC] 'sdhci: add init_readonly_registers() to initialize the CAPAB register'
008/17:[----] [--] 'sdhci: add a 'spec_version property' (default to v2)'
009/17:[0006] [FC] 'sdhci: add basic Spec v1 capabilities'
... no change ...

$ git backport-diff with v6
001/16:[down] 'sdhci: add qtest to check the SD capabilities register'
002/16:[0008] [FC] 'sdhci: add check_capab_readonly() qtest'
003/16:[0001] [FC] 'sdhci: add a check_capab_baseclock() qtest'
004/16:[----] [-C] 'sdhci: add a check_capab_sdma() qtest'
005/16:[0078] [FC] 'sdhci: add qtest to check the SD Spec version'
006/16:[down] 'sdhci: add init_readonly_registers() to initialize the CAPAB register'
007/16:[0010] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
008/16:[0049] [FC] 'sdhci: add basic Spec v1 capabilities'
009/16:[0007] [FC] 'sdhci: add max-block-length capability (Spec v1)'
010/16:[0013] [FC] 'sdhci: add clock capabilities (Spec v1)'
011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
012/16:[----] [-C] 'sdhci: Fix 64-bit ADMA2'
013/16:[0036] [FC] 'sdhci: add DMA and 64-bit capabilities (Spec v2)'
014/16:[0010] [FC] 'hw/arm/xilinx_zynq: implement SDHCI Spec v2'
015/16:[0014] [FC] 'hw/arm/exynos4210: implement SDHCI Spec v2'
016/16:[down] 'sdhci: throw an error if capabilities are incorrectly configured'

Philippe Mathieu-Daudé (16):
  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 init_readonly_registers() to initialize the CAPAB register
  sdhci: add a 'spec_version property' (default to v2)
  sdhci: add basic Spec v1 capabilities
  sdhci: add max-block-length capability (Spec v1)
  sdhci: add clock capabilities (Spec v1)
  sdhci: add BLOCK_SIZE_MASK for DMA
  sdhci: add DMA and 64-bit capabilities (Spec v2)
  hw/arm/xilinx_zynq: implement SDHCI Spec v2
  hw/arm/exynos4210: implement SDHCI Spec v2
  sdhci: throw an error if capabilities are incorrectly configured

Sai Pavan Boddu (1):
  sdhci: Fix 64-bit ADMA2

 include/hw/sd/sdhci.h  |  26 ++++++
 hw/sd/sdhci-internal.h |  43 +++++++---
 hw/arm/exynos4210.c    |  21 ++++-
 hw/arm/xilinx_zynq.c   |  70 ++++++++++------
 hw/sd/sdhci.c          | 223 ++++++++++++++++++++++++++++++++-----------------
 tests/sdhci-test.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include |   3 +
 7 files changed, 488 insertions(+), 113 deletions(-)
 create mode 100644 tests/sdhci-test.c

-- 
2.15.1

[Qemu-devel] [PATCH v8 01/17] sdhci: use error_propagate(local_err) in realize()

[Philippe Mathieu-Daudé]

avoid the "errp && *errp" pattern (not recommended in "qapi/error.h" comments).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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

[Qemu-devel] [PATCH v8 02/17] sdhci: add qtest to check the SD capabilities register

[Philippe Mathieu-Daudé]

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..0db6b81545
--- /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

[Qemu-devel] [PATCH v8 03/17] sdhci: add check_capab_readonly() qtest

[Philippe Mathieu-Daudé]

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 0db6b81545..645f61dc48 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

[Qemu-devel] [PATCH v8 04/17] sdhci: add a check_capab_baseclock() qtest

[Philippe Mathieu-Daudé]

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 645f61dc48..bd5090162f 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

[Qemu-devel] [PATCH v8 05/17] sdhci: add a check_capab_sdma() qtest

[Philippe Mathieu-Daudé]

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 bd5090162f..3f6377474c 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

[Qemu-devel] [PATCH v8 06/17] sdhci: add qtest to check the SD Spec version

[Philippe Mathieu-Daudé]

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 3f6377474c..37ada0e1c5 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

[Qemu-devel] [PATCH v8 07/17] sdhci: add init_readonly_registers() to initialize the CAPAB register

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8c9c0fbc2a..caf5e5a1b4 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1174,12 +1174,19 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
     }
 }
 
+static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
+{
+    if (s->capareg == UINT64_MAX) {
+        s->capareg = SDHC_CAPAB_REG_DEFAULT;
+    }
+}
+
 /* --- qdev common --- */
 
 #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
-    /* Capabilities registers provide information on supported features
-     * of this specific host controller implementation */ \
-    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
+    /* deprecated: Capabilities registers provide information on supported
+     * features of this specific host controller implementation */ \
+    DEFINE_PROP_UINT64("capareg", _state, capareg, UINT64_MAX), \
     DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
 
 static void sdhci_initfn(SDHCIState *s)
@@ -1204,6 +1211,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

[Qemu-devel] [PATCH v8 08/17] sdhci: add a 'spec_version property' (default to v2)

[Philippe Mathieu-Daudé]

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          | 13 +++++++++++--
 3 files changed, 15 insertions(+), 4 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..c174a39ecf 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 spec_version;
 } SDHCIState;
 
 #define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index caf5e5a1b4..76714f1143 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 " "
@@ -1176,6 +1177,12 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
 
 static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
 {
+    if (s->spec_version != 2) {
+        error_setg(errp, "Only Spec v2 is supported");
+        return;
+    }
+    s->version = (SDHC_HCVER_VENDOR << 8) | (s->spec_version - 1);
+
     if (s->capareg == UINT64_MAX) {
         s->capareg = SDHC_CAPAB_REG_DEFAULT;
     }
@@ -1184,6 +1191,8 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
 /* --- qdev common --- */
 
 #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
+    DEFINE_PROP_UINT8("sd-spec-version", _state, spec_version, 2), \
+    \
     /* deprecated: Capabilities registers provide information on supported
      * features of this specific host controller implementation */ \
     DEFINE_PROP_UINT64("capareg", _state, capareg, UINT64_MAX), \
-- 
2.15.1

[Qemu-devel] [PATCH v8 09/17] sdhci: add basic Spec v1 capabilities

[Philippe Mathieu-Daudé]

(Note than Spec v2 is supported by default)

we emit a warning for missing capabilities bits.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci-internal.h | 24 ++++++++++++++++++-
 include/hw/sd/sdhci.h  | 11 +++++++++
 hw/sd/sdhci.c          | 65 ++++++++++++++++++++++++++++++++++++++------------
 3 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index b7751c815f..9acafe7b01 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
 
@@ -84,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
@@ -94,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
@@ -116,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
@@ -172,17 +179,32 @@
 
 /* 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)
 #define SDHC_CAPAB_BLOCKSIZE(x)        (((x) >> 16) & 0x3)
+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/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index c174a39ecf..bc80f59d3c 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -92,6 +92,17 @@ typedef struct SDHCIState {
     /* Configurable properties */
     bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
     uint8_t spec_version;
+    struct {
+        /***********
+         * Spec v1
+         ***********/
+        /* Suspend/resume */
+        bool suspend;
+        bool high_speed;
+        bool sdma;
+        /* Voltage */
+        bool v33, v30, v18;
+    } cap;
 } SDHCIState;
 
 #define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 76714f1143..e56ee7778c 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"
@@ -45,12 +46,6 @@
  * 0 - not supported, 1 - supported, other - prohibited.
  */
 #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
@@ -64,9 +59,7 @@
 #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 ||\
+#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 || \
     SDHC_CAPAB_TOUNIT > 1
 #error Capabilities features can have value 0 or 1 only!
 #endif
@@ -91,16 +84,37 @@
 #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_64BITBUS << 28) | (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))
 
+static uint64_t sdhci_init_capareg(SDHCIState *s, Error **errp)
+{
+    uint64_t capareg = 0;
+
+    switch (s->spec_version) {
+    case 2: /* default version */
+
+    /* fallback */
+    case 1:
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, HIGHSPEED, s->cap.high_speed);
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, SDMA, s->cap.sdma);
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, SUSPRESUME, s->cap.suspend);
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V33, s->cap.v33);
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V30, s->cap.v30);
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V18, s->cap.v18);
+        break;
+
+    default:
+        error_setg(errp, "Unsupported spec version: %u", s->spec_version);
+    }
+
+    return capareg;
+}
+
 static uint8_t sdhci_slotint(SDHCIState *s)
 {
     return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
@@ -1027,7 +1041,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);
@@ -1177,14 +1191,27 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
 
 static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
 {
+    uint64_t capab;
+    Error *local_err = NULL;
+
     if (s->spec_version != 2) {
         error_setg(errp, "Only Spec v2 is supported");
         return;
     }
     s->version = (SDHC_HCVER_VENDOR << 8) | (s->spec_version - 1);
 
+    capab = sdhci_init_capareg(s, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
     if (s->capareg == UINT64_MAX) {
-        s->capareg = SDHC_CAPAB_REG_DEFAULT;
+        s->capareg = capab;
+    } else {
+        capab = s->capareg & ~capab;
+        if (capab) {
+            warn_report("SDHCI: missing capability mask: 0x%" PRIx64, capab);
+        }
     }
 }
 
@@ -1193,6 +1220,14 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
 #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
     DEFINE_PROP_UINT8("sd-spec-version", _state, spec_version, 2), \
     \
+    /* Spec v1 properties */ \
+    DEFINE_PROP_BOOL("sdma", _state, cap.sdma, true), \
+    DEFINE_PROP_BOOL("suspend", _state, cap.suspend, false), \
+    DEFINE_PROP_BOOL("high-speed", _state, cap.high_speed, true), \
+    DEFINE_PROP_BOOL("3v3", _state, cap.v33, true), \
+    DEFINE_PROP_BOOL("3v0", _state, cap.v30, false), \
+    DEFINE_PROP_BOOL("1v8", _state, cap.v18, true), \
+    \
     /* deprecated: Capabilities registers provide information on supported
      * features of this specific host controller implementation */ \
     DEFINE_PROP_UINT64("capareg", _state, capareg, UINT64_MAX), \
-- 
2.15.1

[Qemu-devel] [PATCH v8 10/17] sdhci: add max-block-length capability (Spec v1)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
 hw/sd/sdhci-internal.h |  1 -
 include/hw/sd/sdhci.h  |  2 ++
 hw/sd/sdhci.c          | 36 +++++++++++-------------------------
 3 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 9acafe7b01..c5e26bf8f3 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -188,7 +188,6 @@ FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR,    4, 1);
 #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, TOCLKFREQ,           0, 6);
 FIELD(SDHC_CAPAB, TOUNIT,              7, 1);
 FIELD(SDHC_CAPAB, BASECLKFREQ,         8, 8);
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index bc80f59d3c..d3d438879c 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -96,6 +96,8 @@ typedef struct SDHCIState {
         /***********
          * Spec v1
          ***********/
+        /* Maximum host controller R/W buffers size (512, 1024, 2048 bytes) */
+        uint16_t max_blk_len;
         /* Suspend/resume */
         bool suspend;
         bool high_speed;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e56ee7778c..ced2f66aee 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -48,9 +48,6 @@
 #define SDHC_CAPAB_64BITBUS       0ul        /* 64-bit System Bus 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
@@ -64,16 +61,6 @@
 #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!
@@ -85,7 +72,7 @@
 
 #define SDHC_CAPAB_REG_DEFAULT                                 \
    ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_ADMA1 << 20) |   \
-    (SDHC_CAPAB_ADMA2 << 19) | (MAX_BLOCK_LENGTH << 16) |      \
+    (SDHC_CAPAB_ADMA2 << 19) |                                 \
     (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \
     (SDHC_CAPAB_TOCLKFREQ))
 
@@ -94,12 +81,20 @@
 static uint64_t sdhci_init_capareg(SDHCIState *s, Error **errp)
 {
     uint64_t capareg = 0;
+    uint32_t val;
 
     switch (s->spec_version) {
     case 2: /* default version */
 
     /* fallback */
     case 1:
+        val = ctz32(s->cap.max_blk_len >> 9);
+        if (val >= 0b11) {
+            error_setg(errp, "block size can be 512, 1024 or 2048 only");
+            return 0;
+        }
+        capareg = FIELD_DP64(capareg, SDHC_CAPAB, MAXBLOCKLENGTH, val);
+
         capareg = FIELD_DP64(capareg, SDHC_CAPAB, HIGHSPEED, s->cap.high_speed);
         capareg = FIELD_DP64(capareg, SDHC_CAPAB, SDMA, s->cap.sdma);
         capareg = FIELD_DP64(capareg, SDHC_CAPAB, SUSPRESUME, s->cap.suspend);
@@ -1176,17 +1171,7 @@ static const MemoryRegionOps sdhci_mmio_ops = {
 
 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;
-    }
+    return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
 }
 
 static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
@@ -1221,6 +1206,7 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
     DEFINE_PROP_UINT8("sd-spec-version", _state, spec_version, 2), \
     \
     /* Spec v1 properties */ \
+    DEFINE_PROP_UINT16("max-block-length", _state, cap.max_blk_len, 512), \
     DEFINE_PROP_BOOL("sdma", _state, cap.sdma, true), \
     DEFINE_PROP_BOOL("suspend", _state, cap.suspend, false), \
     DEFINE_PROP_BOOL("high-speed", _state, cap.high_speed, true), \
-- 
2.15.1

[Qemu-devel] [PATCH v8 13/17] sdhci: Fix 64-bit ADMA2

[Philippe Mathieu-Daudé]

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 7390b8464b..6277f067da 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -632,8 +632,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

[Qemu-devel] [PATCH v8 15/17] hw/arm/xilinx_zynq: implement SDHCI Spec v2

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
 hw/arm/xilinx_zynq.c | 70 ++++++++++++++++++++++++++++++++++------------------
 tests/sdhci-test.c   |  4 +++
 2 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1836a4ed45..349787aacd 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -165,10 +165,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 +245,51 @@ 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
+         *
+         * - SDMA (single operation DMA)
+         * - ADMA1 (4 KB boundary limited DMA)
+         * - ADMA2
+         *
+         * - up to seven functions in SD1, SD4, but does not support SPI mode
+         * - SD high-speed (SDHS) card
+         * - SD High Capacity (SDHC) card
+         *
+         * - Low-speed, 1 KHz to 400 KHz
+         * - Full-speed, 1 MHz to 50 MHz (25 MB/sec)
+         */
+        dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
+        qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+        qdev_prop_set_uint8(dev, "timeout-freq", 0);
+        qdev_prop_set_uint8(dev, "max-frequency", 0);
+        qdev_prop_set_bit(dev, "suspend", false);
+        qdev_prop_set_bit(dev, "sdma", true);
+        qdev_prop_set_bit(dev, "adma1", true);
+        qdev_prop_set_bit(dev, "adma2", true);
+        qdev_prop_set_bit(dev, "1v8", false);
+        qdev_prop_set_bit(dev, "high-speed", true);
+        qdev_prop_set_uint16(dev, "max-block-length", 1024);
+        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 37ada0e1c5..44ee5651c4 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -41,6 +41,10 @@ static const struct sdhci_t {
     /* Exynos4210 */
     { "arm",    "smdkc210",
         {0x12510000, 2, 0, {1, 0x5e80080} } },
+
+    /* Zynq-7000 */
+    { "arm",    "xilinx-zynq-a9",
+        {0xe0100000, 2, 0, {1, 0x01790080} } },
 };
 
 static struct {
-- 
2.15.1

[Qemu-devel] [PATCH v8 16/17] hw/arm/exynos4210: implement SDHCI Spec v2

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Alistair Francis <alistair.francis@xilinx.com>
---
Krzysztof kindly checked those values with Manaul, the Samsung datasheet :)

 hw/arm/exynos4210.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index e8e1d81e62..e70e234f58 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -75,7 +75,6 @@
 #define EXYNOS4210_INT_COMBINER_BASE_ADDR   0x10448000
 
 /* SD/MMC host controllers */
-#define EXYNOS4210_SDHCI_CAPABILITIES       0x05E80080
 #define EXYNOS4210_SDHCI_BASE_ADDR          0x12510000
 #define EXYNOS4210_SDHCI_ADDR(n)            (EXYNOS4210_SDHCI_BASE_ADDR + \
                                                 0x00010000 * (n))
@@ -377,8 +376,26 @@ 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_uint32(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
+        qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+        qdev_prop_set_uint8(dev, "timeout-freq", 0);
+        qdev_prop_set_uint8(dev, "max-frequency", 0);
+        qdev_prop_set_bit(dev, "suspend", true);
+        qdev_prop_set_bit(dev, "sdma", true);
+        qdev_prop_set_bit(dev, "adma1", false);
+        qdev_prop_set_bit(dev, "adma2", true);
         qdev_init_nofail(dev);
 
         busdev = SYS_BUS_DEVICE(dev);
-- 
2.15.1

[Qemu-devel] [PATCH v8 17/17] sdhci: throw an error if capabilities are incorrectly configured

[Philippe Mathieu-Daudé]

now than the Exynos4210 uses the 'capareg-reserved' property,
we can safely report missing capabilities as error, since
capabilities will only get changed once new SDHCI models
are added.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
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 d0d64f0c52..6232fcd914 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
@@ -1210,7 +1209,8 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
     } else {
         capab = s->capareg & ~capab;
         if (capab) {
-            warn_report("SDHCI: missing capability mask: 0x%" PRIx64, capab);
+            error_setg(errp, "missing capability mask: 0x%" PRIx64, capab);
+            return;
         }
     }
 }
-- 
2.15.1

Re: [Qemu-devel] [PATCH v8 01/17] sdhci: use error_propagate(local_err) in realize()

[Alistair Francis]

On Thu, Jan 18, 2018 at 10:30 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> 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>

Alistair

> ---
>  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
>
>

Re: [Qemu-devel] [PATCH v8 07/17] sdhci: add init_readonly_registers() to initialize the CAPAB register

[Alistair Francis]

On Thu, Jan 18, 2018 at 10:30 AM, 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.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 8c9c0fbc2a..caf5e5a1b4 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1174,12 +1174,19 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>      }
>  }
>
> +static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
> +{
> +    if (s->capareg == UINT64_MAX) {
> +        s->capareg = SDHC_CAPAB_REG_DEFAULT;
> +    }
> +}
> +
>  /* --- qdev common --- */
>
>  #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> -    /* Capabilities registers provide information on supported features
> -     * of this specific host controller implementation */ \
> -    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
> +    /* deprecated: Capabilities registers provide information on supported
> +     * features of this specific host controller implementation */ \
> +    DEFINE_PROP_UINT64("capareg", _state, capareg, UINT64_MAX), \
>      DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
>
>  static void sdhci_initfn(SDHCIState *s)
> @@ -1204,6 +1211,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
>
>

Re: [Qemu-devel] [PATCH v8 09/17] sdhci: add basic Spec v1 capabilities

[Alistair Francis]

On Thu, Jan 18, 2018 at 10:31 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> (Note than Spec v2 is supported by default)
>
> we emit a warning for missing capabilities bits.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sdhci-internal.h | 24 ++++++++++++++++++-
>  include/hw/sd/sdhci.h  | 11 +++++++++
>  hw/sd/sdhci.c          | 65 ++++++++++++++++++++++++++++++++++++++------------
>  3 files changed, 84 insertions(+), 16 deletions(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index b7751c815f..9acafe7b01 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
>
> @@ -84,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
> @@ -94,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
> @@ -116,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
> @@ -172,17 +179,32 @@
>
>  /* 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)
>  #define SDHC_CAPAB_BLOCKSIZE(x)        (((x) >> 16) & 0x3)
> +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/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index c174a39ecf..bc80f59d3c 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -92,6 +92,17 @@ typedef struct SDHCIState {
>      /* Configurable properties */
>      bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
>      uint8_t spec_version;
> +    struct {
> +        /***********
> +         * Spec v1
> +         ***********/
> +        /* Suspend/resume */
> +        bool suspend;
> +        bool high_speed;
> +        bool sdma;
> +        /* Voltage */
> +        bool v33, v30, v18;
> +    } cap;
>  } SDHCIState;
>
>  #define TYPE_PCI_SDHCI "sdhci-pci"
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 76714f1143..e56ee7778c 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"
> @@ -45,12 +46,6 @@
>   * 0 - not supported, 1 - supported, other - prohibited.
>   */
>  #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
> @@ -64,9 +59,7 @@
>  #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 ||\
> +#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 || \
>      SDHC_CAPAB_TOUNIT > 1
>  #error Capabilities features can have value 0 or 1 only!
>  #endif
> @@ -91,16 +84,37 @@
>  #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_64BITBUS << 28) | (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))
>
> +static uint64_t sdhci_init_capareg(SDHCIState *s, Error **errp)
> +{
> +    uint64_t capareg = 0;
> +
> +    switch (s->spec_version) {
> +    case 2: /* default version */
> +
> +    /* fallback */
> +    case 1:
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, HIGHSPEED, s->cap.high_speed);
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, SDMA, s->cap.sdma);
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, SUSPRESUME, s->cap.suspend);
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V33, s->cap.v33);
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V30, s->cap.v30);
> +        capareg = FIELD_DP64(capareg, SDHC_CAPAB, V18, s->cap.v18);
> +        break;
> +
> +    default:
> +        error_setg(errp, "Unsupported spec version: %u", s->spec_version);
> +    }
> +
> +    return capareg;
> +}
> +
>  static uint8_t sdhci_slotint(SDHCIState *s)
>  {
>      return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
> @@ -1027,7 +1041,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);
> @@ -1177,14 +1191,27 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>
>  static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>  {
> +    uint64_t capab;
> +    Error *local_err = NULL;
> +
>      if (s->spec_version != 2) {
>          error_setg(errp, "Only Spec v2 is supported");
>          return;
>      }
>      s->version = (SDHC_HCVER_VENDOR << 8) | (s->spec_version - 1);
>
> +    capab = sdhci_init_capareg(s, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
>      if (s->capareg == UINT64_MAX) {
> -        s->capareg = SDHC_CAPAB_REG_DEFAULT;
> +        s->capareg = capab;
> +    } else {
> +        capab = s->capareg & ~capab;
> +        if (capab) {
> +            warn_report("SDHCI: missing capability mask: 0x%" PRIx64, capab);
> +        }
>      }
>  }
>
> @@ -1193,6 +1220,14 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>  #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>      DEFINE_PROP_UINT8("sd-spec-version", _state, spec_version, 2), \
>      \
> +    /* Spec v1 properties */ \
> +    DEFINE_PROP_BOOL("sdma", _state, cap.sdma, true), \
> +    DEFINE_PROP_BOOL("suspend", _state, cap.suspend, false), \
> +    DEFINE_PROP_BOOL("high-speed", _state, cap.high_speed, true), \
> +    DEFINE_PROP_BOOL("3v3", _state, cap.v33, true), \
> +    DEFINE_PROP_BOOL("3v0", _state, cap.v30, false), \
> +    DEFINE_PROP_BOOL("1v8", _state, cap.v18, true), \
> +    \
>      /* deprecated: Capabilities registers provide information on supported
>       * features of this specific host controller implementation */ \
>      DEFINE_PROP_UINT64("capareg", _state, capareg, UINT64_MAX), \
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH v8 16/17] hw/arm/exynos4210: implement SDHCI Spec v2

[Krzysztof Kozlowski]

On Thu, Jan 18, 2018 at 03:31:07PM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Acked-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> Krzysztof kindly checked those values with Manaul, the Samsung datasheet :)
> 
>  hw/arm/exynos4210.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof