Re: [PATCH] MdeModulePkg/UsbBusDxe: Rebuild the description table after Reset Device

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

> -----Original Message-----
> From: Jiang, Guomin
> Sent: Saturday, April 25, 2020 9:36 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J; Wu, Hao A; Ni, Ray
> Subject: [PATCH] MdeModulePkg/UsbBusDxe: Rebuild the description table
> after Reset Device
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2694
> 
> When the USB fail and then Reset Device, it should rebuild description.
> 
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> index 4b4915c019..9f2d2cc87f 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> @@ -869,6 +869,11 @@ UsbIoPortReset (
> 
> 
>    DEBUG (( EFI_D_INFO, "UsbIoPortReset: device is now ADDRESSED
> at %d\n", Dev->Address));
> 
> 
> 
> +  //
> 
> +  // The description will be invalid after reset, should rebuild it as well.
> 
> +  //
> 
> +  UsbBuildDescTable (Dev);


Hello Guomin,

Thanks for the proposed patch.

Could you help to explain in more detail for the above fix with regard to the
transfer ring not being set properly in the XHCI driver? Thanks.

Also, judging from the function description comments in UsbBuildDescTable():
|>  /**
|>    Build the whole array of descriptors. This function must
|>    be called after UsbGetMaxPacketSize0 returns the max packet
|>    size correctly for endpoint 0.
|>  ...
|>  **/
|>  EFI_STATUS
|>  UsbBuildDescTable (
|>    IN USB_DEVICE           *UsbDev
|>    )

Does function UsbGetMaxPacketSize0() need to be called before
UsbBuildDescTable() in the proposed fix?

Best Regards,
Hao Wu


> 
> +
> 
>    //
> 
>    // Reset the current active configure, after this device
> 
>    // is in CONFIGURED state.
> 
> --
> 2.25.1.windows.1