Hi Hao,
czw., 1 lis 2018 o 08:11 Wu, Hao A <[email protected]> napisał(a): > > Hi Marcin, > > > -----Original Message----- > > From: Marcin Wojtas [mailto:[email protected]] > > Sent: Friday, October 05, 2018 9:25 PM > > To: [email protected] > > Cc: Tian, Feng; Kinney, Michael D; Gao, Liming; [email protected]; > > Wu, > > Hao A; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected] > > Subject: [PATCH v2 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding > > base clock frequency > > > > 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 <[email protected]> > > --- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +++++ > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 12 +++++---- > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 5 +++- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 4 +-- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 +-- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 18 > > +++++++++++-- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 27 > > +++++++++++--------- > > 7 files changed, 52 insertions(+), 24 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 a03160d..f01ba21 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > > @@ -173,12 +173,14 @@ typedef struct { > > > > @param[in] Slot The slot number of the SD card to send the > > command to. > > @param[in] Capability The buffer to store the capability data. > > + @param[in] BaseClkFreq The base clock frequency of host controller > > in > > MHz. > > > > **/ > > VOID > > DumpCapabilityReg ( > > IN UINT8 Slot, > > - IN SD_MMC_HC_SLOT_CAP *Capability > > + IN SD_MMC_HC_SLOT_CAP *Capability, > > + IN UINT32 BaseClkFreq > > ); > > > > /** > > @@ -431,7 +433,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. > > @@ -442,7 +444,7 @@ SdMmcHcClockSupply ( > > IN EFI_PCI_IO_PROTOCOL *PciIo, > > IN UINT8 Slot, > > IN UINT64 ClockFreq, > > - IN SD_MMC_HC_SLOT_CAP Capability > > + IN UINT32 BaseClkFreq > > ); > > > > /** > > @@ -490,7 +492,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. > > @@ -500,7 +502,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 d9daada..27023d3 100644 > > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > @@ -43,6 +43,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. > > @@ -54,7 +56,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 7e75283..27ccd63 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; > > } > > @@ -1099,7 +1099,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 057a4e2..9ea13be 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; > > } > > @@ -1081,7 +1081,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..d7cc0ce 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > > @@ -625,18 +625,32 @@ 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)); > > continue; > > } > > } > > - DumpCapabilityReg (Slot, &Private->Capability[Slot]); > > + > > + // > > + // 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. > > + // > > + ASSERT (Private->BaseClkFreq[Slot] != 0); > > I think the ASSERT here can be dropped. This value will be handled within > function SdMmcHcInitClockFreq(). And the above newly added comments can be > used > in SdMmcHcInitClockFreq() instead. Right, I missed it - I will remove the ASSERT here, as the protection against 0 value is already there. > > > + > > + DumpCapabilityReg (Slot, &Private->Capability[Slot], Private- > > >BaseClkFreq[Slot]); > > My thought on this one is that you can leave DumpCapabilityReg() unchanged and > append a DEBUG() call with DEBUG_INFO level to output the value of > Private->BaseClkFreq for a slot. Ok, I will append the print separately. Thanks, Marcin > > > > Support64BitDma &= Private->Capability[Slot].SysBus64; > > > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > index 38d6202..9f50754 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > @@ -22,12 +22,14 @@ > > > > @param[in] Slot The slot number of the SD card to send the > > command to. > > @param[in] Capability The buffer to store the capability data. > > + @param[in] BaseClkFreq The base clock frequency of host controller > > in > > MHz. > > > > **/ > > VOID > > DumpCapabilityReg ( > > IN UINT8 Slot, > > - IN SD_MMC_HC_SLOT_CAP *Capability > > + IN SD_MMC_HC_SLOT_CAP *Capability, > > + IN UINT32 BaseClkFreq > > ) > > { > > // > > @@ -35,7 +37,10 @@ DumpCapabilityReg ( > > // > > DEBUG ((DEBUG_INFO, " == Slot [%d] Capability is 0x%x ==\n", Slot, > > Capability)); > > DEBUG ((DEBUG_INFO, " Timeout Clk Freq %d%a\n", Capability- > > >TimeoutFreq, (Capability->TimeoutUnit) ? "MHz" : "KHz")); > > - DEBUG ((DEBUG_INFO, " Base Clk Freq %dMHz\n", Capability- > > >BaseClkFreq)); > > + if (Capability->BaseClkFreq != BaseClkFreq) { > > + DEBUG ((DEBUG_INFO, " Controller register value overriden:\n")); > > + } > > + DEBUG ((DEBUG_INFO, " Base Clk Freq %dMHz\n", BaseClkFreq)); > > DEBUG ((DEBUG_INFO, " Max Blk Len %dbytes\n", 512 * (1 << > > Capability->MaxBlkLen))); > > DEBUG ((DEBUG_INFO, " 8-bit Support %a\n", Capability->BusWidth8 ? > > "TRUE" : "FALSE")); > > DEBUG ((DEBUG_INFO, " ADMA2 Support %a\n", Capability->Adma2 ? > > "TRUE" : "FALSE")); > > @@ -721,7 +726,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 +737,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 +750,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; > > } > > @@ -939,7 +942,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. > > @@ -949,7 +952,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; > > @@ -958,7 +961,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 > > // > > @@ -968,7 +971,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; > > } > > > > @@ -1102,7 +1105,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 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
