From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::142; helo=mail-it1-x142.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (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 A719021A07A92 for ; Thu, 8 Nov 2018 03:10:44 -0800 (PST) Received: by mail-it1-x142.google.com with SMTP id e11so1017451itl.5 for ; Thu, 08 Nov 2018 03:10:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=rxqYH0yAa73zEmuhJEb7X3YURijuSMPJgPVSqKruY8A=; b=P+x8T6/8uB5IbC4f3dlVHHPI8f2tGZ56PlvU1jbZttDMNbaEIb0COlNMKCKbwLHHSK GX3rff+r1u7Hb2WUxN0u4rP8dGfQyWPEHubhRWi5773VIHTd6v1Y8N27mXdjJL5TgDzk 44a4H8x/B+r+C6AAcOCIJOMm66ZXaRjLj98dc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=rxqYH0yAa73zEmuhJEb7X3YURijuSMPJgPVSqKruY8A=; b=PQr4qkXybqGxq79sUyRKLTkq8CEt6DP6ahTnxSDXRu3rkvBkikpdYzesVoje2g7Mx4 m9UNQ5+c7XzMEY9IC6jeIQI6Ah2SIbMB2LC0MrgzT0fO+yKShZVb1uH5PUEcbcGTbbiu qNOjOm3hBEweB+HVezwpu9sbfxaxM3pAwJrKBQK0RpWJa0/gridc1C433Q98bSOS18yM XYVsz7M7mRScHOJIyEy7PHbNE4wKlkUyW2PIH0dL2mfv1FdKR1HbozilTAwzChs7/4Ih bRh257bCMHWveaFInksz4n27jvSFVK3SDZfDeldcQQ1GMbpQ5ARCr78kwEnsacIZnZlD j1qw== X-Gm-Message-State: AGRZ1gLqjkHI6hqImEskw6icsMywEdVRxJPN63jeak5KSjAa3+kGrgaY W+z5VfV16Xs/fPGV0mUUvQt/Xqo9Fn9VzDs1DI5TdA== X-Google-Smtp-Source: AJdET5d1TI+9WIAVs/1L0s3NLfP/sa01c0Ypss+eHDjW1IdjjaHxqdm91+6GEFtwH1kxG7ivYVXkJjfF83LaGt1Ur+U= X-Received: by 2002:a02:9f85:: with SMTP id a5-v6mr3717156jam.2.1541675443906; Thu, 08 Nov 2018 03:10:43 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a6b:4f16:0:0:0:0:0 with HTTP; Thu, 8 Nov 2018 03:10:43 -0800 (PST) In-Reply-To: <1541642255-15602-5-git-send-email-mw@semihalf.com> References: <1541642255-15602-1-git-send-email-mw@semihalf.com> <1541642255-15602-5-git-send-email-mw@semihalf.com> From: Ard Biesheuvel Date: Thu, 8 Nov 2018 12:10:43 +0100 Message-ID: To: Marcin Wojtas Cc: "edk2-devel@lists.01.org" , Leif Lindholm , "Wu, Hao A" , "Kinney, Michael D" , "Gao, Liming" , Nadav Haklai , =?UTF-8?B?SmFuIETEhWJyb8Wb?= , Tomasz Michalec , Grzegorz Jaszczyk Subject: Re: [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency 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, 08 Nov 2018 11:10:45 -0000 Content-Type: text/plain; charset="UTF-8" On 8 November 2018 at 02:57, Marcin Wojtas wrote: > Some SdMmc host controllers are run by clocks with different > frequency than it is reflected in Capabilities Register 1. > It is allowed by SDHCI specification ver. 4.2 - if BaseClkFreq > field value of the Capability Register 1 is zero, the clock > frequency must be obtained via another method. > > Because the bitfield is only 8 bits wide, a maximum value > that could be obtained from hardware is 255MHz. > In case the actual frequency exceeds 255MHz, the 8-bit BaseClkFreq > member of SD_MMC_HC_SLOT_CAP structure occurs to be not sufficient > to be used for setting the clock speed in SdMmcHcClockSupply > function. > > This patch adds new UINT32 array ('BaseClkFreq[]') to > SD_MMC_HC_PRIVATE_DATA structure for specifying > the input clock speed for each slot of the host controller. > All routines that are used for clock configuration are > updated accordingly. > > This patch also adds new IN OUT BaseClockFreq field > in the Capability callback of the SdMmcOverride, > protocol which allows to update BaseClkFreq value. > > The patch reuses original commit from edk2-platforms: > 20f6f144d3a8 ("Marvell/Drivers: XenonDxe: Allow overriding base clock > frequency") > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas Reviewed-by: Ard Biesheuvel > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +++++ > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 8 +++---- > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 7 ++++-- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 4 ++-- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 ++-- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 ++++++++++- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 23 ++++++++++---------- > 7 files changed, 43 insertions(+), 22 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > index c683600..8c1a589 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > @@ -118,6 +118,12 @@ typedef struct { > UINT64 MaxCurrent[SD_MMC_HC_MAX_SLOT]; > > UINT32 ControllerVersion; > + > + // > + // Some controllers may require to override base clock frequency > + // value stored in Capabilities Register 1. > + // > + UINT32 BaseClkFreq[SD_MMC_HC_MAX_SLOT]; > } SD_MMC_HC_PRIVATE_DATA; > > #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T') > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > index 1a11d51..8eefc31 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > @@ -423,7 +423,7 @@ SdMmcHcStopClock ( > @param[in] PciIo The PCI IO protocol instance. > @param[in] Slot The slot number of the SD card to send the command to. > @param[in] ClockFreq The max clock frequency to be set. The unit is KHz. > - @param[in] Capability The capability of the slot. > + @param[in] BaseClkFreq The base clock frequency of host controller in MHz. > > @retval EFI_SUCCESS The clock is supplied successfully. > @retval Others The clock isn't supplied successfully. > @@ -434,7 +434,7 @@ SdMmcHcClockSupply ( > IN EFI_PCI_IO_PROTOCOL *PciIo, > IN UINT8 Slot, > IN UINT64 ClockFreq, > - IN SD_MMC_HC_SLOT_CAP Capability > + IN UINT32 BaseClkFreq > ); > > /** > @@ -482,7 +482,7 @@ SdMmcHcSetBusWidth ( > > @param[in] PciIo The PCI IO protocol instance. > @param[in] Slot The slot number of the SD card to send the command to. > - @param[in] Capability The capability of the slot. > + @param[in] BaseClkFreq The base clock frequency of host controller in MHz. > > @retval EFI_SUCCESS The clock is supplied successfully. > @retval Others The clock isn't supplied successfully. > @@ -492,7 +492,7 @@ EFI_STATUS > SdMmcHcInitClockFreq ( > IN EFI_PCI_IO_PROTOCOL *PciIo, > IN UINT8 Slot, > - IN SD_MMC_HC_SLOT_CAP Capability > + IN UINT32 BaseClkFreq > ); > > /** > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > index 6160b5b..0aaf258 100644 > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > @@ -22,7 +22,7 @@ > #define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \ > { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23, 0x23 } } > > -#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION 0x1 > +#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION 0x2 > > typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE; > > @@ -58,6 +58,8 @@ typedef enum { > @param[in] ControllerHandle The EFI_HANDLE of the controller. > @param[in] Slot The 0 based slot index. > @param[in,out] SdMmcHcSlotCapability The SDHCI capability structure. > + @param[in,out] BaseClkFreq The base clock frequency value that > + optionally can be updated. > > @retval EFI_SUCCESS The override function completed successfully. > @retval EFI_NOT_FOUND The specified controller or slot does not exist. > @@ -69,7 +71,8 @@ EFI_STATUS > (EFIAPI * EDKII_SD_MMC_CAPABILITY) ( > IN EFI_HANDLE ControllerHandle, > IN UINT8 Slot, > - IN OUT VOID *SdMmcHcSlotCapability > + IN OUT VOID *SdMmcHcSlotCapability, > + IN OUT UINT32 *BaseClkFreq > ); > > /** > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > index 6fc6871..0393fd4 100755 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > @@ -705,7 +705,7 @@ EmmcSwitchClockFreq ( > // > // Convert the clock freq unit from MHz to KHz. > // > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]); > + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]); > > return Status; > } > @@ -1098,7 +1098,7 @@ EmmcSetBusMode ( > return Status; > } > > - ASSERT (Private->Capability[Slot].BaseClkFreq != 0); > + ASSERT (Private->BaseClkFreq[Slot] != 0); > // > // Check if the Host Controller support 8bits bus width. > // > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > index 5408bbc..a9d0d3a 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > @@ -882,7 +882,7 @@ SdCardSetBusMode ( > } > } > > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability); > + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -1082,7 +1082,7 @@ SdCardIdentification ( > goto Error; > } > > - SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]); > + SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]); > > gBS->Stall (1000); > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > index bf9869d..a87f8de 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > @@ -625,11 +625,16 @@ SdMmcPciHcDriverBindingStart ( > if (EFI_ERROR (Status)) { > continue; > } > + > + Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq; > + > if (mOverride != NULL && mOverride->Capability != NULL) { > Status = mOverride->Capability ( > Controller, > Slot, > - &Private->Capability[Slot]); > + &Private->Capability[Slot], > + &Private->BaseClkFreq[Slot] > + ); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n", > __FUNCTION__, Status)); > @@ -637,6 +642,12 @@ SdMmcPciHcDriverBindingStart ( > } > } > DumpCapabilityReg (Slot, &Private->Capability[Slot]); > + DEBUG (( > + DEBUG_INFO, > + "Slot[%d] Base Clock Frequency: %dMHz\n", > + Slot, > + Private->BaseClkFreq[Slot] > + )); > > Support64BitDma &= Private->Capability[Slot].SysBus64; > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > index 85aa625..040a959 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > @@ -721,7 +721,7 @@ SdMmcHcStopClock ( > @param[in] PciIo The PCI IO protocol instance. > @param[in] Slot The slot number of the SD card to send the command to. > @param[in] ClockFreq The max clock frequency to be set. The unit is KHz. > - @param[in] Capability The capability of the slot. > + @param[in] BaseClkFreq The base clock frequency of host controller in MHz. > > @retval EFI_SUCCESS The clock is supplied successfully. > @retval Others The clock isn't supplied successfully. > @@ -732,11 +732,10 @@ SdMmcHcClockSupply ( > IN EFI_PCI_IO_PROTOCOL *PciIo, > IN UINT8 Slot, > IN UINT64 ClockFreq, > - IN SD_MMC_HC_SLOT_CAP Capability > + IN UINT32 BaseClkFreq > ) > { > EFI_STATUS Status; > - UINT32 BaseClkFreq; > UINT32 SettingFreq; > UINT32 Divisor; > UINT32 Remainder; > @@ -746,9 +745,8 @@ SdMmcHcClockSupply ( > // > // Calculate a divisor for SD clock frequency > // > - ASSERT (Capability.BaseClkFreq != 0); > + ASSERT (BaseClkFreq != 0); > > - BaseClkFreq = Capability.BaseClkFreq; > if (ClockFreq == 0) { > return EFI_INVALID_PARAMETER; > } > @@ -940,7 +938,7 @@ SdMmcHcSetBusWidth ( > > @param[in] PciIo The PCI IO protocol instance. > @param[in] Slot The slot number of the SD card to send the command to. > - @param[in] Capability The capability of the slot. > + @param[in] BaseClkFreq The base clock frequency of host controller in MHz. > > @retval EFI_SUCCESS The clock is supplied successfully. > @retval Others The clock isn't supplied successfully. > @@ -950,16 +948,19 @@ EFI_STATUS > SdMmcHcInitClockFreq ( > IN EFI_PCI_IO_PROTOCOL *PciIo, > IN UINT8 Slot, > - IN SD_MMC_HC_SLOT_CAP Capability > + IN UINT32 BaseClkFreq > ) > { > EFI_STATUS Status; > UINT32 InitFreq; > > // > - // Calculate a divisor for SD clock frequency > + // According to SDHCI specification ver. 4.2, BaseClkFreq field value of > + // the Capability Register 1 can be zero, which means a need for obtaining > + // the clock frequency via another method. Fail in case it is not updated > + // by SW at this point. > // > - if (Capability.BaseClkFreq == 0) { > + if (BaseClkFreq == 0) { > // > // Don't support get Base Clock Frequency information via another method > // > @@ -969,7 +970,7 @@ SdMmcHcInitClockFreq ( > // Supply 400KHz clock frequency at initialization phase. > // > InitFreq = 400; > - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, Capability); > + Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq); > return Status; > } > > @@ -1103,7 +1104,7 @@ SdMmcHcInitHost ( > PciIo = Private->PciIo; > Capability = Private->Capability[Slot]; > > - Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability); > + Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]); > if (EFI_ERROR (Status)) { > return Status; > } > -- > 2.7.4 >