Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

Hi Pete,

On 6/9/20 11:58 AM, Pete Batard wrote:
> Due to the nature of USB init on the Pi platforms, commit
> c8000ecccc83b728baf04ced2fedb870bc3bc1b3 introduced a regression
> with regards to ensuring that USB devices are operational by the
> time the console is up.
> 
> This commit fixes this by adding explicit calls to connect USB
> controllers before console initialization.
> 
> Note that trying to connect a non existent device (e.g. PCI bus
> on the Pi 3) is harmless so there's no need to guard these calls
> according to the devices effectively supported by the platform.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 31 ++++++++++++++++++++
>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 +
>   2 files changed, 32 insertions(+)
> 
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 253614a646c1..d347c733855d 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -314,6 +314,30 @@ AddOutput (
>       ReportText));
>   }
>   
> +/**
> +  This CALLBACK_FUNCTION attempts to connect a handle non-recursively, asking
> +  the matching driver to produce all first-level child handles.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +Connect (
> +  IN EFI_HANDLE   Handle,
> +  IN CONST CHAR16 *ReportText
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = gBS->ConnectController (
> +                  Handle, // ControllerHandle
> +                  NULL,   // DriverImageHandle
> +                  NULL,   // RemainingDevicePath -- produce all children
> +                  FALSE   // Recursive
> +                  );
> +  DEBUG ((EFI_ERROR (Status) ? EFI_D_ERROR : EFI_D_VERBOSE, "%a: %s: %r\n",
> +    __FUNCTION__, ReportText, Status));
> +}
> +
>   STATIC
>   INTN
>   PlatformRegisterBootOption (
> @@ -617,6 +641,13 @@ PlatformBootManagerBeforeConsole (
>     // Dispatch deferred images after EndOfDxe event and ReadyToLock installation.
>     //
>     EfiBootManagerDispatchDeferredImages ();
> +
> +  //
> +  // Ensure that USB is initialized by connecting the PCI root bridge so
> +  // that the xHCI PCI controller gets enumerated (Pi 4) or by connecting
> +  // to the DesignWare USB OTG controller directly.
> +  FilterAndProcess (&gEfiPciRootBridgeIoProtocolGuid, NULL, Connect);
> +  FilterAndProcess (&gEfiUsb2HcProtocolGuid, NULL, Connect);

Are you sure this is necessary?

Connecting the short-form USB device path (mUsbKeyboard) should already 
trigger the code in the BDS that does the same. I.e.,

BmConnectUsbShortFormDevicePath (
)

finds all PCI I/O protocol handles with the USB class code, and connects 
them.

Alternatively, we might use EfiBootManagerConnectAllDefaultConsoles() 
instead of ConnectAll() if that is a cleaner way of fixing stuff



>   }
>   
>   /**
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index e40b3f096a4f..eb44daa4b7b7 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -78,6 +78,7 @@ [Protocols]
>     gEfiDevicePathProtocolGuid
>     gEfiGraphicsOutputProtocolGuid
>     gEfiLoadedImageProtocolGuid
> +  gEfiPciRootBridgeIoProtocolGuid
>     gEfiSimpleFileSystemProtocolGuid
>     gEsrtManagementProtocolGuid
>     gEfiUsb2HcProtocolGuid
>