On Thu, Jan 05, 2017 at 12:03:38PM +0100, Marcin Wojtas wrote:
This patch enables per-interface configuration of used power supply. For this purpose new PCD is added (PcdXenon1v8Enable). Implementation required obtaining current controller global index, with help of a PCD already used by PciEmulation driver (PcdPciESdhci), which indicates enabled controllers.
Porting guide documentation for XenonDxe was introduced.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Marcin Wojtas mw@semihalf.com
Documentation/Marvell/PortingGuide/Xenon.txt | 21 +++++++++++++++ Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 40 ++++++++++++++++++++++++---- Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf | 5 ++++ Platforms/Marvell/Marvell.dec | 3 +++ 4 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 Documentation/Marvell/PortingGuide/Xenon.txt
diff --git a/Documentation/Marvell/PortingGuide/Xenon.txt b/Documentation/Marvell/PortingGuide/Xenon.txt new file mode 100644 index 0000000..566884b --- /dev/null +++ b/Documentation/Marvell/PortingGuide/Xenon.txt @@ -0,0 +1,21 @@ +XenonDxe porting guide +-------------------- +XenonDxe is a driver supporting SdMmc interface on Marvell platforms. +Following PCDs are required to operate:
- gMarvellTokenSpaceGuid.PcdPciESdhci
+Indication of enabled Xenon controllers. It is common PCD with Marvell +PciEmulation driver (see Documentation/Marvell/PortingGuide/PciEmulation.txt):
- gMarvellTokenSpaceGuid.PcdXenon1v8Enable
+Indicates, whether the interface is supplied with 1.8V.
No ','.
+Examples +-------- +Assuming we want to enable both SdMmc ports on Armada 70x0 board, first one is +supplied with 3.3V and second one with 1.8V:
- gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1, 0x1 }
- gMarvellTokenSpaceGuid.PcdXenon1v8Enable|{0x0, 0x1}
Inconsistent spacing across the two lines. All existing docs use the spacing used on the first line.
diff --git a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c index 981eab5..f8a1772 100644 --- a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c +++ b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c @@ -91,6 +91,17 @@ EMMC_DEVICE_PATH mEmmcDpTemplate = { }; // +// Device global index +// +STATIC UINT8 XenonIdx;
A global variable to keep track of which device we touched last? If there is no other way of dealing with this than a static variable, please at least make it function local. And UINTN/UINT32.
+// +// Tables with used Xenon instances and their configuration +// +STATIC UINT8 * CONST XenonDevEnabled = FixedPcdGetPtr (PcdPciESdhci); +STATIC UINT8 * CONST Xenon1v8Enabled = FixedPcdGetPtr (PcdXenon1v8Enable);
+// // Prioritized function list to detect card type. // User could add other card detection logic here. // @@ -528,10 +539,24 @@ SdMmcPciHcDriverBindingStart ( CARD_TYPE_DETECT_ROUTINE *Routine; UINT32 RoutineNum; BOOLEAN Support64BitDma;
- BOOLEAN Support1v8;
DEBUG ((DEBUG_INFO, "SdMmcPciHcDriverBindingStart: Start\n")); //
- // Set current Xenon device index.
The code looks like what it's actually doing is finding the first enabled Xenon device. The comment should describe the logic it is implementing, the source code already describes the mechanism.
- //
- while (!XenonDevEnabled[XenonIdx]) {
- XenonIdx++;
- }
- //
- // Obtain configuration data for this device and increase index afterwards.
Yes, that's what the code does immediately after. The comment should explain why (or be deleted).
/ Leif
- //
- Support1v8 = Xenon1v8Enabled[XenonIdx];
- XenonIdx++;
- // // Open PCI I/O Protocol and save pointer to open protocol // in private data area. //
@@ -609,12 +634,17 @@ SdMmcPciHcDriverBindingStart ( Support64BitDma &= Private->Capability[Slot].SysBus64; //
- // Override capabilities structure - only 4 Bit width bus is supported
- // by HW and also force using SDR25 mode
- // Override capabilities structure according to board configuration. //
- Private->Capability[Slot].Sdr104 = 0;
- Private->Capability[Slot].Ddr50 = 0;
- Private->Capability[Slot].Sdr50 = 0;
- if (Support1v8) {
- Private->Capability[Slot].Voltage33 = 0;
- Private->Capability[Slot].Voltage30 = 0;
- } else {
- Private->Capability[Slot].Sdr104 = 0;
- Private->Capability[Slot].Ddr50 = 0;
- Private->Capability[Slot].Sdr50 = 0;
- }
- Private->Capability[Slot].BusWidth8 = 0;
if (Private->Capability[Slot].BaseClkFreq == 0) { diff --git a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf index fad9fc6..b929187 100644 --- a/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf +++ b/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.inf @@ -40,6 +40,7 @@ [Packages] MdePkg/MdePkg.dec
- OpenPlatformPkg/Platforms/Marvell/Marvell.dec
[LibraryClasses] BaseLib @@ -52,6 +53,10 @@ UefiLib UefiRuntimeServicesTableLib +[Pcd]
- gMarvellTokenSpaceGuid.PcdPciESdhci
- gMarvellTokenSpaceGuid.PcdXenon1v8Enable
[Protocols] gEfiDevicePathProtocolGuid ## TO_START gEfiPciIoProtocolGuid ## TO_START diff --git a/Platforms/Marvell/Marvell.dec b/Platforms/Marvell/Marvell.dec index 313eaa6..980697b 100644 --- a/Platforms/Marvell/Marvell.dec +++ b/Platforms/Marvell/Marvell.dec @@ -219,6 +219,9 @@ gMarvellTokenSpaceGuid.PcdResetRegAddress|0|UINT64|0x40000050 gMarvellTokenSpaceGuid.PcdResetRegMask|0|UINT32|0x4000051 +#SdMmc
- gMarvellTokenSpaceGuid.PcdXenon1v8Enable|{ 0x0 }|VOID*|0x3000036
[Protocols] gMarvellEepromProtocolGuid = { 0x71954bda, 0x60d3, 0x4ef8, { 0x8e, 0x3c, 0x0e, 0x33, 0x9f, 0x3b, 0xc2, 0x2b }} gMarvellMdioProtocolGuid = { 0x40010b03, 0x5f08, 0x496a, { 0xa2, 0x64, 0x10, 0x5e, 0x72, 0xd3, 0x71, 0xaa }} -- 1.8.3.1