public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
@ 2018-06-13 19:57 Marvin Häuser
  2018-06-14  7:03 ` Wu, Hao A
  0 siblings, 1 reply; 4+ messages in thread
From: Marvin Häuser @ 2018-06-13 19:57 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: hao.a.wu@intel.com

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.

V2:
  - Store endpoint data in the USB Debug Port handle sturcture.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c   | 32 ++++++++++++++++++--
 SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf |  6 +++-
 SourceLevelDebugPkg/SourceLevelDebugPkg.dec                                       | 12 ++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
index d996f80f59e3..a9a9ea07a39b 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.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 = (USB_DEBUG_PORT_REGISTER *)((UINTN)Handle->UsbDebugPortMemoryBase + Handle->DebugPortOffset);
   UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
@@ -722,6 +732,24 @@ InitializeUsbDebugHardware (
       return RETURN_DEVICE_ERROR;
     }
 
+    //
+    // Determine the usb debug device endpoints.
+    //
+    InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
+
+    if (InEndpoint == 0) {
+      InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
+    }
+
+    OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
+
+    if (OutEndpoint == 0) {
+      OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
+    }
+
+    Handle->InEndpoint  = InEndpoint;
+    Handle->OutEndpoint = OutEndpoint;
+
     //
     // enable the usb debug feature.
     //
@@ -879,7 +907,7 @@ DebugPortWriteBuffer (
       Sent = (UINT8)(NumberOfBytes - Total);
     }
 
-    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent, OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
+    Status = 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  = DATA1_PID;
   }
   UsbDebugPortRegister->UsbAddress  = 0x7F;
-  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
+  UsbDebugPortRegister->UsbEndPoint = UsbDebugPortHandle->InEndpoint & 0x0F;
 
   //
   // Clearing W/R bit to indicate it's a READ operation
diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf
index 028b04afbf00..eb55e5ee0f63 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.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 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) == 0
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF000|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|0x82|UINT8|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



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
  2018-06-13 19:57 [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration Marvin Häuser
@ 2018-06-14  7:03 ` Wu, Hao A
  2018-06-14 15:45   ` Marvin Häuser
  0 siblings, 1 reply; 4+ messages in thread
From: Wu, Hao A @ 2018-06-14  7:03 UTC (permalink / raw)
  To: Marvin Häuser, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu

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 Of
> Marvin Häuser
> 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
> the 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 <Marvin.Haeuser@outlook.com>
> ---
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica
> tionLibUsb.c   | 32 ++++++++++++++++++--
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica
> tionLibUsb.inf |  6 +++-
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dec                                       | 12
> ++++++++
>  3 files changed, 47 insertions(+), 3 deletions(-)
> 
> 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;
> 
>    UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER *)((UINTN)Handle-
> >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
>    UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> @@ -722,6 +732,24 @@ InitializeUsbDebugHardware (
>        return RETURN_DEVICE_ERROR;
>      }
> 
> +    //
> +    // Determine the usb debug device endpoints.
> +    //
> +    InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
> +
> +    if (InEndpoint == 0) {
> +      InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> +    }
> +
> +    OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
> +
> +    if (OutEndpoint == 0) {
> +      OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> +    }
> +
> +    Handle->InEndpoint  = InEndpoint;
> +    Handle->OutEndpoint = OutEndpoint;
> +
>      //
>      // enable the usb debug feature.
>      //
> @@ -879,7 +907,7 @@ DebugPortWriteBuffer (
>        Sent = (UINT8)(NumberOfBytes - Total);
>      }
> 
> -    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> +    Status = 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  = DATA1_PID;
>    }
>    UsbDebugPortRegister->UsbAddress  = 0x7F;
> -  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
> +  UsbDebugPortRegister->UsbEndPoint = UsbDebugPortHandle->InEndpoint &
> 0x0F;
> 
>    //
>    // 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 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) == 0
> 
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF000|
> 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|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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
  2018-06-14  7:03 ` Wu, Hao A
@ 2018-06-14 15:45   ` Marvin Häuser
  2018-06-15  0:39     ` Wu, Hao A
  0 siblings, 1 reply; 4+ messages in thread
From: Marvin Häuser @ 2018-06-14 15:45 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Wu, Hao A; +Cc: ruiyu.ni@intel.com

Hey Hao,

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 around which report incorrect information.
I assumed that was part of the reason the endpoints were hardcoded in the first place.

Do you want me to submit a V2 without the PCDs?

Thanks,
Marvin.

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Thursday, June 14, 2018 9:03 AM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> 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 Of
> > Marvin Häuser
> > 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 the
> > 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 <Marvin.Haeuser@outlook.com>
> > ---
> >
> >
> 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 = (USB_DEBUG_PORT_REGISTER
> *)((UINTN)Handle-
> > >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
> >    UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> @@
> > -722,6 +732,24 @@ InitializeUsbDebugHardware (
> >        return RETURN_DEVICE_ERROR;
> >      }
> >
> > +    //
> > +    // Determine the usb debug device endpoints.
> > +    //
> > +    InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
> > +
> > +    if (InEndpoint == 0) {
> > +      InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> > +    }
> > +
> > +    OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
> > +
> > +    if (OutEndpoint == 0) {
> > +      OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> > +    }
> > +
> > +    Handle->InEndpoint  = InEndpoint;
> > +    Handle->OutEndpoint = OutEndpoint;
> > +
> >      //
> >      // enable the usb debug feature.
> >      //
> > @@ -879,7 +907,7 @@ DebugPortWriteBuffer (
> >        Sent = (UINT8)(NumberOfBytes - Total);
> >      }
> >
> > -    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> > OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> > +    Status = 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  = DATA1_PID;
> >    }
> >    UsbDebugPortRegister->UsbAddress  = 0x7F;
> > -  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
> > +  UsbDebugPortRegister->UsbEndPoint = 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.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 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) == 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


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
  2018-06-14 15:45   ` Marvin Häuser
@ 2018-06-15  0:39     ` Wu, Hao A
  0 siblings, 0 replies; 4+ messages in thread
From: Wu, Hao A @ 2018-06-15  0:39 UTC (permalink / raw)
  To: Marvin Häuser, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu

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 configuration
in current code.

I think we can directly use the endpoints value from the descriptor, so please
help to work a series without the PCDs, thanks in advance.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Marvin Häuser [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.
> 
> Hey Hao,
> 
> 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 around
> which report incorrect information.
> I assumed that was part of the reason the endpoints were hardcoded in the first
> place.
> 
> Do you want me to submit a V2 without the PCDs?
> 
> Thanks,
> Marvin.
> 
> > -----Original Message-----
> > From: Wu, Hao A <hao.a.wu@intel.com>
> > Sent: Thursday, June 14, 2018 9:03 AM
> > To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> > devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > 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 Of
> > > Marvin Häuser
> > > 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 the
> > > 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 <Marvin.Haeuser@outlook.com>
> > > ---
> > >
> > >
> > 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 = (USB_DEBUG_PORT_REGISTER
> > *)((UINTN)Handle-
> > > >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
> > >    UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> > @@
> > > -722,6 +732,24 @@ InitializeUsbDebugHardware (
> > >        return RETURN_DEVICE_ERROR;
> > >      }
> > >
> > > +    //
> > > +    // Determine the usb debug device endpoints.
> > > +    //
> > > +    InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
> > > +
> > > +    if (InEndpoint == 0) {
> > > +      InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> > > +    }
> > > +
> > > +    OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
> > > +
> > > +    if (OutEndpoint == 0) {
> > > +      OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> > > +    }
> > > +
> > > +    Handle->InEndpoint  = InEndpoint;
> > > +    Handle->OutEndpoint = OutEndpoint;
> > > +
> > >      //
> > >      // enable the usb debug feature.
> > >      //
> > > @@ -879,7 +907,7 @@ DebugPortWriteBuffer (
> > >        Sent = (UINT8)(NumberOfBytes - Total);
> > >      }
> > >
> > > -    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> > > OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> > > +    Status = 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  = DATA1_PID;
> > >    }
> > >    UsbDebugPortRegister->UsbAddress  = 0x7F;
> > > -  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
> > > +  UsbDebugPortRegister->UsbEndPoint = 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.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 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) == 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


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-06-15  0:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-13 19:57 [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration Marvin Häuser
2018-06-14  7:03 ` Wu, Hao A
2018-06-14 15:45   ` Marvin Häuser
2018-06-15  0:39     ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox