[v1] Platform/RPi: User config improvements

[edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms

Posted by Pete Batard 5 years, 11 months ago

Group all SD/MMC settings under the SD/MMC form.
Make CPU settings more prominent.
Harmonise form titles and text content.
Reorder forms in the order they are most likely to be queried.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 24 +++++-----
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 50 ++++++++++----------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 2e79e322e558..77eda96d8136 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -14,11 +14,11 @@
 #string STR_FORM_SET_TITLE_HELP  #language en-US "Press <Enter> to configure system settings."
 
 /*
- * Chipset config.
+ * Chipset configuration.
  */
 
-#string STR_CHIPSET_FORM_TITLE      #language en-US "Chipset Configuration"
-#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings when booted."
+#string STR_CHIPSET_FORM_TITLE      #language en-US "CPU Configuration"
+#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings."
 
 #string STR_CHIPSET_CLOCK_CPU_PROMPT #language en-US "CPU Clock"
 #string STR_CHIPSET_CLOCK_CPU_HELP   #language en-US "CPU Speed"
@@ -30,11 +30,6 @@
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_PROMPT #language en-US "CPU Clock Rate (MHz)"
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_HELP   #language en-US "Adjust the CPU speed.\nMin value: 100 MHz\nMax value: 1600 MHz\n\nWarning: Overclocking can make the system unbootable!"
 
-#string STR_CHIPSET_SD_PROMPT        #language en-US "uSD Routing"
-#string STR_CHIPSET_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
-#string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
-#string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
-
 /*
  * Advanced configuration.
  */
@@ -55,8 +50,13 @@
  * MMC/SD configuration.
  */
 
-#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Tweaks"
-#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS will override settings when booted."
+#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Configuration"
+#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS may override settings."
+
+#string STR_MMC_SD_PROMPT        #language en-US "uSD Routing"
+#string STR_MMC_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
+#string STR_MMC_SD_SDHOST        #language en-US "Broadcom SDHOST"
+#string STR_MMC_SD_ARASAN        #language en-US "Arasan SDHCI"
 
 #string STR_MMC_DISMULTI_PROMPT  #language en-US "Multi-Block Support"
 #string STR_MMC_DISMULTI_HELP    #language en-US "Use CMD18/CMD25 for transfers when possible"
@@ -84,7 +84,7 @@
  * Display settings.
  */
 
-#string STR_DISPLAY_FORM_TITLE      #language en-US "Display"
+#string STR_DISPLAY_FORM_TITLE      #language en-US "Display Configuration"
 #string STR_DISPLAY_FORM_SUBTITLE   #language en-US "UEFI video driver settings"
 
 #string STR_DISPLAY_VMODES_640_PROMPT  #language en-US "Virtual 640x480"
@@ -109,7 +109,7 @@
 /*
  * Debugging settings go here.
  */
-#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging"
+#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging Configuration"
 #string STR_DEBUG_FORM_SUBTITLE     #language en-US "For UEFI/OS Developers"
 
 #string STR_DEBUG_JTAG_PROMPT       #language en-US "JTAG Routing"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index d16058da4926..9c2fd64a8e27 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -65,14 +65,6 @@ typedef struct {
   UINT32 Clock;
 } CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
 
-typedef struct {
-  /*
-   * 0 - uSD slot routed to Broadcom SDHOST.
-   * 1 - uSD slot routed to Arasan SDHCI.
-   */
-  UINT32 Routing;
-} CHIPSET_SD_VARSTORE_DATA;
-
 typedef struct {
   /*
    * Always set by ConfigDxe prior to HII init to reflect
@@ -93,6 +85,14 @@ typedef struct {
   UINT32 Enabled;
 } ADVANCED_DEVICE_TREE_VARSTORE_DATA;
 
+typedef struct {
+  /*
+   * 0 - uSD slot routed to Broadcom SDHOST.
+   * 1 - uSD slot routed to Arasan SDHCI.
+   */
+  UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - Don't disable multi-block.
@@ -155,11 +155,6 @@ formset
       name  = CustomCpuClock,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
-    efivarstore CHIPSET_SD_VARSTORE_DATA,
-      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-      name  = SdIsArasan,
-      guid  = CONFIGDXE_FORM_SET_GUID;
-
     efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = RamMoreThan3GB,
@@ -175,6 +170,11 @@ formset
       name  = OptDeviceTree,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
+    efivarstore MMC_SD_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = SdIsArasan,
+      guid  = CONFIGDXE_FORM_SET_GUID;
+
     efivarstore MMC_DISMULTI_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = MmcDisableMulti,
@@ -228,6 +228,10 @@ formset
            prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
 
+        goto 0x1004,
+            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
+            help = STRING_TOKEN(STR_NULL_STRING);
+
         goto 0x1006,
            prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
@@ -236,10 +240,6 @@ formset
            prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
 
-        goto 0x1004,
-            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
-            help = STRING_TOKEN(STR_NULL_STRING);
-
         goto 0x1005,
             prompt = STRING_TOKEN(STR_DEBUG_FORM_TITLE),
             help = STRING_TOKEN(STR_NULL_STRING);
@@ -269,14 +269,6 @@ formset
               default = 600,
           endnumeric;
         endif;
-
-        oneof varid = SdIsArasan.Routing,
-            prompt      = STRING_TOKEN(STR_CHIPSET_SD_PROMPT),
-            help        = STRING_TOKEN(STR_CHIPSET_SD_HELP),
-            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
-            option text = STRING_TOKEN(STR_CHIPSET_SD_ARASAN), value = 1, flags = 0;
-            option text = STRING_TOKEN(STR_CHIPSET_SD_SDHOST), value = 0, flags = DEFAULT;
-        endoneof;
     endform;
 
     form formid = 0x1006,
@@ -306,6 +298,14 @@ formset
         title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
         subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);
 
+        oneof varid = SdIsArasan.Routing,
+            prompt      = STRING_TOKEN(STR_MMC_SD_PROMPT),
+            help        = STRING_TOKEN(STR_MMC_SD_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_MMC_SD_ARASAN), value = 1, flags = 0;
+            option text = STRING_TOKEN(STR_MMC_SD_SDHOST), value = 0, flags = DEFAULT;
+        endoneof;
+
         oneof varid = MmcDisableMulti.DisableMulti,
             prompt      = STRING_TOKEN(STR_MMC_DISMULTI_PROMPT),
             help        = STRING_TOKEN(STR_MMC_DISMULTI_HELP),
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55308): https://edk2.groups.io/g/devel/message/55308
Mute This Topic: https://groups.io/mt/71697501/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms

Posted by Ard Biesheuvel 5 years, 11 months ago

On Tue, 3 Mar 2020 at 11:34, Pete Batard <pete@akeo.ie> wrote:
>
> Group all SD/MMC settings under the SD/MMC form.
> Make CPU settings more prominent.
> Harmonise form titles and text content.
> Reorder forms in the order they are most likely to be queried.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 24 +++++-----
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 50 ++++++++++----------
>  2 files changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 2e79e322e558..77eda96d8136 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -14,11 +14,11 @@
>  #string STR_FORM_SET_TITLE_HELP  #language en-US "Press <Enter> to configure system settings."
>
>  /*
> - * Chipset config.
> + * Chipset configuration.
>   */
>
> -#string STR_CHIPSET_FORM_TITLE      #language en-US "Chipset Configuration"
> -#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings when booted."
> +#string STR_CHIPSET_FORM_TITLE      #language en-US "CPU Configuration"
> +#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings."
>
>  #string STR_CHIPSET_CLOCK_CPU_PROMPT #language en-US "CPU Clock"
>  #string STR_CHIPSET_CLOCK_CPU_HELP   #language en-US "CPU Speed"
> @@ -30,11 +30,6 @@
>  #string STR_CHIPSET_CUSTOM_CPU_CLOCK_PROMPT #language en-US "CPU Clock Rate (MHz)"
>  #string STR_CHIPSET_CUSTOM_CPU_CLOCK_HELP   #language en-US "Adjust the CPU speed.\nMin value: 100 MHz\nMax value: 1600 MHz\n\nWarning: Overclocking can make the system unbootable!"
>
> -#string STR_CHIPSET_SD_PROMPT        #language en-US "uSD Routing"
> -#string STR_CHIPSET_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
> -#string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
> -#string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
> -
>  /*
>   * Advanced configuration.
>   */
> @@ -55,8 +50,13 @@
>   * MMC/SD configuration.
>   */
>
> -#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Tweaks"
> -#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS will override settings when booted."
> +#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Configuration"
> +#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS may override settings."
> +
> +#string STR_MMC_SD_PROMPT        #language en-US "uSD Routing"
> +#string STR_MMC_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
> +#string STR_MMC_SD_SDHOST        #language en-US "Broadcom SDHOST"
> +#string STR_MMC_SD_ARASAN        #language en-US "Arasan SDHCI"
>
>  #string STR_MMC_DISMULTI_PROMPT  #language en-US "Multi-Block Support"
>  #string STR_MMC_DISMULTI_HELP    #language en-US "Use CMD18/CMD25 for transfers when possible"
> @@ -84,7 +84,7 @@
>   * Display settings.
>   */
>
> -#string STR_DISPLAY_FORM_TITLE      #language en-US "Display"
> +#string STR_DISPLAY_FORM_TITLE      #language en-US "Display Configuration"
>  #string STR_DISPLAY_FORM_SUBTITLE   #language en-US "UEFI video driver settings"
>
>  #string STR_DISPLAY_VMODES_640_PROMPT  #language en-US "Virtual 640x480"
> @@ -109,7 +109,7 @@
>  /*
>   * Debugging settings go here.
>   */
> -#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging"
> +#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging Configuration"
>  #string STR_DEBUG_FORM_SUBTITLE     #language en-US "For UEFI/OS Developers"
>
>  #string STR_DEBUG_JTAG_PROMPT       #language en-US "JTAG Routing"
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index d16058da4926..9c2fd64a8e27 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -65,14 +65,6 @@ typedef struct {
>    UINT32 Clock;
>  } CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
>
> -typedef struct {
> -  /*
> -   * 0 - uSD slot routed to Broadcom SDHOST.
> -   * 1 - uSD slot routed to Arasan SDHCI.
> -   */
> -  UINT32 Routing;
> -} CHIPSET_SD_VARSTORE_DATA;
> -
>  typedef struct {
>    /*
>     * Always set by ConfigDxe prior to HII init to reflect
> @@ -93,6 +85,14 @@ typedef struct {
>    UINT32 Enabled;
>  } ADVANCED_DEVICE_TREE_VARSTORE_DATA;
>
> +typedef struct {
> +  /*
> +   * 0 - uSD slot routed to Broadcom SDHOST.
> +   * 1 - uSD slot routed to Arasan SDHCI.
> +   */
> +  UINT32 Routing;
> +} MMC_SD_VARSTORE_DATA;
> +
>  typedef struct {
>    /*
>     * 0 - Don't disable multi-block.
> @@ -155,11 +155,6 @@ formset
>        name  = CustomCpuClock,
>        guid  = CONFIGDXE_FORM_SET_GUID;
>
> -    efivarstore CHIPSET_SD_VARSTORE_DATA,
> -      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> -      name  = SdIsArasan,
> -      guid  = CONFIGDXE_FORM_SET_GUID;
> -
>      efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
>        attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>        name  = RamMoreThan3GB,
> @@ -175,6 +170,11 @@ formset
>        name  = OptDeviceTree,
>        guid  = CONFIGDXE_FORM_SET_GUID;
>
> +    efivarstore MMC_SD_VARSTORE_DATA,
> +      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +      name  = SdIsArasan,
> +      guid  = CONFIGDXE_FORM_SET_GUID;
> +
>      efivarstore MMC_DISMULTI_VARSTORE_DATA,
>        attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>        name  = MmcDisableMulti,
> @@ -228,6 +228,10 @@ formset
>             prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
>             help = STRING_TOKEN(STR_NULL_STRING);
>
> +        goto 0x1004,
> +            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
> +            help = STRING_TOKEN(STR_NULL_STRING);
> +
>          goto 0x1006,
>             prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
>             help = STRING_TOKEN(STR_NULL_STRING);
> @@ -236,10 +240,6 @@ formset
>             prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
>             help = STRING_TOKEN(STR_NULL_STRING);
>
> -        goto 0x1004,
> -            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
> -            help = STRING_TOKEN(STR_NULL_STRING);
> -
>          goto 0x1005,
>              prompt = STRING_TOKEN(STR_DEBUG_FORM_TITLE),
>              help = STRING_TOKEN(STR_NULL_STRING);
> @@ -269,14 +269,6 @@ formset
>                default = 600,
>            endnumeric;
>          endif;
> -
> -        oneof varid = SdIsArasan.Routing,
> -            prompt      = STRING_TOKEN(STR_CHIPSET_SD_PROMPT),
> -            help        = STRING_TOKEN(STR_CHIPSET_SD_HELP),
> -            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> -            option text = STRING_TOKEN(STR_CHIPSET_SD_ARASAN), value = 1, flags = 0;
> -            option text = STRING_TOKEN(STR_CHIPSET_SD_SDHOST), value = 0, flags = DEFAULT;
> -        endoneof;
>      endform;
>
>      form formid = 0x1006,
> @@ -306,6 +298,14 @@ formset
>          title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
>          subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);
>
> +        oneof varid = SdIsArasan.Routing,
> +            prompt      = STRING_TOKEN(STR_MMC_SD_PROMPT),
> +            help        = STRING_TOKEN(STR_MMC_SD_HELP),
> +            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> +            option text = STRING_TOKEN(STR_MMC_SD_ARASAN), value = 1, flags = 0;
> +            option text = STRING_TOKEN(STR_MMC_SD_SDHOST), value = 0, flags = DEFAULT;
> +        endoneof;
> +
>          oneof varid = MmcDisableMulti.DisableMulti,
>              prompt      = STRING_TOKEN(STR_MMC_DISMULTI_PROMPT),
>              help        = STRING_TOKEN(STR_MMC_DISMULTI_HELP),
> --
> 2.21.0.windows.1
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55313): https://edk2.groups.io/g/devel/message/55313
Mute This Topic: https://groups.io/mt/71697501/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size
[edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB
[edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice
[edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice
[edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms