> -----Original Message-----
> From: edk2-devel [mailto:[email protected]] On Behalf Of
> Marcin Wojtas
> Sent: Friday, September 07, 2018 5:10 PM
> To: [email protected]
> Cc: Tian, Feng; [email protected]; [email protected]; Gao, Liming; Kinney,
> Michael D
> Subject: [edk2] [PATCH v2 1/4] MdeModulePkg/SdMmcPciHcDxe: Fix HS200
> operation
> 
> When switching to any of high speed modes (HS, HS200, HS400)
> there is need to set HS_ENABLE bit in Host Control 1 register
> which allow Host Controller to output CMD and DAT lines on
> both edges of clock. In Linux it is done after switching bus
> width in sdhci_set_ios().

I am checking both the SD and eMMC specs for this. As far as I can tell,
since the eMMC cards that support HS200 & HS400 will have 1.8V/1.2V
signaling. This makes a match within the SD specs for UHS-I mode (rather
than High Speed mode which is 3.3V signaling).

Switching the host controller to UHS-I mode requires configuring the '1.8V
Signaling Enable' (BIT3) and 'UHS Mode Select' (BIT2~0) fields of the Host
Control 2 Register. And the setting of the 'High Speed Enable' (BIT2)
field of the Host Control 1 Register seems not required to me.

Do you met with issues when switching the eMMC device to HS200/HS400 mode
when not setting BIT2 of the Host Control Register?

> 
> Also according to JESD84-B50-1 chapter 6.6.4 "HS200 timing mode
> selection" (documentation about eMMC4.5 standard) there is
> no need to disable clock when switching to HS200.

Yes, you are right that the eMMC spec does not require such a reset of the
reset SD Clock Enable (BIT2) of the Clock Control Register.

I think the code here is taking reference to the SD Host Controller
Simplified Spec 3.0:

Under the description of 'High Speed Enable' (BIT2) of the Host Control 1
Register (Table 2-16):

* If Preset Value Enable in the Host Control 2 register is set to 1, Host
* Driver needs to reset SD Clock Enable before changing this field to
* avoid generating clock glitches. After setting this field, the Host
* Driver sets SD Clock Enable again.

The spec makes very clear that the reset SD Clock Enable is only needed
when Preset Value Enable in the Host Control 2 register is set to 1.

But for the description of 'UHS Mode Select' (BIT2~0) of the Host Control
2 Register (Table 2-32):

* If Preset Value Enable in the Host Control 2 register is set to 1, Host
* Controller sets SDCLK Frequency Select, Clock Generator Select in the
* Clock Control register and Driver Strength Select according to Preset
* Value registers. In this case, one of preset value registers is selected
* by this field. Host Driver needs to reset SD Clock Enable before changing
* this field to avoid generating clock glitch. After setting this field,
* Host Driver sets SD Clock Enable again.

It's not that clear whether this reset is needed under all cases. So the
driver just play it safe here to added reset of the SD Clock Enable bit
during the switch of HS200 mode for eMMC devices.

So do you met with issues here for this? If not, I prefer to keep the
re-enabling of the clock logic here for stability consideration.

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <[email protected]>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h |  5 ++++
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c  | 30 +++----------------
> -
>  2 files changed, 9 insertions(+), 26 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index e389d52..e3fadb5 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -63,6 +63,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define SD_MMC_HC_CTRL_VER            0xFE
> 
>  //
> +// SD Host Control 1 Register bits description
> +//
> +#define SD_MMC_HC_HOST_CTRL1_HS_ENABLE  (1 << 2)
> +
> +//
>  // The transfer modes supported by SD Host Controller
>  // Simplified Spec 3.0 Table 1-2
>  //
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index c5fd214..b3903b4 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -816,8 +816,8 @@ EmmcSwitchToHS200 (
>  {
>    EFI_STATUS          Status;
>    UINT8               HsTiming;
> +  UINT8               HostCtrl1;
>    UINT8               HostCtrl2;
> -  UINT16              ClockCtrl;
> 
>    if ((BusWidth != 4) && (BusWidth != 8)) {
>      return EFI_INVALID_PARAMETER;
> @@ -828,12 +828,10 @@ EmmcSwitchToHS200 (
>      return Status;
>    }
>    //
> -  // Set to HS200/SDR104 timing
> -  //
> -  //
> -  // Stop bus clock at first
> +  // Set to High Speed timing
>    //
> -  Status = SdMmcHcStopClock (PciIo, Slot);
> +  HostCtrl1 = SD_MMC_HC_HOST_CTRL1_HS_ENABLE;
> +  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof
> (HostCtrl1), &HostCtrl1);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> @@ -853,26 +851,6 @@ EmmcSwitchToHS200 (
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> -  //
> -  // Wait Internal Clock Stable in the Clock Control register to be 1 before 
> set
> SD Clock Enable bit
> -  //
> -  Status = SdMmcHcWaitMmioSet (
> -             PciIo,
> -             Slot,
> -             SD_MMC_HC_CLOCK_CTRL,
> -             sizeof (ClockCtrl),
> -             BIT1,
> -             BIT1,
> -             SD_MMC_HC_GENERIC_TIMEOUT
> -             );
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -  //
> -  // Set SD Clock Enable in the Clock Control register to 1
> -  //
> -  ClockCtrl = BIT2;
> -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
> 
>    HsTiming = 2;
>    Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> ClockFreq);
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to