[PATCH 0/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[PATCH 0/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Yuquan Wang]

As the default xhci_sysbus just supports only one usb slot, it can not
meet the working requirement of this bord. Therefore, we extend the 
slots of xhci to 64.

Yuquan Wang (1):
  hw/arm/sbsa-ref: set 'slots' property of xhci

 hw/arm/sbsa-ref.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.34.1

[PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Yuquan Wang]

This extends the slots of xhci to 64, since the default xhci_sysbus
just supports one slot.

Signed-off-by: Wang Yuquan <wangyuquan1236@phytium.com.cn>
Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
---
 hw/arm/sbsa-ref.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 64e1cbce17..bc89eb4806 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -611,6 +611,7 @@ static void create_xhci(const SBSAMachineState *sms)
     hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;
     int irq = sbsa_ref_irqmap[SBSA_XHCI];
     DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);
+    qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);
 
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-- 
2.34.1

Re: [PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Richard Henderson]

On 7/10/23 07:37, Yuquan Wang wrote:
> This extends the slots of xhci to 64, since the default xhci_sysbus
> just supports one slot.
> 
> Signed-off-by: Wang Yuquan <wangyuquan1236@phytium.com.cn>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
> ---
>   hw/arm/sbsa-ref.c | 1 +
>   1 file changed, 1 insertion(+)


hw/usb/hcd-xhci-nec.c:    DEFINE_PROP_UINT32("slots", XHCINecState, slots, XHCI_MAXSLOTS),
hw/usb/hcd-xhci-sysbus.c:    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, 
XHCI_MAXSLOTS),

The default is XCHI_MAXSLOTS, not 1.  So I can't see why you'd need this.

r~

> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 64e1cbce17..bc89eb4806 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -611,6 +611,7 @@ static void create_xhci(const SBSAMachineState *sms)
>       hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;
>       int irq = sbsa_ref_irqmap[SBSA_XHCI];
>       DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);
> +    qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);
>   
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);

Re: [PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Marcin Juszkiewicz]

W dniu 10.07.2023 o 09:28, Richard Henderson pisze:
> 
> hw/usb/hcd-xhci-nec.c:    DEFINE_PROP_UINT32("slots", XHCINecState, 
> slots, XHCI_MAXSLOTS),
> hw/usb/hcd-xhci-sysbus.c:    DEFINE_PROP_UINT32("slots", 
> XHCISysbusState, xhci.numslots, XHCI_MAXSLOTS),
> 
> The default is XCHI_MAXSLOTS, not 1.  So I can't see why you'd need this.

There are two systems using XHCI: i386/microvm and arm/sbsa. First
one sets amount of slots already.

Without this patch Linux complains that there is only one port and
refuses to connect second usb device:

xhci-hcd PNP0D10:00: Error while assigning device slot ID: No Slots Available Error
xhci-hcd PNP0D10:00: Max number of devices this xHCI host supports is 1.
usb usb1-port2: couldn't allocate usb_device

So it looks like default being XHCI_MAXSLOTS is not applied somewhere.

Re: [PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Marcin Juszkiewicz]

W dniu 10.07.2023 o 08:37, Yuquan Wang pisze:
> This extends the slots of xhci to 64, since the default xhci_sysbus
> just supports one slot.
> 
> Signed-off-by: Wang Yuquan <wangyuquan1236@phytium.com.cn>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
> ---
>   hw/arm/sbsa-ref.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 64e1cbce17..bc89eb4806 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -611,6 +611,7 @@ static void create_xhci(const SBSAMachineState *sms)
>       hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;
>       int irq = sbsa_ref_irqmap[SBSA_XHCI];
>       DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);
> +    qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);

Looks like tab instead of spaces.

>   
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);

Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Reviewed-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>


Before:

xhci-hcd PNP0D10:00: Error while assigning device slot ID: No Slots Available Error
xhci-hcd PNP0D10:00: Max number of devices this xHCI host supports is 1.
usb usb1-port2: couldn't allocate usb_device


After:

input: QEMU QEMU USB Keyboard as /devices/platform/PNP0D10:00/usb1/1-1/1-1:1.0/0003:0627:0001.0001/input/input0
hid-generic 0003:0627:0001.0001: input,hidraw0: USB HID v1.11 Keyboard [QEMU QEMU USB Keyboard] on usb-PNP0D10:00-1/input0
input: QEMU QEMU USB Tablet as /devices/platform/PNP0D10:00/usb1/1-2/1-2:1.0/0003:0627:0001.0002/input/input1
hid-generic 0003:0627:0001.0002: input,hidraw1: USB HID v0.01 Mouse [QEMU QEMU USB Tablet] on usb-PNP0D10:00-2/input0

Re: [PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Richard Henderson]

On 7/10/23 08:33, Marcin Juszkiewicz wrote:
> W dniu 10.07.2023 o 09:28, Richard Henderson pisze:
>>
>> hw/usb/hcd-xhci-nec.c:    DEFINE_PROP_UINT32("slots", XHCINecState, slots, XHCI_MAXSLOTS),
>> hw/usb/hcd-xhci-sysbus.c:    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, 
>> XHCI_MAXSLOTS),
>>
>> The default is XCHI_MAXSLOTS, not 1.  So I can't see why you'd need this.
> 
> There are two systems using XHCI: i386/microvm and arm/sbsa. First
> one sets amount of slots already.
> 
> Without this patch Linux complains that there is only one port and
> refuses to connect second usb device:
> 
> xhci-hcd PNP0D10:00: Error while assigning device slot ID: No Slots Available Error
> xhci-hcd PNP0D10:00: Max number of devices this xHCI host supports is 1.
> usb usb1-port2: couldn't allocate usb_device
> 
> So it looks like default being XHCI_MAXSLOTS is not applied somewhere.

It looks like the problem is that we're performing init and realize in one step.  The 
defaults would be applied later, along with other properties from the command-line.

As this is the last full day before softfreeze, and we have many other instances of the 
same pattern, I'll give this an

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 1/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Philippe Mathieu-Daudé]

On 10/7/23 09:33, Marcin Juszkiewicz wrote:
> W dniu 10.07.2023 o 09:28, Richard Henderson pisze:
>>
>> hw/usb/hcd-xhci-nec.c:    DEFINE_PROP_UINT32("slots", XHCINecState, 
>> slots, XHCI_MAXSLOTS),
>> hw/usb/hcd-xhci-sysbus.c:    DEFINE_PROP_UINT32("slots", 
>> XHCISysbusState, xhci.numslots, XHCI_MAXSLOTS),
>>
>> The default is XCHI_MAXSLOTS, not 1.  So I can't see why you'd need this.
> 
> There are two systems using XHCI: i386/microvm and arm/sbsa. First
> one sets amount of slots already.
> 
> Without this patch Linux complains that there is only one port and
> refuses to connect second usb device:
> 
> xhci-hcd PNP0D10:00: Error while assigning device slot ID: No Slots 
> Available Error
> xhci-hcd PNP0D10:00: Max number of devices this xHCI host supports is 1.
> usb usb1-port2: couldn't allocate usb_device
> 
> So it looks like default being XHCI_MAXSLOTS is not applied somewhere.

Or something sets it to 1 elsewhere.

At a glance xhci_sysbus_class_init() doesn't call
device_class_set_parent_FOO().

Re: [PATCH 0/1] hw/arm/sbsa-ref: set 'slots' property of xhci

[Peter Maydell]

On Mon, 10 Jul 2023 at 07:38, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>
> As the default xhci_sysbus just supports only one usb slot, it can not
> meet the working requirement of this bord. Therefore, we extend the
> slots of xhci to 64.
>
> Yuquan Wang (1):
>   hw/arm/sbsa-ref: set 'slots' property of xhci
>
>  hw/arm/sbsa-ref.c | 1 +
>  1 file changed, 1 insertion(+)



Applied to target-arm.next, thanks.

-- PMM