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.
signature.asc
Description: OpenPGP digital signature
