From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web12.4402.1591699301231706922 for ; Tue, 09 Jun 2020 03:41:41 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=tL3Go6az; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.65, mailfrom: pete@akeo.ie) Received: by mail-wm1-f65.google.com with SMTP id c71so2321609wmd.5 for ; Tue, 09 Jun 2020 03:41:41 -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=SdChG3VmzlAnaMprC9tPxb0DIDgCWjCAHJmf/KV5+oI=; b=tL3Go6azO5e+kgNrZRR1SgdmQkfrQSt8rxxa/WKdp7+mmrJS1pKNNL5fmaligprKqm tsdPYnYHBDeN2SaK9lzGdShkIIdZOGujK+kdDrZ335ACA10Fkc/XpwDAKSK6yfboH4wO 5MZa+n4sQ8XaplHEh/1sTA0OjjaNHMkYacrFCl+I8t8ChjXDeI71JDAZpH/RHf7V6zk+ kMhM5vbv0UslFykP7QOIi08JWwjRvDB/eLa4LLJkipWSoOnXg+F8gmYsX0BNpolO92/p DF1DaHo4YBvgkfQUavSP3rVosaEhlEz48nidc3T5DUxhAZXT1hb1RIao5UQDkpqysYN9 1LTQ== 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=SdChG3VmzlAnaMprC9tPxb0DIDgCWjCAHJmf/KV5+oI=; b=AmAnVkrpUX14XVs3OwFwWy8+GB0ufi3cCH6CjQZV9qbrQ7gNGiC/v8NpOic+HI0nIG 9DsQu9r1F2pxNsPDplrhoaM8TcJ/CibwP5LxWhQyDnKlI52awm0B013P0+mo4U497OHq z81KPG4nB7mRWme1YF9Oj/xv9TGSBp0R4LiqydGytoMg+HcK5N4Gg+DSFohhXCsCwXy7 qna9QaEmD51SSBEGodMFJjaSBgRbRppMyIxcgiTTmnspiszO2tL3DE/x6J8rBQ86MGsT JtcAErf6xyhAuqGp8fbboOBVG4mC+wSeFne5vGNwN484RHhgvQDTu4Hs/PjNrFN3dtnW gwSg== X-Gm-Message-State: AOAM531cHYqTtj3Cxf8zFp+zyY07eMr5eIKjIaIYb8c6L8N57uX8rPFH VS30Gphuy/rMeICihptoPly8FA== X-Google-Smtp-Source: ABdhPJz2tie6w2u014+8q5E3o3I9k7UrqKxVAmCjpGqaM3vGXtZZR30VBHsSb+/LhS3cY0+edUfjIQ== X-Received: by 2002:a1c:4189:: with SMTP id o131mr3462939wma.110.1591699299232; Tue, 09 Jun 2020 03:41:39 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.48.247]) by smtp.googlemail.com with ESMTPSA id v19sm2415700wml.26.2020.06.09.03.41.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 09 Jun 2020 03:41:38 -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> <9f6a8b88-02ef-91d6-54b8-45599347a63d@arm.com> From: "Pete Batard" Message-ID: <9aaefd0c-f496-5f9c-28f0-354529e6cab4@akeo.ie> Date: Tue, 9 Jun 2020 11:41:37 +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: <9f6a8b88-02ef-91d6-54b8-45599347a63d@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit On 2020.06.09 11:40, Ard Biesheuvel wrote: > On 6/9/20 12:36 PM, Pete Batard wrote: >> 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... >> > > Fair enough > > Reviewed-by: Ard Biesheuvel > > Pushed as 678f6bff3c46..2d07a49e4532 Thanks! /Pete