On 04/10/2016 15:20, Thomas Huth wrote:
> On 28.09.2016 20:51, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lviv...@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...
No, but this is the only test using PCI. OHCI test tests only if we can
plug and hotplug a device.
>
>> ---
>> 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.
yes.
Thanks,
Laurent
