* [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present @ 2019-07-31 6:25 Marcin Wojtas 2019-07-31 11:37 ` Laszlo Ersek 2019-07-31 16:51 ` Ni, Ray 0 siblings, 2 replies; 7+ messages in thread From: Marcin Wojtas @ 2019-07-31 6:25 UTC (permalink / raw) To: devel Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, feng.tian, michael.d.kinney, liming.gao, lersek Until now, during the USB device enumeration when its PortState USB_PORT_STAT_CONNECTION bit was not set, the stack was not informed that the device is not present. Fix that by returning appropriate error code. Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c index be9d9bd..ab1db15 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( if (!USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at port %d\n", Port)); + Status = EFI_NOT_FOUND; goto ON_ERROR; } else if (USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_SUPER_SPEED)){ Child->Speed = EFI_USB_SPEED_SUPER; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-07-31 6:25 [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present Marcin Wojtas @ 2019-07-31 11:37 ` Laszlo Ersek 2019-08-02 1:05 ` [edk2-devel] " Wu, Hao A 2019-07-31 16:51 ` Ni, Ray 1 sibling, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2019-07-31 11:37 UTC (permalink / raw) To: Marcin Wojtas, devel Cc: leif.lindholm, ard.biesheuvel, jsd, jaz, feng.tian, michael.d.kinney, liming.gao On 07/31/19 08:25, Marcin Wojtas wrote: > Until now, during the USB device enumeration when its PortState > USB_PORT_STAT_CONNECTION bit was not set, the stack was not informed > that the device is not present. Fix that by returning appropriate > error code. > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > index be9d9bd..ab1db15 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > if (!USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at port %d\n", Port)); > + Status = EFI_NOT_FOUND; > goto ON_ERROR; > } else if (USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_SUPER_SPEED)){ > Child->Speed = EFI_USB_SPEED_SUPER; > I think the patch is correct, based on a quite superficial analysis (i.e. without actual knowledge of USB specifics on my part). The reason is that Status is EFI_SUCCESS when the "goto" statement is reached, due to the preceding context Status = HubApi->GetPortStatus (HubIf, Port, &PortState); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: failed to get speed of port %d\n", Port)); goto ON_ERROR; } And, the ON_ERROR label is documented as: ON_ERROR: // // If reach here, it means the enumeration process on a given port is interrupted due to error. // [...] // return Status; We shouldn't report success when there is no device present on the port. I think EFI_NOT_FOUND is a suitable error code; while it is not listed explicitly in the leading comment on the function, it does fit under @retval Others Failed to enumerate the device. Marcin, can you please remove the "Change-Id" tag from the commit message? (Or the MdeModulePkg maintainers could do that, just before they push the patch.) With "Change-Id" removed: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-07-31 11:37 ` Laszlo Ersek @ 2019-08-02 1:05 ` Wu, Hao A 0 siblings, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2019-08-02 1:05 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, Marcin Wojtas Cc: leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, Tian, Feng, Kinney, Michael D, Gao, Liming > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Laszlo Ersek > Sent: Wednesday, July 31, 2019 7:37 PM > To: Marcin Wojtas; devel@edk2.groups.io > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; jsd@semihalf.com; > jaz@semihalf.com; Tian, Feng; Kinney, Michael D; Gao, Liming > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > when the device is not present > > On 07/31/19 08:25, Marcin Wojtas wrote: > > Until now, during the USB device enumeration when its PortState > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not > informed > > that the device is not present. Fix that by returning appropriate > > error code. > > > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > index be9d9bd..ab1db15 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > > > if (!USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_CONNECTION)) { > > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at > port %d\n", Port)); > > + Status = EFI_NOT_FOUND; > > goto ON_ERROR; > > } else if (USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_SUPER_SPEED)){ > > Child->Speed = EFI_USB_SPEED_SUPER; > > > > I think the patch is correct, based on a quite superficial analysis (i.e. without > actual knowledge of USB specifics on my part). > > The reason is that Status is EFI_SUCCESS when the "goto" statement is > reached, due to the preceding context > > Status = HubApi->GetPortStatus (HubIf, Port, &PortState); > > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: failed to get speed of > port %d\n", Port)); > goto ON_ERROR; > } > > And, the ON_ERROR label is documented as: > > ON_ERROR: > // > // If reach here, it means the enumeration process on a given port is > interrupted due to error. > // [...] > // > return Status; > > We shouldn't report success when there is no device present on the port. > > I think EFI_NOT_FOUND is a suitable error code; while it is not listed explicitly > in the leading comment on the function, it does fit under > > @retval Others Failed to enumerate the device. > > Marcin, can you please remove the "Change-Id" tag from the commit > message? (Or the MdeModulePkg maintainers could do that, just before > they push the patch.) Thanks Laszlo, I will help to remove the 'Change-Id' tag when pushing this patch. Best Regards, Hao Wu > > With "Change-Id" removed: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks > Laszlo > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-07-31 6:25 [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present Marcin Wojtas 2019-07-31 11:37 ` Laszlo Ersek @ 2019-07-31 16:51 ` Ni, Ray 2019-08-02 1:03 ` Wu, Hao A 1 sibling, 1 reply; 7+ messages in thread From: Ni, Ray @ 2019-07-31 16:51 UTC (permalink / raw) To: devel@edk2.groups.io, mw@semihalf.com Cc: leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, Tian, Feng, Kinney, Michael D, Gao, Liming, lersek@redhat.com Marcin, What does the failure look like without your fix? Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marcin Wojtas > Sent: Wednesday, July 31, 2019 2:25 PM > To: devel@edk2.groups.io > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; mw@semihalf.com; jsd@semihalf.com; jaz@semihalf.com; Tian, Feng > <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; > lersek@redhat.com > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present > > Until now, during the USB device enumeration when its PortState > USB_PORT_STAT_CONNECTION bit was not set, the stack was not informed > that the device is not present. Fix that by returning appropriate > error code. > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > index be9d9bd..ab1db15 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > if (!USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at port %d\n", Port)); > + Status = EFI_NOT_FOUND; > goto ON_ERROR; > } else if (USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_SUPER_SPEED)){ > Child->Speed = EFI_USB_SPEED_SUPER; > -- > 2.7.4 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-07-31 16:51 ` Ni, Ray @ 2019-08-02 1:03 ` Wu, Hao A 2019-08-02 7:11 ` Marcin Wojtas 0 siblings, 1 reply; 7+ messages in thread From: Wu, Hao A @ 2019-08-02 1:03 UTC (permalink / raw) To: devel@edk2.groups.io, Ni, Ray, mw@semihalf.com Cc: leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, Kinney, Michael D, Gao, Liming, lersek@redhat.com > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, > Ray > Sent: Thursday, August 01, 2019 12:51 AM > To: devel@edk2.groups.io; mw@semihalf.com > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; jsd@semihalf.com; > jaz@semihalf.com; Tian, Feng; Kinney, Michael D; Gao, Liming; > lersek@redhat.com > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > when the device is not present > > Marcin, > What does the failure look like without your fix? >From my observation of the code: A. Changes are made in UsbEnumerateNewDev(); B. Caller of UsbEnumerateNewDev(): UsbEnumeratePort(); C. UsbEnumeratePort() directly returns the status from UsbEnumerateNewDev() as its own return value; D. Callers of UsbEnumeratePort(): UsbHubEnumeration() and UsbRootHubEnumeration(); E. Both of them do not care about the return status from UsbEnumeratePort(). I think there is no direct failure without the proposed fix. However, let us wait the confirmation from Marcin. Also, I think the proposed patch is a good refinement. If future updates to the driver requires checking the return status during the above code path, then proper status will be returned. Best Regards, Hao Wu > > Thanks, > Ray > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > Marcin Wojtas > > Sent: Wednesday, July 31, 2019 2:25 PM > > To: devel@edk2.groups.io > > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; > mw@semihalf.com; jsd@semihalf.com; jaz@semihalf.com; Tian, Feng > > <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; > Gao, Liming <liming.gao@intel.com>; > > lersek@redhat.com > > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > when the device is not present > > > > Until now, during the USB device enumeration when its PortState > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not > informed > > that the device is not present. Fix that by returning appropriate > > error code. > > > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > index be9d9bd..ab1db15 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > > > if (!USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_CONNECTION)) { > > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at > port %d\n", Port)); > > + Status = EFI_NOT_FOUND; > > goto ON_ERROR; > > } else if (USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_SUPER_SPEED)){ > > Child->Speed = EFI_USB_SPEED_SUPER; > > -- > > 2.7.4 > > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-08-02 1:03 ` Wu, Hao A @ 2019-08-02 7:11 ` Marcin Wojtas 2019-08-05 1:37 ` Wu, Hao A 0 siblings, 1 reply; 7+ messages in thread From: Marcin Wojtas @ 2019-08-02 7:11 UTC (permalink / raw) To: edk2-devel-groups-io, Wu, Hao A Cc: Ni, Ray, leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, Kinney, Michael D, Gao, Liming, lersek@redhat.com [-- Attachment #1: Type: text/plain, Size: 3662 bytes --] Hi Hao, pt., 2 sie 2019 o 03:04 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Ni, > > Ray > > Sent: Thursday, August 01, 2019 12:51 AM > > To: devel@edk2.groups.io; mw@semihalf.com > > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; > jsd@semihalf.com; > > jaz@semihalf.com; Tian, Feng; Kinney, Michael D; Gao, Liming; > > lersek@redhat.com > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > > when the device is not present > > > > Marcin, > > What does the failure look like without your fix? > > > From my observation of the code: > > A. Changes are made in UsbEnumerateNewDev(); > B. Caller of UsbEnumerateNewDev(): UsbEnumeratePort(); > C. UsbEnumeratePort() directly returns the status from UsbEnumerateNewDev() > as its own return value; > D. Callers of UsbEnumeratePort(): UsbHubEnumeration() and > UsbRootHubEnumeration(); > E. Both of them do not care about the return status from > UsbEnumeratePort(). > > I think there is no direct failure without the proposed fix. > However, let us wait the confirmation from Marcin. > > Indeed, the code path guarantees that nothing is done with the UsbEnumeratePort() Status. The patch is not a fix for the upstream code, however current behavior was problematic for a solution I was implementing on top of it (i.e. disabling USB hot-plug feature for a certain platform). Best regards, Marcin > Also, I think the proposed patch is a good refinement. If future updates to > the driver requires checking the return status during the above code path, > then proper status will be returned. > > Best Regards, > Hao Wu > > > > > > Thanks, > > Ray > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > Marcin Wojtas > > > Sent: Wednesday, July 31, 2019 2:25 PM > > > To: devel@edk2.groups.io > > > Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; > > mw@semihalf.com; jsd@semihalf.com; jaz@semihalf.com; Tian, Feng > > > <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; > > Gao, Liming <liming.gao@intel.com>; > > > lersek@redhat.com > > > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > > when the device is not present > > > > > > Until now, during the USB device enumeration when its PortState > > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not > > informed > > > that the device is not present. Fix that by returning appropriate > > > error code. > > > > > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > --- > > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > index be9d9bd..ab1db15 100644 > > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > > > > > if (!USB_BIT_IS_SET (PortState.PortStatus, > > USB_PORT_STAT_CONNECTION)) { > > > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at > > port %d\n", Port)); > > > + Status = EFI_NOT_FOUND; > > > goto ON_ERROR; > > > } else if (USB_BIT_IS_SET (PortState.PortStatus, > > USB_PORT_STAT_SUPER_SPEED)){ > > > Child->Speed = EFI_USB_SPEED_SUPER; > > > -- > > > 2.7.4 > > > > > > > > > > > > > > > > > > > > [-- Attachment #2: Type: text/html, Size: 5973 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present 2019-08-02 7:11 ` Marcin Wojtas @ 2019-08-05 1:37 ` Wu, Hao A 0 siblings, 0 replies; 7+ messages in thread From: Wu, Hao A @ 2019-08-05 1:37 UTC (permalink / raw) To: Marcin Wojtas, edk2-devel-groups-io Cc: Ni, Ray, leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, Kinney, Michael D, Gao, Liming, lersek@redhat.com [-- Attachment #1: Type: text/plain, Size: 4635 bytes --] Reviewed-by: Hao A Wu <hao.a.wu@intel.com> And patch pushed via commit 1702e2ce5a5bc2eb4514f6b1c0d68927b920528a. Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com] Sent: Friday, August 02, 2019 3:12 PM To: edk2-devel-groups-io; Wu, Hao A Cc: Ni, Ray; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; jsd@semihalf.com; jaz@semihalf.com; Kinney, Michael D; Gao, Liming; lersek@redhat.com Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present Hi Hao, pt., 2 sie 2019 o 03:04 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): > -----Original Message----- > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io>] On Behalf Of Ni, > Ray > Sent: Thursday, August 01, 2019 12:51 AM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; mw@semihalf.com<mailto:mw@semihalf.com> > Cc: leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; jsd@semihalf.com<mailto:jsd@semihalf.com>; > jaz@semihalf.com<mailto:jaz@semihalf.com>; Tian, Feng; Kinney, Michael D; Gao, Liming; > lersek@redhat.com<mailto:lersek@redhat.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > when the device is not present > > Marcin, > What does the failure look like without your fix? From my observation of the code: A. Changes are made in UsbEnumerateNewDev(); B. Caller of UsbEnumerateNewDev(): UsbEnumeratePort(); C. UsbEnumeratePort() directly returns the status from UsbEnumerateNewDev() as its own return value; D. Callers of UsbEnumeratePort(): UsbHubEnumeration() and UsbRootHubEnumeration(); E. Both of them do not care about the return status from UsbEnumeratePort(). I think there is no direct failure without the proposed fix. However, let us wait the confirmation from Marcin. Indeed, the code path guarantees that nothing is done with the UsbEnumeratePort() Status. The patch is not a fix for the upstream code, however current behavior was problematic for a solution I was implementing on top of it (i.e. disabling USB hot-plug feature for a certain platform). Best regards, Marcin Also, I think the proposed patch is a good refinement. If future updates to the driver requires checking the return status during the above code path, then proper status will be returned. Best Regards, Hao Wu > > Thanks, > Ray > > > -----Original Message----- > > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of > Marcin Wojtas > > Sent: Wednesday, July 31, 2019 2:25 PM > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > > Cc: leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; > mw@semihalf.com<mailto:mw@semihalf.com>; jsd@semihalf.com<mailto:jsd@semihalf.com>; jaz@semihalf.com<mailto:jaz@semihalf.com>; Tian, Feng > > <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; > Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; > > lersek@redhat.com<mailto:lersek@redhat.com> > > Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error > when the device is not present > > > > Until now, during the USB device enumeration when its PortState > > USB_PORT_STAT_CONNECTION bit was not set, the stack was not > informed > > that the device is not present. Fix that by returning appropriate > > error code. > > > > Change-Id: I588f82b987993e9755f64ce76cde9eb690ef1d54 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com<mailto:mw@semihalf.com>> > > --- > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > index be9d9bd..ab1db15 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c > > @@ -719,6 +719,7 @@ UsbEnumerateNewDev ( > > > > if (!USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_CONNECTION)) { > > DEBUG ((EFI_D_ERROR, "UsbEnumerateNewDev: No device present at > port %d\n", Port)); > > + Status = EFI_NOT_FOUND; > > goto ON_ERROR; > > } else if (USB_BIT_IS_SET (PortState.PortStatus, > USB_PORT_STAT_SUPER_SPEED)){ > > Child->Speed = EFI_USB_SPEED_SUPER; > > -- > > 2.7.4 > > > > > > > > > [-- Attachment #2: Type: text/html, Size: 29562 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-05 1:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-31 6:25 [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present Marcin Wojtas 2019-07-31 11:37 ` Laszlo Ersek 2019-08-02 1:05 ` [edk2-devel] " Wu, Hao A 2019-07-31 16:51 ` Ni, Ray 2019-08-02 1:03 ` Wu, Hao A 2019-08-02 7:11 ` Marcin Wojtas 2019-08-05 1:37 ` 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