public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marcin Wojtas" <mw@semihalf.com>
To: devel@edk2.groups.io
Cc: leif.lindholm@linaro.org, ard.biesheuvel@linaro.org,
	nadavh@marvell.com, mw@semihalf.com, jsd@semihalf.com,
	jaz@semihalf.com, kostap@marvell.com, Jici.Gao@arm.com
Subject: [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime
Date: Wed, 17 Apr 2019 20:05:55 +0200	[thread overview]
Message-ID: <1555524356-1740-3-git-send-email-mw@semihalf.com> (raw)
In-Reply-To: <1555524356-1740-1-git-send-email-mw@semihalf.com>

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


  parent reply	other threads:[~2019-04-17 18:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-04-17 18:27   ` [edk2-platforms: PATCH 2/3] Marvell/Drivers: MvSpiOrionDxe: Keep clock enabled at runtime 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1555524356-1740-3-git-send-email-mw@semihalf.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox