Re: [edk2-platform][PATCH V2 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized

["Wu, Hao A" <hao.a.wu@intel.com>]

Sorry,

The proposed patch still causes CI test failure: https://github.com/tianocore/edk2/pull/3724.

Could you help to follow steps 10 and 11 in the below wiki page:
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-developer-process-for-the-edk-ii-project
to make sure the patch can pass the CI test? Thanks in advance.

Also, for below added code:
  ZeroMem (SwitchResp, 64 * sizeof (SwitchResp));
My take is that it will zero memories beyond the scope of local variable 'SwitchResp', could you help to double check on this?

Best Regards,
Hao Wu

> -----Original Message-----
> From: Chevron Li <chevron.li@bayhubtech.com>
> Sent: Tuesday, December 6, 2022 8:53 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> shirley.her@bayhubtech.com; shaper.liu@bayhubtech.com;
> xiaoguang.yu@bayhubtech.com; Chevron Li (WH)
> <chevron.li@bayhubtech.com>
> Subject: [edk2-platform][PATCH V2 1/1] MdeModulePkg: SdMmcPciHcDxe:
> Fix issue that SD1.0 cards can't be recognized
> 
> From: "Chevron Li (WH)" <chevron.li@bayhubtech.com>
> 
> 
> SD1.0 cards don't support CMD8 and CMD6
> 
> CMD8 result can be used to distinguish the card is SD1.0 or not.
> 
> CMD8 result can be used to decide following CMD6 is sent or skip.
> 
> 
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> 
> Cc: Ray Ni <ray.ni@intel.com>
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Signed-off-by: Chevron Li <chevron.li@bayhubtech.com>
> 
> ---
> 
> Changes in V2:
> 
> 1.Update description comment for input parameter 'SdVersion1'.
> 
> 2.Add variables initialize operation to avoid unexpected value.
> 
> 3.Use TRUE replace with 1 to assign value for BOOLEAN variable.
> 
> ---
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 34 +++++++++++++--
> ----
> 
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> 
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> 
> index f5a3607e47..262f675ea2 100644
> 
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> 
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> 
> @@ -1074,6 +1074,7 @@ SdGetTargetBusMode (
> 
>    @param[in] Slot           The slot number of the SD card to send the
> command to.
> 
>    @param[in] Rca            The relative device address to be assigned.
> 
>    @param[in] S18A           The boolean to show if it's a UHS-I SD card.
> 
> +  @param[in] SdVersion1     The boolean to show if it's a Version 1 SD card.
> 
> 
> 
>    @retval EFI_SUCCESS       The operation is done correctly.
> 
>    @retval Others            The operation fails.
> 
> @@ -1085,7 +1086,8 @@ SdCardSetBusMode (
> 
>    IN EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru,
> 
>    IN UINT8                          Slot,
> 
>    IN UINT16                         Rca,
> 
> -  IN BOOLEAN                        S18A
> 
> +  IN BOOLEAN                        S18A,
> 
> +  IN BOOLEAN                        SdVersion1
> 
>    )
> 
>  {
> 
>    EFI_STATUS              Status;
> 
> @@ -1095,6 +1097,8 @@ SdCardSetBusMode (
> 
>    SD_MMC_HC_PRIVATE_DATA  *Private;
> 
>    SD_MMC_BUS_SETTINGS     BusMode;
> 
> 
> 
> +  ZeroMem (SwitchResp, 64 * sizeof (SwitchResp));
> 
> +
> 
>    Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> 
> 
> 
>    Capability = &Private->Capability[Slot];
> 
> @@ -1117,10 +1121,13 @@ SdCardSetBusMode (
> 
> 
> 
>    //
> 
>    // Get the supported bus speed from SWITCH cmd return data group #1.
> 
> +  // SdVersion1 don't support the SWITCH cmd
> 
>    //
> 
> -  Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore,
> 0xF, FALSE, SwitchResp);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -    return Status;
> 
> +  if(SdVersion1 == FALSE) {
> 
> +    Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore,
> 0xF, FALSE, SwitchResp);
> 
> +    if (EFI_ERROR (Status)) {
> 
> +      return Status;
> 
> +    }
> 
>    }
> 
> 
> 
>    SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode);
> 
> @@ -1141,9 +1148,14 @@ SdCardSetBusMode (
> 
>      }
> 
>    }
> 
> 
> 
> -  Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -    return Status;
> 
> +  //
> 
> +  // SdVersion1 don't support the SWITCH cmd
> 
> +  //
> 
> +  if(SdVersion1 == FALSE){
> 
> +    Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
> 
> +    if (EFI_ERROR (Status)) {
> 
> +      return Status;
> 
> +    }
> 
>    }
> 
> 
> 
>    Status = SdMmcSetDriverStrength (Private->PciIo, Slot,
> BusMode.DriverStrength.Sd);
> 
> @@ -1214,8 +1226,10 @@ SdCardIdentification (
> 
>    UINT8                          HostCtrl2;
> 
>    UINTN                          Retry;
> 
>    BOOLEAN                        ForceVoltage33;
> 
> +  BOOLEAN                        SdVersion1;
> 
> 
> 
>    ForceVoltage33 = FALSE;
> 
> +  SdVersion1 = FALSE;
> 
> 
> 
>    PciIo    = Private->PciIo;
> 
>    PassThru = &Private->PassThru;
> 
> @@ -1231,12 +1245,12 @@ Voltage33Retry:
> 
>    }
> 
> 
> 
>    //
> 
> -  // 2. Send Cmd8 to the device
> 
> +  // 2. Send Cmd8 to the device, the command will fail for SdVersion1;
> 
>    //
> 
>    Status = SdCardVoltageCheck (PassThru, Slot, 0x1, 0xFF);
> 
>    if (EFI_ERROR (Status)) {
> 
> +    SdVersion1 = TRUE;
> 
>      DEBUG ((DEBUG_INFO, "SdCardIdentification: Executing Cmd8 fails
> with %r\n", Status));
> 
> -    return Status;
> 
>    }
> 
> 
> 
>    //
> 
> @@ -1426,7 +1440,7 @@ Voltage33Retry:
> 
>    DEBUG ((DEBUG_INFO, "SdCardIdentification: Found a SD device at slot
> [%d]\n", Slot));
> 
>    Private->Slot[Slot].CardType = SdCardType;
> 
> 
> 
> -  Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) !=
> 0));
> 
> +  Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) != 0),
> SdVersion1);
> 
> 
> 
>    return Status;
> 
> 
> 
> 
> 
> base-commit: 7bee2498910a9034faaf90802c49188afb7582dc
> 
> --
> 
> 2.18.0.windows.1
> 
>