Hi Yuquan,
I am mostly happy with the below, but there are a couple of things I'd
like to change (comments inline).
On Fri, Sep 22, 2023 at 21:41:15 +0800, Yuquan Wang wrote:
> As sbsa-ref does not have DRAM below 4G, it cannot utilize EHCI
> that only has 32-bit DMA capablity and sbsa-ref board uses xhci
> to replace ehci.
>
> This updates DSDT to match the platform xhci controller with two
> usb ports changed from type A to type C.
>
> This also registers the non-discoverable XHCI for sbsa-ref.
Ah, I failed to notice this on previous review. Since we didn't map
EHCI at all (probably because it was non-functioning), this
registering becomes a separate change, which should be in a separate
patch.
This is just mechanics, so I'll rework it myself.
> Signed-off-by: Yuquan Wang <wangyuquan1...@phytium.com.cn>
> ---
> Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 4 +-
> Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 6 +--
> .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 4 +-
> .../SbsaQemuPlatformDxe.inf | 2 +
> .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 41 +++++++++++++++----
> Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 15 +++----
> 6 files changed, 49 insertions(+), 23 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> index aab2894e6455..913d1d75ef29 100644
> --- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> +++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> @@ -32,8 +32,8 @@ [PcdsFixedAtBuild.common]
> # Non discoverable devices Pcds
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase|0|UINT64|0x00000001
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize|0x10000|UINT32|0x00000002
> -
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase|0|UINT64|0x00000003
> -
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x10000|UINT32|0x00000004
> +
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciBase|0|UINT64|0x00000003
> +
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciSize|0x10000|UINT32|0x00000004
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x10000000000|UINT64|0x00000005
>
> # PCDs complementing PCIe layout pulled into ACPI tables
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index be406144c242..948e42326c33 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -429,11 +429,11 @@ [PcdsFixedAtBuild.common]
> # Initial Device Tree Location
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x10000000000
>
> - # Non discoverable devices (AHCI,EHCI)
> + # Non discoverable devices (AHCI,XHCI)
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase|0x60100000
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize|0x00010000
> - gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase|0x60110000
> - gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x00010000
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciBase|0x60110000
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciSize|0x00010000
>
> # PL011 - Serial Terminal
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x60000000
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> index 97021f7971c7..abd10b6c3098 100644
> --- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> @@ -69,8 +69,8 @@ [FixedPcd]
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize
> - gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase
> - gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciBase
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciSize
>
> [Pcd]
> gArmTokenSpaceGuid.PcdGicDistributorBase
> diff --git
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> index 0e3b11d60426..19534b7a274a 100644
> ---
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> +++
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> @@ -37,6 +37,8 @@ [LibraryClasses]
> [Pcd]
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciBase
> + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformXhciSize
>
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor
> gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
> diff --git
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> index ddcca2b7243c..4a04dbeb07ae 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> @@ -26,8 +26,9 @@ InitializeSbsaQemuPlatformDxe (
> )
> {
> EFI_STATUS Status;
> - UINTN Size;
> - VOID* Base;
> + UINTN AhciSize, XhciSize;
> + VOID* AhciBase;
> + VOID* XhciBase;
There is no need for AhciSize/Base and XhciSize/Base to be live
simultaneously in the code. And the patch becomes much cleaner if we
cimply reuse the existing Base/Size variables.
Again, that is purely mechanical, so I have reworked it myself.
Reviewed-by: Leif Lindholm <quic_llind...@quicinc.com>
Pushed as cbe0e979e45a..d03a60523a60.
Thanks!
/
Leif
> UINTN Arg0;
> UINTN Arg1;
> UINTN SmcResult;
> @@ -35,13 +36,13 @@ InitializeSbsaQemuPlatformDxe (
>
> DEBUG ((DEBUG_INFO, "%a: InitializeSbsaQemuPlatformDxe called\n",
> __FUNCTION__));
>
> - Base = (VOID*)(UINTN)PcdGet64 (PcdPlatformAhciBase);
> - ASSERT (Base != NULL);
> - Size = (UINTN)PcdGet32 (PcdPlatformAhciSize);
> - ASSERT (Size != 0);
> + AhciBase = (VOID*)(UINTN)PcdGet64 (PcdPlatformAhciBase);
> + ASSERT (AhciBase != NULL);
> + AhciSize = (UINTN)PcdGet32 (PcdPlatformAhciSize);
> + ASSERT (AhciSize != 0);
>
> DEBUG ((DEBUG_INFO, "%a: Got platform AHCI %llx %u\n",
> - __FUNCTION__, Base, Size));
> + __FUNCTION__, AhciBase, AhciSize));
>
> Status = RegisterNonDiscoverableMmioDevice (
> NonDiscoverableDeviceTypeAhci,
> @@ -49,11 +50,33 @@ InitializeSbsaQemuPlatformDxe (
> NULL,
> NULL,
> 1,
> - Base, Size);
> + AhciBase, AhciSize);
>
> if (EFI_ERROR(Status)) {
> DEBUG ((DEBUG_ERROR, "%a: NonDiscoverable: Cannot install AHCI device
> @%p (Staus == %r)\n",
> - __FUNCTION__, Base, Status));
> + __FUNCTION__, AhciBase, Status));
> + return Status;
> + }
> +
> + XhciBase = (VOID*)(UINTN)PcdGet64 (PcdPlatformXhciBase);
> + ASSERT (XhciBase != NULL);
> + XhciSize = (UINTN)PcdGet32 (PcdPlatformXhciSize);
> + ASSERT (XhciSize != 0);
> +
> + DEBUG ((DEBUG_INFO, "%a: Got platform XHCI %llx %u\n",
> + __FUNCTION__, XhciBase, XhciSize));
> +
> + Status = RegisterNonDiscoverableMmioDevice (
> + NonDiscoverableDeviceTypeXhci,
> + NonDiscoverableDeviceDmaTypeCoherent,
> + NULL,
> + NULL,
> + 1,
> + XhciBase, XhciSize);
> +
> + if (EFI_ERROR(Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: NonDiscoverable: Cannot install XHCI device
> @%p (Staus == %r)\n",
> + __FUNCTION__, XhciBase, Status));
> return Status;
> }
>
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> index e50772fcf76d..543b5782580a 100644
> --- a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> @@ -68,18 +68,19 @@ DefinitionBlock ("DsdtTable.aml", "DSDT",
> }
> }
>
> - // USB EHCI Host Controller
> + // USB XHCI Host Controller
> Device (USB0) {
> - Name (_HID, "LNRO0D20")
> - Name (_CID, "PNP0D20")
> + Name (_HID, "PNP0D10") // _HID: Hardware ID
> + Name (_UID, 0x00) // _UID: Unique ID
> + Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute
> Method (_STA) {
> Return (0xF)
> }
> Method (_CRS, 0x0, Serialized) {
> Name (RBUF, ResourceTemplate() {
> Memory32Fixed (ReadWrite,
> - FixedPcdGet32 (PcdPlatformEhciBase),
> - FixedPcdGet32 (PcdPlatformEhciSize))
> + FixedPcdGet32 (PcdPlatformXhciBase),
> + FixedPcdGet32 (PcdPlatformXhciSize))
> Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> 43 }
> })
> Return (RBUF)
> @@ -146,7 +147,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT",
> Name (_ADR, 0x00000003)
> Name (_UPC, Package() {
> 0xFF, // Port is connectable
> - 0x00, // Port connector is A
> + 0x09, // Type C connector - USB2 and SS with
> Switch
> 0x00000000,
> 0x00000000
> })
> @@ -165,7 +166,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT",
> Name (_ADR, 0x00000004)
> Name (_UPC, Package() {
> 0xFF, // Port is connectable
> - 0x00, // Port connector is A
> + 0x09, // Type C connector - USB2 and SS with
> Switch
> 0x00000000,
> 0x00000000
> })
> --
> 2.34.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109118): https://edk2.groups.io/g/devel/message/109118
Mute This Topic: https://groups.io/mt/101521163/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
