From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 40DF921B02822 for ; Thu, 13 Sep 2018 07:43:30 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2018 07:43:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,369,1531810800"; d="scan'208";a="89719126" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga001.fm.intel.com with ESMTP; 13 Sep 2018 07:43:10 -0700 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 13 Sep 2018 07:43:08 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 13 Sep 2018 07:43:08 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.143]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0319.002; Thu, 13 Sep 2018 22:43:05 +0800 From: "Wu, Hao A" To: Marcin Wojtas , "edk2-devel@lists.01.org" CC: "tm@semihalf.com" , "nadavh@marvell.com" , "Gao, Liming" , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH v2 1/4] MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation Thread-Index: AQHURorVogOAaOsbUUiEQH3PKok+aaTuNSnA Date: Thu, 13 Sep 2018 14:43:04 +0000 Message-ID: References: <1536311416-2751-1-git-send-email-mw@semihalf.com> <1536311416-2751-2-git-send-email-mw@semihalf.com> In-Reply-To: <1536311416-2751-2-git-send-email-mw@semihalf.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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: Thu, 13 Sep 2018 14:43:31 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----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; Kinney, > Michael D > Subject: [edk2] [PATCH v2 1/4] MdeModulePkg/SdMmcPciHcDxe: Fix HS200 > operation >=20 > 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? >=20 > 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. >=20 > 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(-) >=20 > 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 >=20 > // > +// 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; >=20 > 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 be= fore 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); >=20 > HsTiming =3D 2; > Status =3D EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, > ClockFreq); > -- > 2.7.4 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel