* [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection [not found] <20200106041207.29728-1-matt.devillier@gmail.com> @ 2020-01-06 4:12 ` MrChromebox 2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A 2020-01-06 6:52 ` Wu, Hao A 2020-01-06 4:12 ` [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: " MrChromebox 2020-01-06 4:12 ` [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: " MrChromebox 2 siblings, 2 replies; 7+ messages in thread From: MrChromebox @ 2020-01-06 4:12 UTC (permalink / raw) To: devel; +Cc: Matt DeVillier 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)) { // // We only care interrupt endpoint here // -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection 2020-01-06 4:12 ` [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection MrChromebox @ 2020-01-06 5:31 ` Wu, Hao A 2020-01-06 6:52 ` Wu, Hao A 1 sibling, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2020-01-06 5:31 UTC (permalink / raw) To: devel@edk2.groups.io, matt.devillier@gmail.com > -----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> 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 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection 2020-01-06 4:12 ` [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection MrChromebox 2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A @ 2020-01-06 6:52 ` Wu, Hao A 1 sibling, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2020-01-06 6:52 UTC (permalink / raw) To: devel@edk2.groups.io, matt.devillier@gmail.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 > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: Fix endpoint selection [not found] <20200106041207.29728-1-matt.devillier@gmail.com> 2020-01-06 4:12 ` [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection MrChromebox @ 2020-01-06 4:12 ` MrChromebox 2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A 2020-01-06 4:12 ` [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: " MrChromebox 2 siblings, 1 reply; 7+ messages in thread From: MrChromebox @ 2020-01-06 4:12 UTC (permalink / raw) To: devel; +Cc: Matt DeVillier 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. Signed-off-by: Matt DeVillier <matt.devillier@gmail.com> --- MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c index 677815a8ad..143ff15eb0 100644 --- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c +++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c @@ -203,7 +203,7 @@ USBMouseDriverBindingStart ( EndpointNumber = UsbMouseDevice->InterfaceDescriptor.NumEndpoints; // - // Traverse endpoints to find interrupt endpoint + // Traverse endpoints to find interrupt endpoint IN // Found = FALSE; for (Index = 0; Index < EndpointNumber; Index++) { @@ -213,7 +213,8 @@ USBMouseDriverBindingStart ( &EndpointDescriptor ); - if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) { + if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) && + ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0)) { // // We only care interrupt endpoint here // -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: Fix endpoint selection 2020-01-06 4:12 ` [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: " MrChromebox @ 2020-01-06 5:31 ` Wu, Hao A 0 siblings, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2020-01-06 5:31 UTC (permalink / raw) To: devel@edk2.groups.io, matt.devillier@gmail.com > -----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 2/3] MdeModulePkg/Usb/UsbMouse: 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. > > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com> > --- > MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > index 677815a8ad..143ff15eb0 100644 > --- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > +++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > @@ -203,7 +203,7 @@ USBMouseDriverBindingStart ( > EndpointNumber = UsbMouseDevice->InterfaceDescriptor.NumEndpoints; > > // > - // Traverse endpoints to find interrupt endpoint > + // Traverse endpoints to find interrupt endpoint IN > // > Found = FALSE; > for (Index = 0; Index < EndpointNumber; Index++) { > @@ -213,7 +213,8 @@ USBMouseDriverBindingStart ( > &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> 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 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection [not found] <20200106041207.29728-1-matt.devillier@gmail.com> 2020-01-06 4:12 ` [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection MrChromebox 2020-01-06 4:12 ` [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: " MrChromebox @ 2020-01-06 4:12 ` MrChromebox 2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A 2 siblings, 1 reply; 7+ messages in thread From: MrChromebox @ 2020-01-06 4:12 UTC (permalink / raw) To: devel; +Cc: Matt DeVillier 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. Signed-off-by: Matt DeVillier <matt.devillier@gmail.com> --- MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c index 8953e7031c..9cd0e4cd53 100644 --- a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c +++ b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c @@ -203,7 +203,7 @@ USBMouseAbsolutePointerDriverBindingStart ( EndpointNumber = UsbMouseAbsolutePointerDevice->InterfaceDescriptor.NumEndpoints; // - // Traverse endpoints to find interrupt endpoint + // Traverse endpoints to find interrupt endpoint IN // Found = FALSE; for (Index = 0; Index < EndpointNumber; Index++) { @@ -213,7 +213,8 @@ USBMouseAbsolutePointerDriverBindingStart ( &EndpointDescriptor ); - if ((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) { + if (((EndpointDescriptor.Attributes & (BIT0 | BIT1)) == USB_ENDPOINT_INTERRUPT) && + ((EndpointDescriptor.EndpointAddress & USB_ENDPOINT_DIR_IN) != 0)) { // // We only care interrupt endpoint here // -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection 2020-01-06 4:12 ` [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: " MrChromebox @ 2020-01-06 5:31 ` Wu, Hao A 0 siblings, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2020-01-06 5:31 UTC (permalink / raw) To: devel@edk2.groups.io, matt.devillier@gmail.com Best Regards, Hao Wu > -----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 3/3] > MdeModulePkg/UsbMouseAbsolutePointer: 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. > > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com> > --- > > MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolu > tePointer.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git > a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs > olutePointer.c > b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs > olutePointer.c > index 8953e7031c..9cd0e4cd53 100644 > --- > a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs > olutePointer.c > +++ > b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbs > olutePointer.c > @@ -203,7 +203,7 @@ USBMouseAbsolutePointerDriverBindingStart ( > EndpointNumber = UsbMouseAbsolutePointerDevice- > >InterfaceDescriptor.NumEndpoints; > > // > - // Traverse endpoints to find interrupt endpoint > + // Traverse endpoints to find interrupt endpoint IN > // > Found = FALSE; > for (Index = 0; Index < EndpointNumber; Index++) { > @@ -213,7 +213,8 @@ USBMouseAbsolutePointerDriverBindingStart ( > &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> Best Regards, Hao Wu > // > // We only care interrupt endpoint here > // > -- > 2.20.1 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-06 6:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200106041207.29728-1-matt.devillier@gmail.com>
2020-01-06 4:12 ` [PATCH v3 1/3] MdeModulePkg/Usb/EfiKey: Fix endpoint selection MrChromebox
2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A
2020-01-06 6:52 ` Wu, Hao A
2020-01-06 4:12 ` [PATCH v3 2/3] MdeModulePkg/Usb/UsbMouse: " MrChromebox
2020-01-06 5:31 ` [edk2-devel] " Wu, Hao A
2020-01-06 4:12 ` [PATCH v3 3/3] MdeModulePkg/UsbMouseAbsolutePointer: " MrChromebox
2020-01-06 5:31 ` [edk2-devel] " 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