Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 6/6] tests: enable ohci/uhci/xhci tests on PPC64

[Thomas Huth <thuth@redhat.com>]

On 28.09.2016 20:51, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Meta-question: Do we need to test UHCI on ppc at all? AFAIK most (all?)
ppc-based machines were rather based on OHCI instead...

> ---
>  tests/Makefile.include    |  8 +++++++-
>  tests/libqos/usb.c        |  2 +-
>  tests/usb-hcd-uhci-test.c | 24 ++++++++++++++++--------
>  3 files changed, 24 insertions(+), 10 deletions(-)
[...]
> diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
> index c24063e..4b951ce 100644
> --- a/tests/usb-hcd-uhci-test.c
> +++ b/tests/usb-hcd-uhci-test.c
> @@ -9,9 +9,13 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
> +#include "libqos/libqos.h"
>  #include "libqos/usb.h"
> +#include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "hw/usb/uhci-regs.h"
>  
> +static QOSState *qs;
>  
>  static void test_uhci_init(void)
>  {
> @@ -19,13 +23,10 @@ static void test_uhci_init(void)
>  
>  static void test_port(int port)
>  {
> -    QPCIBus *pcibus;
>      struct qhc uhci;
>  
>      g_assert(port > 0);
> -    pcibus = qpci_init_pc(NULL);
> -    g_assert(pcibus != NULL);
> -    qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
> +    qusb_pci_init_one(qs->pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
>      uhci_port_test(&uhci, port - 1, UHCI_PORT_CCS);
>  }
>  
> @@ -75,6 +76,7 @@ static void test_usb_storage_hotplug(void)
>  
>  int main(int argc, char **argv)
>  {
> +    const char *arch = qtest_get_arch();
>      int ret;
>  
>      g_test_init(&argc, &argv, NULL);
> @@ -84,11 +86,17 @@ int main(int argc, char **argv)
>      qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug);
>      qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
>  
> -    qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
> -                " -drive id=drive0,if=none,file=/dev/null,format=raw"
> -                " -device usb-tablet,bus=uhci.0,port=1");
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qs = qtest_pc_boot("-device piix3-usb-uhci,id=uhci,addr=1d.0"
> +                           " -drive id=drive0,if=none,file=/dev/null,format=raw"
> +                           " -device usb-tablet,bus=uhci.0,port=1");
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qs = qtest_spapr_boot("-device piix3-usb-uhci,id=uhci,addr=1d.0"
> +                           " -drive id=drive0,if=none,file=/dev/null,format=raw"
> +                           " -device usb-tablet,bus=uhci.0,port=1");
> +    }

The "-device ..." string looks the same for both machine types ... so
IMHO it would be somewhat nicer to define this only once and then use
the common string for both machine instead of specifying it here twice.

 Thomas