Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c: remove Get/SetConfig calls

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Friday, January 22, 2021 9:17 AM
> To: devel@edk2.groups.io; patrick.rudolph@9elements.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c:
> remove Get/SetConfig calls
> 
> Sorry, please grant me some time for this patch series. I will try to provide
> feedbacks before end of next week.
> Meanwhile, could you help to provide the information on what kind of tests
> have been performed for these 2 patches? Thanks in advance.
> 
> Best Regards,
> Hao Wu
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Patrick
> > Rudolph
> > Sent: Wednesday, January 20, 2021 11:59 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/Usb/Keyboard.c:
> > remove Get/SetConfig calls
> >
> > From: Matt DeVillier <matt.devillier@gmail.com>
> >
> > SetConfig is already called during device enumeration, no need to do
> > it again here.


I found that the InitUSBKeyboard() function, which includes this Get/SetConfig
calls, is triggered during the keyboard reset flow. Even though the
configuration for the keyboard will be set during enumeration, I think it would
be better to keep this reconfiguration in the reset flow.

Best Regards,
Hao Wu


> >
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > ---
> >  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 37 --------------------
> >  1 file changed, 37 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > index 5faf82ea57..77e20b203f 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > @@ -801,10 +801,8 @@ InitUSBKeyboard (
> >    IN OUT USB_KB_DEV   *UsbKeyboardDevice
> >
> >    )
> >
> >  {
> >
> > -  UINT16              ConfigValue;
> >
> >    UINT8               Protocol;
> >
> >    EFI_STATUS          Status;
> >
> > -  UINT32              TransferResult;
> >
> >
> >
> >    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> >
> >      EFI_PROGRESS_CODE,
> >
> > @@ -816,41 +814,6 @@ InitUSBKeyboard (
> >    InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
> >
> >    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
> > (EFI_KEY_DATA));
> >
> >
> >
> > -  //
> >
> > -  // Use the config out of the descriptor
> >
> > -  // Assumed the first config is the correct one and this is not
> > always the case
> >
> > -  //
> >
> > -  Status = UsbGetConfiguration (
> >
> > -             UsbKeyboardDevice->UsbIo,
> >
> > -             &ConfigValue,
> >
> > -             &TransferResult
> >
> > -             );
> >
> > -  if (EFI_ERROR (Status)) {
> >
> > -    ConfigValue = 0x01;
> >
> > -    //
> >
> > -    // Uses default configuration to configure the USB Keyboard device.
> >
> > -    //
> >
> > -    Status = UsbSetConfiguration (
> >
> > -               UsbKeyboardDevice->UsbIo,
> >
> > -               ConfigValue,
> >
> > -               &TransferResult
> >
> > -               );
> >
> > -    if (EFI_ERROR (Status)) {
> >
> > -      //
> >
> > -      // If configuration could not be set here, it means
> >
> > -      // the keyboard interface has some errors and could
> >
> > -      // not be initialized
> >
> > -      //
> >
> > -      REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> >
> > -        EFI_ERROR_CODE | EFI_ERROR_MINOR,
> >
> > -        (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> >
> > -        UsbKeyboardDevice->DevicePath
> >
> > -        );
> >
> > -
> >
> > -      return EFI_DEVICE_ERROR;
> >
> > -    }
> >
> > -  }
> >
> > -
> >
> >    UsbGetProtocolRequest (
> >
> >      UsbKeyboardDevice->UsbIo,
> >
> >      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > --
> > 2.26.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#70585):
> > https://edk2.groups.io/g/devel/message/70585
> > Mute This Topic: https://groups.io/mt/79981643/1768737
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
>