Hi Jeremy, We test many different device and found some device haven't reproduced the issue.
We need to figure out the root cause rather than work around. Best Regards Guomin > -----Original Message----- > From: Jeremy Linton <jeremy.lin...@arm.com> > Sent: Wednesday, July 15, 2020 8:19 AM > To: Jiang, Guomin <guomin.ji...@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A > <hao.a...@intel.com>; Ni, Ray <ray...@intel.com> > Subject: Re: [PATCH v3 1/1] MdeModulePkg/UsbBusDxe: Rebuild the > description table after Reset Device > > Hi, > > On 5/9/20 3:26 AM, Guomin Jiang wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2694 > > > > When the USB fail and then Reset Device, it should rebuild description. > > > I pulled the latest edk2 a few hours ago and I'm still seeing the assert > TrsRing > != 0 messages on a real XHCI controller with the jmicron JBOD I mentioned > earlier. > > Is there a newer version of this patch? > > Thanks, > > > > > > > Signed-off-by: Guomin Jiang <guomin.ji...@intel.com> > > Cc: Jian J Wang <jian.j.w...@intel.com> > > Cc: Hao A Wu <hao.a...@intel.com> > > Cc: Ray Ni <ray...@intel.com> > > --- > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 7 ++ > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 152 > +++++++++++++++++++++++ > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 14 +++ > > 3 files changed, 173 insertions(+) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > index 4b4915c019ad..7bb9373a6adf 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > @@ -874,6 +874,13 @@ UsbIoPortReset ( > > // is in CONFIGURED state. > > // > > if (Dev->ActiveConfig != NULL) { > > + Status = UsbRebuildDescTable (Dev); > > + > > + if (EFI_ERROR (Status)) { > > + DEBUG (( DEBUG_ERROR, "UsbIoPortReset: failed to rebuild desc table > - %r\n", Status)); > > + goto ON_EXIT; > > + } > > + > > Status = UsbSetConfig (Dev, > > Dev->ActiveConfig->Desc.ConfigurationValue); > > > > if (EFI_ERROR (Status)) { > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > > index b08188b1bc78..d8e5e50b7c5a 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > > @@ -900,6 +900,158 @@ UsbBuildDescTable ( > > return EFI_SUCCESS; > > } > > > > +/** > > + Copy the interface descriptor > > + > > + @param[out] DescBuf The buffer of raw descriptor. > > + @param[in] SrcBuf The buffer of raw descriptor > > +**/ > > +VOID > > +UsbCopyInterfaceDesc ( > > + OUT USB_INTERFACE_DESC *DescBuf, > > + IN USB_INTERFACE_DESC *SrcBuf > > + ) > > +{ > > + UINTN Index, IndexI; > > + > > + ASSERT ((DescBuf != NULL) && (SrcBuf != NULL)); > > + > > + if (DescBuf->NumOfSetting == SrcBuf->NumOfSetting) { > > + DescBuf->ActiveIndex = SrcBuf->ActiveIndex; > > + > > + for (Index = 0; Index < DescBuf->NumOfSetting; Index++) { > > + CopyMem (&DescBuf->Settings[Index]->Desc, > > + &SrcBuf->Settings[Index]->Desc, > > + sizeof(EFI_USB_INTERFACE_DESCRIPTOR)); > > + > > + if (DescBuf->Settings[Index]->Desc.NumEndpoints == > > + SrcBuf->Settings[Index]->Desc.NumEndpoints) { > > + > > + for (IndexI = 0; IndexI < DescBuf->Settings[Index]- > >Desc.NumEndpoints; > > + IndexI++) { > > + CopyMem (DescBuf->Settings[Index]->Endpoints[IndexI], > > + SrcBuf->Settings[Index]->Endpoints[IndexI], > > + sizeof(USB_ENDPOINT_DESC)); > > + } > > + } > > + } > > + } > > +} > > + > > +/** > > + Copy the configuration descriptor and its interfaces. > > + > > + @param[out] DescBuf The buffer of raw descriptor. > > + @param[in] SrcBuf The buffer of raw descriptor > > +**/ > > +VOID > > +UsbCopyConfigDesc ( > > + OUT USB_CONFIG_DESC *DescBuf, > > + IN USB_CONFIG_DESC *SrcBuf > > + ) > > +{ > > + UINTN Index; > > + > > + ASSERT ((DescBuf != NULL) && (SrcBuf != NULL)); > > + > > + if (DescBuf->Desc.NumInterfaces == SrcBuf->Desc.NumInterfaces) { > > + CopyMem (&DescBuf->Desc, &SrcBuf->Desc, > > + sizeof(EFI_USB_CONFIG_DESCRIPTOR)); > > + > > + for (Index = 0; Index < DescBuf->Desc.NumInterfaces; Index++) { > > + UsbCopyInterfaceDesc (DescBuf->Interfaces[Index], SrcBuf- > >Interfaces[Index]); > > + } > > + } > > +} > > + > > +/** > > + Rebuild the whole array of descriptors. > > + > > + @param[in] UsbDev The Usb device. > > + > > + @retval EFI_SUCCESS The descriptor table is build. > > + @retval EFI_DEVICE_ERROR Invalid config > > + @retval Others Command error when get descriptor. > > +**/ > > +EFI_STATUS > > +UsbRebuildDescTable ( > > + IN USB_DEVICE *UsbDev > > + ) > > +{ > > + EFI_USB_CONFIG_DESCRIPTOR *Config; > > + USB_CONFIG_DESC *ConfigDesc; > > + UINT8 NumConfig; > > + EFI_STATUS Status; > > + UINT8 Index; > > + > > + // > > + // Override the device descriptor, Current device fail but the > > + original // descriptor pointer array is still exist. > > + // > > + Status = UsbCtrlGetDesc ( > > + UsbDev, > > + USB_DESC_TYPE_DEVICE, > > + 0, > > + 0, > > + UsbDev->DevDesc, > > + sizeof (EFI_USB_DEVICE_DESCRIPTOR) > > + ); > > + > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "UsbRebuildDescTable: failed to override > device descriptor - %r\n", Status)); > > + return Status; > > + } > > + > > + NumConfig = UsbDev->DevDesc->Desc.NumConfigurations; > > + if (NumConfig == 0) { > > + return EFI_DEVICE_ERROR; > > + } > > + > > + DEBUG ((DEBUG_INFO, "UsbReuildDescTable: device has %d > > + configures\n", NumConfig)); > > + > > + // > > + // Read each configurations, then parse them // for (Index = 0; > > + Index < NumConfig; Index++) { > > + Config = UsbGetOneConfig (UsbDev, Index); > > + > > + if (Config == NULL) { > > + DEBUG ((DEBUG_INFO, "UsbRebuildDescTable: failed to get > > + configure (index %d)\n", Index)); > > + > > + // > > + // If we can get the default descriptor, it is likely that the > > + // device is still operational. > > + // > > + if (Index == 0) { > > + return EFI_DEVICE_ERROR; > > + } > > + > > + break; > > + } > > + > > + ConfigDesc = UsbParseConfigDesc ((UINT8 *) Config, > > + Config->TotalLength); > > + > > + FreePool (Config); > > + > > + if (ConfigDesc == NULL) { > > + DEBUG ((DEBUG_ERROR, "UsbRebuildDescTable: failed to parse > > + configure (index %d)\n", Index)); > > + > > + // > > + // If we can get the default descriptor, it is likely that the > > + // device is still operational. > > + // > > + if (Index == 0) { > > + return EFI_DEVICE_ERROR; > > + } > > + > > + break; > > + } else { > > + UsbCopyConfigDesc (UsbDev->DevDesc->Configs[Index], ConfigDesc); > > + UsbFreeConfigDesc (ConfigDesc); > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > > > /** > > Set the device's address. > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > > index 7b0c77fdc79c..54367133241a 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > > @@ -169,6 +169,20 @@ UsbBuildDescTable ( > > IN USB_DEVICE *UsbDev > > ); > > > > +/** > > + Rebuild the whole array of descriptors. > > + > > + @param[in] UsbDev The Usb device. > > + > > + @retval EFI_SUCCESS The descriptor table is build. > > + @retval EFI_DEVICE_ERROR Invalid config > > + @retval Others Command error when get descriptor. > > +**/ > > +EFI_STATUS > > +UsbRebuildDescTable ( > > + IN USB_DEVICE *UsbDev > > + ); > > + > > /** > > Set the device's address. > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62571): https://edk2.groups.io/g/devel/message/62571 Mute This Topic: https://groups.io/mt/74091912/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
