Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/sdbus-test.c     | 151 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include |   2 +
 2 files changed, 153 insertions(+)
 create mode 100644 tests/sdbus-test.c

diff --git a/tests/sdbus-test.c b/tests/sdbus-test.c
new file mode 100644
index 0000000000..9c38be13cb
--- /dev/null
+++ b/tests/sdbus-test.c
@@ -0,0 +1,151 @@
+/*
+ * QTest testcase for the SD/MMC cards
+ *
+ * Copyright (c) 2017 Philippe Mathieu-Daudé
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "qemu/cutils.h"
+
+#include "libqtest.h"
+#include "libqos/sdbus.h"
+
+enum {
+    PROTO_SD,
+    PROTO_MMC,
+    PROTO_SPI,
+    PROTO_COUNT
+};
+
+static const char *proto_name[PROTO_COUNT] = {
+    [PROTO_SD]  = "sd",
+    [PROTO_MMC] = "mmc",
+    [PROTO_SPI] = "spi"
+};
+
+static const char *machines[PROTO_COUNT] = {
+    [PROTO_SD] = "nuri",
+    //[PROTO_MMC] = "vexpress-a9",
+    //[PROTO_SPI] = "lm3s6965evb"
+};
+
+static const uint64_t sizes[] = {
+    //512 * M_BYTE,
+    //1 * G_BYTE,
+    4 * G_BYTE,
+    //64 * G_BYTE,
+};
+
+typedef struct {
+    int protocol;
+    uint64_t size;
+} TestCase;
+
+static void test1(SDBusAdapter *mmc, uint64_t size)
+{
+    uint8_t *response;
+    uint16_t rca;
+    ssize_t sz;
+
+    sz = sdbus_do_cmd(mmc, GO_IDLE_STATE, 0, NULL);
+    g_assert_cmpuint(sz, ==, 0);
+
+    sz = sdbus_do_cmd(mmc, SEND_IF_COND, 0x1aa, NULL);
+    //g_assert_cmpuint(sz, ==, 0);
+
+    sz = sdbus_do_acmd(mmc, SEND_OP_COND, 0x40300000, 0, NULL);
+    g_assert_cmpuint(sz, ==, 4);
+
+    /* CID */
+    sz = sdbus_do_cmd(mmc, ALL_SEND_CID, 0, &response);
+    g_assert_cmpuint(sz, ==, 16);
+    g_assert_cmpmem (&response[3], 5, "QEMU!", 5);
+    g_assert_cmphex(be32_to_cpu(*(uint32_t *)&response[9]), ==, 0xdeadbeef);
+    g_free(response);
+
+    /* RCA */
+    sz = sdbus_do_cmd(mmc, SEND_RELATIVE_ADDR, 0, &response);
+    g_assert_cmpuint(sz, ==, 4);
+    rca = be16_to_cpu(*(uint16_t *)response);
+    g_assert_cmphex(rca, ==, 0x4567);
+    g_free(response);
+
+    /* CSD */
+    sz = sdbus_do_cmd(mmc, SEND_CSD, rca << 16, &response);
+    g_assert_cmpuint(sz, ==, 16);
+    g_assert_cmphex(response[3], ==, 0x32);
+    g_assert_cmphex(response[4], ==, 0x5b); /* class */
+    g_assert_cmphex(response[5], ==, 0x59);
+    /* (SDHC test) */
+    g_assert_cmphex(be32_to_cpu(*(uint32_t *)&response[6]),
+                    ==, (size >> 19) - 1);
+    g_assert_cmphex(response[10], ==, 0x7f);
+    g_assert_cmphex(response[11], ==, 0x80);
+    g_assert_cmphex(response[12], ==, 0x0a);
+    g_assert_cmphex(response[13], ==, 0x40);
+    g_assert_cmphex(response[14], ==, 0);
+    g_assert_cmphex(response[15], ==, 0);
+    g_free(response);
+
+    sz = sdbus_do_cmd(mmc, SELECT_CARD, rca << 16, NULL);
+
+    sz = sdbus_do_acmd(mmc, SEND_SCR, 0, rca, &response);
+    g_assert_cmpuint(sz, ==, 4);
+    g_free(response);
+
+    // TODO 8x: sdcard_read_data len 512
+
+    //sz = sdbus_do_acmd(mmc, SEND_STATUS, 0, rca, &response);
+    //g_free(response);
+}
+
+static void sdcard_tests(gconstpointer data)
+{
+    const TestCase *test = data;
+    SDBusAdapter *sdbus;
+
+    global_qtest = qtest_startf("-machine %s "
+                         "-drive if=sd,driver=null-co,size=%lu,id=mmc0",
+                         machines[test->protocol], test->size);
+    sdbus = qmp_sdbus_create("sd-bus");
+
+    test1(sdbus, test->size);
+    g_free(sdbus);
+
+    qtest_quit(global_qtest);
+}
+
+int main(int argc, char **argv)
+{
+    const char *arch = qtest_get_arch();
+    int iproto, isize;
+    gchar *path;
+    TestCase *test;
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
+        for (iproto = 0; iproto < PROTO_COUNT; iproto++) {
+            if (!machines[iproto]) {
+                continue;
+            }
+            for (isize = 0; isize < ARRAY_SIZE(sizes); isize++) {
+                test = g_new(TestCase, 1);
+
+                test->protocol = iproto;
+                test->size = sizes[isize];
+
+                path = g_strdup_printf("sdcard/%s/%lu", proto_name[iproto], sizes[isize]);
+                qtest_add_data_func(path, test, sdcard_tests);
+                g_free(path);
+                // g_free(test)?
+            }
+        }
+    }
+
+    return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 409784a189..e4434cdfff 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -348,6 +348,7 @@ check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
+check-qtest-arm-y = tests/sdbus-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
@@ -748,6 +749,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
+tests/sdbus-test$(EXESUF): tests/sdbus-test.o $(libqos-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
-- 
2.15.1

On Wed, Jan 03, 2018 at 06:49:25PM -0300, Philippe Mathieu-Daudé wrote:
> +static const char *machines[PROTO_COUNT] = {
> +    [PROTO_SD] = "nuri",
> +    //[PROTO_MMC] = "vexpress-a9",
> +    //[PROTO_SPI] = "lm3s6965evb"
> +};
> +
> +static const uint64_t sizes[] = {
> +    //512 * M_BYTE,
> +    //1 * G_BYTE,
> +    4 * G_BYTE,
> +    //64 * G_BYTE,
> +};

Why are these commented out?

> +static void test1(SDBusAdapter *mmc, uint64_t size)
> +{
> +    uint8_t *response;
> +    uint16_t rca;
> +    ssize_t sz;
> +
> +    sz = sdbus_do_cmd(mmc, GO_IDLE_STATE, 0, NULL);
> +    g_assert_cmpuint(sz, ==, 0);
> +
> +    sz = sdbus_do_cmd(mmc, SEND_IF_COND, 0x1aa, NULL);
> +    //g_assert_cmpuint(sz, ==, 0);

Why is this commented out?

> +    // TODO 8x: sdcard_read_data len 512
> +
> +    //sz = sdbus_do_acmd(mmc, SEND_STATUS, 0, rca, &response);
> +    //g_free(response);

Why is there commenteded out code and TODO here?  Please either
implement this stuff or remove it from the patch.

> +int main(int argc, char **argv)
> +{
> +    const char *arch = qtest_get_arch();
> +    int iproto, isize;
> +    gchar *path;
> +    TestCase *test;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
> +        for (iproto = 0; iproto < PROTO_COUNT; iproto++) {
> +            if (!machines[iproto]) {
> +                continue;
> +            }
> +            for (isize = 0; isize < ARRAY_SIZE(sizes); isize++) {
> +                test = g_new(TestCase, 1);
> +
> +                test->protocol = iproto;
> +                test->size = sizes[isize];
> +
> +                path = g_strdup_printf("sdcard/%s/%lu", proto_name[iproto], sizes[isize]);
> +                qtest_add_data_func(path, test, sdcard_tests);
> +                g_free(path);
> +                // g_free(test)?

Please remove this.  qtest_add_data_func() keeps the pointer to test so
it must not be freed.

Hi Stefan,

On 01/05/2018 12:31 PM, Stefan Hajnoczi wrote:
> On Wed, Jan 03, 2018 at 06:49:25PM -0300, Philippe Mathieu-Daudé wrote:
>> +static const char *machines[PROTO_COUNT] = {
>> +    [PROTO_SD] = "nuri",
>> +    //[PROTO_MMC] = "vexpress-a9",
>> +    //[PROTO_SPI] = "lm3s6965evb"
>> +};
>> +
>> +static const uint64_t sizes[] = {
>> +    //512 * M_BYTE,
>> +    //1 * G_BYTE,
>> +    4 * G_BYTE,
>> +    //64 * G_BYTE,
>> +};
> 
> Why are these commented out?

As I didn't feel good feedback for the previous Python qtests, I
prefered to send this as RFC and to see if this was the good way to go
before spending more time learning QDECREF() and friends :)

However I again forgot to replace RFC -> NOTFORMERGE in the subject :(

These parameters are commented out because the current sd.c code doesn't
work with those cases :(
So to show the qtests is useful and pass Travis/Patchew build, they are
commented, and to show the current model is broken (until I succeed to
fix it), one can uncomment these and see failing tests :)

> 
>> +static void test1(SDBusAdapter *mmc, uint64_t size)
>> +{
>> +    uint8_t *response;
>> +    uint16_t rca;
>> +    ssize_t sz;
>> +
>> +    sz = sdbus_do_cmd(mmc, GO_IDLE_STATE, 0, NULL);
>> +    g_assert_cmpuint(sz, ==, 0);
>> +
>> +    sz = sdbus_do_cmd(mmc, SEND_IF_COND, 0x1aa, NULL);
>> +    //g_assert_cmpuint(sz, ==, 0);
> 
> Why is this commented out?

Actually this test is SD oriented, in MMC mode this command is incorrect.
I'll split this test in 3 (the SD/MMC/SPI protocols).

> 
>> +    // TODO 8x: sdcard_read_data len 512
>> +
>> +    //sz = sdbus_do_acmd(mmc, SEND_STATUS, 0, rca, &response);
>> +    //g_free(response);
> 
> Why is there commenteded out code and TODO here?  Please either
> implement this stuff or remove it from the patch.

Yes, I have to handle this differently for MMC vs SD/SPI.

> 
>> +int main(int argc, char **argv)
>> +{
>> +    const char *arch = qtest_get_arch();
>> +    int iproto, isize;
>> +    gchar *path;
>> +    TestCase *test;
>> +
>> +    g_test_init(&argc, &argv, NULL);
>> +
>> +    if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
>> +        for (iproto = 0; iproto < PROTO_COUNT; iproto++) {
>> +            if (!machines[iproto]) {
>> +                continue;
>> +            }
>> +            for (isize = 0; isize < ARRAY_SIZE(sizes); isize++) {
>> +                test = g_new(TestCase, 1);
>> +
>> +                test->protocol = iproto;
>> +                test->size = sizes[isize];
>> +
>> +                path = g_strdup_printf("sdcard/%s/%lu", proto_name[iproto], sizes[isize]);
>> +                qtest_add_data_func(path, test, sdcard_tests);
>> +                g_free(path);
>> +                // g_free(test)?
> 
> Please remove this.  qtest_add_data_func() keeps the pointer to test so
> it must not be freed.

Ok!

Thanks for your review :)

Phil.

On Fri, Jan 05, 2018 at 12:55:41PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Stefan,
> 
> On 01/05/2018 12:31 PM, Stefan Hajnoczi wrote:
> > On Wed, Jan 03, 2018 at 06:49:25PM -0300, Philippe Mathieu-Daudé wrote:
> >> +static const char *machines[PROTO_COUNT] = {
> >> +    [PROTO_SD] = "nuri",
> >> +    //[PROTO_MMC] = "vexpress-a9",
> >> +    //[PROTO_SPI] = "lm3s6965evb"
> >> +};
> >> +
> >> +static const uint64_t sizes[] = {
> >> +    //512 * M_BYTE,
> >> +    //1 * G_BYTE,
> >> +    4 * G_BYTE,
> >> +    //64 * G_BYTE,
> >> +};
> > 
> > Why are these commented out?
> 
> As I didn't feel good feedback for the previous Python qtests, I
> prefered to send this as RFC and to see if this was the good way to go
> before spending more time learning QDECREF() and friends :)
> 
> However I again forgot to replace RFC -> NOTFORMERGE in the subject :(
> 
> These parameters are commented out because the current sd.c code doesn't
> work with those cases :(
> So to show the qtests is useful and pass Travis/Patchew build, they are
> commented, and to show the current model is broken (until I succeed to
> fix it), one can uncomment these and see failing tests :)

If you want to commit commented out code then please include a comment
that explains why it's commented out.  That way readers understand why
it's there.

Also please run checkpatch.pl against all patches.  It can be run as a
commit hook so all your code is checked at commit time:
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html