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.151; helo=mga17.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (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 F3AD021130022 for ; Thu, 14 Jun 2018 17:40:04 -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 fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jun 2018 17:40:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,224,1526367600"; d="scan'208";a="64239537" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga001.fm.intel.com with ESMTP; 14 Jun 2018 17:39:58 -0700 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 14 Jun 2018 17:39:58 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 14 Jun 2018 17:39:58 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.87]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.116]) with mapi id 14.03.0319.002; Fri, 15 Jun 2018 08:39:56 +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+fQgACST4CAAJPfoA== Date: Fri, 15 Jun 2018 00:39:56 +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: Fri, 15 Jun 2018 00:40:05 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Marvin, Actually, the device we own has the IN/OUT endpoints with 0x82 and 0x01 respectively. I double checked the value returned in the USB Debug Device Descriptor for = the device I own, and the values are correct, matching the hard-code configurat= ion in current code. I think we can directly use the endpoints value from the descriptor, so ple= ase help to work a series without the PCDs, thanks in advance. Best Regards, Hao Wu > -----Original Message----- > From: Marvin H=E4user [mailto:Marvin.Haeuser@outlook.com] > Sent: Thursday, June 14, 2018 11:46 PM > To: edk2-devel@lists.01.org; Wu, Hao A > Cc: Ni, Ruiyu > Subject: RE: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: > Add endpoint configuration. >=20 > Hey Hao, >=20 > I don't require hardcoded endpoint information, but I have only one debug > device to test with in the first place. > The static endpoint option was added in case there are Debug Devices arou= nd > which report incorrect information. > I assumed that was part of the reason the endpoints were hardcoded in the= first > place. >=20 > Do you want me to submit a V2 without the PCDs? >=20 > Thanks, > Marvin. >=20 > > -----Original Message----- > > From: Wu, Hao A > > Sent: Thursday, June 14, 2018 9:03 AM > > To: Marvin H=E4user ; edk2- > > devel@lists.01.org > > Cc: Ni, Ruiyu > > Subject: RE: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: > > Add endpoint configuration. > > > > Hi Marvin, > > > > One thing to confirm with you. For your using case, do you need to hard= - > > code 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 O= f > > > 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. > > > > > > 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 th= e > > > former hardcoded values. > > > > > > V2: > > > - Store endpoint data in the USB Debug Port handle sturcture. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Marvin Haeuser > > > --- > > > > > > > > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun > > ica > > > tionLibUsb.c | 32 ++++++++++++++++++-- > > > > > > > > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun > > ica > > > tionLibUsb.inf | 6 +++- > > > SourceLevelDebugPkg/SourceLevelDebugPkg.dec = | > 12 > > > ++++++++ > > > 3 files changed, 47 insertions(+), 3 deletions(-) > > > > > > diff --git > > > > > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.c > > > > > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.c > > > index d996f80f59e3..a9a9ea07a39b 100644 > > > --- > > > > > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.c > > > +++ > > > > > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > 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; > > > > > > 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; > > > } > > > > > > + // > > > + // 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); > > > } > > > > > > - Status =3D UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total,= Sent, > > > OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle); > > > + Status =3D UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, > > > + Sent, > > > OUTPUT_PID, 0x7F, UsbDebugPortHandle->OutEndpoint, > > UsbDebugPortHandle- > > > >BulkOutToggle); > > > > > > 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; > > > > > > // > > > // Clearing W/R bit to indicate it's a READ operation diff --git > > > > > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.inf > > > > > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.inf > > > index 028b04afbf00..eb55e5ee0f63 100644 > > > --- > > > > > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > cationLibUsb.inf > > > +++ > > > > > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm > > uni > > > 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.de= c > > > 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 debug > > > 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 > > > > > > [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 > > > > > > > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF0 > > 00| > > > UINT32|0x00000003 > > > > > > + ## 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|0x > > 82|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|0 > > x01 > > > |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 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel