| Message ID | 20180130103240.4669-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [edk2,edk2-platforms] Silicon/Socionext/SynQuacer: add configurable eMMC support | expand |
On Tue, Jan 30, 2018 at 10:32:40AM +0000, Ard Biesheuvel wrote: > Implement support for the SynQuacer eMMC controller. This involves an > implementation of the SD/MMC override protocol to handle a couple of > quirks that would otherwise prevent this IP from being driven by the > generic SDHCI driver. > > Also, add a HII page to the PlatformDxe driver that allows eMMC support > to be enabled, and wire it up for both DeveloperBox and EVB. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > Now that the core support for the SD/MMC override protocol is finally > merged, resubmit this again. I dropped Leif's R-b given that I have > now added DeveloperBox, as well as a HII option to enable eMMC. Couple of minor comments/suggestions below and a question. > Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 + > Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 7 + > Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 + > Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 7 + > Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 1 - > Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 - > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 203 ++++++++++++++++++++ > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 5 + > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 9 + > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 4 + > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni | 6 + > Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr | 8 + > Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h | 6 +- > Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 23 ++- > Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 1 + > 15 files changed, 287 insertions(+), 13 deletions(-) > > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc > index 2d46b4515749..7e69eaba9b70 100644 > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc > @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] > # > PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf > PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf > + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf > > [LibraryClasses.common.UEFI_APPLICATION] > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf > @@ -549,6 +550,13 @@ [Components.common] > MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > # > + # eMMC support > + # > + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf > + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf > + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf > + > + # > # AHCI Support > # > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > index 8443986fc3e7..b668f42c7962 100644 > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf > @@ -148,6 +148,13 @@ [FV.FvMain] > INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > # > + # eMMC support > + # > + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf > + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf > + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf > + > + # > # AHCI Support > # > INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf > diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc > index 6241b5bbd0b2..e35c17f0bcb7 100644 > --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc > +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc > @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] > # > PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf > PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf > + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf > > [LibraryClasses.common.UEFI_APPLICATION] > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf > @@ -549,6 +550,13 @@ [Components.common] > MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > # > + # eMMC support > + # > + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf > + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf > + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf > + > + # > # AHCI Support > # > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf > diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf > index cc61cf42ccd4..ba2f32328c2b 100644 > --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf > +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf > @@ -150,6 +150,13 @@ [FV.FvMain] > INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > # > + # eMMC support > + # > + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf > + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf > + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf > + > + # > # AHCI Support > # > INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf > diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi > index fae3f033f98f..f1daac74973f 100644 > --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi > +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi > @@ -567,6 +567,5 @@ > clocks = <&clk_alw_c_0 &clk_alw_b_0>; > clock-names = "core", "iface"; > dma-coherent; > - status = "disabled"; > }; > }; > diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts > index 97fddfedcb46..0c6826f52c35 100644 > --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts > +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts > @@ -31,10 +31,6 @@ > "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31"; > }; > > -&sdhci { > - status = "okay"; > -}; > - > &mdio_netsec { > phy_netsec: ethernet-phy@1 { > compatible = "ethernet-phy-ieee802.3-c22"; > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c > new file mode 100644 > index 000000000000..29a3ebb369fb > --- /dev/null > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c > @@ -0,0 +1,203 @@ > + /** @file > + SynQuacer DXE platform driver - eMMC support > + > + Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR> > + > + This program and the accompanying materials are licensed and made available > + under the terms and conditions of the BSD License which accompanies this > + distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +**/ > + > +#include "PlatformDxe.h" > + > +// F_SDH30 extended Controller registers > +#define F_SDH30_AHB_CONFIG 0x100 > +#define F_SDH30_AHB_BIGED BIT6 > +#define F_SDH30_BUSLOCK_DMA BIT5 > +#define F_SDH30_BUSLOCK_EN BIT4 > +#define F_SDH30_SIN BIT3 > +#define F_SDH30_AHB_INCR_16 BIT2 > +#define F_SDH30_AHB_INCR_8 BIT1 > +#define F_SDH30_AHB_INCR_4 BIT0 > + > +#define F_SDH30_TUNING_SETTING 0x108 > +#define F_SDH30_CMD_CHK_DIS BIT16 > + > +#define F_SDH30_IO_CONTROL2 0x114 > +#define F_SDH30_MSEL_O_1_8 BIT18 > +#define F_SDH30_CRES_O_DN BIT19 > + > +#define F_SDH30_ESD_CONTROL 0x124 > +#define F_SDH30_EMMC_RST BIT1 > +#define F_SDH30_EMMC_HS200 BIT24 > +#define F_SDH30_CMD_DAT_DELAY BIT9 > + > +#define F_SDH30_TUNING_SETTING 0x108 > +#define F_SDH30_CMD_CHK_DIS BIT16 > + > +#define F_SDH30_IO_CONTROL2 0x114 > +#define F_SDH30_MSEL_O_1_8 BIT18 > +#define F_SDH30_CRES_O_DN BIT19 > + > +#define F_SDH30_ESD_CONTROL 0x124 > +#define F_SDH30_EMMC_RST BIT1 > +#define F_SDH30_EMMC_HS200 BIT24 > +#define F_SDH30_CMD_DAT_DELAY BIT9 > + > +#define SD_HC_CLOCK_CTRL 0x2C > +#define SYNQUACER_CLOCK_CTRL_VAL 0xBC01 > + > +#define SD_HC_CAP_SDR104 BIT33 > + > +#define ESD_CONTROL_RESET_DELAY (20 * 1000) > +#define IO_CONTROL2_SETTLE_US 3000 > + > +STATIC EFI_HANDLE mSdMmcControllerHandle; > + > +/** > + > + Override function for SDHCI capability bits > + > + @param[in] PassThru A pointer to the > + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @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. > + > + @retval EFI_SUCCESS The override function completed successfully. > + @retval EFI_NOT_FOUND The specified controller or slot does not exist. > + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +SynQuacerSdMmcCapability ( > + IN EFI_HANDLE ControllerHandle, > + IN UINT8 Slot, > + IN OUT VOID *SdMmcHcSlotCapability > + ) > +{ > + UINT64 Capability; > + > + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { This test pattern repeats below, does it suggest a macro? > + return EFI_SUCCESS; > + } > + > + // > + // Clear the SDR104 capability bit. This avoids the need for a HS200 tuning > + // quirk that is difficult to support using the generic driver. > + // > + Capability = ReadUnaligned64 (SdMmcHcSlotCapability); > + Capability &= ~(UINT64)SD_HC_CAP_SDR104; > + WriteUnaligned64 (SdMmcHcSlotCapability, Capability); > + > + return EFI_SUCCESS; > +} > + > +/** > + > + Override function for SDHCI controller operations > + > + @param[in] ControllerHandle The EFI_HANDLE of the controller. > + @param[in] Slot The 0 based slot index. > + @param[in] PhaseType The type of operation and whether the > + hook is invoked right before (pre) or > + right after (post) > + > + @retval EFI_SUCCESS The override function completed successfully. > + @retval EFI_NOT_FOUND The specified controller or slot does not exist. > + @retval EFI_INVALID_PARAMETER PhaseType is invalid > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +SynQuacerSdMmcNotifyPhase ( > + IN EFI_HANDLE ControllerHandle, > + IN UINT8 Slot, > + IN EDKII_SD_MMC_PHASE_TYPE PhaseType > + ) > +{ > + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { > + return EFI_SUCCESS; > + } > + > + switch (PhaseType) { > + case EdkiiSdMmcResetPre: > + // Soft reset does not complete unless the clock is already enabled. > + MmioWrite16 (SYNQUACER_EMMC_BASE + SD_HC_CLOCK_CTRL, > + SYNQUACER_CLOCK_CTRL_VAL); This function varies between this ^ type of wrapped indentation of args... > + break; > + > + case EdkiiSdMmcInitHostPre: > + // init vendor specific regs > + MmioAnd16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, > + ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN)); ... and this ^ type of wrapped indentation of args. I don't really mind which, but I would prefer consistency. > + > + MmioOr16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, > + F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 | > + F_SDH30_AHB_INCR_4); > + > + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, ~F_SDH30_EMMC_RST); > + MemoryFence (); > + gBS->Stall (ESD_CONTROL_RESET_DELAY); > + > + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, > + F_SDH30_EMMC_RST | F_SDH30_CMD_DAT_DELAY | F_SDH30_EMMC_HS200); > + > + gBS->Stall (IO_CONTROL2_SETTLE_US); > + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_CRES_O_DN); > + MemoryFence (); > + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_MSEL_O_1_8); > + MemoryFence (); > + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, ~F_SDH30_CRES_O_DN); > + MemoryFence (); > + gBS->Stall (IO_CONTROL2_SETTLE_US); > + > + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_TUNING_SETTING, > + F_SDH30_CMD_CHK_DIS); > + break; > + > + default: > + break; > + } > + return EFI_SUCCESS; > +} > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > index 40e42a4d1864..49d9deee57ea 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf > @@ -23,6 +23,7 @@ [Defines] > ENTRY_POINT = PlatformDxeEntryPoint > > [Sources] > + Emmc.c > Pci.c > PlatformDxe.c > PlatformDxeHii.uni > @@ -39,6 +40,7 @@ [Packages] > > [LibraryClasses] > ArmGenericTimerCounterLib > + BaseLib > BaseMemoryLib > DebugLib > DevicePathLib > @@ -46,6 +48,7 @@ [LibraryClasses] > HiiLib > IoLib > MemoryAllocationLib > + NonDiscoverableDeviceRegistrationLib > PcdLib > UefiBootServicesTableLib > UefiDriverEntryPoint > @@ -62,6 +65,7 @@ [Guids] > > [Protocols] > gEdkiiNonDiscoverableDeviceProtocolGuid ## PRODUCES > + gEdkiiSdMmcOverrideProtocolGuid ## PRODUCES > gEfiPciIoProtocolGuid ## CONSUMES > gPcf8563RealTimeClockLibI2cMasterProtocolGuid ## PRODUCES > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > index b274d12ed2c6..2eca8bbba8c3 100644 > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > @@ -27,3 +27,9 @@ > > #string STR_PCIE_MAX_SPEED_UNLIMITED #language en-US "Unlimited" > #string STR_PCIE_MAX_SPEED_GEN1 #language en-US "Gen1 (2.5 GT/s)" > + > +#string STR_EMMC_ENABLE_PROMPT #language en-US "Enable on-board eMMC" > +#string STR_EMMC_ENABLE_HELP #language en-US "Enable the on-board eMMC for booting and for use by the OS." > + > +#string STR_EMMC_DISABLED #language en-US "Disabled" > +#string STR_EMMC_ENABLED #language en-US "Enabled" Perhaps a random question, but ... Why am I seeing this in cleartext in the patch? Is it really a unicode file? / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 30 January 2018 at 11:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Jan 30, 2018 at 10:32:40AM +0000, Ard Biesheuvel wrote: >> Implement support for the SynQuacer eMMC controller. This involves an >> implementation of the SD/MMC override protocol to handle a couple of >> quirks that would otherwise prevent this IP from being driven by the >> generic SDHCI driver. >> >> Also, add a HII page to the PlatformDxe driver that allows eMMC support >> to be enabled, and wire it up for both DeveloperBox and EVB. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> Now that the core support for the SD/MMC override protocol is finally >> merged, resubmit this again. I dropped Leif's R-b given that I have >> now added DeveloperBox, as well as a HII option to enable eMMC. > > Couple of minor comments/suggestions below and a question. > >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 + >> Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 7 + >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 + >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 7 + >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 1 - >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 - >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 203 ++++++++++++++++++++ >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 5 + >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 9 + >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 4 + >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni | 6 + >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr | 8 + >> Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h | 6 +- >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 23 ++- >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 1 + >> 15 files changed, 287 insertions(+), 13 deletions(-) >> > >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> index 2d46b4515749..7e69eaba9b70 100644 >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc >> @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] >> # >> PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf >> PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf >> + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf >> >> [LibraryClasses.common.UEFI_APPLICATION] >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf >> @@ -549,6 +550,13 @@ [Components.common] >> MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> >> # >> + # eMMC support >> + # >> + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf >> + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf >> + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf >> + >> + # >> # AHCI Support >> # >> MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf >> index 8443986fc3e7..b668f42c7962 100644 >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf >> @@ -148,6 +148,13 @@ [FV.FvMain] >> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> >> # >> + # eMMC support >> + # >> + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf >> + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf >> + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf >> + >> + # >> # AHCI Support >> # >> INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> index 6241b5bbd0b2..e35c17f0bcb7 100644 >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc >> @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] >> # >> PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf >> PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf >> + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf >> >> [LibraryClasses.common.UEFI_APPLICATION] >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf >> @@ -549,6 +550,13 @@ [Components.common] >> MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> >> # >> + # eMMC support >> + # >> + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf >> + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf >> + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf >> + >> + # >> # AHCI Support >> # >> MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> index cc61cf42ccd4..ba2f32328c2b 100644 >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf >> @@ -150,6 +150,13 @@ [FV.FvMain] >> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> >> # >> + # eMMC support >> + # >> + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf >> + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf >> + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf >> + >> + # >> # AHCI Support >> # >> INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf >> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi >> index fae3f033f98f..f1daac74973f 100644 >> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi >> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi >> @@ -567,6 +567,5 @@ >> clocks = <&clk_alw_c_0 &clk_alw_b_0>; >> clock-names = "core", "iface"; >> dma-coherent; >> - status = "disabled"; >> }; >> }; >> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts >> index 97fddfedcb46..0c6826f52c35 100644 >> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts >> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts >> @@ -31,10 +31,6 @@ >> "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31"; >> }; >> >> -&sdhci { >> - status = "okay"; >> -}; >> - >> &mdio_netsec { >> phy_netsec: ethernet-phy@1 { >> compatible = "ethernet-phy-ieee802.3-c22"; >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c >> new file mode 100644 >> index 000000000000..29a3ebb369fb >> --- /dev/null >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c >> @@ -0,0 +1,203 @@ >> + /** @file >> + SynQuacer DXE platform driver - eMMC support >> + >> + Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR> >> + >> + This program and the accompanying materials are licensed and made available >> + under the terms and conditions of the BSD License which accompanies this >> + distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> +**/ >> + >> +#include "PlatformDxe.h" >> + >> +// F_SDH30 extended Controller registers >> +#define F_SDH30_AHB_CONFIG 0x100 >> +#define F_SDH30_AHB_BIGED BIT6 >> +#define F_SDH30_BUSLOCK_DMA BIT5 >> +#define F_SDH30_BUSLOCK_EN BIT4 >> +#define F_SDH30_SIN BIT3 >> +#define F_SDH30_AHB_INCR_16 BIT2 >> +#define F_SDH30_AHB_INCR_8 BIT1 >> +#define F_SDH30_AHB_INCR_4 BIT0 >> + >> +#define F_SDH30_TUNING_SETTING 0x108 >> +#define F_SDH30_CMD_CHK_DIS BIT16 >> + >> +#define F_SDH30_IO_CONTROL2 0x114 >> +#define F_SDH30_MSEL_O_1_8 BIT18 >> +#define F_SDH30_CRES_O_DN BIT19 >> + >> +#define F_SDH30_ESD_CONTROL 0x124 >> +#define F_SDH30_EMMC_RST BIT1 >> +#define F_SDH30_EMMC_HS200 BIT24 >> +#define F_SDH30_CMD_DAT_DELAY BIT9 >> + >> +#define F_SDH30_TUNING_SETTING 0x108 >> +#define F_SDH30_CMD_CHK_DIS BIT16 >> + >> +#define F_SDH30_IO_CONTROL2 0x114 >> +#define F_SDH30_MSEL_O_1_8 BIT18 >> +#define F_SDH30_CRES_O_DN BIT19 >> + >> +#define F_SDH30_ESD_CONTROL 0x124 >> +#define F_SDH30_EMMC_RST BIT1 >> +#define F_SDH30_EMMC_HS200 BIT24 >> +#define F_SDH30_CMD_DAT_DELAY BIT9 >> + >> +#define SD_HC_CLOCK_CTRL 0x2C >> +#define SYNQUACER_CLOCK_CTRL_VAL 0xBC01 >> + >> +#define SD_HC_CAP_SDR104 BIT33 >> + >> +#define ESD_CONTROL_RESET_DELAY (20 * 1000) >> +#define IO_CONTROL2_SETTLE_US 3000 >> + >> +STATIC EFI_HANDLE mSdMmcControllerHandle; >> + >> +/** >> + >> + Override function for SDHCI capability bits >> + >> + @param[in] PassThru A pointer to the >> + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. >> + @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. >> + >> + @retval EFI_SUCCESS The override function completed successfully. >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. >> + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL >> + >> +**/ >> +STATIC >> +EFI_STATUS >> +EFIAPI >> +SynQuacerSdMmcCapability ( >> + IN EFI_HANDLE ControllerHandle, >> + IN UINT8 Slot, >> + IN OUT VOID *SdMmcHcSlotCapability >> + ) >> +{ >> + UINT64 Capability; >> + >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { > > This test pattern repeats below, does it suggest a macro? > I don't see how that would clear things up tbh, and the pattern occurs only twice #define IS_OUR_QUIRKY_SDMMC_CONTROLLER(Handle, Slot) \ ((Handle) == mSdMmcControllerHandle && (Slot) == 0) if (!IS_OUR_QUIRKY_SDMMC_CONTROLLER(ControllerHandle, Slot) { return EFI_SUCCESS; } I can change it if you want, or add a comment if the condition is not self-explanatory enough. >> + return EFI_SUCCESS; >> + } >> + >> + // >> + // Clear the SDR104 capability bit. This avoids the need for a HS200 tuning >> + // quirk that is difficult to support using the generic driver. >> + // >> + Capability = ReadUnaligned64 (SdMmcHcSlotCapability); >> + Capability &= ~(UINT64)SD_HC_CAP_SDR104; >> + WriteUnaligned64 (SdMmcHcSlotCapability, Capability); >> + >> + return EFI_SUCCESS; >> +} >> + >> +/** >> + >> + Override function for SDHCI controller operations >> + >> + @param[in] ControllerHandle The EFI_HANDLE of the controller. >> + @param[in] Slot The 0 based slot index. >> + @param[in] PhaseType The type of operation and whether the >> + hook is invoked right before (pre) or >> + right after (post) >> + >> + @retval EFI_SUCCESS The override function completed successfully. >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. >> + @retval EFI_INVALID_PARAMETER PhaseType is invalid >> + >> +**/ >> +STATIC >> +EFI_STATUS >> +EFIAPI >> +SynQuacerSdMmcNotifyPhase ( >> + IN EFI_HANDLE ControllerHandle, >> + IN UINT8 Slot, >> + IN EDKII_SD_MMC_PHASE_TYPE PhaseType >> + ) >> +{ >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { >> + return EFI_SUCCESS; >> + } >> + >> + switch (PhaseType) { >> + case EdkiiSdMmcResetPre: >> + // Soft reset does not complete unless the clock is already enabled. >> + MmioWrite16 (SYNQUACER_EMMC_BASE + SD_HC_CLOCK_CTRL, >> + SYNQUACER_CLOCK_CTRL_VAL); > > This function varies between this ^ type of wrapped indentation of args... > >> + break; >> + >> + case EdkiiSdMmcInitHostPre: >> + // init vendor specific regs >> + MmioAnd16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, >> + ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN)); > > ... and this ^ type of wrapped indentation of args. > > I don't really mind which, but I would prefer consistency. > Will fix >> + >> + MmioOr16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, >> + F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 | >> + F_SDH30_AHB_INCR_4); >> + >> + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, ~F_SDH30_EMMC_RST); >> + MemoryFence (); >> + gBS->Stall (ESD_CONTROL_RESET_DELAY); >> + >> + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, >> + F_SDH30_EMMC_RST | F_SDH30_CMD_DAT_DELAY | F_SDH30_EMMC_HS200); >> + >> + gBS->Stall (IO_CONTROL2_SETTLE_US); >> + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_CRES_O_DN); >> + MemoryFence (); >> + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_MSEL_O_1_8); >> + MemoryFence (); >> + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, ~F_SDH30_CRES_O_DN); >> + MemoryFence (); >> + gBS->Stall (IO_CONTROL2_SETTLE_US); >> + >> + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_TUNING_SETTING, >> + F_SDH30_CMD_CHK_DIS); >> + break; >> + >> + default: >> + break; >> + } >> + return EFI_SUCCESS; >> +} > >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf >> index 40e42a4d1864..49d9deee57ea 100644 >> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf >> @@ -23,6 +23,7 @@ [Defines] >> ENTRY_POINT = PlatformDxeEntryPoint >> >> [Sources] >> + Emmc.c >> Pci.c >> PlatformDxe.c >> PlatformDxeHii.uni >> @@ -39,6 +40,7 @@ [Packages] >> >> [LibraryClasses] >> ArmGenericTimerCounterLib >> + BaseLib >> BaseMemoryLib >> DebugLib >> DevicePathLib >> @@ -46,6 +48,7 @@ [LibraryClasses] >> HiiLib >> IoLib >> MemoryAllocationLib >> + NonDiscoverableDeviceRegistrationLib >> PcdLib >> UefiBootServicesTableLib >> UefiDriverEntryPoint >> @@ -62,6 +65,7 @@ [Guids] >> >> [Protocols] >> gEdkiiNonDiscoverableDeviceProtocolGuid ## PRODUCES >> + gEdkiiSdMmcOverrideProtocolGuid ## PRODUCES >> gEfiPciIoProtocolGuid ## CONSUMES >> gPcf8563RealTimeClockLibI2cMasterProtocolGuid ## PRODUCES >> >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> index b274d12ed2c6..2eca8bbba8c3 100644 >> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> @@ -27,3 +27,9 @@ >> >> #string STR_PCIE_MAX_SPEED_UNLIMITED #language en-US "Unlimited" >> #string STR_PCIE_MAX_SPEED_GEN1 #language en-US "Gen1 (2.5 GT/s)" >> + >> +#string STR_EMMC_ENABLE_PROMPT #language en-US "Enable on-board eMMC" >> +#string STR_EMMC_ENABLE_HELP #language en-US "Enable the on-board eMMC for booting and for use by the OS." >> + >> +#string STR_EMMC_DISABLED #language en-US "Disabled" >> +#string STR_EMMC_ENABLED #language en-US "Enabled" > > Perhaps a random question, but ... > Why am I seeing this in cleartext in the patch? Is it really a unicode file? > Given that we support UTF-8 in .uni files these days, and the fact that all characters used are in the 7-bit ASCII range, it doesn't really make a difference, I guess. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Jan 30, 2018 at 11:14:31AM +0000, Ard Biesheuvel wrote: > On 30 January 2018 at 11:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Tue, Jan 30, 2018 at 10:32:40AM +0000, Ard Biesheuvel wrote: > >> Implement support for the SynQuacer eMMC controller. This involves an > >> implementation of the SD/MMC override protocol to handle a couple of > >> quirks that would otherwise prevent this IP from being driven by the > >> generic SDHCI driver. > >> > >> Also, add a HII page to the PlatformDxe driver that allows eMMC support > >> to be enabled, and wire it up for both DeveloperBox and EVB. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> Now that the core support for the SD/MMC override protocol is finally > >> merged, resubmit this again. I dropped Leif's R-b given that I have > >> now added DeveloperBox, as well as a HII option to enable eMMC. > > > > Couple of minor comments/suggestions below and a question. > > > >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 + > >> Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 7 + > >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 + > >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 7 + > >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 1 - > >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 - > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 203 ++++++++++++++++++++ > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 5 + > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 9 + > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 4 + > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni | 6 + > >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr | 8 + > >> Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h | 6 +- > >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 23 ++- > >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 1 + > >> 15 files changed, 287 insertions(+), 13 deletions(-) > >> > > > >> +/** > >> + > >> + Override function for SDHCI capability bits > >> + > >> + @param[in] PassThru A pointer to the > >> + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > >> + @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. > >> + > >> + @retval EFI_SUCCESS The override function completed successfully. > >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. > >> + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL > >> + > >> +**/ > >> +STATIC > >> +EFI_STATUS > >> +EFIAPI > >> +SynQuacerSdMmcCapability ( > >> + IN EFI_HANDLE ControllerHandle, > >> + IN UINT8 Slot, > >> + IN OUT VOID *SdMmcHcSlotCapability > >> + ) > >> +{ > >> + UINT64 Capability; > >> + > >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { > > > > This test pattern repeats below, does it suggest a macro? > > > > I don't see how that would clear things up tbh, and the pattern occurs > only twice > > #define IS_OUR_QUIRKY_SDMMC_CONTROLLER(Handle, Slot) \ > ((Handle) == mSdMmcControllerHandle && (Slot) == 0) > > if (!IS_OUR_QUIRKY_SDMMC_CONTROLLER(ControllerHandle, Slot) { > return EFI_SUCCESS; > } > > I can change it if you want, or add a comment if the condition is not > self-explanatory enough. It's just an awful lot of logical operations on a single line. 'ControllerHandle != mSdMmcControllerHandle' is reasonably easy to figure out, but '|| Slot != 0' looks a bit random there. A comment would be sufficient. Another option would be to reduce the number of logical operations by two by doing if (ControllerHandle == mSdMmcControllerHandle && Slot == 0) { and doing the body inside the if-statement. That's a bit uglier in the next function, but I would expect it follows the paradigm of "handle most likely case first"? > >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > >> index b274d12ed2c6..2eca8bbba8c3 100644 > >> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni > >> @@ -27,3 +27,9 @@ > >> > >> #string STR_PCIE_MAX_SPEED_UNLIMITED #language en-US "Unlimited" > >> #string STR_PCIE_MAX_SPEED_GEN1 #language en-US "Gen1 (2.5 GT/s)" > >> + > >> +#string STR_EMMC_ENABLE_PROMPT #language en-US "Enable on-board eMMC" > >> +#string STR_EMMC_ENABLE_HELP #language en-US "Enable the on-board eMMC for booting and for use by the OS." > >> + > >> +#string STR_EMMC_DISABLED #language en-US "Disabled" > >> +#string STR_EMMC_ENABLED #language en-US "Enabled" > > > > Perhaps a random question, but ... > > Why am I seeing this in cleartext in the patch? Is it really a unicode file? > > Given that we support UTF-8 in .uni files these days, and the fact > that all characters used are in the 7-bit ASCII range, it doesn't > really make a difference, I guess. Fair enough. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 30 January 2018 at 11:47, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Jan 30, 2018 at 11:14:31AM +0000, Ard Biesheuvel wrote: >> On 30 January 2018 at 11:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > On Tue, Jan 30, 2018 at 10:32:40AM +0000, Ard Biesheuvel wrote: >> >> Implement support for the SynQuacer eMMC controller. This involves an >> >> implementation of the SD/MMC override protocol to handle a couple of >> >> quirks that would otherwise prevent this IP from being driven by the >> >> generic SDHCI driver. >> >> >> >> Also, add a HII page to the PlatformDxe driver that allows eMMC support >> >> to be enabled, and wire it up for both DeveloperBox and EVB. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> --- >> >> Now that the core support for the SD/MMC override protocol is finally >> >> merged, resubmit this again. I dropped Leif's R-b given that I have >> >> now added DeveloperBox, as well as a HII option to enable eMMC. >> > >> > Couple of minor comments/suggestions below and a question. >> > >> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 + >> >> Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 7 + >> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 + >> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 7 + >> >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 1 - >> >> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 - >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 203 ++++++++++++++++++++ >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 5 + >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 9 + >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 4 + >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni | 6 + >> >> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr | 8 + >> >> Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h | 6 +- >> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 23 ++- >> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 1 + >> >> 15 files changed, 287 insertions(+), 13 deletions(-) >> >> >> > >> >> +/** >> >> + >> >> + Override function for SDHCI capability bits >> >> + >> >> + @param[in] PassThru A pointer to the >> >> + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. >> >> + @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. >> >> + >> >> + @retval EFI_SUCCESS The override function completed successfully. >> >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. >> >> + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL >> >> + >> >> +**/ >> >> +STATIC >> >> +EFI_STATUS >> >> +EFIAPI >> >> +SynQuacerSdMmcCapability ( >> >> + IN EFI_HANDLE ControllerHandle, >> >> + IN UINT8 Slot, >> >> + IN OUT VOID *SdMmcHcSlotCapability >> >> + ) >> >> +{ >> >> + UINT64 Capability; >> >> + >> >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { >> > >> > This test pattern repeats below, does it suggest a macro? >> > >> >> I don't see how that would clear things up tbh, and the pattern occurs >> only twice >> >> #define IS_OUR_QUIRKY_SDMMC_CONTROLLER(Handle, Slot) \ >> ((Handle) == mSdMmcControllerHandle && (Slot) == 0) >> >> if (!IS_OUR_QUIRKY_SDMMC_CONTROLLER(ControllerHandle, Slot) { >> return EFI_SUCCESS; >> } >> >> I can change it if you want, or add a comment if the condition is not >> self-explanatory enough. > > It's just an awful lot of logical operations on a single line. > 'ControllerHandle != mSdMmcControllerHandle' is reasonably easy to > figure out, but '|| Slot != 0' looks a bit random there. > > A comment would be sufficient. > > Another option would be to reduce the number of logical operations by > two by doing > if (ControllerHandle == mSdMmcControllerHandle && Slot == 0) { > and doing the body inside the if-statement. > > That's a bit uglier in the next function, but I would expect it > follows the paradigm of "handle most likely case first"? > Actually, come to think of it, the slot number is completely redundant. The non-discoverable SDHCI controller we expose only implements a single slot, so something is terribly wrong if ControllerHandle == mSdMmcControllerHandle && Slot != 0. So I will reduce the sequence to if (ControllerHandle != mSdMmcControllerHandle) { return EFI_SUCCESS; } ASSERT (Slot == 0); instead. Ok? >> >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> >> index b274d12ed2c6..2eca8bbba8c3 100644 >> >> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni >> >> @@ -27,3 +27,9 @@ >> >> >> >> #string STR_PCIE_MAX_SPEED_UNLIMITED #language en-US "Unlimited" >> >> #string STR_PCIE_MAX_SPEED_GEN1 #language en-US "Gen1 (2.5 GT/s)" >> >> + >> >> +#string STR_EMMC_ENABLE_PROMPT #language en-US "Enable on-board eMMC" >> >> +#string STR_EMMC_ENABLE_HELP #language en-US "Enable the on-board eMMC for booting and for use by the OS." >> >> + >> >> +#string STR_EMMC_DISABLED #language en-US "Disabled" >> >> +#string STR_EMMC_ENABLED #language en-US "Enabled" >> > >> > Perhaps a random question, but ... >> > Why am I seeing this in cleartext in the patch? Is it really a unicode file? >> >> Given that we support UTF-8 in .uni files these days, and the fact >> that all characters used are in the 7-bit ASCII range, it doesn't >> really make a difference, I guess. > > Fair enough. > > / > Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Jan 30, 2018 at 11:52:41AM +0000, Ard Biesheuvel wrote: > >> >> +STATIC > >> >> +EFI_STATUS > >> >> +EFIAPI > >> >> +SynQuacerSdMmcCapability ( > >> >> + IN EFI_HANDLE ControllerHandle, > >> >> + IN UINT8 Slot, > >> >> + IN OUT VOID *SdMmcHcSlotCapability > >> >> + ) > >> >> +{ > >> >> + UINT64 Capability; > >> >> + > >> >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { > >> > > >> > This test pattern repeats below, does it suggest a macro? > >> > > >> > >> I don't see how that would clear things up tbh, and the pattern occurs > >> only twice > >> > >> #define IS_OUR_QUIRKY_SDMMC_CONTROLLER(Handle, Slot) \ > >> ((Handle) == mSdMmcControllerHandle && (Slot) == 0) > >> > >> if (!IS_OUR_QUIRKY_SDMMC_CONTROLLER(ControllerHandle, Slot) { > >> return EFI_SUCCESS; > >> } > >> > >> I can change it if you want, or add a comment if the condition is not > >> self-explanatory enough. > > > > It's just an awful lot of logical operations on a single line. > > 'ControllerHandle != mSdMmcControllerHandle' is reasonably easy to > > figure out, but '|| Slot != 0' looks a bit random there. > > > > A comment would be sufficient. > > > > Another option would be to reduce the number of logical operations by > > two by doing > > if (ControllerHandle == mSdMmcControllerHandle && Slot == 0) { > > and doing the body inside the if-statement. > > > > That's a bit uglier in the next function, but I would expect it > > follows the paradigm of "handle most likely case first"? > > > > Actually, come to think of it, the slot number is completely > redundant. The non-discoverable SDHCI controller we expose only > implements a single slot, so something is terribly wrong if > ControllerHandle == mSdMmcControllerHandle && Slot != 0. > > So I will reduce the sequence to > > if (ControllerHandle != mSdMmcControllerHandle) { > return EFI_SUCCESS; > } > > ASSERT (Slot == 0); > > instead. Ok? Sure, that works for me. With that (in both places), and the indentation fix: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 30 January 2018 at 12:52, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Jan 30, 2018 at 11:52:41AM +0000, Ard Biesheuvel wrote: >> >> >> +STATIC >> >> >> +EFI_STATUS >> >> >> +EFIAPI >> >> >> +SynQuacerSdMmcCapability ( >> >> >> + IN EFI_HANDLE ControllerHandle, >> >> >> + IN UINT8 Slot, >> >> >> + IN OUT VOID *SdMmcHcSlotCapability >> >> >> + ) >> >> >> +{ >> >> >> + UINT64 Capability; >> >> >> + >> >> >> + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { >> >> > >> >> > This test pattern repeats below, does it suggest a macro? >> >> > >> >> >> >> I don't see how that would clear things up tbh, and the pattern occurs >> >> only twice >> >> >> >> #define IS_OUR_QUIRKY_SDMMC_CONTROLLER(Handle, Slot) \ >> >> ((Handle) == mSdMmcControllerHandle && (Slot) == 0) >> >> >> >> if (!IS_OUR_QUIRKY_SDMMC_CONTROLLER(ControllerHandle, Slot) { >> >> return EFI_SUCCESS; >> >> } >> >> >> >> I can change it if you want, or add a comment if the condition is not >> >> self-explanatory enough. >> > >> > It's just an awful lot of logical operations on a single line. >> > 'ControllerHandle != mSdMmcControllerHandle' is reasonably easy to >> > figure out, but '|| Slot != 0' looks a bit random there. >> > >> > A comment would be sufficient. >> > >> > Another option would be to reduce the number of logical operations by >> > two by doing >> > if (ControllerHandle == mSdMmcControllerHandle && Slot == 0) { >> > and doing the body inside the if-statement. >> > >> > That's a bit uglier in the next function, but I would expect it >> > follows the paradigm of "handle most likely case first"? >> > >> >> Actually, come to think of it, the slot number is completely >> redundant. The non-discoverable SDHCI controller we expose only >> implements a single slot, so something is terribly wrong if >> ControllerHandle == mSdMmcControllerHandle && Slot != 0. >> >> So I will reduce the sequence to >> >> if (ControllerHandle != mSdMmcControllerHandle) { >> return EFI_SUCCESS; >> } >> >> ASSERT (Slot == 0); >> >> instead. Ok? > > Sure, that works for me. > With that (in both places), and the indentation fix: > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Thanks. Pushed as c733b7ef291f _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc index 2d46b4515749..7e69eaba9b70 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] # PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf [LibraryClasses.common.UEFI_APPLICATION] PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf @@ -549,6 +550,13 @@ [Components.common] MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf # + # eMMC support + # + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf + + # # AHCI Support # MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf index 8443986fc3e7..b668f42c7962 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf @@ -148,6 +148,13 @@ [FV.FvMain] INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf # + # eMMC support + # + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf + + # # AHCI Support # INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc index 6241b5bbd0b2..e35c17f0bcb7 100644 --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc @@ -179,6 +179,7 @@ [LibraryClasses.common.DXE_DRIVER] # PciSegmentLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciSegmentLib/SynQuacerPciSegmentLib.inf PciHostBridgeLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf + NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf [LibraryClasses.common.UEFI_APPLICATION] PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf @@ -549,6 +550,13 @@ [Components.common] MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf # + # eMMC support + # + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf + MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf + MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf + + # # AHCI Support # MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf index cc61cf42ccd4..ba2f32328c2b 100644 --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf @@ -150,6 +150,13 @@ [FV.FvMain] INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf # + # eMMC support + # + INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf + INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf + INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf + + # # AHCI Support # INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi index fae3f033f98f..f1daac74973f 100644 --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi @@ -567,6 +567,5 @@ clocks = <&clk_alw_c_0 &clk_alw_b_0>; clock-names = "core", "iface"; dma-coherent; - status = "disabled"; }; }; diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts index 97fddfedcb46..0c6826f52c35 100644 --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts @@ -31,10 +31,6 @@ "PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31"; }; -&sdhci { - status = "okay"; -}; - &mdio_netsec { phy_netsec: ethernet-phy@1 { compatible = "ethernet-phy-ieee802.3-c22"; diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c new file mode 100644 index 000000000000..29a3ebb369fb --- /dev/null +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c @@ -0,0 +1,203 @@ + /** @file + SynQuacer DXE platform driver - eMMC support + + Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR> + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +**/ + +#include "PlatformDxe.h" + +// F_SDH30 extended Controller registers +#define F_SDH30_AHB_CONFIG 0x100 +#define F_SDH30_AHB_BIGED BIT6 +#define F_SDH30_BUSLOCK_DMA BIT5 +#define F_SDH30_BUSLOCK_EN BIT4 +#define F_SDH30_SIN BIT3 +#define F_SDH30_AHB_INCR_16 BIT2 +#define F_SDH30_AHB_INCR_8 BIT1 +#define F_SDH30_AHB_INCR_4 BIT0 + +#define F_SDH30_TUNING_SETTING 0x108 +#define F_SDH30_CMD_CHK_DIS BIT16 + +#define F_SDH30_IO_CONTROL2 0x114 +#define F_SDH30_MSEL_O_1_8 BIT18 +#define F_SDH30_CRES_O_DN BIT19 + +#define F_SDH30_ESD_CONTROL 0x124 +#define F_SDH30_EMMC_RST BIT1 +#define F_SDH30_EMMC_HS200 BIT24 +#define F_SDH30_CMD_DAT_DELAY BIT9 + +#define F_SDH30_TUNING_SETTING 0x108 +#define F_SDH30_CMD_CHK_DIS BIT16 + +#define F_SDH30_IO_CONTROL2 0x114 +#define F_SDH30_MSEL_O_1_8 BIT18 +#define F_SDH30_CRES_O_DN BIT19 + +#define F_SDH30_ESD_CONTROL 0x124 +#define F_SDH30_EMMC_RST BIT1 +#define F_SDH30_EMMC_HS200 BIT24 +#define F_SDH30_CMD_DAT_DELAY BIT9 + +#define SD_HC_CLOCK_CTRL 0x2C +#define SYNQUACER_CLOCK_CTRL_VAL 0xBC01 + +#define SD_HC_CAP_SDR104 BIT33 + +#define ESD_CONTROL_RESET_DELAY (20 * 1000) +#define IO_CONTROL2_SETTLE_US 3000 + +STATIC EFI_HANDLE mSdMmcControllerHandle; + +/** + + Override function for SDHCI capability bits + + @param[in] PassThru A pointer to the + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. + @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. + + @retval EFI_SUCCESS The override function completed successfully. + @retval EFI_NOT_FOUND The specified controller or slot does not exist. + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL + +**/ +STATIC +EFI_STATUS +EFIAPI +SynQuacerSdMmcCapability ( + IN EFI_HANDLE ControllerHandle, + IN UINT8 Slot, + IN OUT VOID *SdMmcHcSlotCapability + ) +{ + UINT64 Capability; + + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { + return EFI_SUCCESS; + } + + // + // Clear the SDR104 capability bit. This avoids the need for a HS200 tuning + // quirk that is difficult to support using the generic driver. + // + Capability = ReadUnaligned64 (SdMmcHcSlotCapability); + Capability &= ~(UINT64)SD_HC_CAP_SDR104; + WriteUnaligned64 (SdMmcHcSlotCapability, Capability); + + return EFI_SUCCESS; +} + +/** + + Override function for SDHCI controller operations + + @param[in] ControllerHandle The EFI_HANDLE of the controller. + @param[in] Slot The 0 based slot index. + @param[in] PhaseType The type of operation and whether the + hook is invoked right before (pre) or + right after (post) + + @retval EFI_SUCCESS The override function completed successfully. + @retval EFI_NOT_FOUND The specified controller or slot does not exist. + @retval EFI_INVALID_PARAMETER PhaseType is invalid + +**/ +STATIC +EFI_STATUS +EFIAPI +SynQuacerSdMmcNotifyPhase ( + IN EFI_HANDLE ControllerHandle, + IN UINT8 Slot, + IN EDKII_SD_MMC_PHASE_TYPE PhaseType + ) +{ + if (ControllerHandle != mSdMmcControllerHandle || Slot != 0) { + return EFI_SUCCESS; + } + + switch (PhaseType) { + case EdkiiSdMmcResetPre: + // Soft reset does not complete unless the clock is already enabled. + MmioWrite16 (SYNQUACER_EMMC_BASE + SD_HC_CLOCK_CTRL, + SYNQUACER_CLOCK_CTRL_VAL); + break; + + case EdkiiSdMmcInitHostPre: + // init vendor specific regs + MmioAnd16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, + ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN)); + + MmioOr16 (SYNQUACER_EMMC_BASE + F_SDH30_AHB_CONFIG, + F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 | + F_SDH30_AHB_INCR_4); + + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, ~F_SDH30_EMMC_RST); + MemoryFence (); + gBS->Stall (ESD_CONTROL_RESET_DELAY); + + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_ESD_CONTROL, + F_SDH30_EMMC_RST | F_SDH30_CMD_DAT_DELAY | F_SDH30_EMMC_HS200); + + gBS->Stall (IO_CONTROL2_SETTLE_US); + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_CRES_O_DN); + MemoryFence (); + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, F_SDH30_MSEL_O_1_8); + MemoryFence (); + MmioAnd32 (SYNQUACER_EMMC_BASE + F_SDH30_IO_CONTROL2, ~F_SDH30_CRES_O_DN); + MemoryFence (); + gBS->Stall (IO_CONTROL2_SETTLE_US); + + MmioOr32 (SYNQUACER_EMMC_BASE + F_SDH30_TUNING_SETTING, + F_SDH30_CMD_CHK_DIS); + break; + + default: + break; + } + return EFI_SUCCESS; +} + +STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = { + EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION, + SynQuacerSdMmcCapability, + SynQuacerSdMmcNotifyPhase, +}; + +EFI_STATUS +EFIAPI +RegisterEmmc ( + VOID + ) +{ + EFI_STATUS Status; + EFI_HANDLE Handle; + + Status = RegisterNonDiscoverableMmioDevice ( + NonDiscoverableDeviceTypeSdhci, + NonDiscoverableDeviceDmaTypeCoherent, + NULL, + &mSdMmcControllerHandle, + 1, + SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ); + ASSERT_EFI_ERROR (Status); + + Handle = NULL; + Status = gBS->InstallProtocolInterface (&Handle, + &gEdkiiSdMmcOverrideProtocolGuid, + EFI_NATIVE_INTERFACE, (VOID **)&mSdMmcOverride); + ASSERT_EFI_ERROR (Status); + + return EFI_SUCCESS; +} diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c index b9394aa19f1a..aab830dc3a5a 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c @@ -322,5 +322,10 @@ PlatformDxeEntryPoint ( Status = EnableSettingsForm (); ASSERT_EFI_ERROR (Status); + if (mHiiSettings->EnableEmmc == EMMC_ENABLED) { + Status = RegisterEmmc (); + ASSERT_EFI_ERROR (Status); + } + return EFI_SUCCESS; } diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h index 5fb1437757b9..a391d2f67c29 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h @@ -19,6 +19,7 @@ #include <Guid/SynQuacerPlatformFormSet.h> #include <IndustryStandard/Pci.h> #include <Library/ArmGenericTimerCounterLib.h> +#include <Library/BaseLib.h> #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> #include <Library/DevicePathLib.h> @@ -26,6 +27,7 @@ #include <Library/HiiLib.h> #include <Library/IoLib.h> #include <Library/MemoryAllocationLib.h> +#include <Library/NonDiscoverableDeviceRegistrationLib.h> #include <Library/PcdLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Library/UefiLib.h> @@ -35,6 +37,7 @@ #include <Platform/VarStore.h> #include <Protocol/NonDiscoverableDevice.h> #include <Protocol/PciIo.h> +#include <Protocol/SdMmcOverride.h> extern UINT8 PlatformDxeHiiBin[]; extern UINT8 PlatformDxeStrings[]; @@ -48,4 +51,10 @@ RegisterPcieNotifier ( VOID ); +EFI_STATUS +EFIAPI +RegisterEmmc ( + VOID + ); + #endif diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf index 40e42a4d1864..49d9deee57ea 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf @@ -23,6 +23,7 @@ [Defines] ENTRY_POINT = PlatformDxeEntryPoint [Sources] + Emmc.c Pci.c PlatformDxe.c PlatformDxeHii.uni @@ -39,6 +40,7 @@ [Packages] [LibraryClasses] ArmGenericTimerCounterLib + BaseLib BaseMemoryLib DebugLib DevicePathLib @@ -46,6 +48,7 @@ [LibraryClasses] HiiLib IoLib MemoryAllocationLib + NonDiscoverableDeviceRegistrationLib PcdLib UefiBootServicesTableLib UefiDriverEntryPoint @@ -62,6 +65,7 @@ [Guids] [Protocols] gEdkiiNonDiscoverableDeviceProtocolGuid ## PRODUCES + gEdkiiSdMmcOverrideProtocolGuid ## PRODUCES gEfiPciIoProtocolGuid ## CONSUMES gPcf8563RealTimeClockLibI2cMasterProtocolGuid ## PRODUCES diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni index b274d12ed2c6..2eca8bbba8c3 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni @@ -27,3 +27,9 @@ #string STR_PCIE_MAX_SPEED_UNLIMITED #language en-US "Unlimited" #string STR_PCIE_MAX_SPEED_GEN1 #language en-US "Gen1 (2.5 GT/s)" + +#string STR_EMMC_ENABLE_PROMPT #language en-US "Enable on-board eMMC" +#string STR_EMMC_ENABLE_HELP #language en-US "Enable the on-board eMMC for booting and for use by the OS." + +#string STR_EMMC_DISABLED #language en-US "Disabled" +#string STR_EMMC_ENABLED #language en-US "Enabled" diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr index 52f554b61e3c..ea35e902b2d7 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr @@ -62,6 +62,14 @@ formset option text = STRING_TOKEN(STR_PCIE_MAX_SPEED_GEN1), value = PCIE_MAX_SPEED_GEN1, flags = 0; endoneof; + oneof varid = SynQuacerPlatformSettings.EnableEmmc, + prompt = STRING_TOKEN(STR_EMMC_ENABLE_PROMPT), + help = STRING_TOKEN(STR_EMMC_ENABLE_HELP), + flags = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED, + option text = STRING_TOKEN(STR_EMMC_DISABLED), value = EMMC_DISABLED, flags = DEFAULT; + option text = STRING_TOKEN(STR_EMMC_ENABLED), value = EMMC_ENABLED, flags = 0; + endoneof; + subtitle text = STRING_TOKEN(STR_NULL_STRING); endform; diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h b/Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h index 5944613e7465..fbbcbd7d3eec 100644 --- a/Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h +++ b/Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h @@ -16,14 +16,18 @@ #define SYNQUACER_PLATFORM_VARIABLE_NAME L"SynQuacerPlatformSettings" +#define EMMC_DISABLED 0x0 +#define EMMC_ENABLED 0x1 + #define PCIE_MAX_SPEED_UNLIMITED 0x0 #define PCIE_MAX_SPEED_GEN1 0x1 typedef struct { + UINT8 EnableEmmc; UINT8 PcieSlot0MaxSpeed; UINT8 PcieSlot1MaxSpeed; UINT8 PcieSlot2MaxSpeed; - UINT8 Reserved[5]; + UINT8 Reserved[4]; } SYNQUACER_PLATFORM_VARSTORE_DATA; #endif diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c index cebf30c9b6ee..897d06743708 100644 --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c @@ -19,8 +19,9 @@ #include <Library/DebugLib.h> #include <Library/DxeServicesLib.h> #include <Library/MemoryAllocationLib.h> +#include <Platform/VarStore.h> -// add enough space for two instances of 'status = "disabled"' +// add enough space for three instances of 'status = "disabled"' #define DTB_PADDING 64 STATIC @@ -65,12 +66,14 @@ DtPlatformLoadDtb ( OUT UINTN *DtbSize ) { - EFI_STATUS Status; - VOID *OrigDtb; - VOID *CopyDtb; - UINTN OrigDtbSize; - UINTN CopyDtbSize; - INT32 Rc; + EFI_STATUS Status; + VOID *OrigDtb; + VOID *CopyDtb; + UINTN OrigDtbSize; + UINTN CopyDtbSize; + INT32 Rc; + UINT64 SettingsVal; + SYNQUACER_PLATFORM_VARSTORE_DATA *Settings; Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid, EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize); @@ -98,6 +101,12 @@ DtPlatformLoadDtb ( DisableDtNode (CopyDtb, "/pcie@70000000"); } + SettingsVal = PcdGet64 (PcdPlatformSettings); + Settings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&SettingsVal; + if (Settings->EnableEmmc == EMMC_DISABLED) { + DisableDtNode (CopyDtb, "/sdhci@52300000"); + } + *Dtb = CopyDtb; *DtbSize = CopyDtbSize; diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf index e1f564f73078..548d62fd5c0a 100644 --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf @@ -37,6 +37,7 @@ [LibraryClasses] [Pcd] gSynQuacerTokenSpaceGuid.PcdPcieEnableMask + gSynQuacerTokenSpaceGuid.PcdPlatformSettings [Guids] gDtPlatformDefaultDtbFileGuid
Implement support for the SynQuacer eMMC controller. This involves an implementation of the SD/MMC override protocol to handle a couple of quirks that would otherwise prevent this IP from being driven by the generic SDHCI driver. Also, add a HII page to the PlatformDxe driver that allows eMMC support to be enabled, and wire it up for both DeveloperBox and EVB. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Now that the core support for the SD/MMC override protocol is finally merged, resubmit this again. I dropped Leif's R-b given that I have now added DeveloperBox, as well as a HII option to enable eMMC. Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 + Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 7 + Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 + Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf | 7 + Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 1 - Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 - Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 203 ++++++++++++++++++++ Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 5 + Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h | 9 + Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 4 + Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni | 6 + Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr | 8 + Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h | 6 +- Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 23 ++- Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 1 + 15 files changed, 287 insertions(+), 13 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel