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::d42; helo=mail-io1-xd42.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) (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 739FE2118AA97 for ; Fri, 2 Nov 2018 02:39:41 -0700 (PDT) Received: by mail-io1-xd42.google.com with SMTP id a5-v6so920394ioq.8 for ; Fri, 02 Nov 2018 02:39:41 -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=35Mv162PRcAfKDYkPud/U8G7h7IGtwMBvLiecuHXUZ0=; b=eFCidxyghq2v4gPLlGmKYeF8/KEB+nKEMwXH6avFGJq1lSvA83S6pate7feZ2TH1wP iysS/5x8ZssLj0aWp9PxyjmY/hiuP3jo1DeT6s4xtukB/CRQQlNDSbIukZDV4YrT0HDm t5wE/IMVxZblpCn5QWH/mykG0TOS2WwcKLD1QBJdLRO4ylDJc1ThvK5NoFBwas2ECYEk +K4R8V2CwAzCvQiQKUcbcGtBdVQiciRWELcVQlRRq9XNXncm4QN/TSfVPWDSD6bNA3Q+ g7033w8KoX8IuTYBeQsGe+4jHdhMhox6yNQQ49p3mxFWrP1DGnc6enLdpM7R+5MCZKNv 457w== 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=35Mv162PRcAfKDYkPud/U8G7h7IGtwMBvLiecuHXUZ0=; b=SazAevf6CtEj1ckxPnGGIdlwslPtuFDEiAwW16O+4bSHMcwOR3rwX2AaNWzwTrxTad OYQ22efXhcvHr34i+TRGYL+uslWPKvRSKlJGYmQvoVl6Qs2hTuqkvBCvKjo0ieHEvC8H CPPda5/gn4dP9CIDR/WNR6B4nHKAzMw8QXp9scKpCbmK4Ynp0/ByhvfKdH29QZytdcVJ nXpYt6wWy307ir+dimU3weDKbzOAtXgGVx/18OPIEdX7+YmpF4+sUl3j+b0trd61j3B3 G8JmlO2KH2qZgWTKMQ4vPzQq9waTDag2NrSUWGy1nfU0wt6ziLT026fYXkwlpBYwGsiN 1SMg== X-Gm-Message-State: AGRZ1gJc7rwXQPYLsWQoDLLsqW5jAno7p6/xZbsRHVEVZkGr5GMTf4Ki 7bv++rjf0Lod6Iaa6i8PpR1ZHUEAvPICjj85VhUMxA== X-Google-Smtp-Source: AJdET5d7BWHqdexRyNg9gKTjHgw7VrSeGjbBbYv2MQk0FKPZWZ2xuoAL5KuVoL/s0LnBP0e5KbemEe36VPVdfNCTsCY= X-Received: by 2002:a6b:8ec9:: with SMTP id q192-v6mr8144821iod.248.1541151579208; Fri, 02 Nov 2018 02:39:39 -0700 (PDT) MIME-Version: 1.0 References: <1538745911-22484-1-git-send-email-mw@semihalf.com> <1538745911-22484-4-git-send-email-mw@semihalf.com> In-Reply-To: From: Marcin Wojtas Date: Fri, 2 Nov 2018 10:39:28 +0100 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 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride 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: Fri, 02 Nov 2018 09:39:41 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Hao, czw., 1 lis 2018 o 08:06 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, October 05, 2018 9:25 PM > > To: edk2-devel@lists.01.org > > Cc: Tian, Feng; tm@semihalf.com; Wu, Hao A; nadavh@marvell.com; Gao, > > Liming; Kinney, Michael D > > Subject: [edk2] [PATCH v2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add > > SwitchClockFreqPost to SdMmcOverride > > > > From: Tomasz Michalec > > > > Some SD Host Controlers need to do additional opperations after clock > > frequency switch. > > > > This patch add new callback type to NotifyPhase of the SdMmcOverride > > protocol. It is called after EmmcSwitchClockFreq and SdMmcHcClockSupply= . > > Hi Marcin, > > Just curious, I had a quick glance at the implementation of the > XenonSwitchClockFreqPost() in your platform part changes. Are those opera= tions > within the function mandatory during the HC initialization? Are they main= ly for > performance or stability consideration? As for Marvellt he Xenon controller is pretty complicated IP, which consists of standard Sd/Mmc part and the dedicated PHY, that's responsible for signal integrity for all bus modes. It requires additional configuration, depending on the mode. > > I am wondering if this kind of customization is common among the SD & eMM= C devices. Well, in Linux this clock tuning after switching to certain bus mode. This driver simply does it in 'set_ios' callback, whose custom implementation (platform-specific code surrounding generic sdhci_set_ios call) is _very_ common among all drivers/mmc/host drivers. Do you have any objections to the patch itself, given above explanation? Best regards, Marcin > > Best Regards, > Hao Wu > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 1 + > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 60 > > ++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++ > > 3 files changed, 79 insertions(+) > > > > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > index 25db98a..d9daada 100644 > > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > @@ -33,6 +33,7 @@ typedef enum { > > EdkiiSdMmcInitHostPre, > > EdkiiSdMmcInitHostPost, > > EdkiiSdMmcUhsSignaling, > > + EdkiiSdMmcSwitchClockFreqPost, > > } EDKII_SD_MMC_PHASE_TYPE; > > > > /** > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > index 05bd4a0..7e75283 100755 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > @@ -796,6 +796,27 @@ EmmcSwitchToHighSpeed ( > > > > HsTiming =3D 1; > > Status =3D EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming= , > > ClockFreq); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcSwitchClockFreqPost, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC switch clock freq post notifier callback failed - = %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } > > > > return Status; > > } > > @@ -905,6 +926,24 @@ EmmcSwitchToHS200 ( > > return Status; > > } > > > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcSwitchClockFreqPost, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC switch clock freq post notifier callback failed - = %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } > > + > > Status =3D EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth); > > > > return Status; > > @@ -989,6 +1028,27 @@ EmmcSwitchToHS400 ( > > > > HsTiming =3D 3; > > Status =3D EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming= , > > ClockFreq); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcSwitchClockFreqPost, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC switch clock freq post notifier callback failed - = %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } > > > > return Status; > > } > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > index 5645a71..057a4e2 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > @@ -887,6 +887,24 @@ SdCardSetBusMode ( > > return Status; > > } > > > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcSwitchClockFreqPost, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC switch clock freq post notifier callback failed - = %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } > > + > > if ((AccessMode =3D=3D 3) || ((AccessMode =3D=3D 2) && (Capability- > > >TuningSDR50 !=3D 0))) { > > Status =3D SdCardTuningClock (PciIo, PassThru, Slot); > > if (EFI_ERROR (Status)) { > > -- > > 2.7.4 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel