From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web11.4340.1591698991223470109 for ; Tue, 09 Jun 2020 03:36:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=EcfIIe4u; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.221.65, mailfrom: pete@akeo.ie) Received: by mail-wr1-f65.google.com with SMTP id j10so20703921wrw.8 for ; Tue, 09 Jun 2020 03:36:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=gCt7M23r+lsqAUDpKxab740FNojdCuoOR+JGmLs0G9M=; b=EcfIIe4u4mnaBHn1gmW7MZ4CO1dQ6U3yge8/HW5B1p+Rwsi7DHfsVaFOGPqcMOZAna Am99E5CUFnPFOpV5uBfE/wbaW2D3e025KxAlsjPf90BiUSIbEduncIvQGCdwpOyuytgY /uCIR60ZPHR/oBbl0twv+thY4RuBl/O7yg8/1xtxWtFmmX/P3G3GIFm0iO/jBDgWk4JI 0Fznjv7fis881PBqJ+trMJ+08rwD7PdtY+vU8/oa2NjhyqqXrq6HUSQYAkfIcipfqtHK jOHSvRN57y8YFBhxpkZ7rqCPy72X29R6o93TZkbDrYz+fsbOONhliwi+qxRFGwfD/0rj TKiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gCt7M23r+lsqAUDpKxab740FNojdCuoOR+JGmLs0G9M=; b=k5LgaOJDq/kJAnEpKIwVStf7DUwASQ3r8DXiDw7YyPhSsCrtwK088jf2N8iqF2qRHg th2qeV+RKginVfdf0rjn9mEH37bCk96mLeMhoi433Q5W1oLmat2xtARusDZSfxfN2tlg Z+Y1UyN/men6SGVkh88obWfVGZiioN8HrXKYA6h+MWoAdNEF34FURt6062AOQRiRFnQh RKFl89Ig++B41czeKwAHYxzXpvzAQfSw+UgtwCK56eMcDORZ+NLYhvjCy9nctIbnxJEs Ft++tM4QDOQ0KYAIrA7hJon6o86h+gGxCuLrb+ldf2JHACupZverWhgucomoRXV4lvzP KbPg== X-Gm-Message-State: AOAM530p8G3gBdHICihSgrDHbzjKkAf2qi9T+TRkB08UGwt99ZoxLem5 ZSuBO5rjxZykInfwSINS/qJKTQ== X-Google-Smtp-Source: ABdhPJxFGJKrQQDTowFrLCKezl0io8A3I9B+P5ue4o6/akeZOfHSd4sg8BkrMqe1tWVR46I3yjBTbQ== X-Received: by 2002:a5d:4d89:: with SMTP id b9mr3925964wru.210.1591698989764; Tue, 09 Jun 2020 03:36:29 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.48.247]) by smtp.googlemail.com with ESMTPSA id y25sm2528753wmi.2.2020.06.09.03.36.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 09 Jun 2020 03:36:29 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console To: Ard Biesheuvel , devel@edk2.groups.io Cc: leif@nuviainc.com References: <20200609095850.3000-1-pete@akeo.ie> From: "Pete Batard" Message-ID: Date: Tue, 9 Jun 2020 11:36:27 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit Hi Ard, On 2020.06.09 11:06, Ard Biesheuvel wrote: > 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? Not really. But I don't have any better options at the moment, and I consider that regression needs to be fixed as a matter of urgency because it makes the platform next to useless at the moment. > Connecting the short-form USB device path (mUsbKeyboard) should already > trigger the code in the BDS that does the same. I.e., > > BmConnectUsbShortFormDevicePath ( > ) I tried that, before the call to EfiBootManagerUpdateConsoleVariable (...&mUsbKeyboard...) but it didn't seem to work... > 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 That would be my preferred choice if it worked, because someone may create their own console input device (e.g. using GPIO) and we'd want to have a way to connect everything that's relevant rather than just USB. Unfortunately, adding this call didn't seem to do anything either. At this stage, because I validated that what's being proposed works on both Pi 3 and Pi 4 (both platforms are currently broken for keyboard input right now), and because there's only so much time I can devote to testing alternatives right now, I'd push for this fix to be applied until we come up with a better solution... Regards, /Pete > > > >>   } >>   /** >> 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 >> >