From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.3970.1591697222190099697 for ; Tue, 09 Jun 2020 03:07:02 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 79CC61FB; Tue, 9 Jun 2020 03:07:01 -0700 (PDT) Received: from [192.168.1.69] (unknown [10.37.8.184]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B6CE83F73D; Tue, 9 Jun 2020 03:06:58 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console To: Pete Batard , devel@edk2.groups.io Cc: leif@nuviainc.com References: <20200609095850.3000-1-pete@akeo.ie> From: "Ard Biesheuvel" Message-ID: Date: Tue, 9 Jun 2020 12:06:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: <20200609095850.3000-1-pete@akeo.ie> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > --- > 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 >