Re: [PATCH v5 08/10] hw/fsi: Added qtest

[Thomas Huth]

On 11/10/2023 17.13, Ninad Palsule wrote:
> Added basic qtests for FSI model.
>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> v3:
>   - Added new qtest as per Cedric's comment.
> V4:
>   - Remove MAINTAINER and documentation changes from this commit
> ---
>   tests/qtest/fsi-test.c  | 210 ++++++++++++++++++++++++++++++++++++++++
>   tests/qtest/meson.build |   2 +
>   2 files changed, 212 insertions(+)
>   create mode 100644 tests/qtest/fsi-test.c
>
> diff --git a/tests/qtest/fsi-test.c b/tests/qtest/fsi-test.c
> new file mode 100644
> index 0000000000..30bb7475c7
> --- /dev/null
> +++ b/tests/qtest/fsi-test.c
> @@ -0,0 +1,210 @@
...
> +int main(int argc, char **argv)
> +{
> +    int ret = -1;
> +    QTestState *s;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    s = qtest_init("-machine ast2600-evb ");
> +    if (s == NULL) {
> +        return -ENOMEM;

returning -ENOMEM here does not make too much sense ... and actually 
qtest_init() cannot return NULL. So please drop this if-statement.

> +    }
> +
> +    /* Tests for OPB/FSI0 */
> +    qtest_add_data_func("/fsi-test/test_fsi0_master_regs", s,
> +                        test_fsi0_master_regs);
> +
> +    qtest_add_data_func("/fsi-test/test_fsi0_getcfam_addr0", s,
> +                        test_fsi0_getcfam_addr0);
> +
> +    /* Tests for OPB/FSI1 */
> +    qtest_add_data_func("/fsi-test/test_fsi1_master_regs", s,
> +                        test_fsi1_master_regs);
> +
> +    qtest_add_data_func("/fsi-test/test_fsi1_getcfam_addr0", s,
> +                        test_fsi1_getcfam_addr0);
> +
> +    ret = g_test_run();
> +    qtest_quit(s);
> +
> +    return ret;
> +}
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index b071d400b3..5976081b44 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -207,6 +207,7 @@ qtests_arm = \
>     (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : 
> []) + \
>     (config_all_devices.has_key('CONFIG_VEXPRESS') ? ['test-arm-mptimer'] : []) 
> + \
>     (config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) + \
> +  (config_all_devices.has_key('CONFIG_FSI_APB2OPB_ASPEED') ? ['fsi-test'] : 
> []) + \
>     ['arm-cpu-features',
>      'boot-serial-test']
>   
> @@ -318,6 +319,7 @@ qtests = {
>     'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
>     'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
>     'netdev-socket': files('netdev-socket.c', '../unit/socket-helpers.c'),
> +  'fsi-test': files('fsi-test.c'),

I think this hunk here is not required - you only need to specify additional 
dependencies here, not the main file.

 Thomas