* [edk2-platforms: PATCH 0/3] Armada7k8k FVB improvements
@ 2019-04-17 18:05 Marcin Wojtas
2019-04-17 18:05 ` [edk2-platforms: PATCH 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Marcin Wojtas @ 2019-04-17 18:05 UTC (permalink / raw)
To: devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
Jici.Gao
Hi,
Here's another short patchset from my stack. The changes
introduce support additional method of preventing hang
when accessing variables from the OS.
Also a support is added for platforms with the SPI whose
memory-mapped access is not configured (or even possible).
The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/fvb-upstream-r20190417
I'm looking forward to your comments or remarks.
Best regards,
Marcin
Hanna Hawa (1):
Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit
Kornel Duleba (1):
Marvell/Drivers: Add non-mmio mode to MvFvbDxe
Marcin Wojtas (1):
Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime
Silicon/Marvell/Marvell.dec | 13 +-
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +-
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 +-
Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf | 3 +
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 +
Silicon/Marvell/Include/Protocol/Spi.h | 10 ++
Silicon/Marvell/Include/Protocol/SpiFlash.h | 7 +
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 180 ++++++++++++++++----
Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c | 11 ++
Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c | 63 ++++++-
10 files changed, 269 insertions(+), 46 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [edk2-platforms: PATCH 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit 2019-04-17 18:05 [edk2-platforms: PATCH 0/3] Armada7k8k FVB improvements Marcin Wojtas @ 2019-04-17 18:05 ` Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2 siblings, 0 replies; 8+ messages in thread From: Marcin Wojtas @ 2019-04-17 18:05 UTC (permalink / raw) To: devel Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap, Jici.Gao, Hanna Hawa From: Hanna Hawa <hannah@marvell.com> Update PCD paramters to be 64 bit, so that to add more flexibility for the platforms in terms of configuring memory-mapped SPI access. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Marvell.dec | 2 +- Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 6 +++--- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 6 +++--- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 18 +++++++++--------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec index c927078..7210ba2 100644 --- a/Silicon/Marvell/Marvell.dec +++ b/Silicon/Marvell/Marvell.dec @@ -139,7 +139,7 @@ #SPI gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 - gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT32|0x3000059 + gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index 0ced400..a1ebb81 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -396,11 +396,11 @@ # Variable store - default values # gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0xF93C0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0xF93D0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0xF93E0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 !if $(CAPSULE_ENABLE) diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf index fd3f2f7..ef10bfd 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf @@ -76,11 +76,11 @@ gMarvellSpiMasterProtocolGuid [FixedPcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize gMarvellTokenSpaceGuid.PcdSpiMemoryBase diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c index 1a41a4f..cb006cd 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c @@ -145,12 +145,12 @@ MvFvbInitFvAndVariableStoreHeaders ( // FirmwareVolumeHeader->FvLength is declared to have the Variable area // AND the FTW working area AND the FTW Spare contiguous. // - ASSERT (PcdGet32 (PcdFlashNvStorageVariableBase) + + ASSERT (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) == - PcdGet32 (PcdFlashNvStorageFtwWorkingBase)); - ASSERT (PcdGet32 (PcdFlashNvStorageFtwWorkingBase) + + PcdGet64 (PcdFlashNvStorageFtwWorkingBase64)); + ASSERT (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) + PcdGet32 (PcdFlashNvStorageFtwWorkingSize) == - PcdGet32 (PcdFlashNvStorageFtwSpareBase)); + PcdGet64 (PcdFlashNvStorageFtwSpareBase64)); // Check if the size of the area is at least one block size ASSERT ((PcdGet32 (PcdFlashNvStorageVariableSize) > 0) && @@ -161,9 +161,9 @@ MvFvbInitFvAndVariableStoreHeaders ( (PcdGet32 (PcdFlashNvStorageFtwSpareSize) / BlockSize > 0)); // Ensure the Variable areas are aligned on block size boundaries - ASSERT ((PcdGet32 (PcdFlashNvStorageVariableBase) % BlockSize) == 0); - ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingBase) % BlockSize) == 0); - ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareBase) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageVariableBase64) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageFtwSpareBase64) % BlockSize) == 0); // // EFI_FIRMWARE_VOLUME_HEADER @@ -1009,8 +1009,8 @@ MvFvbConfigureFlashInstance ( } // Fill remaining flash description - FlashInstance->DeviceBaseAddress = PcdGet32 (PcdSpiMemoryBase); - FlashInstance->RegionBaseAddress = FixedPcdGet32 (PcdFlashNvStorageVariableBase); + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); + FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - FlashInstance->DeviceBaseAddress; FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime 2019-04-17 18:05 [edk2-platforms: PATCH 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas @ 2019-04-17 18:05 ` Marcin Wojtas 2019-04-17 18:27 ` Ard Biesheuvel 2019-04-17 18:05 ` [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2 siblings, 1 reply; 8+ messages in thread From: Marcin Wojtas @ 2019-04-17 18:05 UTC (permalink / raw) To: devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap, Jici.Gao Linux MTD subsystem enables the SPI controller clock only when explicitly accessing the device. Because of that, using variables stored in the SPI flash could cause a hang when this clock remained disabled. Such issue was prevented when booting with the default DTB - see commit 124073e4d440 ("Marvell/Armada7k8k: DeviceTree: Use fixed clocks"). However in case someone wishes to try a different DTB, the issue can become problematic again. Fix above problem by enabling the clock when accessing SPI controller registers or mapped memory region AtRuntime. For this purpose add required callbacks to the SpiMaster and SpiFlash protocols that are called from MvFvbDxe driver. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Marvell.dec | 3 + Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf | 3 + Silicon/Marvell/Include/Protocol/Spi.h | 10 ++++ Silicon/Marvell/Include/Protocol/SpiFlash.h | 7 +++ Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 34 +++++++++++ Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c | 11 ++++ Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c | 63 +++++++++++++++++++- 7 files changed, 129 insertions(+), 2 deletions(-) diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec index 7210ba2..201a62f 100644 --- a/Silicon/Marvell/Marvell.dec +++ b/Silicon/Marvell/Marvell.dec @@ -138,6 +138,9 @@ gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183 #SPI + gMarvellTokenSpaceGuid.PcdSpiClockFixed|TRUE|BOOLEAN|0x30000054 + gMarvellTokenSpaceGuid.PcdSpiClockMask|0|UINT32|0x30000055 + gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf index 4779371..1857484 100644 --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf @@ -59,7 +59,10 @@ UefiRuntimeLib [FixedPcd] + gMarvellTokenSpaceGuid.PcdSpiClockFixed gMarvellTokenSpaceGuid.PcdSpiClockFrequency + gMarvellTokenSpaceGuid.PcdSpiClockMask + gMarvellTokenSpaceGuid.PcdSpiClockRegBase gMarvellTokenSpaceGuid.PcdSpiMaxFrequency gMarvellTokenSpaceGuid.PcdSpiRegBase diff --git a/Silicon/Marvell/Include/Protocol/Spi.h b/Silicon/Marvell/Include/Protocol/Spi.h index abbad19..4d66f7f 100644 --- a/Silicon/Marvell/Include/Protocol/Spi.h +++ b/Silicon/Marvell/Include/Protocol/Spi.h @@ -54,6 +54,9 @@ typedef struct { UINT32 AddrSize; NOR_FLASH_INFO *Info; UINTN HostRegisterBaseAddress; + BOOLEAN HostClockFixed; + UINTN HostClockEnableRegister; + UINT32 HostClockEnableMask; UINTN CoreClock; } SPI_DEVICE; @@ -107,6 +110,12 @@ EFI_STATUS IN SPI_DEVICE *SpiDev ); +typedef +EFI_STATUS +(EFIAPI *MV_SPI_POWER_ON) ( + IN SPI_DEVICE *SpiDev + ); + struct _MARVELL_SPI_MASTER_PROTOCOL { MV_SPI_INIT Init; MV_SPI_READ_WRITE ReadWrite; @@ -114,6 +123,7 @@ struct _MARVELL_SPI_MASTER_PROTOCOL { MV_SPI_SETUP_DEVICE SetupDevice; MV_SPI_FREE_DEVICE FreeDevice; MV_SPI_CONFIG_RT ConfigRuntime; + MV_SPI_POWER_ON ControllerPowerOn; }; #endif // __MARVELL_SPI_MASTER_PROTOCOL_H__ diff --git a/Silicon/Marvell/Include/Protocol/SpiFlash.h b/Silicon/Marvell/Include/Protocol/SpiFlash.h index e703330..0336dda 100644 --- a/Silicon/Marvell/Include/Protocol/SpiFlash.h +++ b/Silicon/Marvell/Include/Protocol/SpiFlash.h @@ -102,6 +102,12 @@ EFI_STATUS IN UINTN EndPercentage ); +typedef +EFI_STATUS +(EFIAPI *MV_SPI_FLASH_POWER_ON) ( + IN SPI_DEVICE *SpiDev + ); + struct _MARVELL_SPI_FLASH_PROTOCOL { MV_SPI_FLASH_INIT Init; MV_SPI_FLASH_READ_ID ReadId; @@ -110,6 +116,7 @@ struct _MARVELL_SPI_FLASH_PROTOCOL { MV_SPI_FLASH_ERASE Erase; MV_SPI_FLASH_UPDATE Update; MV_SPI_FLASH_UPDATE_WITH_PROGRESS UpdateWithProgress; + MV_SPI_FLASH_POWER_ON FlashPowerOn; }; #endif // __MV_SPI_FLASH__ diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c index cb006cd..b57f415 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c @@ -311,6 +311,11 @@ MvFvbGetAttributes ( FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } + FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)FlashInstance->RegionBaseAddress; FlashFvbAttributes = (EFI_FVB_ATTRIBUTES_2 *)&(FwVolHeader->Attributes); @@ -349,10 +354,18 @@ MvFvbSetAttributes ( EFI_FVB_ATTRIBUTES_2 OldAttributes; EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; + FVB_DEVICE *FlashInstance; UINT32 Capabilities; UINT32 OldStatus; UINT32 NewStatus; + FlashInstance = INSTANCE_FROM_FVB_THIS (This); + + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } + // // Obtain attributes from FVB header // @@ -468,6 +481,11 @@ MvFvbGetPhysicalAddress ( FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } + *Address = FlashInstance->RegionBaseAddress; return EFI_SUCCESS; @@ -588,6 +606,10 @@ MvFvbRead ( FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } // Cache the block size to avoid de-referencing pointers all the time BlockSize = FlashInstance->Media.BlockSize; @@ -697,6 +719,11 @@ MvFvbWrite ( FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset + Offset, FlashInstance->StartLba + Lba, FlashInstance->Media.BlockSize); @@ -770,6 +797,11 @@ MvFvbEraseBlocks ( FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // Ensure proper access to the flash device from OS level + if (EfiAtRuntime ()) { + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); + } + Status = EFI_SUCCESS; // Detect WriteDisabled state @@ -882,11 +914,13 @@ MvFvbVirtualNotifyEvent ( // Convert SPI device description EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.Info); EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostRegisterBaseAddress); + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostClockEnableRegister); EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice); // Convert SpiFlashProtocol EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Erase); EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Write); + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->FlashPowerOn); EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol); return; diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c index d81f6e3..4cc897f 100755 --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c @@ -548,6 +548,15 @@ MvSpiFlashInit ( } EFI_STATUS +EFIAPI +MvSpiFlashPowerOn ( + IN SPI_DEVICE *Slave + ) +{ + return SpiMasterProtocol->ControllerPowerOn (Slave); +} + +EFI_STATUS MvSpiFlashInitProtocol ( IN MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol ) @@ -560,6 +569,7 @@ MvSpiFlashInitProtocol ( SpiFlashProtocol->Erase = MvSpiFlashErase; SpiFlashProtocol->Update = MvSpiFlashUpdate; SpiFlashProtocol->UpdateWithProgress = MvSpiFlashUpdateWithProgress; + SpiFlashProtocol->FlashPowerOn = MvSpiFlashPowerOn; return EFI_SUCCESS; } @@ -586,6 +596,7 @@ MvSpiFlashVirtualNotifyEvent ( // EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ReadWrite); EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->Transfer); + EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ControllerPowerOn); EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol); return; diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c index cbf403f..3f7f67e 100755 --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c @@ -323,6 +323,11 @@ MvSpiSetupSlave ( } Slave->HostRegisterBaseAddress = PcdGet32 (PcdSpiRegBase); + Slave->HostClockFixed = PcdGetBool (PcdSpiClockFixed); + if (!Slave->HostClockFixed) { + Slave->HostClockEnableRegister = PcdGet64 (PcdSpiClockRegBase); + Slave->HostClockEnableMask = PcdGet32 (PcdSpiClockMask); + } Slave->CoreClock = PcdGet32 (PcdSpiClockFrequency); Slave->MaxFreq = PcdGet32 (PcdSpiMaxFrequency); @@ -344,12 +349,26 @@ MvSpiFreeSlave ( EFI_STATUS EFIAPI +MvSpiControllerPowerOn ( + IN SPI_DEVICE *Slave + ) +{ + if (!Slave->HostClockFixed) { + MmioOr32 (Slave->HostClockEnableRegister, Slave->HostClockEnableMask); + } + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI MvSpiConfigRuntime ( IN SPI_DEVICE *Slave ) { EFI_STATUS Status; UINTN AlignedAddress; + UINTN ClockEnableAlignedAddress; // // Host register base may be not aligned to the page size, @@ -373,11 +392,50 @@ MvSpiConfigRuntime ( EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); - gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); - return Status; + goto ErrorSetHostMemorySpace; + } + + // + // In case a clock feeding the SPI interface is always on, skip its + // configuration for Runtime. + // + if (Slave->HostClockFixed) { + return EFI_SUCCESS; + } + + // + // Make sure about an alignment of the memory space needed for clock enabling. + // Add one aligned page of memory space which covers the clock enabling + // register. + // + ClockEnableAlignedAddress = Slave->HostClockEnableRegister & ~(SIZE_4KB - 1); + + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, + ClockEnableAlignedAddress, + SIZE_4KB, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); + goto ErrorSetHostMemorySpace; + } + + Status = gDS->SetMemorySpaceAttributes (ClockEnableAlignedAddress, + SIZE_4KB, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); + goto ErrorSetClockMemorySpace; } return EFI_SUCCESS; + +ErrorSetClockMemorySpace: + gDS->RemoveMemorySpace (ClockEnableAlignedAddress, SIZE_4KB); + +ErrorSetHostMemorySpace: + gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); + + return Status; } STATIC @@ -393,6 +451,7 @@ SpiMasterInitProtocol ( SpiMasterProtocol->Transfer = MvSpiTransfer; SpiMasterProtocol->ReadWrite = MvSpiReadWrite; SpiMasterProtocol->ConfigRuntime = MvSpiConfigRuntime; + SpiMasterProtocol->ControllerPowerOn = MvSpiControllerPowerOn; return EFI_SUCCESS; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime 2019-04-17 18:05 ` [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime Marcin Wojtas @ 2019-04-17 18:27 ` Ard Biesheuvel 2019-04-17 18:30 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2019-04-17 18:27 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao On Wed, 17 Apr 2019 at 11:06, Marcin Wojtas <mw@semihalf.com> wrote: > > Linux MTD subsystem enables the SPI controller clock > only when explicitly accessing the device. Because of that, > using variables stored in the SPI flash could cause a hang > when this clock remained disabled. > > Such issue was prevented when booting with the default > DTB - see commit 124073e4d440 ("Marvell/Armada7k8k: DeviceTree: > Use fixed clocks"). However in case someone wishes to > try a different DTB, the issue can become problematic again. > > Fix above problem by enabling the clock when accessing > SPI controller registers or mapped memory region > AtRuntime. For this purpose add required callbacks > to the SpiMaster and SpiFlash protocols that are > called from MvFvbDxe driver. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> This is really not the right solution. Linux runs UEFI runtime services on a single core with interrupts enabled, and it thinks it owns the SPI subsystem if you describe it in the DT, and may decide to access it at the same time, either in interrupt ot softirq context, or from another core. > --- > Silicon/Marvell/Marvell.dec | 3 + > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf | 3 + > Silicon/Marvell/Include/Protocol/Spi.h | 10 ++++ > Silicon/Marvell/Include/Protocol/SpiFlash.h | 7 +++ > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 34 +++++++++++ > Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c | 11 ++++ > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c | 63 +++++++++++++++++++- > 7 files changed, 129 insertions(+), 2 deletions(-) > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > index 7210ba2..201a62f 100644 > --- a/Silicon/Marvell/Marvell.dec > +++ b/Silicon/Marvell/Marvell.dec > @@ -138,6 +138,9 @@ > gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183 > > #SPI > + gMarvellTokenSpaceGuid.PcdSpiClockFixed|TRUE|BOOLEAN|0x30000054 > + gMarvellTokenSpaceGuid.PcdSpiClockMask|0|UINT32|0x30000055 > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > index 4779371..1857484 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > @@ -59,7 +59,10 @@ > UefiRuntimeLib > > [FixedPcd] > + gMarvellTokenSpaceGuid.PcdSpiClockFixed > gMarvellTokenSpaceGuid.PcdSpiClockFrequency > + gMarvellTokenSpaceGuid.PcdSpiClockMask > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency > gMarvellTokenSpaceGuid.PcdSpiRegBase > > diff --git a/Silicon/Marvell/Include/Protocol/Spi.h b/Silicon/Marvell/Include/Protocol/Spi.h > index abbad19..4d66f7f 100644 > --- a/Silicon/Marvell/Include/Protocol/Spi.h > +++ b/Silicon/Marvell/Include/Protocol/Spi.h > @@ -54,6 +54,9 @@ typedef struct { > UINT32 AddrSize; > NOR_FLASH_INFO *Info; > UINTN HostRegisterBaseAddress; > + BOOLEAN HostClockFixed; > + UINTN HostClockEnableRegister; > + UINT32 HostClockEnableMask; > UINTN CoreClock; > } SPI_DEVICE; > > @@ -107,6 +110,12 @@ EFI_STATUS > IN SPI_DEVICE *SpiDev > ); > > +typedef > +EFI_STATUS > +(EFIAPI *MV_SPI_POWER_ON) ( > + IN SPI_DEVICE *SpiDev > + ); > + > struct _MARVELL_SPI_MASTER_PROTOCOL { > MV_SPI_INIT Init; > MV_SPI_READ_WRITE ReadWrite; > @@ -114,6 +123,7 @@ struct _MARVELL_SPI_MASTER_PROTOCOL { > MV_SPI_SETUP_DEVICE SetupDevice; > MV_SPI_FREE_DEVICE FreeDevice; > MV_SPI_CONFIG_RT ConfigRuntime; > + MV_SPI_POWER_ON ControllerPowerOn; > }; > > #endif // __MARVELL_SPI_MASTER_PROTOCOL_H__ > diff --git a/Silicon/Marvell/Include/Protocol/SpiFlash.h b/Silicon/Marvell/Include/Protocol/SpiFlash.h > index e703330..0336dda 100644 > --- a/Silicon/Marvell/Include/Protocol/SpiFlash.h > +++ b/Silicon/Marvell/Include/Protocol/SpiFlash.h > @@ -102,6 +102,12 @@ EFI_STATUS > IN UINTN EndPercentage > ); > > +typedef > +EFI_STATUS > +(EFIAPI *MV_SPI_FLASH_POWER_ON) ( > + IN SPI_DEVICE *SpiDev > + ); > + > struct _MARVELL_SPI_FLASH_PROTOCOL { > MV_SPI_FLASH_INIT Init; > MV_SPI_FLASH_READ_ID ReadId; > @@ -110,6 +116,7 @@ struct _MARVELL_SPI_FLASH_PROTOCOL { > MV_SPI_FLASH_ERASE Erase; > MV_SPI_FLASH_UPDATE Update; > MV_SPI_FLASH_UPDATE_WITH_PROGRESS UpdateWithProgress; > + MV_SPI_FLASH_POWER_ON FlashPowerOn; > }; > > #endif // __MV_SPI_FLASH__ > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > index cb006cd..b57f415 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > @@ -311,6 +311,11 @@ MvFvbGetAttributes ( > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > + > FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)FlashInstance->RegionBaseAddress; > FlashFvbAttributes = (EFI_FVB_ATTRIBUTES_2 *)&(FwVolHeader->Attributes); > > @@ -349,10 +354,18 @@ MvFvbSetAttributes ( > EFI_FVB_ATTRIBUTES_2 OldAttributes; > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > + FVB_DEVICE *FlashInstance; > UINT32 Capabilities; > UINT32 OldStatus; > UINT32 NewStatus; > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > + > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > + > // > // Obtain attributes from FVB header > // > @@ -468,6 +481,11 @@ MvFvbGetPhysicalAddress ( > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > + > *Address = FlashInstance->RegionBaseAddress; > > return EFI_SUCCESS; > @@ -588,6 +606,10 @@ MvFvbRead ( > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > > // Cache the block size to avoid de-referencing pointers all the time > BlockSize = FlashInstance->Media.BlockSize; > @@ -697,6 +719,11 @@ MvFvbWrite ( > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > + > DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset + Offset, > FlashInstance->StartLba + Lba, > FlashInstance->Media.BlockSize); > @@ -770,6 +797,11 @@ MvFvbEraseBlocks ( > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + // Ensure proper access to the flash device from OS level > + if (EfiAtRuntime ()) { > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > + } > + > Status = EFI_SUCCESS; > > // Detect WriteDisabled state > @@ -882,11 +914,13 @@ MvFvbVirtualNotifyEvent ( > // Convert SPI device description > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.Info); > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostRegisterBaseAddress); > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostClockEnableRegister); > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice); > > // Convert SpiFlashProtocol > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Erase); > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Write); > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->FlashPowerOn); > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol); > > return; > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > index d81f6e3..4cc897f 100755 > --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > @@ -548,6 +548,15 @@ MvSpiFlashInit ( > } > > EFI_STATUS > +EFIAPI > +MvSpiFlashPowerOn ( > + IN SPI_DEVICE *Slave > + ) > +{ > + return SpiMasterProtocol->ControllerPowerOn (Slave); > +} > + > +EFI_STATUS > MvSpiFlashInitProtocol ( > IN MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol > ) > @@ -560,6 +569,7 @@ MvSpiFlashInitProtocol ( > SpiFlashProtocol->Erase = MvSpiFlashErase; > SpiFlashProtocol->Update = MvSpiFlashUpdate; > SpiFlashProtocol->UpdateWithProgress = MvSpiFlashUpdateWithProgress; > + SpiFlashProtocol->FlashPowerOn = MvSpiFlashPowerOn; > > return EFI_SUCCESS; > } > @@ -586,6 +596,7 @@ MvSpiFlashVirtualNotifyEvent ( > // > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ReadWrite); > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->Transfer); > + EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ControllerPowerOn); > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol); > > return; > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > index cbf403f..3f7f67e 100755 > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > @@ -323,6 +323,11 @@ MvSpiSetupSlave ( > } > > Slave->HostRegisterBaseAddress = PcdGet32 (PcdSpiRegBase); > + Slave->HostClockFixed = PcdGetBool (PcdSpiClockFixed); > + if (!Slave->HostClockFixed) { > + Slave->HostClockEnableRegister = PcdGet64 (PcdSpiClockRegBase); > + Slave->HostClockEnableMask = PcdGet32 (PcdSpiClockMask); > + } > Slave->CoreClock = PcdGet32 (PcdSpiClockFrequency); > Slave->MaxFreq = PcdGet32 (PcdSpiMaxFrequency); > > @@ -344,12 +349,26 @@ MvSpiFreeSlave ( > > EFI_STATUS > EFIAPI > +MvSpiControllerPowerOn ( > + IN SPI_DEVICE *Slave > + ) > +{ > + if (!Slave->HostClockFixed) { > + MmioOr32 (Slave->HostClockEnableRegister, Slave->HostClockEnableMask); > + } > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > MvSpiConfigRuntime ( > IN SPI_DEVICE *Slave > ) > { > EFI_STATUS Status; > UINTN AlignedAddress; > + UINTN ClockEnableAlignedAddress; > > // > // Host register base may be not aligned to the page size, > @@ -373,11 +392,50 @@ MvSpiConfigRuntime ( > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > - gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > - return Status; > + goto ErrorSetHostMemorySpace; > + } > + > + // > + // In case a clock feeding the SPI interface is always on, skip its > + // configuration for Runtime. > + // > + if (Slave->HostClockFixed) { > + return EFI_SUCCESS; > + } > + > + // > + // Make sure about an alignment of the memory space needed for clock enabling. > + // Add one aligned page of memory space which covers the clock enabling > + // register. > + // > + ClockEnableAlignedAddress = Slave->HostClockEnableRegister & ~(SIZE_4KB - 1); > + > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > + ClockEnableAlignedAddress, > + SIZE_4KB, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > + goto ErrorSetHostMemorySpace; > + } > + > + Status = gDS->SetMemorySpaceAttributes (ClockEnableAlignedAddress, > + SIZE_4KB, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > + goto ErrorSetClockMemorySpace; > } > > return EFI_SUCCESS; > + > +ErrorSetClockMemorySpace: > + gDS->RemoveMemorySpace (ClockEnableAlignedAddress, SIZE_4KB); > + > +ErrorSetHostMemorySpace: > + gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > + > + return Status; > } > > STATIC > @@ -393,6 +451,7 @@ SpiMasterInitProtocol ( > SpiMasterProtocol->Transfer = MvSpiTransfer; > SpiMasterProtocol->ReadWrite = MvSpiReadWrite; > SpiMasterProtocol->ConfigRuntime = MvSpiConfigRuntime; > + SpiMasterProtocol->ControllerPowerOn = MvSpiControllerPowerOn; > > return EFI_SUCCESS; > } > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime 2019-04-17 18:27 ` Ard Biesheuvel @ 2019-04-17 18:30 ` Ard Biesheuvel 2019-04-17 19:33 ` Marcin Wojtas 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2019-04-17 18:30 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao On Wed, 17 Apr 2019 at 11:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Wed, 17 Apr 2019 at 11:06, Marcin Wojtas <mw@semihalf.com> wrote: > > > > Linux MTD subsystem enables the SPI controller clock > > only when explicitly accessing the device. Because of that, > > using variables stored in the SPI flash could cause a hang > > when this clock remained disabled. > > > > Such issue was prevented when booting with the default > > DTB - see commit 124073e4d440 ("Marvell/Armada7k8k: DeviceTree: > > Use fixed clocks"). However in case someone wishes to > > try a different DTB, the issue can become problematic again. > > > > Fix above problem by enabling the clock when accessing > > SPI controller registers or mapped memory region > > AtRuntime. For this purpose add required callbacks > > to the SpiMaster and SpiFlash protocols that are > > called from MvFvbDxe driver. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > This is really not the right solution. Linux runs UEFI runtime > services on a single core with interrupts enabled, and it thinks it > owns the SPI subsystem if you describe it in the DT, and may decide to > access it at the same time, either in interrupt ot softirq context, or > from another core. > Note that this applies to just the clock as well. > > > --- > > Silicon/Marvell/Marvell.dec | 3 + > > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf | 3 + > > Silicon/Marvell/Include/Protocol/Spi.h | 10 ++++ > > Silicon/Marvell/Include/Protocol/SpiFlash.h | 7 +++ > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 34 +++++++++++ > > Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c | 11 ++++ > > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c | 63 +++++++++++++++++++- > > 7 files changed, 129 insertions(+), 2 deletions(-) > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > index 7210ba2..201a62f 100644 > > --- a/Silicon/Marvell/Marvell.dec > > +++ b/Silicon/Marvell/Marvell.dec > > @@ -138,6 +138,9 @@ > > gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183 > > > > #SPI > > + gMarvellTokenSpaceGuid.PcdSpiClockFixed|TRUE|BOOLEAN|0x30000054 > > + gMarvellTokenSpaceGuid.PcdSpiClockMask|0|UINT32|0x30000055 > > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > index 4779371..1857484 100644 > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > @@ -59,7 +59,10 @@ > > UefiRuntimeLib > > > > [FixedPcd] > > + gMarvellTokenSpaceGuid.PcdSpiClockFixed > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency > > + gMarvellTokenSpaceGuid.PcdSpiClockMask > > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency > > gMarvellTokenSpaceGuid.PcdSpiRegBase > > > > diff --git a/Silicon/Marvell/Include/Protocol/Spi.h b/Silicon/Marvell/Include/Protocol/Spi.h > > index abbad19..4d66f7f 100644 > > --- a/Silicon/Marvell/Include/Protocol/Spi.h > > +++ b/Silicon/Marvell/Include/Protocol/Spi.h > > @@ -54,6 +54,9 @@ typedef struct { > > UINT32 AddrSize; > > NOR_FLASH_INFO *Info; > > UINTN HostRegisterBaseAddress; > > + BOOLEAN HostClockFixed; > > + UINTN HostClockEnableRegister; > > + UINT32 HostClockEnableMask; > > UINTN CoreClock; > > } SPI_DEVICE; > > > > @@ -107,6 +110,12 @@ EFI_STATUS > > IN SPI_DEVICE *SpiDev > > ); > > > > +typedef > > +EFI_STATUS > > +(EFIAPI *MV_SPI_POWER_ON) ( > > + IN SPI_DEVICE *SpiDev > > + ); > > + > > struct _MARVELL_SPI_MASTER_PROTOCOL { > > MV_SPI_INIT Init; > > MV_SPI_READ_WRITE ReadWrite; > > @@ -114,6 +123,7 @@ struct _MARVELL_SPI_MASTER_PROTOCOL { > > MV_SPI_SETUP_DEVICE SetupDevice; > > MV_SPI_FREE_DEVICE FreeDevice; > > MV_SPI_CONFIG_RT ConfigRuntime; > > + MV_SPI_POWER_ON ControllerPowerOn; > > }; > > > > #endif // __MARVELL_SPI_MASTER_PROTOCOL_H__ > > diff --git a/Silicon/Marvell/Include/Protocol/SpiFlash.h b/Silicon/Marvell/Include/Protocol/SpiFlash.h > > index e703330..0336dda 100644 > > --- a/Silicon/Marvell/Include/Protocol/SpiFlash.h > > +++ b/Silicon/Marvell/Include/Protocol/SpiFlash.h > > @@ -102,6 +102,12 @@ EFI_STATUS > > IN UINTN EndPercentage > > ); > > > > +typedef > > +EFI_STATUS > > +(EFIAPI *MV_SPI_FLASH_POWER_ON) ( > > + IN SPI_DEVICE *SpiDev > > + ); > > + > > struct _MARVELL_SPI_FLASH_PROTOCOL { > > MV_SPI_FLASH_INIT Init; > > MV_SPI_FLASH_READ_ID ReadId; > > @@ -110,6 +116,7 @@ struct _MARVELL_SPI_FLASH_PROTOCOL { > > MV_SPI_FLASH_ERASE Erase; > > MV_SPI_FLASH_UPDATE Update; > > MV_SPI_FLASH_UPDATE_WITH_PROGRESS UpdateWithProgress; > > + MV_SPI_FLASH_POWER_ON FlashPowerOn; > > }; > > > > #endif // __MV_SPI_FLASH__ > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > index cb006cd..b57f415 100644 > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > @@ -311,6 +311,11 @@ MvFvbGetAttributes ( > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > + > > FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)FlashInstance->RegionBaseAddress; > > FlashFvbAttributes = (EFI_FVB_ATTRIBUTES_2 *)&(FwVolHeader->Attributes); > > > > @@ -349,10 +354,18 @@ MvFvbSetAttributes ( > > EFI_FVB_ATTRIBUTES_2 OldAttributes; > > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > > + FVB_DEVICE *FlashInstance; > > UINT32 Capabilities; > > UINT32 OldStatus; > > UINT32 NewStatus; > > > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > + > > // > > // Obtain attributes from FVB header > > // > > @@ -468,6 +481,11 @@ MvFvbGetPhysicalAddress ( > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > + > > *Address = FlashInstance->RegionBaseAddress; > > > > return EFI_SUCCESS; > > @@ -588,6 +606,10 @@ MvFvbRead ( > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > > > // Cache the block size to avoid de-referencing pointers all the time > > BlockSize = FlashInstance->Media.BlockSize; > > @@ -697,6 +719,11 @@ MvFvbWrite ( > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > + > > DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset + Offset, > > FlashInstance->StartLba + Lba, > > FlashInstance->Media.BlockSize); > > @@ -770,6 +797,11 @@ MvFvbEraseBlocks ( > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > + // Ensure proper access to the flash device from OS level > > + if (EfiAtRuntime ()) { > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > + } > > + > > Status = EFI_SUCCESS; > > > > // Detect WriteDisabled state > > @@ -882,11 +914,13 @@ MvFvbVirtualNotifyEvent ( > > // Convert SPI device description > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.Info); > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostRegisterBaseAddress); > > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostClockEnableRegister); > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice); > > > > // Convert SpiFlashProtocol > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Erase); > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Write); > > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->FlashPowerOn); > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol); > > > > return; > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > index d81f6e3..4cc897f 100755 > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > @@ -548,6 +548,15 @@ MvSpiFlashInit ( > > } > > > > EFI_STATUS > > +EFIAPI > > +MvSpiFlashPowerOn ( > > + IN SPI_DEVICE *Slave > > + ) > > +{ > > + return SpiMasterProtocol->ControllerPowerOn (Slave); > > +} > > + > > +EFI_STATUS > > MvSpiFlashInitProtocol ( > > IN MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol > > ) > > @@ -560,6 +569,7 @@ MvSpiFlashInitProtocol ( > > SpiFlashProtocol->Erase = MvSpiFlashErase; > > SpiFlashProtocol->Update = MvSpiFlashUpdate; > > SpiFlashProtocol->UpdateWithProgress = MvSpiFlashUpdateWithProgress; > > + SpiFlashProtocol->FlashPowerOn = MvSpiFlashPowerOn; > > > > return EFI_SUCCESS; > > } > > @@ -586,6 +596,7 @@ MvSpiFlashVirtualNotifyEvent ( > > // > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ReadWrite); > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->Transfer); > > + EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ControllerPowerOn); > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol); > > > > return; > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > index cbf403f..3f7f67e 100755 > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > @@ -323,6 +323,11 @@ MvSpiSetupSlave ( > > } > > > > Slave->HostRegisterBaseAddress = PcdGet32 (PcdSpiRegBase); > > + Slave->HostClockFixed = PcdGetBool (PcdSpiClockFixed); > > + if (!Slave->HostClockFixed) { > > + Slave->HostClockEnableRegister = PcdGet64 (PcdSpiClockRegBase); > > + Slave->HostClockEnableMask = PcdGet32 (PcdSpiClockMask); > > + } > > Slave->CoreClock = PcdGet32 (PcdSpiClockFrequency); > > Slave->MaxFreq = PcdGet32 (PcdSpiMaxFrequency); > > > > @@ -344,12 +349,26 @@ MvSpiFreeSlave ( > > > > EFI_STATUS > > EFIAPI > > +MvSpiControllerPowerOn ( > > + IN SPI_DEVICE *Slave > > + ) > > +{ > > + if (!Slave->HostClockFixed) { > > + MmioOr32 (Slave->HostClockEnableRegister, Slave->HostClockEnableMask); > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > MvSpiConfigRuntime ( > > IN SPI_DEVICE *Slave > > ) > > { > > EFI_STATUS Status; > > UINTN AlignedAddress; > > + UINTN ClockEnableAlignedAddress; > > > > // > > // Host register base may be not aligned to the page size, > > @@ -373,11 +392,50 @@ MvSpiConfigRuntime ( > > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > if (EFI_ERROR (Status)) { > > DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > - gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > > - return Status; > > + goto ErrorSetHostMemorySpace; > > + } > > + > > + // > > + // In case a clock feeding the SPI interface is always on, skip its > > + // configuration for Runtime. > > + // > > + if (Slave->HostClockFixed) { > > + return EFI_SUCCESS; > > + } > > + > > + // > > + // Make sure about an alignment of the memory space needed for clock enabling. > > + // Add one aligned page of memory space which covers the clock enabling > > + // register. > > + // > > + ClockEnableAlignedAddress = Slave->HostClockEnableRegister & ~(SIZE_4KB - 1); > > + > > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > + ClockEnableAlignedAddress, > > + SIZE_4KB, > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > + goto ErrorSetHostMemorySpace; > > + } > > + > > + Status = gDS->SetMemorySpaceAttributes (ClockEnableAlignedAddress, > > + SIZE_4KB, > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > + goto ErrorSetClockMemorySpace; > > } > > > > return EFI_SUCCESS; > > + > > +ErrorSetClockMemorySpace: > > + gDS->RemoveMemorySpace (ClockEnableAlignedAddress, SIZE_4KB); > > + > > +ErrorSetHostMemorySpace: > > + gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > > + > > + return Status; > > } > > > > STATIC > > @@ -393,6 +451,7 @@ SpiMasterInitProtocol ( > > SpiMasterProtocol->Transfer = MvSpiTransfer; > > SpiMasterProtocol->ReadWrite = MvSpiReadWrite; > > SpiMasterProtocol->ConfigRuntime = MvSpiConfigRuntime; > > + SpiMasterProtocol->ControllerPowerOn = MvSpiControllerPowerOn; > > > > return EFI_SUCCESS; > > } > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime 2019-04-17 18:30 ` Ard Biesheuvel @ 2019-04-17 19:33 ` Marcin Wojtas 0 siblings, 0 replies; 8+ messages in thread From: Marcin Wojtas @ 2019-04-17 19:33 UTC (permalink / raw) To: Ard Biesheuvel Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao Ard, śr., 17 kwi 2019 o 20:30 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a): > > On Wed, 17 Apr 2019 at 11:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > On Wed, 17 Apr 2019 at 11:06, Marcin Wojtas <mw@semihalf.com> wrote: > > > > > > Linux MTD subsystem enables the SPI controller clock > > > only when explicitly accessing the device. Because of that, > > > using variables stored in the SPI flash could cause a hang > > > when this clock remained disabled. > > > > > > Such issue was prevented when booting with the default > > > DTB - see commit 124073e4d440 ("Marvell/Armada7k8k: DeviceTree: > > > Use fixed clocks"). However in case someone wishes to > > > try a different DTB, the issue can become problematic again. > > > > > > Fix above problem by enabling the clock when accessing > > > SPI controller registers or mapped memory region > > > AtRuntime. For this purpose add required callbacks > > > to the SpiMaster and SpiFlash protocols that are > > > called from MvFvbDxe driver. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > This is really not the right solution. Linux runs UEFI runtime > > services on a single core with interrupts enabled, and it thinks it > > owns the SPI subsystem if you describe it in the DT, and may decide to > > access it at the same time, either in interrupt ot softirq context, or > > from another core. > > I'm aware of that. Maybe I phrased the commit message not clear enough, but this patch actually handles a case where the only owner/user of the SPI device is UEFI. OS, when using variables, is totally unaware of it, hence there is noone, who can enable the clock feeding SPI interface. To overcome that we can: a. Use DT with fake clock tree (and so unused clocks won't be disabled in HW) - we already do it in the default DT provided by UEFI. b. Append clk_ignore_unused to commandline on every boot - even if doable, I cannot think of any nice implementations. c. Make sure in UEFI the SPI clock is enabled every time the OS tries to access the variables. This patch implements c. > > Note that this applies to just the clock as well. So if c. is illegal, do you see any other option that dropping the patch? Best regards, Marcin > > > > > > --- > > > Silicon/Marvell/Marvell.dec | 3 + > > > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf | 3 + > > > Silicon/Marvell/Include/Protocol/Spi.h | 10 ++++ > > > Silicon/Marvell/Include/Protocol/SpiFlash.h | 7 +++ > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 34 +++++++++++ > > > Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c | 11 ++++ > > > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c | 63 +++++++++++++++++++- > > > 7 files changed, 129 insertions(+), 2 deletions(-) > > > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > > index 7210ba2..201a62f 100644 > > > --- a/Silicon/Marvell/Marvell.dec > > > +++ b/Silicon/Marvell/Marvell.dec > > > @@ -138,6 +138,9 @@ > > > gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183 > > > > > > #SPI > > > + gMarvellTokenSpaceGuid.PcdSpiClockFixed|TRUE|BOOLEAN|0x30000054 > > > + gMarvellTokenSpaceGuid.PcdSpiClockMask|0|UINT32|0x30000055 > > > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 > > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > > index 4779371..1857484 100644 > > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > > > @@ -59,7 +59,10 @@ > > > UefiRuntimeLib > > > > > > [FixedPcd] > > > + gMarvellTokenSpaceGuid.PcdSpiClockFixed > > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency > > > + gMarvellTokenSpaceGuid.PcdSpiClockMask > > > + gMarvellTokenSpaceGuid.PcdSpiClockRegBase > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency > > > gMarvellTokenSpaceGuid.PcdSpiRegBase > > > > > > diff --git a/Silicon/Marvell/Include/Protocol/Spi.h b/Silicon/Marvell/Include/Protocol/Spi.h > > > index abbad19..4d66f7f 100644 > > > --- a/Silicon/Marvell/Include/Protocol/Spi.h > > > +++ b/Silicon/Marvell/Include/Protocol/Spi.h > > > @@ -54,6 +54,9 @@ typedef struct { > > > UINT32 AddrSize; > > > NOR_FLASH_INFO *Info; > > > UINTN HostRegisterBaseAddress; > > > + BOOLEAN HostClockFixed; > > > + UINTN HostClockEnableRegister; > > > + UINT32 HostClockEnableMask; > > > UINTN CoreClock; > > > } SPI_DEVICE; > > > > > > @@ -107,6 +110,12 @@ EFI_STATUS > > > IN SPI_DEVICE *SpiDev > > > ); > > > > > > +typedef > > > +EFI_STATUS > > > +(EFIAPI *MV_SPI_POWER_ON) ( > > > + IN SPI_DEVICE *SpiDev > > > + ); > > > + > > > struct _MARVELL_SPI_MASTER_PROTOCOL { > > > MV_SPI_INIT Init; > > > MV_SPI_READ_WRITE ReadWrite; > > > @@ -114,6 +123,7 @@ struct _MARVELL_SPI_MASTER_PROTOCOL { > > > MV_SPI_SETUP_DEVICE SetupDevice; > > > MV_SPI_FREE_DEVICE FreeDevice; > > > MV_SPI_CONFIG_RT ConfigRuntime; > > > + MV_SPI_POWER_ON ControllerPowerOn; > > > }; > > > > > > #endif // __MARVELL_SPI_MASTER_PROTOCOL_H__ > > > diff --git a/Silicon/Marvell/Include/Protocol/SpiFlash.h b/Silicon/Marvell/Include/Protocol/SpiFlash.h > > > index e703330..0336dda 100644 > > > --- a/Silicon/Marvell/Include/Protocol/SpiFlash.h > > > +++ b/Silicon/Marvell/Include/Protocol/SpiFlash.h > > > @@ -102,6 +102,12 @@ EFI_STATUS > > > IN UINTN EndPercentage > > > ); > > > > > > +typedef > > > +EFI_STATUS > > > +(EFIAPI *MV_SPI_FLASH_POWER_ON) ( > > > + IN SPI_DEVICE *SpiDev > > > + ); > > > + > > > struct _MARVELL_SPI_FLASH_PROTOCOL { > > > MV_SPI_FLASH_INIT Init; > > > MV_SPI_FLASH_READ_ID ReadId; > > > @@ -110,6 +116,7 @@ struct _MARVELL_SPI_FLASH_PROTOCOL { > > > MV_SPI_FLASH_ERASE Erase; > > > MV_SPI_FLASH_UPDATE Update; > > > MV_SPI_FLASH_UPDATE_WITH_PROGRESS UpdateWithProgress; > > > + MV_SPI_FLASH_POWER_ON FlashPowerOn; > > > }; > > > > > > #endif // __MV_SPI_FLASH__ > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > index cb006cd..b57f415 100644 > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > @@ -311,6 +311,11 @@ MvFvbGetAttributes ( > > > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > + > > > FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)FlashInstance->RegionBaseAddress; > > > FlashFvbAttributes = (EFI_FVB_ATTRIBUTES_2 *)&(FwVolHeader->Attributes); > > > > > > @@ -349,10 +354,18 @@ MvFvbSetAttributes ( > > > EFI_FVB_ATTRIBUTES_2 OldAttributes; > > > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > > > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > > > + FVB_DEVICE *FlashInstance; > > > UINT32 Capabilities; > > > UINT32 OldStatus; > > > UINT32 NewStatus; > > > > > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > + > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > + > > > // > > > // Obtain attributes from FVB header > > > // > > > @@ -468,6 +481,11 @@ MvFvbGetPhysicalAddress ( > > > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > + > > > *Address = FlashInstance->RegionBaseAddress; > > > > > > return EFI_SUCCESS; > > > @@ -588,6 +606,10 @@ MvFvbRead ( > > > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > > > > // Cache the block size to avoid de-referencing pointers all the time > > > BlockSize = FlashInstance->Media.BlockSize; > > > @@ -697,6 +719,11 @@ MvFvbWrite ( > > > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > + > > > DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset + Offset, > > > FlashInstance->StartLba + Lba, > > > FlashInstance->Media.BlockSize); > > > @@ -770,6 +797,11 @@ MvFvbEraseBlocks ( > > > > > > FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > > > > + // Ensure proper access to the flash device from OS level > > > + if (EfiAtRuntime ()) { > > > + FlashInstance->SpiFlashProtocol->FlashPowerOn (&FlashInstance->SpiDevice); > > > + } > > > + > > > Status = EFI_SUCCESS; > > > > > > // Detect WriteDisabled state > > > @@ -882,11 +914,13 @@ MvFvbVirtualNotifyEvent ( > > > // Convert SPI device description > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.Info); > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostRegisterBaseAddress); > > > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice.HostClockEnableRegister); > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiDevice); > > > > > > // Convert SpiFlashProtocol > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Erase); > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->Write); > > > + EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol->FlashPowerOn); > > > EfiConvertPointer (0x0, (VOID**)&mFvbDevice->SpiFlashProtocol); > > > > > > return; > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > > index d81f6e3..4cc897f 100755 > > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c > > > @@ -548,6 +548,15 @@ MvSpiFlashInit ( > > > } > > > > > > EFI_STATUS > > > +EFIAPI > > > +MvSpiFlashPowerOn ( > > > + IN SPI_DEVICE *Slave > > > + ) > > > +{ > > > + return SpiMasterProtocol->ControllerPowerOn (Slave); > > > +} > > > + > > > +EFI_STATUS > > > MvSpiFlashInitProtocol ( > > > IN MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol > > > ) > > > @@ -560,6 +569,7 @@ MvSpiFlashInitProtocol ( > > > SpiFlashProtocol->Erase = MvSpiFlashErase; > > > SpiFlashProtocol->Update = MvSpiFlashUpdate; > > > SpiFlashProtocol->UpdateWithProgress = MvSpiFlashUpdateWithProgress; > > > + SpiFlashProtocol->FlashPowerOn = MvSpiFlashPowerOn; > > > > > > return EFI_SUCCESS; > > > } > > > @@ -586,6 +596,7 @@ MvSpiFlashVirtualNotifyEvent ( > > > // > > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ReadWrite); > > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->Transfer); > > > + EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol->ControllerPowerOn); > > > EfiConvertPointer (0x0, (VOID**)&SpiMasterProtocol); > > > > > > return; > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > > index cbf403f..3f7f67e 100755 > > > --- a/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > > +++ b/Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > > > @@ -323,6 +323,11 @@ MvSpiSetupSlave ( > > > } > > > > > > Slave->HostRegisterBaseAddress = PcdGet32 (PcdSpiRegBase); > > > + Slave->HostClockFixed = PcdGetBool (PcdSpiClockFixed); > > > + if (!Slave->HostClockFixed) { > > > + Slave->HostClockEnableRegister = PcdGet64 (PcdSpiClockRegBase); > > > + Slave->HostClockEnableMask = PcdGet32 (PcdSpiClockMask); > > > + } > > > Slave->CoreClock = PcdGet32 (PcdSpiClockFrequency); > > > Slave->MaxFreq = PcdGet32 (PcdSpiMaxFrequency); > > > > > > @@ -344,12 +349,26 @@ MvSpiFreeSlave ( > > > > > > EFI_STATUS > > > EFIAPI > > > +MvSpiControllerPowerOn ( > > > + IN SPI_DEVICE *Slave > > > + ) > > > +{ > > > + if (!Slave->HostClockFixed) { > > > + MmioOr32 (Slave->HostClockEnableRegister, Slave->HostClockEnableMask); > > > + } > > > + > > > + return EFI_SUCCESS; > > > +} > > > + > > > +EFI_STATUS > > > +EFIAPI > > > MvSpiConfigRuntime ( > > > IN SPI_DEVICE *Slave > > > ) > > > { > > > EFI_STATUS Status; > > > UINTN AlignedAddress; > > > + UINTN ClockEnableAlignedAddress; > > > > > > // > > > // Host register base may be not aligned to the page size, > > > @@ -373,11 +392,50 @@ MvSpiConfigRuntime ( > > > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > if (EFI_ERROR (Status)) { > > > DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > > - gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > > > - return Status; > > > + goto ErrorSetHostMemorySpace; > > > + } > > > + > > > + // > > > + // In case a clock feeding the SPI interface is always on, skip its > > > + // configuration for Runtime. > > > + // > > > + if (Slave->HostClockFixed) { > > > + return EFI_SUCCESS; > > > + } > > > + > > > + // > > > + // Make sure about an alignment of the memory space needed for clock enabling. > > > + // Add one aligned page of memory space which covers the clock enabling > > > + // register. > > > + // > > > + ClockEnableAlignedAddress = Slave->HostClockEnableRegister & ~(SIZE_4KB - 1); > > > + > > > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > > + ClockEnableAlignedAddress, > > > + SIZE_4KB, > > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > > + goto ErrorSetHostMemorySpace; > > > + } > > > + > > > + Status = gDS->SetMemorySpaceAttributes (ClockEnableAlignedAddress, > > > + SIZE_4KB, > > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > > + goto ErrorSetClockMemorySpace; > > > } > > > > > > return EFI_SUCCESS; > > > + > > > +ErrorSetClockMemorySpace: > > > + gDS->RemoveMemorySpace (ClockEnableAlignedAddress, SIZE_4KB); > > > + > > > +ErrorSetHostMemorySpace: > > > + gDS->RemoveMemorySpace (AlignedAddress, SIZE_4KB); > > > + > > > + return Status; > > > } > > > > > > STATIC > > > @@ -393,6 +451,7 @@ SpiMasterInitProtocol ( > > > SpiMasterProtocol->Transfer = MvSpiTransfer; > > > SpiMasterProtocol->ReadWrite = MvSpiReadWrite; > > > SpiMasterProtocol->ConfigRuntime = MvSpiConfigRuntime; > > > + SpiMasterProtocol->ControllerPowerOn = MvSpiControllerPowerOn; > > > > > > return EFI_SUCCESS; > > > } > > > -- > > > 2.7.4 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-17 18:05 [edk2-platforms: PATCH 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime Marcin Wojtas @ 2019-04-17 18:05 ` Marcin Wojtas 2019-04-20 17:45 ` Ard Biesheuvel 2 siblings, 1 reply; 8+ messages in thread From: Marcin Wojtas @ 2019-04-17 18:05 UTC (permalink / raw) To: devel Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap, Jici.Gao, Kornel Duleba From: Kornel Duleba <mindal@semihalf.com> This path enables support for reading variables directly from flash without relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that allows to switch between the modes. When in non-memory-mapped mode the driver will copy the variables from flash to previously allocated buffer and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 and PcdFlashNvStorageFtwSpareBase64 accordingly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Marvell.dec | 8 ++ Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 132 +++++++++++++++----- 5 files changed, 132 insertions(+), 36 deletions(-) diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec index 201a62f..70929d2 100644 --- a/Silicon/Marvell/Marvell.dec +++ b/Silicon/Marvell/Marvell.dec @@ -58,6 +58,12 @@ gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } + # + # Generic FaultTolerantWriteDxe driver use variables, + # whose setting is done in MvFvbDxe driver in case + # the SPI contents are not mapped in memory. + # + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } [LibraryClasses] ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h @@ -143,6 +149,8 @@ gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index a1ebb81..9974d5f 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -256,6 +256,11 @@ # USB support gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE +[PcdsDynamicDefault.common] + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 + [PcdsFixedAtBuild.common] gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" gArmPlatformTokenSpaceGuid.PcdCoreCount|4 @@ -396,11 +401,10 @@ # Variable store - default values # gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 !if $(CAPSULE_ENABLE) diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf index ef10bfd..c85e8a6 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf @@ -76,13 +76,22 @@ gMarvellSpiMasterProtocolGuid [FixedPcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize gMarvellTokenSpaceGuid.PcdSpiMemoryBase + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped + gMarvellTokenSpaceGuid.PcdSpiVariableOffset + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 [Depex] - gEfiCpuArchProtocolGuid + # + # Generic FaultTolerantWriteDxe driver use variables, + # whose setting is done in MvFvbDxe driver in case + # the SPI contents are not mapped in memory. + # + BEFORE gFaultTolerantWriteDxeFileGuid diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h index 31e6e44..e8df9a5 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h @@ -55,6 +55,7 @@ typedef struct { UINT32 Signature; + BOOLEAN IsMemoryMapped; UINTN DeviceBaseAddress; UINTN RegionBaseAddress; UINTN Size; diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c index b57f415..b1dfd62 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { FVB_FLASH_SIGNATURE, // Signature + FALSE, // IsMemoryMapped ... NEED TO BE FILLED 0, // DeviceBaseAddress ... NEED TO BE FILLED 0, // RegionBaseAddress ... NEED TO BE FILLED SIZE_256KB, // Size @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS | EFI_FVB2_STICKY_WRITE | - EFI_FVB2_MEMORY_MAPPED | EFI_FVB2_ERASE_POLARITY | EFI_FVB2_WRITE_STATUS | EFI_FVB2_WRITE_ENABLED_CAP; + if (FlashInstance->IsMemoryMapped) { + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; + } + FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY); FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; @@ -382,12 +386,15 @@ MvFvbSetAttributes ( EFI_FVB2_WRITE_ENABLED_CAP | \ EFI_FVB2_LOCK_CAP | \ EFI_FVB2_STICKY_WRITE | \ - EFI_FVB2_MEMORY_MAPPED | \ EFI_FVB2_ERASE_POLARITY | \ EFI_FVB2_READ_LOCK_CAP | \ EFI_FVB2_WRITE_LOCK_CAP | \ EFI_FVB2_ALIGNMENT; + if (FlashInstance->IsMemoryMapped) { + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; + } + // // Some attributes of FV is read only can *not* be set // @@ -714,6 +721,7 @@ MvFvbWrite ( IN UINT8 *Buffer ) { + EFI_STATUS Status; FVB_DEVICE *FlashInstance; UINTN DataOffset; @@ -728,10 +736,27 @@ MvFvbWrite ( FlashInstance->StartLba + Lba, FlashInstance->Media.BlockSize); - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, - DataOffset, - *NumBytes, - Buffer); + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, + DataOffset, + *NumBytes, + Buffer); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, + "%a: Failed to write to Spi device\n", + __FUNCTION__)); + return Status; + } + + // Update shadow buffer + if (!FlashInstance->IsMemoryMapped) { + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, + FlashInstance->StartLba + Lba, + FlashInstance->Media.BlockSize); + + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); + } + + return EFI_SUCCESS; } /** @@ -1009,6 +1034,9 @@ MvFvbConfigureFlashInstance ( ) { EFI_STATUS Status; + UINTN *NumBytes; + UINTN DataOffset; + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; // Locate SPI protocols @@ -1043,25 +1071,62 @@ MvFvbConfigureFlashInstance ( } // Fill remaining flash description - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - - FlashInstance->DeviceBaseAddress; - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + - PcdGet32(PcdFlashNvStorageFtwSpareSize); + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); + + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); FlashInstance->Media.MediaId = 0; FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; FlashInstance->Media.LastBlock = FlashInstance->Size / FlashInstance->Media.BlockSize - 1; + if (FlashInstance->IsMemoryMapped) { + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); + } else { + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); + + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, + SIZE_64KB); + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { + return EFI_OUT_OF_RESOURCES; + } + + PcdSet64 (PcdFlashNvStorageVariableBase64, + (UINT64) FlashInstance->RegionBaseAddress); + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, + (UINT64) FlashInstance->RegionBaseAddress + + VariableSize); + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, + (UINT64) FlashInstance->RegionBaseAddress + + VariableSize + + FtwWorkingSize); + + // Fill the buffer with data from flash + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, + FlashInstance->StartLba, + FlashInstance->Media.BlockSize); + *NumBytes = FlashInstance->FvbSize; + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, + DataOffset, + *NumBytes, + (VOID *)FlashInstance->RegionBaseAddress); + if (EFI_ERROR (Status)) { + goto ErrorFreeAllocatedPages; + } + } + Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, NULL); if (EFI_ERROR (Status)) { - return Status; + goto ErrorFreeAllocatedPages; } Status = MvFvbPrepareFvHeader (FlashInstance); @@ -1077,6 +1142,12 @@ ErrorPrepareFvbHeader: &gEfiFirmwareVolumeBlockProtocolGuid, NULL); +ErrorFreeAllocatedPages: + if (!FlashInstance->IsMemoryMapped) { + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, + MemorySize); + } + return Status; } @@ -1128,24 +1199,27 @@ MvFvbEntryPoint ( // // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME // - RuntimeMmioRegionSize = mFvbDevice->FvbSize; RegionBaseAddress = mFvbDevice->RegionBaseAddress; - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, - RegionBaseAddress, - RuntimeMmioRegionSize, - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); - goto ErrorAddSpace; - } + if (mFvbDevice->IsMemoryMapped) { + RuntimeMmioRegionSize = mFvbDevice->FvbSize; + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, + RegionBaseAddress, + RuntimeMmioRegionSize, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); + goto ErrorAddSpace; + } - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, - RuntimeMmioRegionSize, - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); - goto ErrorSetMemAttr; + + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, + RuntimeMmioRegionSize, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); + goto ErrorSetMemAttr; + } } // -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-17 18:05 ` [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas @ 2019-04-20 17:45 ` Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2019-04-20 17:45 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba On Wed, 17 Apr 2019 at 20:06, Marcin Wojtas <mw@semihalf.com> wrote: > > From: Kornel Duleba <mindal@semihalf.com> > > This path enables support for reading variables directly from flash without > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > allows to switch between the modes. When in non-memory-mapped mode the > driver will copy the variables from flash to previously allocated buffer > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> How does this work with VariablePei? > --- > Silicon/Marvell/Marvell.dec | 8 ++ > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 132 +++++++++++++++----- > 5 files changed, 132 insertions(+), 36 deletions(-) > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > index 201a62f..70929d2 100644 > --- a/Silicon/Marvell/Marvell.dec > +++ b/Silicon/Marvell/Marvell.dec > @@ -58,6 +58,12 @@ > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > + # > + # Generic FaultTolerantWriteDxe driver use variables, > + # whose setting is done in MvFvbDxe driver in case > + # the SPI contents are not mapped in memory. > + # > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > [LibraryClasses] > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > @@ -143,6 +149,8 @@ > gMarvellTokenSpaceGuid.PcdSpiClockRegBase|0|UINT64|0x30000056 > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > index a1ebb81..9974d5f 100644 > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > @@ -256,6 +256,11 @@ > # USB support > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > +[PcdsDynamicDefault.common] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > + > [PcdsFixedAtBuild.common] > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > @@ -396,11 +401,10 @@ > # Variable store - default values > # > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > !if $(CAPSULE_ENABLE) > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > index ef10bfd..c85e8a6 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > @@ -76,13 +76,22 @@ > gMarvellSpiMasterProtocolGuid > > [FixedPcd] > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > [Depex] > - gEfiCpuArchProtocolGuid > + # > + # Generic FaultTolerantWriteDxe driver use variables, > + # whose setting is done in MvFvbDxe driver in case > + # the SPI contents are not mapped in memory. > + # > + BEFORE gFaultTolerantWriteDxeFileGuid > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > index 31e6e44..e8df9a5 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > @@ -55,6 +55,7 @@ typedef struct { > > UINT32 Signature; > > + BOOLEAN IsMemoryMapped; > UINTN DeviceBaseAddress; > UINTN RegionBaseAddress; > UINTN Size; > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > index b57f415..b1dfd62 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { > > FVB_FLASH_SIGNATURE, // Signature > > + FALSE, // IsMemoryMapped ... NEED TO BE FILLED > 0, // DeviceBaseAddress ... NEED TO BE FILLED > 0, // RegionBaseAddress ... NEED TO BE FILLED > SIZE_256KB, // Size > @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( > FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | > EFI_FVB2_READ_STATUS | > EFI_FVB2_STICKY_WRITE | > - EFI_FVB2_MEMORY_MAPPED | > EFI_FVB2_ERASE_POLARITY | > EFI_FVB2_WRITE_STATUS | > EFI_FVB2_WRITE_ENABLED_CAP; > > + if (FlashInstance->IsMemoryMapped) { > + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; > + } > + > FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + > sizeof (EFI_FV_BLOCK_MAP_ENTRY); > FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; > @@ -382,12 +386,15 @@ MvFvbSetAttributes ( > EFI_FVB2_WRITE_ENABLED_CAP | \ > EFI_FVB2_LOCK_CAP | \ > EFI_FVB2_STICKY_WRITE | \ > - EFI_FVB2_MEMORY_MAPPED | \ > EFI_FVB2_ERASE_POLARITY | \ > EFI_FVB2_READ_LOCK_CAP | \ > EFI_FVB2_WRITE_LOCK_CAP | \ > EFI_FVB2_ALIGNMENT; > > + if (FlashInstance->IsMemoryMapped) { > + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; > + } > + > // > // Some attributes of FV is read only can *not* be set > // > @@ -714,6 +721,7 @@ MvFvbWrite ( > IN UINT8 *Buffer > ) > { > + EFI_STATUS Status; > FVB_DEVICE *FlashInstance; > UINTN DataOffset; > > @@ -728,10 +736,27 @@ MvFvbWrite ( > FlashInstance->StartLba + Lba, > FlashInstance->Media.BlockSize); > > - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > - DataOffset, > - *NumBytes, > - Buffer); > + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > + DataOffset, > + *NumBytes, > + Buffer); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, > + "%a: Failed to write to Spi device\n", > + __FUNCTION__)); > + return Status; > + } > + > + // Update shadow buffer > + if (!FlashInstance->IsMemoryMapped) { > + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, > + FlashInstance->StartLba + Lba, > + FlashInstance->Media.BlockSize); > + > + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); > + } > + > + return EFI_SUCCESS; > } > > /** > @@ -1009,6 +1034,9 @@ MvFvbConfigureFlashInstance ( > ) > { > EFI_STATUS Status; > + UINTN *NumBytes; > + UINTN DataOffset; > + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; > > > // Locate SPI protocols > @@ -1043,25 +1071,62 @@ MvFvbConfigureFlashInstance ( > } > > // Fill remaining flash description > - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); > - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - > - FlashInstance->DeviceBaseAddress; > - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + > - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > - PcdGet32(PcdFlashNvStorageFtwSpareSize); > + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); > + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); > + > + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); > + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; > + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); > > FlashInstance->Media.MediaId = 0; > FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; > FlashInstance->Media.LastBlock = FlashInstance->Size / > FlashInstance->Media.BlockSize - 1; > > + if (FlashInstance->IsMemoryMapped) { > + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); > + } else { > + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); > + > + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize > + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, > + SIZE_64KB); > + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + PcdSet64 (PcdFlashNvStorageVariableBase64, > + (UINT64) FlashInstance->RegionBaseAddress); > + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, > + (UINT64) FlashInstance->RegionBaseAddress > + + VariableSize); > + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, > + (UINT64) FlashInstance->RegionBaseAddress > + + VariableSize > + + FtwWorkingSize); > + > + // Fill the buffer with data from flash > + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, > + FlashInstance->StartLba, > + FlashInstance->Media.BlockSize); > + *NumBytes = FlashInstance->FvbSize; > + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, > + DataOffset, > + *NumBytes, > + (VOID *)FlashInstance->RegionBaseAddress); > + if (EFI_ERROR (Status)) { > + goto ErrorFreeAllocatedPages; > + } > + } > + > Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, > &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, > &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, > NULL); > if (EFI_ERROR (Status)) { > - return Status; > + goto ErrorFreeAllocatedPages; > } > > Status = MvFvbPrepareFvHeader (FlashInstance); > @@ -1077,6 +1142,12 @@ ErrorPrepareFvbHeader: > &gEfiFirmwareVolumeBlockProtocolGuid, > NULL); > > +ErrorFreeAllocatedPages: > + if (!FlashInstance->IsMemoryMapped) { > + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, > + MemorySize); > + } > + > return Status; > } > > @@ -1128,24 +1199,27 @@ MvFvbEntryPoint ( > // > // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > // > - RuntimeMmioRegionSize = mFvbDevice->FvbSize; > RegionBaseAddress = mFvbDevice->RegionBaseAddress; > > - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > - RegionBaseAddress, > - RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > - goto ErrorAddSpace; > - } > + if (mFvbDevice->IsMemoryMapped) { > + RuntimeMmioRegionSize = mFvbDevice->FvbSize; > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > + RegionBaseAddress, > + RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > + goto ErrorAddSpace; > + } > > - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > - RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > - goto ErrorSetMemAttr; > + > + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > + RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > + goto ErrorSetMemAttr; > + } > } > > // > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-20 17:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-17 18:05 [edk2-platforms: PATCH 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime Marcin Wojtas 2019-04-17 18:27 ` Ard Biesheuvel 2019-04-17 18:30 ` Ard Biesheuvel 2019-04-17 19:33 ` Marcin Wojtas 2019-04-17 18:05 ` [edk2-platforms: PATCH 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2019-04-20 17:45 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox