public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console
@ 2020-06-09  9:58 Pete Batard
  2020-06-09 10:06 ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-06-09  9:58 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif

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);
 }
 
 /**
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
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console
  2020-06-09  9:58 [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console Pete Batard
@ 2020-06-09 10:06 ` Ard Biesheuvel
  2020-06-09 10:36   ` Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-06-09 10:06 UTC (permalink / raw)
  To: Pete Batard, devel; +Cc: leif

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console
  2020-06-09 10:06 ` Ard Biesheuvel
@ 2020-06-09 10:36   ` Pete Batard
  2020-06-09 10:40     ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-06-09 10:36 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: leif

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 <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?

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
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console
  2020-06-09 10:36   ` Pete Batard
@ 2020-06-09 10:40     ` Ard Biesheuvel
  2020-06-09 10:41       ` Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-06-09 10:40 UTC (permalink / raw)
  To: Pete Batard, devel; +Cc: leif

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 <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?
> 
> 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 <ard.biesheuvel@arm.com>

Pushed as 678f6bff3c46..2d07a49e4532

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console
  2020-06-09 10:40     ` Ard Biesheuvel
@ 2020-06-09 10:41       ` Pete Batard
  0 siblings, 0 replies; 5+ messages in thread
From: Pete Batard @ 2020-06-09 10:41 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: leif

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 <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?
>>
>> 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 <ard.biesheuvel@arm.com>
> 
> Pushed as 678f6bff3c46..2d07a49e4532

Thanks!

/Pete


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-09 10:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-09  9:58 [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Ensure USB controller init before console Pete Batard
2020-06-09 10:06 ` Ard Biesheuvel
2020-06-09 10:36   ` Pete Batard
2020-06-09 10:40     ` Ard Biesheuvel
2020-06-09 10:41       ` Pete Batard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox