Re: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Hao A,

On 2/20/19 2:11 AM, Wu, Hao A wrote:
> Thanks Mateusz,
> 
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> And pushed via commit 68c67d3a2a33261e41ff0123129b4e9759617f71.

I think you mean "pushed 'after' commit
68c67d3a2a33261e41ff0123129b4e9759617f71".

Commit 68c67d3a2a33261e41ff0123129b4e9759617f71 is:
"BaseTools: Fixed a code bug for Pcd Array", while
this patch is commited as 195f673f6270aaf73dd34b75f1da26451b63c316.

Now about this commit 195f673f6270aaf, I think you have an issue with
your configuration, as the patch author was remplaced from:
Albecki, Mateusz <mateusz.albecki@intel.com>
to:
Albecki, Mateusz </o=Intel/ou=Exchange Administrative Group
(FYDIBOHF23SPDLT)/cn=Recipients/cn=Albecki, Mateusz3be>

Regards,

Phil.

> 
> Best Regards,
> Hao Wu
> 
> 
>> -----Original Message-----
>> From: Albecki, Mateusz
>> Sent: Monday, February 18, 2019 7:12 PM
>> To: edk2-devel@lists.01.org
>> Cc: Albecki, Mateusz; Wu, Hao A
>> Subject: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400
>> switch sequence
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140
>>
>> In eMMC HS400 switch sequence flow eMMC driver attempted
>> to execute SEND_STATUS just after switching bus timing to high
>> speed and before downgrading clock frequency to 52MHz. Since link
>> was at that time in incorrect state SEND_STATUS was failing which
>> made driver think switch to HS400 failed.
>> This change makes driver always change clock frequency after
>> switching bus timing and before executing SEND_STATUS.
>>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Albecki Mateusz <mateusz.albecki@intel.com>
>> ---
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 39
>> +++++++++++++------------
>>  1 file changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>> index 4ef849fd0962..15db8a87a5c4 100644
>> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
>> @@ -642,7 +642,7 @@ EmmcSwitchBusWidth (
>>  }
>>
>>  /**
>> -  Switch the clock frequency to the specified value.
>> +  Switch the bus timing and clock frequency.
>>
>>    Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host
>> Controller
>>    Simplified Spec 3.0 Figure 3-3 for details.
>> @@ -660,7 +660,7 @@ EmmcSwitchBusWidth (
>>
>>  **/
>>  EFI_STATUS
>> -EmmcSwitchClockFreq (
>> +EmmcSwitchBusTiming (
>>    IN EFI_PCI_IO_PROTOCOL                *PciIo,
>>    IN EFI_SD_MMC_PASS_THRU_PROTOCOL      *PassThru,
>>    IN UINT8                              Slot,
>> @@ -689,22 +689,10 @@ EmmcSwitchClockFreq (
>>
>>    Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet);
>>    if (EFI_ERROR (Status)) {
>> -    DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to hstiming %d
>> fails with %r\n", HsTiming, Status));
>> +    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d
>> fails with %r\n", HsTiming, Status));
>>      return Status;
>>    }
>>
>> -  Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
>> -  if (EFI_ERROR (Status)) {
>> -    DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails
>> with %r\n", Status));
>> -    return Status;
>> -  }
>> -  //
>> -  // Check the switch operation is really successful or not.
>> -  //
>> -  if ((DevStatus & BIT7) != 0) {
>> -    DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation
>> fails as DevStatus is 0x%08x\n", DevStatus));
>> -    return EFI_DEVICE_ERROR;
>> -  }
>>    //
>>    // Convert the clock freq unit from MHz to KHz.
>>    //
>> @@ -713,6 +701,19 @@ EmmcSwitchClockFreq (
>>      return Status;
>>    }
>>
>> +  Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Send status fails
>> with %r\n", Status));
>> +    return Status;
>> +  }
>> +  //
>> +  // Check the switch operation is really successful or not.
>> +  //
>> +  if ((DevStatus & BIT7) != 0) {
>> +    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: The switch operation
>> fails as DevStatus is 0x%08x\n", DevStatus));
>> +    return EFI_DEVICE_ERROR;
>> +  }
>> +
>>    if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
>>      Status = mOverride->NotifyPhase (
>>                            Private->ControllerHandle,
>> @@ -799,7 +800,7 @@ EmmcSwitchToHighSpeed (
>>    }
>>
>>    HsTiming = 1;
>> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
>> ClockFreq);
>> +  Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
>> Timing, ClockFreq);
>>
>>    return Status;
>>  }
>> @@ -887,7 +888,7 @@ EmmcSwitchToHS200 (
>>    Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
>> (ClockCtrl), &ClockCtrl);
>>
>>    HsTiming = 2;
>> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
>> ClockFreq);
>> +  Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
>> Timing, ClockFreq);
>>    if (EFI_ERROR (Status)) {
>>      return Status;
>>    }
>> @@ -937,7 +938,7 @@ EmmcSwitchToHS400 (
>>    // Set to Hight Speed timing and set the clock frequency to a value less than
>> 52MHz.
>>    //
>>    HsTiming = 1;
>> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
>> SdMmcMmcHsSdr, 52);
>> +  Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
>> SdMmcMmcHsSdr, 52);
>>    if (EFI_ERROR (Status)) {
>>      return Status;
>>    }
>> @@ -957,7 +958,7 @@ EmmcSwitchToHS400 (
>>    }
>>
>>    HsTiming = 3;
>> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
>> ClockFreq);
>> +  Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
>> Timing, ClockFreq);
>>
>>    return Status;
>>  }
>> --
>> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>