From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 567BE211CE857 for ; Thu, 14 Jun 2018 00:03:32 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jun 2018 00:03:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,222,1526367600"; d="scan'208";a="63980128" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga001.fm.intel.com with ESMTP; 14 Jun 2018 00:03:32 -0700 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 14 Jun 2018 00:03:32 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 14 Jun 2018 00:03:31 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.87]) by shsmsx102.ccr.corp.intel.com ([169.254.2.223]) with mapi id 14.03.0319.002; Thu, 14 Jun 2018 15:03:29 +0800 From: "Wu, Hao A" To: =?iso-8859-1?Q?Marvin_H=E4user?= , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" Thread-Topic: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration. Thread-Index: AQHUA1DQ/mf1XxE7gU6kNmXE15+v2aRfU+fQ Date: Thu, 14 Jun 2018 07:03:29 +0000 Message-ID: References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jun 2018 07:03:32 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Marvin, One thing to confirm with you. For your using case, do you need to hard-cod= e the IN/OUT endpoints for EHCI debug device? If not, I think we can just remove the hard-code endpoints settings in the current code. And use the endpoints returned from the USB Device Descriptor= . By doing so, I think we can avoid adding those 2 PCDs. Please let me know your thoughts on this. Thanks in advance. Best Regards, Hao Wu > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Marvin H=E4user > Sent: Thursday, June 14, 2018 3:58 AM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A > Subject: [edk2] [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: > Add endpoint configuration. >=20 > Currently, DebugCommunicationLibUsb uses the hardcoded endpoints 0x82 > and 0x01 to communicate with the EHCI Debug Device. These, however, > are not standardized and may vary across different hardware. > To solve this problem, two PCDs have been introduced to configure the > in and out endpoints of the EHCI Debug Device. These may be set to 0 > to retrieve the endpoints from the USB Device Descriptor directly. > To ensure maximum compatibility, the PCD defaults have been set to > the former hardcoded values. >=20 > V2: > - Store endpoint data in the USB Debug Port handle sturcture. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marvin Haeuser > --- >=20 > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica > tionLibUsb.c | 32 ++++++++++++++++++-- >=20 > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica > tionLibUsb.inf | 6 +++- > SourceLevelDebugPkg/SourceLevelDebugPkg.dec = | 12 > ++++++++ > 3 files changed, 47 insertions(+), 3 deletions(-) >=20 > diff --git > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.c > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.c > index d996f80f59e3..a9a9ea07a39b 100644 > --- > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.c > +++ > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.c > @@ -132,6 +132,14 @@ typedef struct _USB_DEBUG_PORT_HANDLE{ > // > UINT32 EhciMemoryBase; > // > + // The usb debug device In endpoint. > + // > + UINT8 InEndpoint; > + // > + // The usb debug device Out endpoint. > + // > + UINT8 OutEndpoint; > + // > // The Bulk In endpoint toggle bit. > // > UINT8 BulkInToggle; > @@ -603,6 +611,8 @@ InitializeUsbDebugHardware ( > UINT32 *UsbHCSParam; > UINT8 DebugPortNumber; > UINT8 Length; > + UINT8 InEndpoint; > + UINT8 OutEndpoint; >=20 > UsbDebugPortRegister =3D (USB_DEBUG_PORT_REGISTER *)((UINTN)Handle- > >UsbDebugPortMemoryBase + Handle->DebugPortOffset); > UsbHCSParam =3D (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04); > @@ -722,6 +732,24 @@ InitializeUsbDebugHardware ( > return RETURN_DEVICE_ERROR; > } >=20 > + // > + // Determine the usb debug device endpoints. > + // > + InEndpoint =3D PcdGet8 (PcdUsbDebugPortInEndpoint); > + > + if (InEndpoint =3D=3D 0) { > + InEndpoint =3D UsbDebugPortDescriptor.DebugInEndpoint; > + } > + > + OutEndpoint =3D PcdGet8 (PcdUsbDebugPortOutEndpoint); > + > + if (OutEndpoint =3D=3D 0) { > + OutEndpoint =3D UsbDebugPortDescriptor.DebugOutEndpoint; > + } > + > + Handle->InEndpoint =3D InEndpoint; > + Handle->OutEndpoint =3D OutEndpoint; > + > // > // enable the usb debug feature. > // > @@ -879,7 +907,7 @@ DebugPortWriteBuffer ( > Sent =3D (UINT8)(NumberOfBytes - Total); > } >=20 > - Status =3D UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sen= t, > OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle); > + Status =3D UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sen= t, > OUTPUT_PID, 0x7F, UsbDebugPortHandle->OutEndpoint, UsbDebugPortHandle- > >BulkOutToggle); >=20 > if (RETURN_ERROR(Status)) { > return Total; > @@ -959,7 +987,7 @@ DebugPortPollBuffer ( > UsbDebugPortRegister->SendPid =3D DATA1_PID; > } > UsbDebugPortRegister->UsbAddress =3D 0x7F; > - UsbDebugPortRegister->UsbEndPoint =3D 0x82 & 0x0F; > + UsbDebugPortRegister->UsbEndPoint =3D UsbDebugPortHandle->InEndpoint & > 0x0F; >=20 > // > // Clearing W/R bit to indicate it's a READ operation > diff --git > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.inf > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.inf > index 028b04afbf00..eb55e5ee0f63 100644 > --- > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.inf > +++ > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni > cationLibUsb.inf > @@ -43,9 +43,13 @@ [Pcd] > # The pci address of ehci host controller, in which usb debug feature = is > enabled. > # The format of pci address please refer to SourceLevelDebugPkg.dec > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress = ## > CONSUMES > + # The endpoint that should be used for read transactions from the usb = debug > device. > + gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint > ## CONSUMES > + # The endpoint that should be used for write transactions to the usb d= ebug > device. > + gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint > ## CONSUMES > # The value of data buffer size used for USB debug port handle. > # It should be equal to sizeof (USB_DEBUG_PORT_HANDLE). > - gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize|23 > ## SOMETIMES_CONSUMES > + > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize|25 > ## SOMETIMES_CONSUMES >=20 > [LibraryClasses] > TimerLib > diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > index b89e9c6ad601..76410444f385 100644 > --- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > +++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > @@ -72,6 +72,18 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] > # @Expression 0x80000001 | > (gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress & > 0xF0000FFF) =3D=3D 0 >=20 > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF000| > UINT32|0x00000003 >=20 > + ## The endpoint that should be used for read transactions from the usb > debug device. > + # 0: Determine the endpoint dynamically. > + # other: The endpoint that should be used. > + # @Prompt Configure the endpoint to read from the usb debug device. > + > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint|0x82|U > INT8|0x0000000b > + > + ## The endpoint that should be used for write transactions to the usb = debug > device. > + # 0: Determine the endpoint dynamically. > + # other: The endpoint that should be used. > + # @Prompt Configure the endpoint to write to the usb debug device. > + > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint|0x01 > |UINT8|0x0000000c > + > ## The mask of exception numbers whose handlers would be ignored and > cannot be replaced or > # hooked by Debug Agent Library. Masking INT1/INT3 is invalid. > # @Prompt Configure exception numbers not to be hooked by Debug Agent. > -- > 2.17.1.windows.2 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel