From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4864:20::d2b; helo=mail-io1-xd2b.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd2b.google.com (mail-io1-xd2b.google.com [IPv6:2607:f8b0:4864:20::d2b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2AF5221962301 for ; Sat, 15 Sep 2018 15:24:50 -0700 (PDT) Received: by mail-io1-xd2b.google.com with SMTP id l14-v6so8721205iob.7 for ; Sat, 15 Sep 2018 15:24:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=1IAuNBqXvemXZ2N224FwJQrmXt+Rhec4iqfH0Mqx7bI=; b=PMhWvNDtq0sfZNd9XEsDBiJyhcIPc+KkAgrrzZ7+naWCWqc6aWvsTXVmiT54U3kcOQ dvcRFBxJlJFPcVBhBdQs3zpKFiQvHCg/T0d2CpHo8vHDGPRYazIoJCTxfYxaWfALZRzq MONiTsCn/GdXcX8YWqc9LJiH23pP/MMKsYCpLBU1/U6M6+Pe/rBMmZ+mS/lcZ7m/RLhK r0/gWVs/qRAFD1ET2eVXBPvOEyxHrIQrcYL7mZCaIi8xto7/5P9XDcsg6fovXOKstxTx huS84A3jlGXhTEAAOUEtWLa1j1p7BPsTcENvKiLnE7bKM/aTxA52fsGZElNRCG9+2BkK 4rDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1IAuNBqXvemXZ2N224FwJQrmXt+Rhec4iqfH0Mqx7bI=; b=dUjdR2TLVg96Cl0T7jbTtAIbvjpBZBC9Pa3CtSfVQS0//n/JEVPRNgFK+2FangLqW7 PgLdu/ynmEI/D51/XyX/7J+OpoqdJO/5IbTYPbr0rhHren+8qCrcKRudrj28DtY1SZnE jKgB3zsVARpOvlum57ocGiCMvXefCAr0soRksF+QOcjUGhdlhFy5b/zFb5cfiG87hfnc 720ZwVdXeC/gTTvh3dvHRQ4pAuUreyEMHu68GEjRQ0e33baDN5RfQrVgYm1EZRNTlC8Z 3Ha7JXMyVwjRBb0jjGNweTb40L0PucrO+xgy9xxqdRqwoWW8Y6Gjo9XS0OJbd7hF4RnX FCqw== X-Gm-Message-State: APzg51A9FjFF5Ib/xYk6VL2jWziDBCVIiCvq501XAWNyAVQ2FWOTJ/6l QBJnZokO6t9kx4mGXiWM0wTJXLqD7qtnRRpaOZiXzg== X-Google-Smtp-Source: ANB0VdZYuCLX+UNdSrbCIoPuNycsfKH37TB8NaChRSoW1ZakBgtAnteKTOFlxseGDDh/PmlIKTe2gGOdF0QNS9ChHbo= X-Received: by 2002:a5e:c70c:: with SMTP id f12-v6mr16313778iop.108.1537050289061; Sat, 15 Sep 2018 15:24:49 -0700 (PDT) MIME-Version: 1.0 References: <1536311416-2751-1-git-send-email-mw@semihalf.com> <1536311416-2751-2-git-send-email-mw@semihalf.com> In-Reply-To: From: Marcin Wojtas Date: Sun, 16 Sep 2018 00:24:35 +0200 Message-ID: To: hao.a.wu@intel.com Cc: edk2-devel-01 , Tomasz Michalec , nadavh@marvell.com, "Gao, Liming" , "Kinney, Michael D" Subject: Re: [PATCH v2 1/4] MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Sep 2018 22:24:50 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Hao, Thank you for your input. In v3, I'll withdraw this patch. I tested and I can use HS200 successfully on my boards without it. Both changes (HS enable bit and removing clock disabling) were residues from an old debug, whereas the acutal fix was an additional HW reconfiguration after switching the clock frequency. Best regards, Marcin czw., 13 wrz 2018 o 16:43 Wu, Hao A napisa=C5=82(a): > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Marcin Wojtas > > Sent: Friday, September 07, 2018 5:10 PM > > To: edk2-devel@lists.01.org > > Cc: Tian, Feng; tm@semihalf.com; nadavh@marvell.com; Gao, Liming; Kinne= y, > > 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.8= V > Signaling Enable' (BIT3) and 'UHS Mode Select' (BIT2~0) fields of the Hos= t > 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 th= e > 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 selecte= d > * by this field. Host Driver needs to reset SD Clock Enable before changi= ng > * 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 > > --- > > 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 !=3D 4) && (BusWidth !=3D 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 =3D SdMmcHcStopClock (PciIo, Slot); > > + HostCtrl1 =3D SD_MMC_HC_HOST_CTRL1_HS_ENABLE; > > + Status =3D 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 =3D 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 =3D BIT2; > > - Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof > > (ClockCtrl), &ClockCtrl); > > > > HsTiming =3D 2; > > Status =3D EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming= , > > ClockFreq); > > -- > > 2.7.4 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel