diff mbox series

[v6,8/8] hw/gpio/aspeed: Add test case for AST2700

Message ID 20240930085239.3089901-9-jamin_lin@aspeedtech.com (mailing list archive)
State New, archived
Headers show
Series Support GPIO for AST2700 | expand

Commit Message

Jamin Lin Sept. 30, 2024, 8:52 a.m. UTC 
Add test case to test GPIO output and input pins from A0 to D7 for AST2700.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed_gpio-test.c | 77 ++++++++++++++++++++++++++++++++--
 tests/qtest/meson.build        |  3 ++
 2 files changed, 76 insertions(+), 4 deletions(-)

Comments

Thomas Huth Sept. 30, 2024, 4:36 p.m. UTC | #1 
On 30/09/2024 10.52, Jamin Lin wrote:
> Add test case to test GPIO output and input pins from A0 to D7 for AST2700.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   tests/qtest/aspeed_gpio-test.c | 77 ++++++++++++++++++++++++++++++++--
>   tests/qtest/meson.build        |  3 ++
>   2 files changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
> index d38f51d719..03b3b1c2b2 100644
> --- a/tests/qtest/aspeed_gpio-test.c
> +++ b/tests/qtest/aspeed_gpio-test.c
> @@ -33,6 +33,10 @@
>   #define GPIO_ABCD_DATA_VALUE 0x000
>   #define GPIO_ABCD_DIRECTION  0x004
>   
> +/* AST2700 */
> +#define AST2700_GPIO_BASE 0x14C0B000
> +#define GPIOA0_CONTROL 0x180
> +
>   static void test_set_colocated_pins(const void *data)
>   {
>       QTestState *s = (QTestState *)data;
> @@ -72,17 +76,82 @@ static void test_set_input_pins(const void *data)
>       g_assert_cmphex(value, ==, 0xffffffff);
>   }
>   
> +static void test_2700_output_pins(const void *data)
> +{
> +    QTestState *s = (QTestState *)data;
> +    uint32_t offset = 0;
> +    uint32_t value = 0;
> +    uint32_t pin = 0;
> +
> +    for (char c = 'A'; c <= 'D'; c++) {
> +        for (int i = 0; i < 8; i++) {
> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
> +
> +            /* output direction and output hi */
> +            qtest_writel(s, offset, 0x00000003);
> +            value = qtest_readl(s, offset);
> +            g_assert_cmphex(value, ==, 0x00000003);
> +
> +            /* output direction and output low */
> +            qtest_writel(s, offset, 0x00000002);
> +            value = qtest_readl(s, offset);
> +            g_assert_cmphex(value, ==, 0x00000002);
> +            pin++;
> +        }
> +    }
> +}
> +
> +static void test_2700_input_pins(const void *data)
> +{
> +    QTestState *s = (QTestState *)data;
> +    char name[16];
> +    uint32_t offset = 0;
> +    uint32_t value = 0;
> +    uint32_t pin = 0;
> +
> +    for (char c = 'A'; c <= 'D'; c++) {
> +        for (int i = 0; i < 8; i++) {
> +            sprintf(name, "gpio%c%d", c, i);
> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
> +            /* input direction */
> +            qtest_writel(s, offset, 0);
> +
> +            /* set input */
> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
> +            value = qtest_readl(s, offset);
> +            g_assert_cmphex(value, ==, 0x00002000);
> +
> +            /* clear input */
> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, false);
> +            value = qtest_readl(s, offset);
> +            g_assert_cmphex(value, ==, 0);
> +            pin++;
> +        }
> +    }
> +}

As far as I can see, there is nothing in these two functions that requires 
any of the other code in this file ...

> +
>   int main(int argc, char **argv)
>   {
> +    const char *arch = qtest_get_arch();
>       QTestState *s;
>       int r;
>   
>       g_test_init(&argc, &argv, NULL);
>   
> -    s = qtest_init("-machine ast2600-evb");
> -    qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
> -                        test_set_colocated_pins);
> -    qtest_add_data_func("/ast2600/gpio/set_input_pins", s, test_set_input_pins);
> +    if (strcmp(arch, "aarch64") == 0) {
> +        s = qtest_init("-machine ast2700-evb");
> +        qtest_add_data_func("/ast2700/gpio/input_pins",
> +                            s, test_2700_input_pins);
> +        qtest_add_data_func("/ast2700/gpio/out_pins", s, test_2700_output_pins);
> +    } else {
> +        s = qtest_init("-machine ast2600-evb");
> +        qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
> +                            test_set_colocated_pins);
> +        qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
> +                            test_set_input_pins);
> +    }

... so the more I look at this, the more I think your new test should reside 
in a separate file that only gets executed for aarch64, while this file here 
should stay for arm 32-bit. Or is there a real compelling reason for putting 
your code in this file here?

  Thomas


>       r = g_test_run();
>       qtest_quit(s);
>   
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 310865e49c..292980e3ad 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -209,6 +209,8 @@ qtests_aspeed = \
>     ['aspeed_hace-test',
>      'aspeed_smc-test',
>      'aspeed_gpio-test']
> +qtests_aspeed64 = \
> +  ['aspeed_gpio-test']
>   
>   qtests_stm32l4x5 = \
>     ['stm32l4x5_exti-test',
> @@ -247,6 +249,7 @@ qtests_aarch64 = \
>     (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-i2c-test'] : []) +  \
>     (config_all_accel.has_key('CONFIG_TCG') and                                            \
>      config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
> +  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>     ['arm-cpu-features',
>      'numa-test',
>      'boot-serial-test',
Cédric Le Goater Sept. 30, 2024, 4:48 p.m. UTC | #2 
On 9/30/24 18:36, Thomas Huth wrote:
> On 30/09/2024 10.52, Jamin Lin wrote:
>> Add test case to test GPIO output and input pins from A0 to D7 for AST2700.
>>
>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>> ---
>>   tests/qtest/aspeed_gpio-test.c | 77 ++++++++++++++++++++++++++++++++--
>>   tests/qtest/meson.build        |  3 ++
>>   2 files changed, 76 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
>> index d38f51d719..03b3b1c2b2 100644
>> --- a/tests/qtest/aspeed_gpio-test.c
>> +++ b/tests/qtest/aspeed_gpio-test.c
>> @@ -33,6 +33,10 @@
>>   #define GPIO_ABCD_DATA_VALUE 0x000
>>   #define GPIO_ABCD_DIRECTION  0x004
>> +/* AST2700 */
>> +#define AST2700_GPIO_BASE 0x14C0B000
>> +#define GPIOA0_CONTROL 0x180
>> +
>>   static void test_set_colocated_pins(const void *data)
>>   {
>>       QTestState *s = (QTestState *)data;
>> @@ -72,17 +76,82 @@ static void test_set_input_pins(const void *data)
>>       g_assert_cmphex(value, ==, 0xffffffff);
>>   }
>> +static void test_2700_output_pins(const void *data)
>> +{
>> +    QTestState *s = (QTestState *)data;
>> +    uint32_t offset = 0;
>> +    uint32_t value = 0;
>> +    uint32_t pin = 0;
>> +
>> +    for (char c = 'A'; c <= 'D'; c++) {
>> +        for (int i = 0; i < 8; i++) {
>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
>> +
>> +            /* output direction and output hi */
>> +            qtest_writel(s, offset, 0x00000003);
>> +            value = qtest_readl(s, offset);
>> +            g_assert_cmphex(value, ==, 0x00000003);
>> +
>> +            /* output direction and output low */
>> +            qtest_writel(s, offset, 0x00000002);
>> +            value = qtest_readl(s, offset);
>> +            g_assert_cmphex(value, ==, 0x00000002);
>> +            pin++;
>> +        }
>> +    }
>> +}
>> +
>> +static void test_2700_input_pins(const void *data)
>> +{
>> +    QTestState *s = (QTestState *)data;
>> +    char name[16];
>> +    uint32_t offset = 0;
>> +    uint32_t value = 0;
>> +    uint32_t pin = 0;
>> +
>> +    for (char c = 'A'; c <= 'D'; c++) {
>> +        for (int i = 0; i < 8; i++) {
>> +            sprintf(name, "gpio%c%d", c, i);
>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
>> +            /* input direction */
>> +            qtest_writel(s, offset, 0);
>> +
>> +            /* set input */
>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
>> +            value = qtest_readl(s, offset);
>> +            g_assert_cmphex(value, ==, 0x00002000);
>> +
>> +            /* clear input */
>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, false);
>> +            value = qtest_readl(s, offset);
>> +            g_assert_cmphex(value, ==, 0);
>> +            pin++;
>> +        }
>> +    }
>> +}
> 
> As far as I can see, there is nothing in these two functions that requires any of the other code in this file ...
> 
>> +
>>   int main(int argc, char **argv)
>>   {
>> +    const char *arch = qtest_get_arch();
>>       QTestState *s;
>>       int r;
>>       g_test_init(&argc, &argv, NULL);
>> -    s = qtest_init("-machine ast2600-evb");
>> -    qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
>> -                        test_set_colocated_pins);
>> -    qtest_add_data_func("/ast2600/gpio/set_input_pins", s, test_set_input_pins);
>> +    if (strcmp(arch, "aarch64") == 0) {
>> +        s = qtest_init("-machine ast2700-evb");
>> +        qtest_add_data_func("/ast2700/gpio/input_pins",
>> +                            s, test_2700_input_pins);
>> +        qtest_add_data_func("/ast2700/gpio/out_pins", s, test_2700_output_pins);
>> +    } else {
>> +        s = qtest_init("-machine ast2600-evb");
>> +        qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
>> +                            test_set_colocated_pins);
>> +        qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
>> +                            test_set_input_pins);
>> +    }
> 
> ... so the more I look at this, the more I think your new test should reside in a separate file that only gets executed for aarch64, while this file here should stay for arm 32-bit. Or is there a real compelling reason for putting your code in this file here?

Because it is related to the Aspeed GPIO controllers. Unless we
want to introduce a new set of test files for 64-bit Aspeed SoC
controllers ? why not.

I am ok with both options. Option 1 is simpler to implement, but
there may be reasons to separate the tests based on architecture,
although I'm not aware of any at the moment. Would you rather prefer
option 2 ? How should we name the test file ?


Thanks,

C.





>   Thomas
> 
> 
>>       r = g_test_run();
>>       qtest_quit(s);
>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>> index 310865e49c..292980e3ad 100644
>> --- a/tests/qtest/meson.build
>> +++ b/tests/qtest/meson.build
>> @@ -209,6 +209,8 @@ qtests_aspeed = \
>>     ['aspeed_hace-test',
>>      'aspeed_smc-test',
>>      'aspeed_gpio-test']
>> +qtests_aspeed64 = \
>> +  ['aspeed_gpio-test']
>>   qtests_stm32l4x5 = \
>>     ['stm32l4x5_exti-test',
>> @@ -247,6 +249,7 @@ qtests_aarch64 = \
>>     (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-i2c-test'] : []) +  \
>>     (config_all_accel.has_key('CONFIG_TCG') and                                            \
>>      config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
>> +  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>>     ['arm-cpu-features',
>>      'numa-test',
>>      'boot-serial-test',
>
Thomas Huth Sept. 30, 2024, 5:23 p.m. UTC | #3 
On 30/09/2024 18.48, Cédric Le Goater wrote:
> On 9/30/24 18:36, Thomas Huth wrote:
>> On 30/09/2024 10.52, Jamin Lin wrote:
>>> Add test case to test GPIO output and input pins from A0 to D7 for AST2700.
>>>
>>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>>> ---
>>>   tests/qtest/aspeed_gpio-test.c | 77 ++++++++++++++++++++++++++++++++--
>>>   tests/qtest/meson.build        |  3 ++
>>>   2 files changed, 76 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
>>> index d38f51d719..03b3b1c2b2 100644
>>> --- a/tests/qtest/aspeed_gpio-test.c
>>> +++ b/tests/qtest/aspeed_gpio-test.c
>>> @@ -33,6 +33,10 @@
>>>   #define GPIO_ABCD_DATA_VALUE 0x000
>>>   #define GPIO_ABCD_DIRECTION  0x004
>>> +/* AST2700 */
>>> +#define AST2700_GPIO_BASE 0x14C0B000
>>> +#define GPIOA0_CONTROL 0x180
>>> +
>>>   static void test_set_colocated_pins(const void *data)
>>>   {
>>>       QTestState *s = (QTestState *)data;
>>> @@ -72,17 +76,82 @@ static void test_set_input_pins(const void *data)
>>>       g_assert_cmphex(value, ==, 0xffffffff);
>>>   }
>>> +static void test_2700_output_pins(const void *data)
>>> +{
>>> +    QTestState *s = (QTestState *)data;
>>> +    uint32_t offset = 0;
>>> +    uint32_t value = 0;
>>> +    uint32_t pin = 0;
>>> +
>>> +    for (char c = 'A'; c <= 'D'; c++) {
>>> +        for (int i = 0; i < 8; i++) {
>>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
>>> +
>>> +            /* output direction and output hi */
>>> +            qtest_writel(s, offset, 0x00000003);
>>> +            value = qtest_readl(s, offset);
>>> +            g_assert_cmphex(value, ==, 0x00000003);
>>> +
>>> +            /* output direction and output low */
>>> +            qtest_writel(s, offset, 0x00000002);
>>> +            value = qtest_readl(s, offset);
>>> +            g_assert_cmphex(value, ==, 0x00000002);
>>> +            pin++;
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static void test_2700_input_pins(const void *data)
>>> +{
>>> +    QTestState *s = (QTestState *)data;
>>> +    char name[16];
>>> +    uint32_t offset = 0;
>>> +    uint32_t value = 0;
>>> +    uint32_t pin = 0;
>>> +
>>> +    for (char c = 'A'; c <= 'D'; c++) {
>>> +        for (int i = 0; i < 8; i++) {
>>> +            sprintf(name, "gpio%c%d", c, i);
>>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
>>> +            /* input direction */
>>> +            qtest_writel(s, offset, 0);
>>> +
>>> +            /* set input */
>>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
>>> +            value = qtest_readl(s, offset);
>>> +            g_assert_cmphex(value, ==, 0x00002000);
>>> +
>>> +            /* clear input */
>>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name, false);
>>> +            value = qtest_readl(s, offset);
>>> +            g_assert_cmphex(value, ==, 0);
>>> +            pin++;
>>> +        }
>>> +    }
>>> +}
>>
>> As far as I can see, there is nothing in these two functions that requires 
>> any of the other code in this file ...
>>
>>> +
>>>   int main(int argc, char **argv)
>>>   {
>>> +    const char *arch = qtest_get_arch();
>>>       QTestState *s;
>>>       int r;
>>>       g_test_init(&argc, &argv, NULL);
>>> -    s = qtest_init("-machine ast2600-evb");
>>> -    qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
>>> -                        test_set_colocated_pins);
>>> -    qtest_add_data_func("/ast2600/gpio/set_input_pins", s, 
>>> test_set_input_pins);
>>> +    if (strcmp(arch, "aarch64") == 0) {
>>> +        s = qtest_init("-machine ast2700-evb");
>>> +        qtest_add_data_func("/ast2700/gpio/input_pins",
>>> +                            s, test_2700_input_pins);
>>> +        qtest_add_data_func("/ast2700/gpio/out_pins", s, 
>>> test_2700_output_pins);
>>> +    } else {
>>> +        s = qtest_init("-machine ast2600-evb");
>>> +        qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
>>> +                            test_set_colocated_pins);
>>> +        qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
>>> +                            test_set_input_pins);
>>> +    }
>>
>> ... so the more I look at this, the more I think your new test should 
>> reside in a separate file that only gets executed for aarch64, while this 
>> file here should stay for arm 32-bit. Or is there a real compelling reason 
>> for putting your code in this file here?
> 
> Because it is related to the Aspeed GPIO controllers. Unless we
> want to introduce a new set of test files for 64-bit Aspeed SoC
> controllers ? why not.
> 
> I am ok with both options. Option 1 is simpler to implement, but
> there may be reasons to separate the tests based on architecture,
> although I'm not aware of any at the moment. Would you rather prefer
> option 2 ? How should we name the test file ?

Since all arm 32-bit tests are currently completely separate from the 
aarch64 tests, I think a separate file would be better, indeed.
Simply call it ast2700-gpio-test.c ?

  Thomas
Jamin Lin Oct. 1, 2024, 1:15 a.m. UTC | #4 
Hi Thomas, Cedric

> Subject: Re: [PATCH v6 8/8] hw/gpio/aspeed: Add test case for AST2700
> 
> On 30/09/2024 18.48, Cédric Le Goater wrote:
> > On 9/30/24 18:36, Thomas Huth wrote:
> >> On 30/09/2024 10.52, Jamin Lin wrote:
> >>> Add test case to test GPIO output and input pins from A0 to D7 for
> AST2700.
> >>>
> >>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >>> ---
> >>>   tests/qtest/aspeed_gpio-test.c | 77
> >>> ++++++++++++++++++++++++++++++++--
> >>>   tests/qtest/meson.build        |  3 ++
> >>>   2 files changed, 76 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tests/qtest/aspeed_gpio-test.c
> >>> b/tests/qtest/aspeed_gpio-test.c index d38f51d719..03b3b1c2b2 100644
> >>> --- a/tests/qtest/aspeed_gpio-test.c
> >>> +++ b/tests/qtest/aspeed_gpio-test.c
> >>> @@ -33,6 +33,10 @@
> >>>   #define GPIO_ABCD_DATA_VALUE 0x000
> >>>   #define GPIO_ABCD_DIRECTION  0x004
> >>> +/* AST2700 */
> >>> +#define AST2700_GPIO_BASE 0x14C0B000 #define GPIOA0_CONTROL
> 0x180
> >>> +
> >>>   static void test_set_colocated_pins(const void *data)
> >>>   {
> >>>       QTestState *s = (QTestState *)data; @@ -72,17 +76,82 @@ static
> >>> void test_set_input_pins(const void *data)
> >>>       g_assert_cmphex(value, ==, 0xffffffff);
> >>>   }
> >>> +static void test_2700_output_pins(const void *data) {
> >>> +    QTestState *s = (QTestState *)data;
> >>> +    uint32_t offset = 0;
> >>> +    uint32_t value = 0;
> >>> +    uint32_t pin = 0;
> >>> +
> >>> +    for (char c = 'A'; c <= 'D'; c++) {
> >>> +        for (int i = 0; i < 8; i++) {
> >>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL +
> (pin *
> >>> +4);
> >>> +
> >>> +            /* output direction and output hi */
> >>> +            qtest_writel(s, offset, 0x00000003);
> >>> +            value = qtest_readl(s, offset);
> >>> +            g_assert_cmphex(value, ==, 0x00000003);
> >>> +
> >>> +            /* output direction and output low */
> >>> +            qtest_writel(s, offset, 0x00000002);
> >>> +            value = qtest_readl(s, offset);
> >>> +            g_assert_cmphex(value, ==, 0x00000002);
> >>> +            pin++;
> >>> +        }
> >>> +    }
> >>> +}
> >>> +
> >>> +static void test_2700_input_pins(const void *data) {
> >>> +    QTestState *s = (QTestState *)data;
> >>> +    char name[16];
> >>> +    uint32_t offset = 0;
> >>> +    uint32_t value = 0;
> >>> +    uint32_t pin = 0;
> >>> +
> >>> +    for (char c = 'A'; c <= 'D'; c++) {
> >>> +        for (int i = 0; i < 8; i++) {
> >>> +            sprintf(name, "gpio%c%d", c, i);
> >>> +            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL +
> (pin *
> >>> +4);
> >>> +            /* input direction */
> >>> +            qtest_writel(s, offset, 0);
> >>> +
> >>> +            /* set input */
> >>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name,
> true);
> >>> +            value = qtest_readl(s, offset);
> >>> +            g_assert_cmphex(value, ==, 0x00002000);
> >>> +
> >>> +            /* clear input */
> >>> +            qtest_qom_set_bool(s, "/machine/soc/gpio", name,
> >>> +false);
> >>> +            value = qtest_readl(s, offset);
> >>> +            g_assert_cmphex(value, ==, 0);
> >>> +            pin++;
> >>> +        }
> >>> +    }
> >>> +}
> >>
> >> As far as I can see, there is nothing in these two functions that
> >> requires any of the other code in this file ...
> >>
> >>> +
> >>>   int main(int argc, char **argv)
> >>>   {
> >>> +    const char *arch = qtest_get_arch();
> >>>       QTestState *s;
> >>>       int r;
> >>>       g_test_init(&argc, &argv, NULL);
> >>> -    s = qtest_init("-machine ast2600-evb");
> >>> -    qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
> >>> -                        test_set_colocated_pins);
> >>> -    qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
> >>> test_set_input_pins);
> >>> +    if (strcmp(arch, "aarch64") == 0) {
> >>> +        s = qtest_init("-machine ast2700-evb");
> >>> +        qtest_add_data_func("/ast2700/gpio/input_pins",
> >>> +                            s, test_2700_input_pins);
> >>> +        qtest_add_data_func("/ast2700/gpio/out_pins", s,
> >>> test_2700_output_pins);
> >>> +    } else {
> >>> +        s = qtest_init("-machine ast2600-evb");
> >>> +        qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
> >>> +                            test_set_colocated_pins);
> >>> +        qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
> >>> +                            test_set_input_pins);
> >>> +    }
> >>
> >> ... so the more I look at this, the more I think your new test should
> >> reside in a separate file that only gets executed for aarch64, while
> >> this file here should stay for arm 32-bit. Or is there a real
> >> compelling reason for putting your code in this file here?
> >
> > Because it is related to the Aspeed GPIO controllers. Unless we want
> > to introduce a new set of test files for 64-bit Aspeed SoC controllers
> > ? why not.
> >
> > I am ok with both options. Option 1 is simpler to implement, but there
> > may be reasons to separate the tests based on architecture, although
> > I'm not aware of any at the moment. Would you rather prefer option 2 ?
> > How should we name the test file ?
> 
> Since all arm 32-bit tests are currently completely separate from the
> aarch64 tests, I think a separate file would be better, indeed.
> Simply call it ast2700-gpio-test.c ?
> 

Thanks for suggestion.
Will move it into new file, ast2700-gptio-test.c.

Jamin
>   Thomas
diff mbox series

Patch

diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index d38f51d719..03b3b1c2b2 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -33,6 +33,10 @@ 
 #define GPIO_ABCD_DATA_VALUE 0x000
 #define GPIO_ABCD_DIRECTION  0x004
 
+/* AST2700 */
+#define AST2700_GPIO_BASE 0x14C0B000
+#define GPIOA0_CONTROL 0x180
+
 static void test_set_colocated_pins(const void *data)
 {
     QTestState *s = (QTestState *)data;
@@ -72,17 +76,82 @@  static void test_set_input_pins(const void *data)
     g_assert_cmphex(value, ==, 0xffffffff);
 }
 
+static void test_2700_output_pins(const void *data)
+{
+    QTestState *s = (QTestState *)data;
+    uint32_t offset = 0;
+    uint32_t value = 0;
+    uint32_t pin = 0;
+
+    for (char c = 'A'; c <= 'D'; c++) {
+        for (int i = 0; i < 8; i++) {
+            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
+
+            /* output direction and output hi */
+            qtest_writel(s, offset, 0x00000003);
+            value = qtest_readl(s, offset);
+            g_assert_cmphex(value, ==, 0x00000003);
+
+            /* output direction and output low */
+            qtest_writel(s, offset, 0x00000002);
+            value = qtest_readl(s, offset);
+            g_assert_cmphex(value, ==, 0x00000002);
+            pin++;
+        }
+    }
+}
+
+static void test_2700_input_pins(const void *data)
+{
+    QTestState *s = (QTestState *)data;
+    char name[16];
+    uint32_t offset = 0;
+    uint32_t value = 0;
+    uint32_t pin = 0;
+
+    for (char c = 'A'; c <= 'D'; c++) {
+        for (int i = 0; i < 8; i++) {
+            sprintf(name, "gpio%c%d", c, i);
+            offset = AST2700_GPIO_BASE + GPIOA0_CONTROL + (pin * 4);
+            /* input direction */
+            qtest_writel(s, offset, 0);
+
+            /* set input */
+            qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
+            value = qtest_readl(s, offset);
+            g_assert_cmphex(value, ==, 0x00002000);
+
+            /* clear input */
+            qtest_qom_set_bool(s, "/machine/soc/gpio", name, false);
+            value = qtest_readl(s, offset);
+            g_assert_cmphex(value, ==, 0);
+            pin++;
+        }
+    }
+}
+
+
 int main(int argc, char **argv)
 {
+    const char *arch = qtest_get_arch();
     QTestState *s;
     int r;
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_init("-machine ast2600-evb");
-    qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
-                        test_set_colocated_pins);
-    qtest_add_data_func("/ast2600/gpio/set_input_pins", s, test_set_input_pins);
+    if (strcmp(arch, "aarch64") == 0) {
+        s = qtest_init("-machine ast2700-evb");
+        qtest_add_data_func("/ast2700/gpio/input_pins",
+                            s, test_2700_input_pins);
+        qtest_add_data_func("/ast2700/gpio/out_pins", s, test_2700_output_pins);
+    } else {
+        s = qtest_init("-machine ast2600-evb");
+        qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
+                            test_set_colocated_pins);
+        qtest_add_data_func("/ast2600/gpio/set_input_pins", s,
+                            test_set_input_pins);
+    }
+
     r = g_test_run();
     qtest_quit(s);
 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 310865e49c..292980e3ad 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -209,6 +209,8 @@  qtests_aspeed = \
   ['aspeed_hace-test',
    'aspeed_smc-test',
    'aspeed_gpio-test']
+qtests_aspeed64 = \
+  ['aspeed_gpio-test']
 
 qtests_stm32l4x5 = \
   ['stm32l4x5_exti-test',
@@ -247,6 +249,7 @@  qtests_aarch64 = \
   (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-i2c-test'] : []) +  \
   (config_all_accel.has_key('CONFIG_TCG') and                                            \
    config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
   ['arm-cpu-features',
    'numa-test',
    'boot-serial-test',