MdeModulePkg/UsbBusDxe: Return error when the device is not present

[edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Marcin Wojtas 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44666): https://edk2.groups.io/g/devel/message/44666
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Laszlo Ersek 4 years, 8 months ago

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44677): https://edk2.groups.io/g/devel/message/44677
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Wu, Hao A 4 years, 8 months ago

> -----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
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44823): https://edk2.groups.io/g/devel/message/44823
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Ni, Ray 4 years, 8 months ago

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
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44690): https://edk2.groups.io/g/devel/message/44690
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Wu, Hao A 4 years, 8 months ago

> -----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
> >
> >
> >
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44822): https://edk2.groups.io/g/devel/message/44822
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Marcin Wojtas 4 years, 8 months ago

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
> > >
> > >
> > >
> >
> >
> >
>
>
> 
>
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44852): https://edk2.groups.io/g/devel/message/44852
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: Return error when the device is not present

Posted by Wu, Hao A 4 years, 8 months ago

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
> >
> >
> >
>
>
>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44888): https://edk2.groups.io/g/devel/message/44888
Mute This Topic: https://groups.io/mt/32662408/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-