Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, January 06, 2020 1:32 PM
> To: devel@edk2.groups.io; 'matt.devillier@gmail.com'
> Subject: RE: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix
> endpoint selection
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > MrChromebox
> > Sent: Monday, January 06, 2020 12:12 PM
> > To: devel@edk2.groups.io
> > Cc: Matt DeVillier
> > Subject: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix
> > endpoint selection
> >
> > The endpoint selected by the driver needs to not
> > only be an interrupt type, but have direction IN
> > as required to set up an asynchronous interrupt transfer.
> >
> > Currently, the driver assumes that the first INT endpoint
> > will be of type IN, but that is not true of all devices,
> > and will silently fail on devices which have the OUT endpoint
> > before the IN. Adjust the endpoint selection loop to explictly
> > check for direction IN.
> >
> > Test: detachable keyboard on Google Pixel Slate now works.
> >
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > ---
> >  MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> > index 27685995c2..ccb389067a 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
> > @@ -215,7 +215,7 @@ USBKeyboardDriverBindingStart (
> >    EndpointNumber = UsbKeyboardDevice-
> > >InterfaceDescriptor.NumEndpoints;
> >
> >    //
> > -  // Traverse endpoints to find interrupt endpoint
> > +  // Traverse endpoints to find interrupt endpoint IN
> >    //
> >    Found = FALSE;
> >    for (Index = 0; Index < EndpointNumber; Index++) {
> > @@ -226,7 +226,8 @@ USBKeyboardDriverBindingStart (
> >               &EndpointDescriptor
> >               );
> >
> > -    if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> > USB_ENDPOINT_INTERRUPT) {
> > +    if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) ==
> > USB_ENDPOINT_INTERRUPT) &&
> > +        ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) !=
> 0))
> > {
> 
> 
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>


Series has been pushed via commits 0286fe8176..6cfb6da951.

Best Regards,
Hao Wu


> 
> Since there is only coding style change compared with V1 series, I will keep
> the R-b tag by GuoMinJ <newexplorerj@gmail.com> as well.
> 
> Best Regards,
> Hao Wu
> 
> 
> >        //
> >        // We only care interrupt endpoint here
> >        //
> > --
> > 2.20.1
> >
> >
> >