Re: [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton  wrote:
>
> Its a complete tossup whether removing the mailbox call after we have
> set up the XHCI works for a given kernel+distro in DT mode. So lets
> give users which want to try DT the option of flipping this on/off.
>
> Users that don't want to have to deal with DT, can use ACPI.
>
> Signed-off-by: Jeremy Linton 
> ---
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
>  .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +
>  .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 15 +++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  |  4 
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf|  1 +
>  Platform/RaspberryPi/RPi3/RPi3.dsc|  6 ++
>  Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
>  Platform/RaspberryPi/RaspberryPi.dec  |  1 +
>  9 files changed, 50 insertions(+)
>

This looks ok to me but it doesn't appear to apply on today's edk2-platforms.

> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 3dcf2bac0d..2484787982 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -298,6 +298,16 @@ SetupVariables (
>  Status = PcdSet32S (PcdXhciPci, 1);
>  ASSERT_EFI_ERROR (Status);
>}
> +
> +  Size = sizeof (UINT32);
> +  Status = gRT->GetVariable (L"XhciReload",
> + ,
> + NULL, , );
> +  if (EFI_ERROR (Status)) {
> +Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
>  }
>} else {
>  /*
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 6f6e8f42ac..475e645537 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -96,6 +96,7 @@
>gRaspberryPiTokenSpaceGuid.PcdUartInUse
>gRaspberryPiTokenSpaceGuid.PcdXhciPci
>gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload
>
>  [Depex]
>gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 5ec17072c3..8130638876 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -62,6 +62,11 @@
>  #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
>  #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"
>
> +#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI 
> firmware"
> +#string STR_ADVANCED_XHCIRELOAD_HELP#language en-US "OS should reload 
> XHCI firmware on reset"
> +#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
> +#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
> +
>  #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>  #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
> Tag"
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index f668b7a553..f13b70711d 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -61,6 +61,11 @@ formset
>name  = XhciPci,
>guid  = CONFIGDXE_FORM_SET_GUID;
>
> +efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
> +  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +  name  = XhciReload,
> +  guid  = CONFIGDXE_FORM_SET_GUID;
> +
>  efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>name  = SystemTableMode,
> @@ -228,6 +233,16 @@ formset
>option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 
> 1, flags = 0;
>  endoneof;
>endif;
> +
> +  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
> +oneof varid = XhciReload.Value,
> +  prompt  = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
> +  help= STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
> +  flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> +  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), 
> value = 0, flags = DEFAULT;
> +  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), 
> value = 1, flags = 0;
> +endoneof;
> +  endif;
>  

[edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

2024-01-10 Thread Jeremy Linton
Its a complete tossup whether removing the mailbox call after we have
set up the XHCI works for a given kernel+distro in DT mode. So lets
give users which want to try DT the option of flipping this on/off.

Users that don't want to have to deal with DT, can use ACPI.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 15 +++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  |  4 
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf|  1 +
 Platform/RaspberryPi/RPi3/RPi3.dsc|  6 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
 Platform/RaspberryPi/RaspberryPi.dec  |  1 +
 9 files changed, 50 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3dcf2bac0d..2484787982 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -298,6 +298,16 @@ SetupVariables (
 Status = PcdSet32S (PcdXhciPci, 1);
 ASSERT_EFI_ERROR (Status);
   }
+
+  Size = sizeof (UINT32);
+  Status = gRT->GetVariable (L"XhciReload",
+ ,
+ NULL, , );
+  if (EFI_ERROR (Status)) {
+Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
+ASSERT_EFI_ERROR (Status);
+  }
+
 }
   } else {
 /*
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 6f6e8f42ac..475e645537 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload

 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 5ec17072c3..8130638876 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -62,6 +62,11 @@
 #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
 #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"

+#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI 
firmware"
+#string STR_ADVANCED_XHCIRELOAD_HELP#language en-US "OS should reload XHCI 
firmware on reset"
+#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
Tag"

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f668b7a553..f13b70711d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -61,6 +61,11 @@ formset
   name  = XhciPci,
   guid  = CONFIGDXE_FORM_SET_GUID;

+efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
+  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  name  = XhciReload,
+  guid  = CONFIGDXE_FORM_SET_GUID;
+
 efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
   attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
   name  = SystemTableMode,
@@ -228,6 +233,16 @@ formset
   option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 
1, flags = 0;
 endoneof;
   endif;
+
+  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+oneof varid = XhciReload.Value,
+  prompt  = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
+  help= STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
+  flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), 
value = 0, flags = DEFAULT;
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), 
value = 1, flags = 0;
+endoneof;
+  endif;
 endif;
 #endif
 string varid = AssetTag.AssetTag,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cbbc2ad30d..dd4fc0a05e 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,10 @@ SyncPcie (
 return