From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id E3359D80470 for ; Wed, 27 Sep 2023 10:30:32 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=vmp7ov7Vg5OOOO0IE19QlN+4UIkK+APKrcIUgNGjvaM=; c=relaxed/simple; d=groups.io; h=Date:From:To:CC:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1695810631; v=1; b=GvWFHWb7IXuR+w6LvmoyLow0kH9f5z0szX0YIRyyeL+52aj3vEbRx9pCW16bhYAzAplnkxs7 ai25gkuS8+H8piy+2YMNcr52HIJ+oqRPulzRniVHKKvFWNT0n9lFw+nUCt+gFJ7Hb80KhVgQYau BlLIxhxFKUs9AHvvLGapruBg= X-Received: by 127.0.0.2 with SMTP id KH8KYY7687511xOkxjubirR5; Wed, 27 Sep 2023 03:30:31 -0700 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web10.14121.1695810630957661558 for ; Wed, 27 Sep 2023 03:30:31 -0700 X-Received: from pps.filterd (m0279868.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 38R7BonU008405; Wed, 27 Sep 2023 10:30:30 GMT X-Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3tcfp6gbad-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 27 Sep 2023 10:30:29 +0000 X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 38RAUSm0023091 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 27 Sep 2023 10:30:28 GMT X-Received: from qc-i7.hemma.eciton.net (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.36; Wed, 27 Sep 2023 03:30:27 -0700 Date: Wed, 27 Sep 2023 11:30:23 +0100 From: "Leif Lindholm" To: , CC: , Subject: Re: [edk2-devel] [PATCH edk2-platforms v3 1/1] Platform/SbsaQemu: add XHCI support and replace EHCI Message-ID: References: <20230922134115.326726-1-wangyuquan1236@phytium.com.cn> <20230922134115.326726-2-wangyuquan1236@phytium.com.cn> MIME-Version: 1.0 In-Reply-To: <20230922134115.326726-2-wangyuquan1236@phytium.com.cn> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-ORIG-GUID: Mx_wCBzlisXEkVSepvIz1mxkiVKQYvrG X-Proofpoint-GUID: Mx_wCBzlisXEkVSepvIz1mxkiVKQYvrG Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: jjeW255hGc69I1L1MNGZsJ8bx7686176AA= Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=GvWFHWb7; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.com (policy=none) 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 > --- > 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 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-