From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c06::244; helo=mail-io0-x244.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x244.google.com (mail-io0-x244.google.com [IPv6:2607:f8b0:4001:c06::244]) (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 72A6A2034A876 for ; Fri, 27 Oct 2017 08:26:35 -0700 (PDT) Received: by mail-io0-x244.google.com with SMTP id 97so13493360iok.7 for ; Fri, 27 Oct 2017 08:30:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=wQvCO8+JRicpB/3d8Kvvd/GlkZy9MrYyAJ9gt035Ue4=; b=SU8XAnpz7W1qxP7qLrXQ5YfWrCTOXPfA7ah05YQRP0RHmCfGW0XfxZLwkhnevExAS+ GwVhBHp2ECqesU/LSi+8eiJUD21FLSWylCaPZeB9vxb5+F/ueKbaYZapPcFTBhVerBgZ Id98eZjMj+JwpncBnLfOKEuoZm5UYoB6co8kX4EIs6jZNNbT98xwQPsW0ZkZCHlNoFaq QfACAiBaAU2VHnJ+OIe6MD6taNlubu5HskJetWkPnRrmDqpEJP3+MpIcq9NwcVDBIpCa /XtV/8OEn0PE+4VlKqTAiPCYchpUAOgTjdFjQuu7D2hqQx95L61Q2uPm9Em5CXhJhkgu kQHg== 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=wQvCO8+JRicpB/3d8Kvvd/GlkZy9MrYyAJ9gt035Ue4=; b=G1WYpl1ILYpFUfMYo42Y1hJWUhFb7ixr6ugrjsRQmdeKYiMJfqS+43zwDWxLbGAM8L gwmJisXJYg++NvpkDRK/0FLkjojdUPVJ3ivMf5oFuFAbxSYt+YjRA3JWX2CCBAy7PPsM bn2BB9Lh4de6GznQ/a+Slr+vCCnLeuZ6Y47Go/GuAuyoG4oDmsBiP3QkPOMwWb8aDQVO bJZvwjs4wtb3Rtj5+Xu2ROqCfzMxnDekauiAg7puZrI/hpsO8Qq7jOILgcOqcHw4VaBE XJVZTXdFZiEac45mZxEsKjtN794fSN/eo0Aris9wlf4rot/Kr0twVuLlc05m1iGHqhVA TQiw== X-Gm-Message-State: AMCzsaXCr1w3QUcRaIfaFZXpk4EtLG/NT5N8WlZ7VXTFg937UyE5IJUt 6LL4JVYOFe9INCKjhokCvWNFYYnXYm2XPw30dDmSqw== X-Google-Smtp-Source: ABhQp+TR8oEF8fXVpd88tCnKuOfh42949Tdad54Nx5d7vkSSPGRr7uIFOsI5/3LUdWUnFfyRP1I5/j4avfmCfZnSFK0= X-Received: by 10.36.105.65 with SMTP id e62mr1151885itc.16.1509118221856; Fri, 27 Oct 2017 08:30:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.167.208 with HTTP; Fri, 27 Oct 2017 08:30:21 -0700 (PDT) In-Reply-To: <20171027145144.v26za4zpw4rh5tuo@bivouac.eciton.net> References: <1509066832-5285-1-git-send-email-mw@semihalf.com> <1509066832-5285-10-git-send-email-mw@semihalf.com> <20171027145144.v26za4zpw4rh5tuo@bivouac.eciton.net> From: Marcin Wojtas Date: Fri, 27 Oct 2017 17:30:21 +0200 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , semihalf-dabros-jan Subject: Re: [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2017 15:26:35 -0000 Content-Type: text/plain; charset="UTF-8" 2017-10-27 16:51 GMT+02:00 Leif Lindholm : > On Fri, Oct 27, 2017 at 03:13:51AM +0200, Marcin Wojtas wrote: >> Some SdMmc host controllers are run by clocks with different >> frequency than it is reflected in Capabilities Register 1. >> Because the bitfield is only 8 bits wide, a maximum value >> that could be obtained from hardware is 255(MHz). >> >> 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. >> >> Thanks to above the Xenon host controller driver >> could be modified to configure clock speed relatively >> to actual 400MHz input. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Marcin Wojtas >> --- >> Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c | 4 ++-- >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c | 4 ++-- >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 13 ++++++++---- >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h | 6 ++++++ >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c | 22 ++++++++++---------- >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h | 12 ++++++----- >> 6 files changed, 37 insertions(+), 24 deletions(-) >> >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c >> index 4d4833f..530a01c 100755 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/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; >> } >> @@ -1007,7 +1007,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/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c >> index 9122848..ea7eed7 100644 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c >> @@ -972,7 +972,7 @@ SdCardSetBusMode ( >> return Status; >> } >> >> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability); >> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]); >> if (EFI_ERROR (Status)) { >> return Status; >> } >> @@ -1144,7 +1144,7 @@ SdCardIdentification ( >> goto Error; >> } >> >> - SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]); >> + SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]); >> >> gBS->Stall (1000); >> >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c >> index 981eab5..10e15c5 100644 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c >> @@ -291,7 +291,10 @@ SdMmcPciHcEnumerateDevice ( >> // >> // Reinitialize slot and restart identification process for the new attached device >> // >> - Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]); >> + Status = SdMmcHcInitHost (Private->PciIo, >> + Slot, >> + Private->Capability[Slot], >> + Private->BaseClkFreq[Slot]); >> if (EFI_ERROR (Status)) { >> continue; >> } >> @@ -617,9 +620,11 @@ SdMmcPciHcDriverBindingStart ( >> Private->Capability[Slot].Sdr50 = 0; >> Private->Capability[Slot].BusWidth8 = 0; >> >> - if (Private->Capability[Slot].BaseClkFreq == 0) { >> - Private->Capability[Slot].BaseClkFreq = 0xff; >> - } >> + // >> + // Override inappropriate base clock frequency from Capabilities Register 1. >> + // Actual clock speed of Xenon controller is 400MHz. >> + // >> + Private->BaseClkFreq[Slot] = XENON_MMC_MAX_CLK / 1000 / 1000; >> >> DumpCapabilityReg (Slot, &Private->Capability[Slot]); >> >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h >> index 6a2a279..067b9ac 100644 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h >> @@ -115,6 +115,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/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> index ccbf355..706618d 100644 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c > > If doing this rework, there should probably be an addition to > DumpCapabilityReg to at least point out that Capability->BaseClkFreq > is ignored, rather than just printing the (now unused) value. > Ok, I'll sort that out. > Otherwise, this looks sort of OK. > Great, thanks. Marcin > / > Leif > >> @@ -678,7 +678,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. >> @@ -689,11 +689,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; >> @@ -703,9 +702,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; >> } >> @@ -896,7 +894,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. >> @@ -906,7 +904,7 @@ 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; >> @@ -915,7 +913,7 @@ SdMmcHcInitClockFreq ( >> // >> // Calculate a divisor for SD clock frequency >> // >> - if (Capability.BaseClkFreq == 0) { >> + if (BaseClkFreq == 0) { >> // >> // Don't support get Base Clock Frequency information via another method >> // >> @@ -925,7 +923,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; >> } >> >> @@ -1024,6 +1022,7 @@ SdMmcHcInitTimeoutCtrl ( >> @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 host controller is initialized successfully. >> @retval Others The host controller isn't initialized successfully. >> @@ -1033,12 +1032,13 @@ EFI_STATUS >> SdMmcHcInitHost ( >> IN EFI_PCI_IO_PROTOCOL *PciIo, >> IN UINT8 Slot, >> - IN SD_MMC_HC_SLOT_CAP Capability >> + IN SD_MMC_HC_SLOT_CAP Capability, >> + IN UINT32 BaseClkFreq >> ) >> { >> EFI_STATUS Status; >> >> - Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability); >> + Status = SdMmcHcInitClockFreq (PciIo, Slot, BaseClkFreq); >> if (EFI_ERROR (Status)) { >> return Status; >> } >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h >> index fb62758..a4ec4fe 100644 >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h >> @@ -414,7 +414,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. >> @@ -425,7 +425,7 @@ SdMmcHcClockSupply ( >> IN EFI_PCI_IO_PROTOCOL *PciIo, >> IN UINT8 Slot, >> IN UINT64 ClockFreq, >> - IN SD_MMC_HC_SLOT_CAP Capability >> + IN UINT32 BaseClockFreq >> ); >> >> /** >> @@ -473,7 +473,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. >> @@ -483,7 +483,7 @@ EFI_STATUS >> SdMmcHcInitClockFreq ( >> IN EFI_PCI_IO_PROTOCOL *PciIo, >> IN UINT8 Slot, >> - IN SD_MMC_HC_SLOT_CAP Capability >> + IN UINT32 BaseClockFreq >> ); >> >> /** >> @@ -531,6 +531,7 @@ SdMmcHcInitTimeoutCtrl ( >> @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 host controller is initialized successfully. >> @retval Others The host controller isn't initialized successfully. >> @@ -540,7 +541,8 @@ EFI_STATUS >> SdMmcHcInitHost ( >> IN EFI_PCI_IO_PROTOCOL *PciIo, >> IN UINT8 Slot, >> - IN SD_MMC_HC_SLOT_CAP Capability >> + IN SD_MMC_HC_SLOT_CAP Capability, >> + IN UINT32 BaseClockFreq >> ); >> >> #endif >> -- >> 2.7.4 >>