On Thu, Jan 05, 2017 at 12:03:40PM +0100, Marcin Wojtas wrote:
Xenon controller comprise PHY,
"comprise" -> "contains a".
which can be bypassed, using BIT29 of EMMC_PHY_TIMING_ADJUST register.
Calling out bit numbers in a commit message is a bit too much detail. Just say it can be disabled through that register.
Due to possible hardware issues such operation may have to be enabled in some cases.
This patch enables per-interface configuration of the 'slow mode' usage. For this purpose new PCD is added (PcdXenonSlowModeEnable). Porting guide documentation is updated accordingly.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Marcin Wojtas mw@semihalf.com
Documentation/Marvell/PortingGuide/Xenon.txt | 7 +++++++ Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 5 ++++- Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf | 1 + Drivers/SdMmc/XenonDxe/XenonSdhci.c | 16 ++++++++++++---- Drivers/SdMmc/XenonDxe/XenonSdhci.h | 3 ++- Platforms/Marvell/Marvell.dec | 1 + 6 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/Documentation/Marvell/PortingGuide/Xenon.txt b/Documentation/Marvell/PortingGuide/Xenon.txt index 297e179..159a417 100644 --- a/Documentation/Marvell/PortingGuide/Xenon.txt +++ b/Documentation/Marvell/PortingGuide/Xenon.txt @@ -16,6 +16,10 @@ Indicates, whether the interface is supplied with 1.8V. Indicates, whether the interface is using 8-bit bus.
- gMarvellTokenSpaceGuid.PcdXenonSlowModeEnable
+Indicates, whether the interface is using so called 'slow mode' (PHY bypassing).
Examples
Assuming we want to enable both SdMmc ports on Armada 70x0 board, first one is @@ -26,3 +30,6 @@ supplied with 3.3V and second one with 1.8V: Use 8-bit bus only with first controller: gMarvellTokenSpaceGuid.PcdXenon8BitBusEnable|{ 0x1, 0x0 }
+Use 'slow mode' for both controllers:
- gMarvellTokenSpaceGuid.PcdXenonSlowModeEnable|{ 0x1, 0x1 }
diff --git a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c index 57d41c2..1d5346a 100644 --- a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c +++ b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c @@ -101,6 +101,7 @@ STATIC UINT8 XenonIdx; STATIC UINT8 * CONST XenonDevEnabled = FixedPcdGetPtr (PcdPciESdhci); STATIC UINT8 * CONST Xenon1v8Enabled = FixedPcdGetPtr (PcdXenon1v8Enable); STATIC UINT8 * CONST Xenon8BitBusEnabled = FixedPcdGetPtr (PcdXenon8BitBusEnable); +STATIC UINT8 * CONST XenonSlowModeEnabled = FixedPcdGetPtr (PcdXenonSlowModeEnable); // // Prioritized function list to detect card type. @@ -542,6 +543,7 @@ SdMmcPciHcDriverBindingStart ( BOOLEAN Support64BitDma; BOOLEAN Support1v8; BOOLEAN Support8Bit;
- BOOLEAN SlowMode;
DEBUG ((DEBUG_INFO, "SdMmcPciHcDriverBindingStart: Start\n")); @@ -557,6 +559,7 @@ SdMmcPciHcDriverBindingStart ( // Support1v8 = Xenon1v8Enabled[XenonIdx]; Support8Bit = Xenon8BitBusEnabled[XenonIdx];
- SlowMode = XenonSlowModeEnabled[XenonIdx];
This is also starting to stick out a bit.
Could you instead move the increment of XenonIdx later in function and create a macro. Given a global struct called Config, you could do something like:
#define FEATURE_ENABLED(x, y) (Config.Xenon ## x ## Enabled[y])
And then inline
XenonIdx++; // @@ -672,7 +675,7 @@ SdMmcPciHcDriverBindingStart ( // // Perform Xenon-specific init sequence //
- XenonInit (Private);
- XenonInit (Private, SlowMode);
XenonInit (Private, FEATURE_ENABLED (SlowMode));
?
And adapt preceding patches to the same format?
/ Leif
// // Initialize HC timeout control diff --git a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf index 5b5cdd4..c64479b 100644 --- a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf +++ b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf @@ -57,6 +57,7 @@ gMarvellTokenSpaceGuid.PcdPciESdhci gMarvellTokenSpaceGuid.PcdXenon1v8Enable gMarvellTokenSpaceGuid.PcdXenon8BitBusEnable
- gMarvellTokenSpaceGuid.PcdXenonSlowModeEnable
[Protocols] gEfiDevicePathProtocolGuid ## TO_START diff --git a/Drivers/SdMmc/XenonDxe/XenonSdhci.c b/Drivers/SdMmc/XenonDxe/XenonSdhci.c index 31f207e..fe0b564 100755 --- a/Drivers/SdMmc/XenonDxe/XenonSdhci.c +++ b/Drivers/SdMmc/XenonDxe/XenonSdhci.c @@ -344,7 +344,8 @@ STATIC VOID XenonSetPhy ( IN EFI_PCI_IO_PROTOCOL *PciIo,
- UINT8 Timing
- IN UINT8 Timing,
- IN BOOLEAN SlowMode )
{ UINT32 Var = 0; @@ -368,7 +369,13 @@ XenonSetPhy ( SdMmcHcRwMmio(PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, TRUE, SDHC_REG_SIZE_4B, &Var); // Set SLOW_MODE for PHY
- Var |= OUTPUT_QSN_PHASE_SELECT | QSN_PHASE_SLOW_MODE_BIT;
- if (SlowMode) {
Var |= QSN_PHASE_SLOW_MODE_BIT;- }
- // Set output clock polarity
- Var |= OUTPUT_QSN_PHASE_SELECT;
- SdMmcHcRwMmio(PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, FALSE, SDHC_REG_SIZE_4B, &Var); }
@@ -626,7 +633,8 @@ XenonTransferData ( EFI_STATUS XenonInit (
- IN SD_MMC_HC_PRIVATE_DATA *Private
- IN SD_MMC_HC_PRIVATE_DATA *Private,
- IN BOOLEAN SlowMode )
{ EFI_PCI_IO_PROTOCOL *PciIo = Private->PciIo; @@ -646,7 +654,7 @@ XenonInit ( // Set MAX_CLOCK for configuring PHY XenonSetClk (PciIo, Private, XENON_MMC_MAX_CLK);
- XenonSetPhy (PciIo, MMC_TIMING_UHS_SDR50);
- XenonSetPhy (PciIo, MMC_TIMING_UHS_SDR50, SlowMode);
XenonConfigureInterrupts (PciIo); diff --git a/Drivers/SdMmc/XenonDxe/XenonSdhci.h b/Drivers/SdMmc/XenonDxe/XenonSdhci.h index 2be0ee6..d22838b 100644 --- a/Drivers/SdMmc/XenonDxe/XenonSdhci.h +++ b/Drivers/SdMmc/XenonDxe/XenonSdhci.h @@ -334,7 +334,8 @@ XenonTransferData ( EFI_STATUS XenonInit (
- IN SD_MMC_HC_PRIVATE_DATA *Private
- IN SD_MMC_HC_PRIVATE_DATA *Private,
- IN BOOLEAN SlowMode );
EFI_STATUS diff --git a/Platforms/Marvell/Marvell.dec b/Platforms/Marvell/Marvell.dec index efd67b4..f011de1 100644 --- a/Platforms/Marvell/Marvell.dec +++ b/Platforms/Marvell/Marvell.dec @@ -222,6 +222,7 @@ #SdMmc gMarvellTokenSpaceGuid.PcdXenon1v8Enable|{ 0x0 }|VOID*|0x3000036 gMarvellTokenSpaceGuid.PcdXenon8BitBusEnable|{ 0x0 }|VOID*|0x3000037
- gMarvellTokenSpaceGuid.PcdXenonSlowModeEnable|{ 0x0 }|VOID*|0x3000038
[Protocols] gMarvellEepromProtocolGuid = { 0x71954bda, 0x60d3, 0x4ef8, { 0x8e, 0x3c, 0x0e, 0x33, 0x9f, 0x3b, 0xc2, 0x2b }} -- 1.8.3.1