From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org
Subject: Re: [PATCH edk2-platforms] Silicon/Socionext/SynQuacer: add configurable eMMC support
Date: Tue, 30 Jan 2018 11:00:44 +0000 [thread overview]
Message-ID: <20180130110044.ftayhc37kfh6pw27@bivouac.eciton.net> (raw)
In-Reply-To: <20180130103240.4669-1-ard.biesheuvel@linaro.org>
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
next prev parent reply other threads:[~2018-01-30 10:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-30 10:32 [PATCH edk2-platforms] Silicon/Socionext/SynQuacer: add configurable eMMC support Ard Biesheuvel
2018-01-30 11:00 ` Leif Lindholm [this message]
2018-01-30 11:14 ` Ard Biesheuvel
2018-01-30 11:47 ` Leif Lindholm
2018-01-30 11:52 ` Ard Biesheuvel
2018-01-30 12:52 ` Leif Lindholm
2018-01-30 13:04 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180130110044.ftayhc37kfh6pw27@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox