[PATCH v4 0/1] use XHCI to replace EHCI

[PATCH v4 0/1] use XHCI to replace EHCI

[Yuquan Wang]

Please review the change.
 - sbsa-ref: Replace EHCI with XHCI on sysbus.
This version updates relevant Kconfig and sbsa.rst file.

Yuquan Wang (1):
  hw/arm/sbsa-ref: use XHCI to replace EHCI

 docs/system/arm/sbsa.rst |  2 +-
 hw/arm/Kconfig           |  2 +-
 hw/arm/sbsa-ref.c        | 21 ++++++++++++---------
 3 files changed, 14 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Yuquan Wang]

The current sbsa-ref cannot use EHCI controller which is only
able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
Hence, this uses system bus XHCI to provide a usb controller with
64-bit DMA capablity instead of EHCI.

Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
---
 docs/system/arm/sbsa.rst |  2 +-
 hw/arm/Kconfig           |  2 +-
 hw/arm/sbsa-ref.c        | 21 ++++++++++++---------
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst
index 016776aed8..1751ba0d21 100644
--- a/docs/system/arm/sbsa.rst
+++ b/docs/system/arm/sbsa.rst
@@ -24,7 +24,7 @@ The sbsa-ref board supports:
   - A configurable number of AArch64 CPUs
   - GIC version 3
   - System bus AHCI controller
-  - System bus EHCI controller
+  - System bus XHCI controller
   - CDROM and hard disc on AHCI bus
   - E1000E ethernet card on PCIe bus
   - Bochs display adapter on PCIe bus
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index acc4371a4a..ab884ad319 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -266,7 +266,7 @@ config SBSA_REF
     select PL011 # UART
     select PL031 # RTC
     select PL061 # GPIO
-    select USB_EHCI_SYSBUS
+    select USB_XHCI_SYSBUS
     select WDT_SBSA
 
 config SABRELITE
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index de21200ff9..4fb65704d4 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -40,6 +40,7 @@
 #include "hw/pci-host/gpex.h"
 #include "hw/qdev-properties.h"
 #include "hw/usb.h"
+#include "hw/usb/xhci.h"
 #include "hw/char/pl011.h"
 #include "hw/watchdog/sbsa_gwdt.h"
 #include "net/net.h"
@@ -82,7 +83,7 @@ enum {
     SBSA_SECURE_UART_MM,
     SBSA_SECURE_MEM,
     SBSA_AHCI,
-    SBSA_EHCI,
+    SBSA_XHCI,
 };
 
 struct SBSAMachineState {
@@ -119,7 +120,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
     [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
     /* Space here reserved for more SMMUs */
     [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
-    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
+    [SBSA_XHCI] =               { 0x60110000, 0x00010000 },
     /* Space here reserved for other devices */
     [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
     /* 32-bit address PCIE MMIO space */
@@ -139,7 +140,7 @@ static const int sbsa_ref_irqmap[] = {
     [SBSA_SECURE_UART] = 8,
     [SBSA_SECURE_UART_MM] = 9,
     [SBSA_AHCI] = 10,
-    [SBSA_EHCI] = 11,
+    [SBSA_XHCI] = 11,
     [SBSA_SMMU] = 12, /* ... to 15 */
     [SBSA_GWDT_WS0] = 16,
 };
@@ -575,13 +576,15 @@ static void create_ahci(const SBSAMachineState *sms)
     }
 }
 
-static void create_ehci(const SBSAMachineState *sms)
+static void create_xhci(const SBSAMachineState *sms)
 {
-    hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base;
-    int irq = sbsa_ref_irqmap[SBSA_EHCI];
+    hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;
+    int irq = sbsa_ref_irqmap[SBSA_XHCI];
+    DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);
 
-    sysbus_create_simple("platform-ehci-usb", base,
-                         qdev_get_gpio_in(sms->gic, irq));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(sms->gic, irq));
 }
 
 static void create_smmu(const SBSAMachineState *sms, PCIBus *bus)
@@ -803,7 +806,7 @@ static void sbsa_ref_init(MachineState *machine)
 
     create_ahci(sms);
 
-    create_ehci(sms);
+    create_xhci(sms);
 
     create_pcie(sms);
 
-- 
2.34.1

Re: [PATCH v4 0/1] use XHCI to replace EHCI

[wangyuquan1236]

On Wed, 7 Jun 2023 10:33:13 +0800, Yuquan Wang wrote:



>



> Please review the change.



>  - sbsa-ref: Replace EHCI with XHCI on sysbus.



> This version updates relevant Kconfig and sbsa.rst file.



>



> Yuquan Wang (1):



>   hw/arm/sbsa-ref: use XHCI to replace EHCI



>



>  docs/system/arm/sbsa.rst |  2 +-



>  hw/arm/Kconfig           |  2 +-



>  hw/arm/sbsa-ref.c        | 21 ++++++++++++---------



>  3 files changed, 14 insertions(+), 11 deletions(-)



>



> --



> 2.34.1





Below is the link of corresponding edk2 firmware patchset



for qemu side testing:



https://edk2.groups.io/g/devel/message/105726





Many thanks



Yuquan

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Chen Baozi]

> On Jun 7, 2023, at 10:33, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> 
> The current sbsa-ref cannot use EHCI controller which is only
> able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
> Hence, this uses system bus XHCI to provide a usb controller with
> 64-bit DMA capablity instead of EHCI.
> 
> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>

Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn <mailto:chenbaozi@phytium.com.cn>>

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Marcin Juszkiewicz]

W dniu 7.06.2023 o 04:33, Yuquan Wang pisze:
> The current sbsa-ref cannot use EHCI controller which is only
> able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
> Hence, this uses system bus XHCI to provide a usb controller with
> 64-bit DMA capablity instead of EHCI.
> 
> Signed-off-by: Yuquan Wang<wangyuquan1236@phytium.com.cn>

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

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Peter Maydell]

On Wed, 7 Jun 2023 at 08:25, Chen Baozi <chenbaozi@phytium.com.cn> wrote:
>
>
> > On Jun 7, 2023, at 10:33, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> >
> > The current sbsa-ref cannot use EHCI controller which is only
> > able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
> > Hence, this uses system bus XHCI to provide a usb controller with
> > 64-bit DMA capablity instead of EHCI.
> >
> > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn <mailto:chenbaozi@phytium.com.cn>>

Hi; why have you added yourself as a Signed-off-by: here?
Did you do the work jointly with Yuquan? Or did you intend
to provide some other tag (like a reviewed-by or a tested-by?)

thanks
-- PMM

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Peter Maydell]

On Wed, 7 Jun 2023 at 03:34, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>
> The current sbsa-ref cannot use EHCI controller which is only
> able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
> Hence, this uses system bus XHCI to provide a usb controller with
> 64-bit DMA capablity instead of EHCI.

"capability"

> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>

The change itself looks good. We could probably mention in
the commit message that existing firmware/kernel images
still work (with no USB support) with this change.

Is this the sort of change that we should increase the
machine-version-minor for ? The comment says "updated
when features are added that don't break fw compatibility"
and this sounds like one of those.

Leif, do you think we should bump the minor version here?

thanks
-- PMM

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Chen Baozi]

> On Jun 19, 2023, at 18:28, Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Wed, 7 Jun 2023 at 08:25, Chen Baozi <chenbaozi@phytium.com.cn> wrote:
>> 
>> 
>>> On Jun 7, 2023, at 10:33, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>>> 
>>> The current sbsa-ref cannot use EHCI controller which is only
>>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
>>> Hence, this uses system bus XHCI to provide a usb controller with
>>> 64-bit DMA capablity instead of EHCI.
>>> 
>>> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
>> 
>> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn <mailto:chenbaozi@phytium.com.cn>>
> 
> Hi; why have you added yourself as a Signed-off-by: here?
> Did you do the work jointly with Yuquan? Or did you intend
> to provide some other tag (like a reviewed-by or a tested-by?)

Oh yes, we are. And I asked him to send this patch upstream after we have worked it
out internally first.

Cheers,

Baozi.

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Leif Lindholm]

Hi Peter,

On 2023-06-19 13:47, Peter Maydell wrote:
> On Wed, 7 Jun 2023 at 03:34, Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
>>
>> The current sbsa-ref cannot use EHCI controller which is only
>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM below 4GB.
>> Hence, this uses system bus XHCI to provide a usb controller with
>> 64-bit DMA capablity instead of EHCI.
> 
> "capability"
> 
>> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
> 
> The change itself looks good. We could probably mention in
> the commit message that existing firmware/kernel images
> still work (with no USB support) with this change.
> 
> Is this the sort of change that we should increase the
> machine-version-minor for ? The comment says "updated
> when features are added that don't break fw compatibility"
> and this sounds like one of those.
> 
> Leif, do you think we should bump the minor version here?

I think that makes sense, yes.

/
     Leif

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Yuquan Wang]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

On 2023-06-21 01:24,  Leif wrote:

> Leif, do you think we should bump the minor version here?
 
I think that makes sense, yes.
 
/
     Leif

Thanks for everyone's guidance.
There is a new confusion: Which minor version should I bump to (2 or 3) ?
As I found that Marcin’s latest patch (add ITS support in SBSA GIC
https://lists.nongnu.org/archive/html/qemu-arm/2023-06/msg00709.html )
increased the minor version to 2. 


Many thanks
Yuquan

[-- Attachment #2: Type: text/html, Size: 2187 bytes --]

Re: [PATCH v4 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI

[Marcin Juszkiewicz]

W dniu 21.06.2023 o 05:46, Yuquan Wang pisze:
> On 2023-06-21 01:24, Leif wrote:

> > > Leif, do you think we should bump the minor version here?
> > I think that makes sense, yes.

> There is a new confusion: Which minor version should I bump to (2 or 3) ?
> As I found that Marcin’s latest patch (add ITS support in SBSA GIC
> <https://lists.nongnu.org/archive/html/qemu-arm/2023-06/msg00709.html> )
> increased the minor version to 2.

Please bump platform version to 0.3 one. ITS being 0.2 is already in our 
plans. Not that numbers matter for those components which are provided 
via DeviceTree.