Inline comments below:


> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf Of Sean
> Rhodes
> Sent: Friday, February 11, 2022 4:43 PM
> To: [email protected]
> Cc: Dong, Guo <[email protected]>; Matt DeVillier
> <[email protected]>; Wu, Hao A <[email protected]>; Ni, Ray
> <[email protected]>; Wang, Jian J <[email protected]>; Gao, Liming
> <[email protected]>; Rhodes, Sean <[email protected]>
> Subject: [edk2-devel] [PATCH 1/2] SdMmcPciDxe: Make timeout for SD card
> configurable


1. Please help to update the commit subject to "MdeModulePkg/SdMmcPciHcDxe: 
Make timeout for SD card configurable"?


> 
> From: Matt DeVillier <[email protected]>
> 
> The default 1s timeout can delay boot splash on some hardware with no
> benefit.
> 
> Cc: Hao A Wu <[email protected]>
> Cc: Ray Ni <[email protected]>
> Cc: Jian J Wang <[email protected]>
> Cc: Liming Gao <[email protected]>
> Signed-off-by: Matt DeVillier <[email protected]>
> Signed-off-by: Sean Rhodes <[email protected]>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 2 +-
>  MdeModulePkg/MdeModulePkg.dec                      | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 85e09cf114..c9a21e01bd 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -49,7 +49,7 @@ extern EDKII_SD_MMC_OVERRIDE  *mOverride;
>  //
> 
>  // Generic time out value, 1 microsecond as unit.
> 
>  //
> 
> -#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 1000
> 
> +#define SD_MMC_HC_GENERIC_TIMEOUT  ((PcdGet32
> (PcdSdMmcGenericTimeoutValue)


2. The parentheses in the macro definition are not matched.
How about "... (PcdGet32 (PcdSdMmcGenericTimeoutValue))"?
Could you help to verify the patch before sending it out? Thanks in advance.


> 
> 
> 
>  //
> 
>  // SD/MMC async transfer timer interval, set by experience.
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 463e889e9a..092660f7f0 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1559,6 +1559,9 @@
>    # @Prompt Maximum permitted FwVol section nesting depth (exclusive).
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|
> 0x10|UINT32|0x00000030
> 
> 
> 
> +  ## SD Card timeout


3. Could you help to update the PCD description to:
  ## Indicates the default timeout value for SD/MMC Host Controller operations 
in microseconds.
  # @Prompt SD/MMC Host Controller Operations Timeout (us).

> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|1000
> 000|UINT32|0x00000031
> 
> +


4. I think the patch is missing the PcdLib reference in files SdMmcPciHcDxe.h 
and SdMmcPciHcDxe.inf.

5. The patch is also missing the PcdSdMmcGenericTimeoutValue PCD reference in 
SdMmcPciHcDxe.inf.
Please follow other INF files in edk2 to add the PCD reference.
Also please help to verify the patch before sending it out. Thanks in advance.

6. When adding a new PCD in DEC file, please also help to update the UNI file 
as well.
Could you help to add below lines in file MdeModulePkg.uni:
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_PROMPT 
#language en-US "SD/MMC Host Controller Operations Timeout (us)."

#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_HELP   
#language en-US "Indicates the default timeout value for SD/MMC Host Controller 
operations in microseconds."


Best Regards,
Hao Wu


> 
>  [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> 
>    ## This PCD defines the Console output row. The default value is 25
> according to UEFI spec.
> 
>    #  This PCD could be set to 0 then console output would be at max column
> and max row.
> 
> --
> 2.32.0
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86612): https://edk2.groups.io/g/devel/message/86612
> Mute This Topic: https://groups.io/mt/89066986/1768737
> Group Owner: [email protected]
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86743): https://edk2.groups.io/g/devel/message/86743
Mute This Topic: https://groups.io/mt/89066986/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to