[edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[treffer+groups.io]

DwHcReset expects attributes as the second argument. A reset is
performed if the passed attribute is valid. However 0 is not a valid
attribute and will thus never cause a controller reset.

Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2 controller
as expected.

This enables the USB 2.0 port of the raspberry compute module 4.
---
 Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
index bada13a6cd..bb228e62d9 100644
--- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
+++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
@@ -140,7 +140,7 @@ DriverStart (
    * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784 expects
    * the HCD to do this. There is no agent invoking DwHcReset anymore.
    */
-  DwHcReset (&DwHc->DwUsbOtgHc, 0);
+  DwHcReset (&DwHc->DwUsbOtgHc, EFI_USB_HC_RESET_HOST_CONTROLLER);
   DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
 
   Status = gBS->InstallMultipleProtocolInterfaces (
-- 
2.27.0

Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[Samer El-Haj-Mahmoud]

Thanks for the patch!

+ Andrei and Jeremey for review

I think this may be a side effect of https://github.com/tianocore/edk2-platforms/commit/f8958b86e8863432b815a132a0f0fe82950c6dd1

Previously, the DwHcReset() function did not check for valid Attributes passed in as an argument. So if you pass in 0, the function will still happily reset the controller. That caused UEFI SCT issues (since the function will take in garbage Attributes without checking for their validity, per UEFI spec). The change was to verify the Attributes are valid, and return EFI_UNSUPPORTED if they are not. The only valid attributes for resetting are EFI_USB_HC_RESET_GLOBAL and EFI_USB_HC_RESET_HOST_CONTROLLER.

I think your change makes sense.  But I would like to run more tests.

Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of René
> Treffer via groups.io
> Sent: Thursday, March 11, 2021 7:40 AM
> To: devel@edk2.groups.io
> Cc: Pete Batard <pete@akeo.ie>; Leif Lindholm <leif@nuviainc.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>
> Subject: [edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi:
> Fix dwc2 reset on raspberry pi boards
>
> DwHcReset expects attributes as the second argument. A reset is performed
> if the passed attribute is valid. However 0 is not a valid attribute and will thus
> never cause a controller reset.
>
> Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2
> controller as expected.
>
> This enables the USB 2.0 port of the raspberry compute module 4.
> ---
>  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> index bada13a6cd..bb228e62d9 100644
> --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> @@ -140,7 +140,7 @@ DriverStart (
>     * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784 expects
>     * the HCD to do this. There is no agent invoking DwHcReset anymore.
>     */
> -  DwHcReset (&DwHc->DwUsbOtgHc, 0);
> +  DwHcReset (&DwHc->DwUsbOtgHc,
> EFI_USB_HC_RESET_HOST_CONTROLLER);
>    DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
>
>    Status = gBS->InstallMultipleProtocolInterfaces (
> --
> 2.27.0
>
>
>
> 
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[Samer El-Haj-Mahmoud]

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>


Thanks again for this patch. This is reported to fix :

https://github.com/pftf/RPi4/issues/88#issuecomment-792370668
and
https://github.com/pftf/RPi4/issues/122
and possibly:
https://github.com/pftf/RPi4/issues/132

Thanks,
--Samer



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer
> El-Haj-Mahmoud via groups.io
> Sent: Thursday, March 11, 2021 4:51 PM
> To: devel@edk2.groups.io; treffer+groups.io@measite.de
> Cc: Pete Batard <pete@akeo.ie>; Leif Lindholm <leif@nuviainc.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Andrei Warkentin
> (awarkentin@vmware.com) <awarkentin@vmware.com>; Jeremy Linton
> <Jeremy.Linton@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud@arm.com>
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1]
> Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards
>
> Thanks for the patch!
>
> + Andrei and Jeremey for review
>
> I think this may be a side effect of https://github.com/tianocore/edk2-
> platforms/commit/f8958b86e8863432b815a132a0f0fe82950c6dd1
>
> Previously, the DwHcReset() function did not check for valid Attributes
> passed in as an argument. So if you pass in 0, the function will still happily
> reset the controller. That caused UEFI SCT issues (since the function will take
> in garbage Attributes without checking for their validity, per UEFI spec). The
> change was to verify the Attributes are valid, and return EFI_UNSUPPORTED
> if they are not. The only valid attributes for resetting are
> EFI_USB_HC_RESET_GLOBAL and EFI_USB_HC_RESET_HOST_CONTROLLER.
>
> I think your change makes sense.  But I would like to run more tests.
>
> Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of René
> > Treffer via groups.io
> > Sent: Thursday, March 11, 2021 7:40 AM
> > To: devel@edk2.groups.io
> > Cc: Pete Batard <pete@akeo.ie>; Leif Lindholm <leif@nuviainc.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>
> > Subject: [edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi:
> > Fix dwc2 reset on raspberry pi boards
> >
> > DwHcReset expects attributes as the second argument. A reset is
> > performed if the passed attribute is valid. However 0 is not a valid
> > attribute and will thus never cause a controller reset.
> >
> > Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2
> > controller as expected.
> >
> > This enables the USB 2.0 port of the raspberry compute module 4.
> > ---
> >  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> > b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> > index bada13a6cd..bb228e62d9 100644
> > --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> > +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> > @@ -140,7 +140,7 @@ DriverStart (
> >     * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784
> expects
> >     * the HCD to do this. There is no agent invoking DwHcReset anymore.
> >     */
> > -  DwHcReset (&DwHc->DwUsbOtgHc, 0);
> > +  DwHcReset (&DwHc->DwUsbOtgHc,
> > EFI_USB_HC_RESET_HOST_CONTROLLER);
> >    DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
> >
> >    Status = gBS->InstallMultipleProtocolInterfaces (
> > --
> > 2.27.0
> >
> >
> >
> >
> >
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
>
>
> 
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[Ard Biesheuvel]

On Thu, 11 Mar 2021 at 13:40, René Treffer <treffer+groups.io@measite.de> wrote:
>
> DwHcReset expects attributes as the second argument. A reset is
> performed if the passed attribute is valid. However 0 is not a valid
> attribute and will thus never cause a controller reset.
>
> Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2 controller
> as expected.
>
> This enables the USB 2.0 port of the raspberry compute module 4.

Thanks for the patch (and for the review, Samer)

Please resend this patch (with Samer's Reviewed-by included) with a
signed-off-by line, otherwise I cannot merge this.

Thanks,
Ard.


> ---
>  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> index bada13a6cd..bb228e62d9 100644
> --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> @@ -140,7 +140,7 @@ DriverStart (
>     * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784 expects
>     * the HCD to do this. There is no agent invoking DwHcReset anymore.
>     */
> -  DwHcReset (&DwHc->DwUsbOtgHc, 0);
> +  DwHcReset (&DwHc->DwUsbOtgHc, EFI_USB_HC_RESET_HOST_CONTROLLER);
>    DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
>
>    Status = gBS->InstallMultipleProtocolInterfaces (
> --
> 2.27.0
>

[edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[René Treffer]

DwHcReset expects attributes as the second argument. A reset is
performed if the passed attribute is valued. However 0 is not a valid
attribute and will thus never cause a controller reset.

Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2 controller
as expected.

This enables the USB 2.0 port of the raspberry compute module 4.

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Signed-off-by: René Treffer <treffer@measite.de>
---
 Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
index bada13a6cd..bb228e62d9 100644
--- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
+++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
@@ -140,7 +140,7 @@ DriverStart (
    * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784 expects
    * the HCD to do this. There is no agent invoking DwHcReset anymore.
    */
-  DwHcReset (&DwHc->DwUsbOtgHc, 0);
+  DwHcReset (&DwHc->DwUsbOtgHc, EFI_USB_HC_RESET_HOST_CONTROLLER);
   DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
 
   Status = gBS->InstallMultipleProtocolInterfaces (
-- 
2.27.0

Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix dwc2 reset on raspberry pi boards

[Ard Biesheuvel]

On Fri, 12 Mar 2021 at 18:42, René Treffer <treffer@measite.de> wrote:
>
> DwHcReset expects attributes as the second argument. A reset is
> performed if the passed attribute is valued. However 0 is not a valid
> attribute and will thus never cause a controller reset.
>
> Passing EFI_USB_HC_RESET_HOST_CONTROLLER will reset the dwc2 controller
> as expected.
>
> This enables the USB 2.0 port of the raspberry compute module 4.
>
> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Signed-off-by: René Treffer <treffer@measite.de>

Pushed as abf4c54b2192..2620e05c6fad

Thanks all

> ---
>  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> index bada13a6cd..bb228e62d9 100644
> --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DriverBinding.c
> @@ -140,7 +140,7 @@ DriverStart (
>     * UsbBusDxe as of b4e96b82b4e2e47e95014b51787ba5b43abac784 expects
>     * the HCD to do this. There is no agent invoking DwHcReset anymore.
>     */
> -  DwHcReset (&DwHc->DwUsbOtgHc, 0);
> +  DwHcReset (&DwHc->DwUsbOtgHc, EFI_USB_HC_RESET_HOST_CONTROLLER);
>    DwHcSetState (&DwHc->DwUsbOtgHc, EfiUsbHcStateOperational);
>
>    Status = gBS->InstallMultipleProtocolInterfaces (
> --
> 2.27.0