From: Marcin Wojtas <mw@semihalf.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
nadavh@marvell.com, Neta Zur Hershkovits <neta@marvell.com>,
Kostya Porotchkin <kostap@marvell.com>,
Hua Jing <jinghua@marvell.com>,
semihalf-dabros-jan <jsd@semihalf.com>
Subject: Re: [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
Date: Fri, 27 Oct 2017 17:30:21 +0200 [thread overview]
Message-ID: <CAPv3WKeNhEVMfrzFxtV5cFG1FQ-779sUwKkNNLE26J6sB9-+0Q@mail.gmail.com> (raw)
In-Reply-To: <20171027145144.v26za4zpw4rh5tuo@bivouac.eciton.net>
2017-10-27 16:51 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> 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 <mw@semihalf.com>
>> ---
>> 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
>>
next prev parent reply other threads:[~2017-10-27 15:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-27 1:13 [platforms: PATCH v2 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
2017-10-27 1:13 ` [platforms: PATCH v2 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail Marcin Wojtas
2017-10-27 14:28 ` Leif Lindholm
2017-10-27 1:13 ` [platforms: PATCH v2 02/10] Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest Marcin Wojtas
2017-10-27 14:29 ` Leif Lindholm
2017-10-27 1:13 ` [platforms: PATCH v2 03/10] Marvell/Drivers: MvI2cDxe: Reduce bus occupation time Marcin Wojtas
2017-10-27 1:13 ` [platforms: PATCH v2 04/10] Marvell/Library: MppLib: Prevent overwriting PCD values Marcin Wojtas
2017-10-27 1:13 ` [platforms: PATCH v2 05/10] Marvell/Library: MppLib: Disable the stack protector Marcin Wojtas
2017-10-27 1:13 ` [platforms: PATCH v2 06/10] Marvell/Library: MppLib: Take 0xFF placeholders into account Marcin Wojtas
2017-10-27 14:30 ` Leif Lindholm
2017-10-27 1:13 ` [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link Marcin Wojtas
2017-10-27 14:36 ` Leif Lindholm
2017-10-27 14:49 ` Marcin Wojtas
2017-10-27 1:13 ` [platforms: PATCH v2 08/10] Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting Marcin Wojtas
2017-10-27 14:37 ` Leif Lindholm
2017-10-27 1:13 ` [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency Marcin Wojtas
2017-10-27 14:51 ` Leif Lindholm
2017-10-27 15:30 ` Marcin Wojtas [this message]
2017-10-27 1:13 ` [platforms: PATCH v2 10/10] Marvell/Drivers: XenonDxe: Do not modify FIFO default values Marcin Wojtas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPv3WKeNhEVMfrzFxtV5cFG1FQ-779sUwKkNNLE26J6sB9-+0Q@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox