2017-10-27 16:51 GMT+02:00 Leif Lindholm <[email protected]>:
> 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 <[email protected]>
>> ---
>>  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
>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to