[edk2] [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency

Marcin Wojtas posted 10 patches 7 years, 6 months ago
There is a newer version of this series
[edk2] [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
Posted by Marcin Wojtas 7 years, 6 months ago
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
@@ -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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
Posted by Leif Lindholm 7 years, 6 months ago
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.

Otherwise, this looks sort of OK.

/
    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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
Posted by Marcin Wojtas 7 years, 6 months ago
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
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel