[platforms: PATCH 00/11] Armada 70x0/80x0 SPI improvements

[platforms: PATCH 00/11] Armada 70x0/80x0 SPI improvements

[Marcin Wojtas]

Hi,

This patchset introduces a big rework of MvSpiFlash driver, which
now is capable of autodecting SPI flash device. Its operation is based
on structure fields and that allowed to clean fixed and faulty nature
of hitherto PCD representation. From now on MvSpiFlash is generic
enough to be possibly reused by other platforms.

Other than that, various minor fixes and improvements are implemented.
A PortingGuide refactoring patch was required, because all future
changes around PCDs will modify a single file in a Doxygen-compliant
format.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/spi-upstream-r20170901

I'm looking forward to the comments or remarks.

Best regards,
Marcin

Ard Biesheuvel (3):
  Applications/SpiTool: Fix bug in error test
  Applications/FirmwareUpdate: Fix 32-bit issues
  Applications/SpiTool: Fix 32-bit issues

Joe Zhou (1):
  Drivers/Spi/MvSpiDxe: Fix write bug

Marcin Wojtas (5):
  Applications/SpiTool: Enable configurable CS and SCLK mode
  Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash
  Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter
  Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection
  Drivers/Spi/Devices/MvSpiFlash: Fix bank selection for Spansion

Nir Erez (1):
  Platform/Marvell/Documentation: Refactor PortingGuide

Piotr Król (1):
  Drivers/Spi/MvSpiDxe: Log and return correct error

 .../Marvell/Applications/FirmwareUpdate/FUpdate.c  |  28 +-
 .../Applications/FirmwareUpdate/FUpdate.inf        |   3 -
 .../Marvell/Applications/SpiTool/SpiFlashCmd.c     |  46 +--
 .../Marvell/Applications/SpiTool/SpiFlashCmd.inf   |   3 +-
 Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
 Platform/Marvell/Documentation/Build.txt           |  58 ++++
 Platform/Marvell/Documentation/PortingGuide.txt    | 357 +++++++++++++++++++++
 .../Marvell/Documentation/PortingGuide/ComPhy.txt  |  45 ---
 .../Marvell/Documentation/PortingGuide/I2c.txt     |  20 --
 .../Marvell/Documentation/PortingGuide/Mdio.txt    |   7 -
 .../Marvell/Documentation/PortingGuide/Mpp.txt     |  48 ---
 .../Documentation/PortingGuide/PciEmulation.txt    |  31 --
 .../Marvell/Documentation/PortingGuide/Phy.txt     |  45 ---
 .../Marvell/Documentation/PortingGuide/Pp2.txt     |  35 --
 .../Marvell/Documentation/PortingGuide/Reset.txt   |   7 -
 .../Marvell/Documentation/PortingGuide/Spi.txt     |  16 -
 .../Documentation/PortingGuide/SpiFlash.txt        |  23 --
 .../Marvell/Documentation/PortingGuide/Utmi.txt    |  35 --
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 266 ++++++++++++---
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |   9 +
 .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |   6 -
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c            |   6 +-
 Platform/Marvell/Include/Protocol/Spi.h            |  37 +++
 Platform/Marvell/Include/Protocol/SpiFlash.h       |   4 +-
 Platform/Marvell/Marvell.dec                       |   7 +-
 25 files changed, 722 insertions(+), 427 deletions(-)
 create mode 100644 Platform/Marvell/Documentation/Build.txt
 create mode 100644 Platform/Marvell/Documentation/PortingGuide.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt

-- 
1.8.3.1

[platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide

[Marcin Wojtas]

From: Nir Erez <nerez@marvell.com>

This patch introduces following improvements to the PortingGuide
* Replace split documentation with single file
* Align format to Doxygen constraints

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Documentation/Build.txt           |  58 ++++
 Platform/Marvell/Documentation/PortingGuide.txt    | 371 +++++++++++++++++++++
 .../Marvell/Documentation/PortingGuide/ComPhy.txt  |  45 ---
 .../Marvell/Documentation/PortingGuide/I2c.txt     |  20 --
 .../Marvell/Documentation/PortingGuide/Mdio.txt    |   7 -
 .../Marvell/Documentation/PortingGuide/Mpp.txt     |  48 ---
 .../Documentation/PortingGuide/PciEmulation.txt    |  31 --
 .../Marvell/Documentation/PortingGuide/Phy.txt     |  45 ---
 .../Marvell/Documentation/PortingGuide/Pp2.txt     |  35 --
 .../Marvell/Documentation/PortingGuide/Reset.txt   |   7 -
 .../Marvell/Documentation/PortingGuide/Spi.txt     |  16 -
 .../Documentation/PortingGuide/SpiFlash.txt        |  23 --
 .../Marvell/Documentation/PortingGuide/Utmi.txt    |  35 --
 13 files changed, 429 insertions(+), 312 deletions(-)
 create mode 100644 Platform/Marvell/Documentation/Build.txt
 create mode 100644 Platform/Marvell/Documentation/PortingGuide.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt

diff --git a/Platform/Marvell/Documentation/Build.txt b/Platform/Marvell/Documentation/Build.txt
new file mode 100644
index 0000000..1162e2e
--- /dev/null
+++ b/Platform/Marvell/Documentation/Build.txt
@@ -0,0 +1,58 @@
+UEFI Build Instructions
+=======================
+
+For toolchain versions limitations please refer to edk2 wiki page:
+https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC
+
+Fully supported are gcc4.5 - gcc4.9, so possible {toolchain_name} are:
+	- GCC45
+	- GCC46
+	- GCC47
+	- GCC48
+	- GCC49
+	- GCC5
+
+Supported {platform} are:
+	- Armada70x0
+
+Supported {target} are
+	- DEBUG
+	- RELEASE
+
+Build procedure
+---------------
+1. Prerequisites:
+
+	Clone into edk2 repositories and apply Marvell patches (Please refer to
+	Release notes for instructions).
+
+2. Prepare environment:
+
+	2.1 Several packages will be needed to fully set up an edk2 build environment:
+
+			# sudo apt-get install build-essential uuid-dev
+			# sudo apt-get install lib32stdc++6 lib32z1
+
+	2.2 Set up EDK2 environment
+
+			# source edksetup.sh
+
+	2.3 Build base tools
+
+			# make -C BaseTools
+
+	2.4 Set {toolchain_name}_AARCH64_PREFIX to path to your cross compiler
+
+			# export {toolchain_name}_AARCH64_PREFIX=/path/to/toolchain
+
+		Example:
+		--------
+			# export GCC5_AARCH64_PREFIX=/opt/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-
+
+3. Build EDK2 for selected {platform}:
+
+			# build -a AARCH64 -t {toolchain_name} -b {target} -p OpenPlatformPkg/Platforms/Marvell/Armada/{platform}.dsc
+
+	Example for building edk2 for Armada70x0 platform with GCC5 for DEBUG:
+
+			# build -a AARCH64 -t GCC5 -b DEBUG -p OpenPlatformPkg/Platforms/Marvell/Armada/Armada70x0.dsc
diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
new file mode 100644
index 0000000..8c3579e
--- /dev/null
+++ b/Platform/Marvell/Documentation/PortingGuide.txt
@@ -0,0 +1,371 @@
+UEFI Porting Guide
+==================
+
+This document provides instructions for adding support for new Marvell Armada
+board. For the sake of simplicity new Marvell board will be called "new_board".
+
+1. Create configuration files for new target
+	1.1 Create FDF file for new board
+
+	 - Copy and rename PathToYourOpp/Platforms/Marvell/Armada/Armada70x0.fdf to
+	   PathToYourOpp/Platforms/Marvell/Armada/new_board.fdf
+	 - Change the first no-comment line:
+	   [FD.Armada70x0_EFI] to [FD.{new_board}_EFI]
+
+	1.2 Create DSC file for new board
+
+	 - Add new_board.dsc file to PathToYourOpp/Platforms/Marvell/Armada directory
+	 - Insert following [Defines] section to new_board.dsc:
+
+			[Defines]
+			  PLATFORM_NAME                  = {new_board}
+			  PLATFORM_GUID                  = {newly_generated_GUID}
+			  PLATFORM_VERSION               = 0.1
+			  DSC_SPECIFICATION              = 0x00010019
+			  OUTPUT_DIRECTORY               = {output_directory}
+			  SUPPORTED_ARCHITECTURES        = AARCH64
+			  BUILD_TARGETS                  = DEBUG|RELEASE
+			  SKUID_IDENTIFIER               = DEFAULT
+			  FLASH_DEFINITION               = {path_to_fdf_file}
+
+	 - Add "!include Armada.dsc.inc" entry to new_board.dsc
+
+2. Driver support
+ - According to content of files from PathToYourOpp/Documentation/Marvell/PortingGuide
+   insert PCD entries into new_board.dsc for every needed interface (as listed below).
+
+3. Compilation
+ - Refer to PathToYourOpp/Documentation/Marvell/Build.txt. Remember to change
+   {platform} to new_board in order to point build system to newly created DSC file.
+
+4. Output file
+ - Output files (and among others FD file, which may be used by ATF) are
+   generated under directory pointed by "OUTPUT_DIRECTORY" entry (see point 1.2).
+
+
+COMPHY configuration
+====================
+In order to configure ComPhy library, following PCDs are available:
+
+  - gMarvellTokenSpaceGuid.PcdComPhyDevices
+
+This array indicates, which ones of the ComPhy chips defined in
+MVHW_COMPHY_DESC template will be configured.
+
+Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
+important, but configuration will be set for first PcdComPhyChipCount chips).
+
+Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
+settings for this chip. Their format is unicode string, containing settings
+for up to 10 lanes. Setting for each one is separated with semicolon.
+These PCDs together describe outputs of PHY integrated in simple cihp.
+Below is example for the first chip (Chip0).
+
+  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
+	(Unicode string indicating PHY types. Currently supported are:
+
+		{ L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
+		  L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
+		  L"SGMII1", L"SGMII2", L"SGMII3", L"QSGMII",
+		  L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
+		  L"XAUI0", L"XAUI1", L"XAUI2", L"XAUI3", L"RXAUI0",
+		  L"RXAUI1", L"KR" } )
+
+  - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
+	(Indicates PHY speeds in MHz. Currently supported are:
+		{ 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 1031 }  )
+
+  - gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
+	(Indicates lane polarity invert)
+
+Example
+-------
+
+		  #ComPhy
+		  gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
+		  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
+		  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
+
+
+PHY Driver configuration
+========================
+MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
+Currently only 1518 series PHYs are supported. Following PCDs are required:
+
+  - gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
+	(list of values corresponding to PHY_CONNECTION enum)
+  - gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
+	(boolean - if true, driver waits for autonegotiation on startup)
+  - gMarvellTokenSpaceGuid.PcdPhyDeviceIds
+	(list of values corresponding to MV_PHY_DEVICE_ID enum)
+
+PHY_CONNECTION enum type is defined as follows:
+
+		typedef enum {
+			0    PHY_CONNECTION_RGMII,
+			1    PHY_CONNECTION_RGMII_ID,
+			2    PHY_CONNECTION_RGMII_TXID,
+			3    PHY_CONNECTION_RGMII_RXID,
+			4    PHY_CONNECTION_SGMII,
+			5    PHY_CONNECTION_RTBI,
+			6    PHY_CONNECTION_XAUI,
+			7    PHY_CONNECTION_RXAUI
+		} PHY_CONNECTION;
+
+MV_PHY_DEVICE_ID:
+
+		typedef enum {
+			0    MV_PHY_DEVICE_1512,
+		} MV_PHY_DEVICE_ID;
+
+It should be extended when adding support for other PHY models.
+Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
+
+ gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
+
+with disabled autonegotiation:
+
+ gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
+
+assuming, that PHY models are 1512:
+
+ gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
+
+
+MDIO configuration
+==================
+MDIO driver provides access to network PHYs' registers via EFI_MDIO_READ and
+EFI_MDIO_WRITE functions (EFI_MDIO_PROTOCOL). Following PCD is required:
+
+  - gMarvellTokenSpaceGuid.PcdMdioBaseAddress
+	(base address of SMI management register)
+
+
+I2C configuration
+=================
+In order to enable driver on a new platform, following steps need to be taken:
+ - add following line to .dsc file:
+   OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
+ - add following line to .fdf file:
+   INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
+ - add PCDs with relevant values to .dsc file:
+	- gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57 }
+		(addresses of I2C slave devices on bus)
+	- gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0 }
+		(buses to which accoring slaves are attached)
+	- gMarvellTokenSpaceGuid.PcdI2cBusCount|2
+		(number of SoC's I2C buses)
+	- gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
+		(base addresses of I2C controller buses)
+	- gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
+		(I2C host controller clock frequency)
+	- gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
+		(baud rate used in I2C transmission)
+
+
+PciEmulation configuration
+==========================
+Installation of various NonDiscoverable devices via PciEmulation driver is performed
+via set of PCDs. Following are available:
+
+  - gMarvellTokenSpaceGuid.PcdPciEXhci
+	(Indicates, which Xhci devices are used)
+
+  - gMarvellTokenSpaceGuid.PcdPciEAhci
+	(Indicates, which Ahci devices are used)
+
+  - gMarvellTokenSpaceGuid.PcdPciESdhci
+	(Indicates, which Sdhci devices are used)
+
+All above PCD's correspond to hardware description in a dedicated structure:
+
+STATIC PCI_E_PLATFORM_DESC A70x0PlatDescTemplate
+
+in Platforms/Marvell/PciEmulation/PciEmulation.c file. It comprises device
+count, base addresses, register region size and DMA-coherency type.
+
+Example
+-------
+
+Assuming we want to enable second XHCI port and one SDHCI port on Armada
+70x0 board, following needs to be declared:
+
+		gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x0 0x1 }
+		gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1 }
+
+
+SATA configuration
+==================
+There is one additional PCD for AHCI:
+
+  - gMarvellTokenSpaceGuid.PcdSataBaseAddress
+	(Base address of SATA controller register space - used in SATA ComPhy init
+	 sequence)
+
+
+Pp2Dxe configuration
+====================
+Pp2Dxe is driver supporting PP2 NIC on Marvell platforms. Following PCDs
+are required to operate:
+
+  - gMarvellTokenSpaceGuid.PcdPp2Controllers
+	(Array with used controllers
+	 Set to 0x1 for enabled, 0x0 for disabled)
+
+  - gMarvellTokenSpaceGuid.PcdPp2Port2Controller
+	(Array specifying, to which controller the port belongs to)
+
+  - gMarvellTokenSpaceGuid.PcdPhySmiAddresses
+	(Addresses of PHY devices)
+
+  - gMarvellTokenSpaceGuid.PcdPp2PortIds
+	(Identificators of PP2 ports)
+
+  - gMarvellTokenSpaceGuid.PcdPp2GopIndexes
+	(Indexes used in GOP operation)
+
+  - gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
+	(Set to 0x1 for always-up interface, 0x0 otherwise)
+
+  - gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
+	(Values corresponding to PHY_SPEED enum.
+	 PHY_SPEED is defined as follows:
+
+		  typedef enum {
+			  0  NO_SPEED,
+			  1  SPEED_10,
+			  2  SPEED_100,
+			  3  SPEED_1000,
+			  4  SPEED_2500,
+			  5  SPEED_10000
+		  } PHY_SPEED;
+
+
+UTMI PHY configuration
+======================
+In order to configure UTMI, following PCDs are available:
+
+  - gMarvellTokenSpaceGuid.PcdUtmiPhyCount
+	(Indicates how many UTMI PHYs are available on platform)
+
+Next four PCDs are in unicode string format containing settings for all devices
+separated with semicolon.
+
+  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
+	(Indicates base address of the UTMI unit)
+
+  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
+	(Indicates address of USB Configuration register)
+
+  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
+	(Indicates address of external UTMI configuration)
+
+  - gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
+	(Indicates type of the connected USB port)
+
+Example
+-------
+
+		# UtmiPhy
+		  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
+		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
+		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
+		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
+		  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
+
+
+SPI driver configuration
+========================
+Following PCDs are available for configuration of spi driver:
+
+  - gMarvellTokenSpaceGuid.PcdSpiClockFrequency
+	(Frequency (in Hz) of SPI clock)
+
+  - gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
+	(Max SCLK line frequency (in Hz) (max transfer frequency) )
+
+  - gMarvellTokenSpaceGuid.PcdSpiDefaultMode
+	(default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h) )
+
+
+SpiFlash configuration
+======================
+Folowing PCDs for spi flash driver configuration must be set properly:
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
+	(Size of SPI flash address in bytes (3 or 4) )
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
+	(Size of minimal erase block in bytes)
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
+	(Size of SPI flash page)
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashId
+	(Id of SPI flash)
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
+	(Spi flash polling flag)
+
+
+MPP configuration
+=================
+Multi-Purpose Ports (MPP) are configurable through platform PCDs.
+In order to set desired pin multiplexing, .dsc file needs to be modified.
+(OpenPlatformPkg/Platforms/Marvell/Armada/{platform_name}.dsc - please refer to
+Documentation/Build.txt for currently supported {platftorm_name} )
+Following PCDs are available:
+
+  - gMarvellTokenSpaceGuid.PcdMppChipCount
+	(Indicates how many different chips are placed on board. So far up to 4 chips
+	 are supported)
+
+Every MPP PCD has <Num> part where
+ <Num> stands for chip ID (order is not important, but configuration will be
+ set for first PcdMppChipCount chips).
+
+Below is example for the first chip (Chip0).
+
+  - gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag
+	(Indicates that register order is reversed. (Needs to be used only for AP806-Z1) )
+
+  - gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress
+	(This is base address for MPP configuration register)
+
+  - gMarvellTokenSpaceGuid.PcdChip0MppPinCount
+	(Defines how many MPP pins are available)
+
+  - gMarvellTokenSpaceGuid.PcdChip0MppSel0
+  - gMarvellTokenSpaceGuid.PcdChip0MppSel1
+  - gMarvellTokenSpaceGuid.PcdChip0MppSel2
+	(This registers defines functions of 10 pins in ascending order)
+
+Examples
+--------
+
+		# APN806-A0 MPP SET
+		 gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag|FALSE
+		 gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress|0xF06F4000
+		 gMarvellTokenSpaceGuid.PcdChip0MppRegCount|3
+		 gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x0 }
+		 gMarvellTokenSpaceGuid.PcdChip0MppSel1|{ 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 }
+
+Set pin 6 and 7 to 0xa function:
+		 gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xa, 0xa, 0x0, 0x0 }
+
+
+MarvellResetSystemLib configuration
+===================================
+This simple library allows to mask given bits in given reg at UEFI 'reset'
+command call. These variables are configurable through PCDs:
+
+  - gMarvellTokenSpaceGuid.PcdResetRegAddress
+  - gMarvellTokenSpaceGuid.PcdResetRegMask
+
+
+Ramdisk configuration
+=====================
+There is one PCD available for Ramdisk configuration
+
+  - gMarvellTokenSpaceGuid.PcdRamDiskSize
+	(Defines size of Ramdisk)
diff --git a/Platform/Marvell/Documentation/PortingGuide/ComPhy.txt b/Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
deleted file mode 100644
index a96015e..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-COMPHY configuration
----------------------------
-In order to configure ComPhy library, following PCDs are available:
-
-  gMarvellTokenSpaceGuid.PcdComPhyDevices
-
-This array indicates, which ones of the ComPhy chips defined in
-MVHW_COMPHY_DESC template will be configured.
-
-Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
-important, but configuration will be set for first PcdComPhyChipCount chips).
-
-Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
-settings for this chip. Their format is unicode string, containing settings
-for up to 10 lanes. Setting for each one is separated with semicolon.
-These PCDs together describe outputs of PHY integrated in simple cihp.
-Below is example for the first chip (Chip0).
-
-  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
-
-Unicode string indicating PHY types. Currently supported are:
-
-{ L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
-L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
-L"SGMII1", L"SGMII2", L"SGMII3",
-L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
-L"RXAUI0", L"RXAUI1", L"SFI" }
-
-  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
-
-Indicates PHY speeds in MHz. Currently supported are:
-
-{ 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 10310 }
-
-  gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
-
-Indicates lane polarity invert.
-
-Example
--------
-  #ComPhy
-  gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
-  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
-
diff --git a/Platform/Marvell/Documentation/PortingGuide/I2c.txt b/Platform/Marvell/Documentation/PortingGuide/I2c.txt
deleted file mode 100644
index 020ffb4..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/I2c.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-1. Porting I2C driver to a new SOC
-----------------------------------
-In order to enable driver on a new platform, following steps need to be taken:
- - add following line to .dsc file:
-   Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
- - add following line to .fdf file:
-   INF Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
- - add PCDs with relevant values to .dsc file:
-     gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57 }
-   (addresses of I2C slave devices on bus)
-     gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0 }
-   (buses to which accoring slaves are attached)
-     gMarvellTokenSpaceGuid.PcdI2cBusCount|2
-   (number of SoC's I2C buses)
-     gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
-   (base addresses of I2C controller buses)
-     gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
-   (I2C host controller clock frequency)
-     gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
-   (baud rate used in I2C transmission)
diff --git a/Platform/Marvell/Documentation/PortingGuide/Mdio.txt b/Platform/Marvell/Documentation/PortingGuide/Mdio.txt
deleted file mode 100644
index c341d9e..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Mdio.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-MDIO driver configuration
--------------------------
-MDIO driver provides access to network PHYs' registers via MARVELL_MDIO_READ and
-MARVELL_MDIO_WRITE functions (MARVELL_MDIO_PROTOCOL). Following PCD is required:
-
-  gMarvellTokenSpaceGuid.PcdMdioBaseAddress
-  (base address of SMI management register)
diff --git a/Platform/Marvell/Documentation/PortingGuide/Mpp.txt b/Platform/Marvell/Documentation/PortingGuide/Mpp.txt
deleted file mode 100644
index 68f0e9d..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Mpp.txt
+++ /dev/null
@@ -1,48 +0,0 @@
-MPP configuration
------------------
-Multi-Purpose Ports (MPP) are configurable through platform PCDs.
-In order to set desired pin multiplexing, .dsc file needs to be modified.
-(Platform/Marvell/Armada/{platform_name}.dsc - please refer to
-Documentation/Build.txt for currently supported {platftorm_name} )
-Following PCDs are available:
-
- gMarvellTokenSpaceGuid.PcdMppChipCount
-
-Indicates how many different chips are placed on board. So far up to 4 chips
-are supported.
-
-Every MPP PCD has <Num> part where
- <Num> stands for chip ID (order is not important, but configuration will be
- set for first PcdMppChipCount chips).
-
-Below is example for the first chip (Chip0).
-
- gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag
-
-Indicates that register order is reversed. (Needs to be used only for AP806-Z1)
-
- gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress
-
-This is base address for MPP configuration register.
-
- gMarvellTokenSpaceGuid.PcdChip0MppPinCount
-
-Defines how many MPP pins are available.
-
- gMarvellTokenSpaceGuid.PcdChip0MppSel0
- gMarvellTokenSpaceGuid.PcdChip0MppSel1
- gMarvellTokenSpaceGuid.PcdChip0MppSel2
-
-This registers defines functions of 10 pins in ascending order.
-
-Examples
---------
-#APN806-A0 MPP SET
- gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag|FALSE
- gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress|0xF06F4000
- gMarvellTokenSpaceGuid.PcdChip0MppRegCount|3
- gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x0 }
- gMarvellTokenSpaceGuid.PcdChip0MppSel1|{ 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 }
-
-Set pin 6 and 7 to 0xa function:
- gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xa, 0xa, 0x0, 0x0 }
diff --git a/Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt b/Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
deleted file mode 100644
index ec1afbc..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-PciEmulation configuration
---------------------------
-Installation of various NonDiscoverable devices via PciEmulation driver is performed
-via set of PCDs. Following are available:
-
- gMarvellTokenSpaceGuid.PcdPciEXhci
-
-Indicates, which Xhci devices are used.
-
- gMarvellTokenSpaceGuid.PcdPciEAhci
-
-Indicates, which Ahci devices are used.
-
- gMarvellTokenSpaceGuid.PcdPciESdhci
-
-Indicates, which Sdhci devices are used.
-
-All above PCD's correspond to hardware description in a dedicated structure:
-
-STATIC PCI_E_PLATFORM_DESC A70x0PlatDescTemplate
-
-in Platforms/Marvell/PciEmulation/PciEmulation.c file. It comprises device
-count, base addresses, register region size and DMA-coherency type.
-
-Examples
---------
-Assuming we want to enable second XHCI port and one SDHCI port on Armada
-70x0 board, following needs to be declared:
-
-  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x0 0x1 }
-  gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1 }
diff --git a/Platform/Marvell/Documentation/PortingGuide/Phy.txt b/Platform/Marvell/Documentation/PortingGuide/Phy.txt
deleted file mode 100644
index 69dae02..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Phy.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-PHY driver configuration
-------------------------
-MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
-Currently only 1512 series PHYs are supported. Following PCDs are required:
-
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
-  (list of values corresponding to PHY_CONNECTION enum)
-  gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
-  (boolean - if true, driver waits for autonegotiation on startup)
-  gMarvellTokenSpaceGuid.PcdPhyDeviceIds
-  (list of values corresponding to MV_PHY_DEVICE_ID enum)
-
-PHY_CONNECTION enum type is defined as follows:
-
-  typedef enum {
-0    PHY_CONNECTION_RGMII,
-1    PHY_CONNECTION_RGMII_ID,
-2    PHY_CONNECTION_RGMII_TXID,
-3    PHY_CONNECTION_RGMII_RXID,
-4    PHY_CONNECTION_SGMII,
-5    PHY_CONNECTION_RTBI,
-6    PHY_CONNECTION_XAUI,
-7    PHY_CONNECTION_RXAUI
-  } PHY_CONNECTION;
-
-MV_PHY_DEVICE_ID:
-
-  typedef enum {
-0    MV_PHY_DEVICE_1512,
-  } MV_PHY_DEVICE_ID;
-
-It should be extended when adding support for other PHY
-models.
-
-Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
-
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
-
-with disabled autonegotiation:
-
-  gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
-
-assuming, that PHY models are 1512:
-
-  gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
diff --git a/Platform/Marvell/Documentation/PortingGuide/Pp2.txt b/Platform/Marvell/Documentation/PortingGuide/Pp2.txt
deleted file mode 100644
index f05ba27..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Pp2.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-Pp2Dxe porting guide
---------------------
-Pp2Dxe is driver supporting PP2 NIC on Marvell platforms. Following PCDs
-are required to operate:
-
-Array with used controllers - Set to 0x1 for enabled, 0x0 for disabled:
-  gMarvellTokenSpaceGuid.PcdPp2Controllers
-
-Array specifying, to which controller the port belongs to:
-  gMarvellTokenSpaceGuid.PcdPp2Port2Controller
-
-Addresses of PHY devices:
-  gMarvellTokenSpaceGuid.PcdPhySmiAddresses
-
-Identificators of PP2 ports:
-  gMarvellTokenSpaceGuid.PcdPp2PortIds
-
-Indexes used in GOP operation:
-  gMarvellTokenSpaceGuid.PcdPp2GopIndexes
-
-Set to 0x1 for always-up interface, 0x0 otherwise:
-  gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
-
-Values corresponding to PHY_SPEED enum:
-  gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
-
-PHY_SPEED (in Mbps) is defined as follows:
-  typedef enum {
-  0  NO_SPEED,
-  1  SPEED_10,
-  2  SPEED_100,
-  3  SPEED_1000,
-  4  SPEED_2500,
-  5  SPEED_10000
-  } PHY_SPEED;
diff --git a/Platform/Marvell/Documentation/PortingGuide/Reset.txt b/Platform/Marvell/Documentation/PortingGuide/Reset.txt
deleted file mode 100644
index 30dec86..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Reset.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-MarvellResetSystemLib configuration
------------------------------------
-This simple library allows to mask given bits in given reg at UEFI 'reset'
-command call. These variables are configurable through PCDs:
-
-  gMarvellTokenSpaceGuid.PcdResetRegAddress
-  gMarvellTokenSpaceGuid.PcdResetRegMask
diff --git a/Platform/Marvell/Documentation/PortingGuide/Spi.txt b/Platform/Marvell/Documentation/PortingGuide/Spi.txt
deleted file mode 100644
index be498a6..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Spi.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Spi driver configuration
-------------------------
-Following PCDs are available for configuration of spi driver:
-
- gMarvellTokenSpaceGuid.PcdSpiClockFrequency
-
-Frequency (in Hz) of SPI clock
-
- gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
-
-Max SCLK line frequency (in Hz) (max transfer frequency)
-
- gMarvellTokenSpaceGuid.PcdSpiDefaultMode
-
-default SCLK mode (see SPI_MODE enum in file
-Platform/Marvell/Drivers/Spi/MvSpi.h)
diff --git a/Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt b/Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
deleted file mode 100644
index 226db40..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-SpiFlash driver configuration
------------------------------
-Folowing PCDs for spi flash driver configuration must be set properly:
-
- gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
-
-Size of SPI flash address in bytes (3 or 4)
-
- gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
-
-Size of minimal erase block in bytes
-
- gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
-
-Size of SPI flash page
-
- gMarvellTokenSpaceGuid.PcdSpiFlashId
-
-Id of SPI flash
-
- gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
-
-Spi flash polling flag
diff --git a/Platform/Marvell/Documentation/PortingGuide/Utmi.txt b/Platform/Marvell/Documentation/PortingGuide/Utmi.txt
deleted file mode 100644
index cff4843..0000000
--- a/Platform/Marvell/Documentation/PortingGuide/Utmi.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-UTMI PHY configuration
-----------------------
-In order to configure UTMI, following PCDs are available:
-
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount
-
-Indicates how many UTMI PHYs are available on platform.
-
-Next four PCDs are in unicode string format containing settings for all devices
-separated with semicolon.
-
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
-
-Indicates base address of the UTMI unit.
-
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
-
-Indicates address of USB Configuration register.
-
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
-
-Indicates address of external UTMI configuration.
-
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
-
-Indicates type of the connected USB port.
-
-Example
--------
-#UtmiPhy
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
-- 
1.8.3.1

[platforms: PATCH 02/11] Drivers/Spi/MvSpiDxe: Log and return correct error

[Marcin Wojtas]

From: Piotr Król <piotr.krol@3mdeb.com>

Make log information clear where it came from and return correct code to
be interpreted by caller.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
index aab20fc..0872f61 100755
--- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
+++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
@@ -240,7 +240,8 @@ MvSpiTransfer (
     }
 
     if (Iterator >= SPI_TIMEOUT) {
-      DEBUG ((DEBUG_ERROR, "Timeout\n"));
+      DEBUG ((DEBUG_ERROR, "EfiSpiTransfer: Timeout\n"));
+      return EFI_TIMEOUT;
     }
   }
 
-- 
1.8.3.1

[platforms: PATCH 03/11] Drivers/Spi/MvSpiDxe: Fix write bug

[Marcin Wojtas]

From: Joe Zhou <shjzhou@marvell.com>

This patch prevents possible NULL pointer dereference
during SPI transfers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Joe Zhou <shjzhou@marvell.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
index 0872f61..6ddfcf6 100755
--- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
+++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
@@ -226,9 +226,8 @@ MvSpiTransfer (
     // Wait for memory ready
     for (Iterator = 0; Iterator < SPI_TIMEOUT; Iterator++) {
       if (MmioRead32 (SpiRegBase + SPI_INT_CAUSE_REG)) {
-        *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
-
         if (DataInPtr != NULL) {
+          *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
           DataInPtr++;
         }
         if (DataOutPtr != NULL) {
-- 
1.8.3.1

[platforms: PATCH 04/11] Applications/SpiTool: Enable configurable CS and SCLK mode

[Marcin Wojtas]

Until now transfer SCLK mode and CS were fixed, when using
shell 'sf' command. This patch enables their configuration.
Update porting guide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c   | 6 +++++-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf | 2 ++
 Platform/Marvell/Documentation/PortingGuide.txt       | 9 +++++----
 Platform/Marvell/Marvell.dec                          | 2 ++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index 184e3d7..b6dc54f 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -218,6 +218,7 @@ EFI_STATUS              Status;
   CONST CHAR16          *LengthStr = NULL, *FileStr = NULL;
   BOOLEAN               AddrFlag = FALSE, LengthFlag = TRUE, FileFlag = FALSE;
   UINT8                 Flag = 0, CheckFlag = 0;
+  UINT8                 Mode, Cs;
 
   Status = gBS->LocateProtocol (
     &gMarvellSpiFlashProtocolGuid,
@@ -283,8 +284,11 @@ EFI_STATUS              Status;
     }
   }
 
+  Mode = PcdGet32 (PcdSpiFlashMode);
+  Cs = PcdGet32 (PcdSpiFlashCs);
+
   // Setup new spi device
-  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, 0, 0);
+  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, Cs, Mode);
     if (Slave == NULL) {
       Print(L"sf: Cannot allocate SPI device!\n");
       return SHELL_ABORTED;
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
index 41b7b7c..c1ab770 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
@@ -66,6 +66,8 @@
 
 [Pcd]
  gMarvellTokenSpaceGuid.PcdSpiFlashId
+ gMarvellTokenSpaceGuid.PcdSpiFlashCs
+ gMarvellTokenSpaceGuid.PcdSpiFlashMode
 
 [Protocols]
  gMarvellSpiFlashProtocolGuid
diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
index 8c3579e..3b79bd2 100644
--- a/Platform/Marvell/Documentation/PortingGuide.txt
+++ b/Platform/Marvell/Documentation/PortingGuide.txt
@@ -284,10 +284,6 @@ Following PCDs are available for configuration of spi driver:
   - gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
 	(Max SCLK line frequency (in Hz) (max transfer frequency) )
 
-  - gMarvellTokenSpaceGuid.PcdSpiDefaultMode
-	(default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h) )
-
-
 SpiFlash configuration
 ======================
 Folowing PCDs for spi flash driver configuration must be set properly:
@@ -307,6 +303,11 @@ Folowing PCDs for spi flash driver configuration must be set properly:
   - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
 	(Spi flash polling flag)
 
+  - gMarvellTokenSpaceGuid.PcdSpiFlashMode
+	(Default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h))
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashCs
+	(Chip select used for communication with the Flash)
 
 MPP configuration
 =================
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 4e2dd6d..869e376 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -128,6 +128,8 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
+  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
+  gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
 
 #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x0 }|VOID*|0x30000098
-- 
1.8.3.1

[platforms: PATCH 05/11] Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash

[Marcin Wojtas]

This patch makes use of recently added SPI configuration
PCDs and sets CS with SCLK mode on Armada 7040 DB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada70x0.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index f519196..df2ebdb 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -98,6 +98,8 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
+  gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
+  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
 
   #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-- 
1.8.3.1

[platforms: PATCH 06/11] Applications/SpiTool: Fix bug in error test

[Marcin Wojtas]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Fix a misplaced closing parenthesis.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index b6dc54f..e6e1007 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -378,7 +378,7 @@ EFI_STATUS              Status;
       FilePath = (CHAR16 *) FileStr;
       Status = ShellIsFile (FilePath);
       // When read file into flash, file doesn't have to exist
-      if (EFI_ERROR(Status && !(Flag & READ_FILE))) {
+      if (EFI_ERROR (Status) && !(Flag & READ_FILE)) {
         Print (L"sf: Wrong FilePath parameter!\n");
         return SHELL_ABORTED;
       }
-- 
1.8.3.1

[platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues

[Marcin Wojtas]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Fix casting and related issues to make this code build for 32-bit ARM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index edb6986..0951734 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -172,6 +172,7 @@ PrepareFirmwareImage (
   EFI_STATUS           Status;
   UINT64               OpenMode;
   UINTN                *Buffer;
+  UINT64               Size;
 
   // Parse string from commandline
   FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
@@ -195,11 +196,13 @@ PrepareFirmwareImage (
       return EFI_DEVICE_ERROR;
     }
 
-  Status = FileHandleGetSize (*FileHandle, FileSize);
+  Status = FileHandleGetSize (*FileHandle, &Size);
     if (EFI_ERROR (Status)) {
       Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
     }
 
+  *FileSize = (UINTN)Size;
+
   // Read Image header into buffer
   Buffer = AllocateZeroPool (*FileSize);
 
-- 
1.8.3.1

[platforms: PATCH 08/11] Applications/SpiTool: Fix 32-bit issues

[Marcin Wojtas]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Fix casting and related issues to make this code build for 32-bit ARM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index e6e1007..ee14270 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -418,7 +418,7 @@ EFI_STATUS              Status;
     }
   }
 
-  Buffer = (UINT8 *) Address;
+  Buffer = (UINT8 *)(UINTN)Address;
   if (FileFlag) {
     Buffer = FileBuffer;
   }
-- 
1.8.3.1

[platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter

[Marcin Wojtas]

Although, hitherto support allowed for using configurable EraseSize,
the erase command was fixed to CMD_ERASE_64K. Also it was
assumed that EraseSize equals SectorSize, which is not true
for some flash devices. Fix both issues by adding new PCD
(gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
this parameter properly in MvSpiFlashUpdate routine instead
of the EraseSize. Also erase command is adjusted to the settings.
Update PortingGuide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Documentation/PortingGuide.txt    |  3 +++
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
 .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
 Platform/Marvell/Marvell.dec                       |  1 +
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
index 3b79bd2..f637fee 100644
--- a/Platform/Marvell/Documentation/PortingGuide.txt
+++ b/Platform/Marvell/Documentation/PortingGuide.txt
@@ -297,6 +297,9 @@ Folowing PCDs for spi flash driver configuration must be set properly:
   - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
 	(Size of SPI flash page)
 
+  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
+	(Size of SPI flash sector, 65536 bytes by default)
+
   - gMarvellTokenSpaceGuid.PcdSpiFlashId
 	(Id of SPI flash)
 
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index 9a04493..f3fdba4 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -191,7 +191,21 @@ MvSpiFlashErase (
     return EFI_DEVICE_ERROR;
   }
 
-  Cmd[0] = CMD_ERASE_64K;
+  switch (EraseSize) {
+  case SPI_ERASE_SIZE_4K:
+    Cmd[0] = CMD_ERASE_4K;
+    break;
+  case SPI_ERASE_SIZE_32K:
+    Cmd[0] = CMD_ERASE_32K;
+    break;
+  case SPI_ERASE_SIZE_64K:
+    Cmd[0] = CMD_ERASE_64K;
+    break;
+  default:
+    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
   while (Length) {
     EraseAddr = Offset;
 
@@ -353,14 +367,14 @@ MvSpiFlashUpdate (
   )
 {
   EFI_STATUS Status;
-  UINT64 EraseSize, ToUpdate, Scale = 1;
+  UINT64 SectorSize, ToUpdate, Scale = 1;
   UINT8 *TmpBuf, *End;
 
-  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
+  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
 
   End = Buf + ByteCount;
 
-  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
+  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
   if (TmpBuf == NULL) {
     DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
     return EFI_OUT_OF_RESOURCES;
@@ -370,9 +384,9 @@ MvSpiFlashUpdate (
     Scale = (End - Buf) / 100;
 
   for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
-    ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
+    ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
     Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
-    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize);
+    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize);
 
     if (EFI_ERROR (Status)) {
       DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
index 3889643..646598a 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
@@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_READ_ARRAY_FAST             0x0b
 #define CMD_PAGE_PROGRAM                0x02
 #define CMD_BANK_WRITE                  0xc5
+#define CMD_ERASE_4K                    0x20
+#define CMD_ERASE_32K                   0x52
 #define CMD_ERASE_64K                   0xd8
 #define CMD_4B_ADDR_ENABLE              0xb7
 
@@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define SPI_TRANSFER_BEGIN              0x01  // Assert CS before transfer
 #define SPI_TRANSFER_END                0x02  // Deassert CS after transfers
 
+#define SPI_ERASE_SIZE_4K               4096
+#define SPI_ERASE_SIZE_32K              32768
+#define SPI_ERASE_SIZE_64K              65536
+
 #define SPI_FLASH_16MB_BOUN             0x1000000
 
 typedef enum {
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
index d035d47..4519b02 100644
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
@@ -58,6 +58,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
   gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
+  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
 
 [Protocols]
   gMarvellSpiMasterProtocolGuid
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 869e376..fc00f1a 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -127,6 +127,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
+  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
   gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
-- 
1.8.3.1

[platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection

[Marcin Wojtas]

Hitherto mechanism of fixing SPI flash model in the PCDs,
occured to be very inefficient and problematic. Enable
dynamic detection by reworking MvSpiFlashReadId() command,
which now reads the Id and goes through newly added table
with JEDEC compliant devices and their description.

On the occasion fix the ReadId process by using master's
ReadWrite routine instead of pure Transfer - no longer
swapping and byte shifting is needed. Simplify code by
using local array instead of dynamic allocation. Also
reduced number of ReadId arguments allowed for cleaning
Fupdate and Sf shell commands probe routines.

Additionally, use SpiFlashInfo fields instead of PCDs,
and configure needed settings in MvSpiFlashInit.

Update PortingGuide documentation accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 .../Marvell/Applications/FirmwareUpdate/FUpdate.c  |  23 +-
 .../Applications/FirmwareUpdate/FUpdate.inf        |   3 -
 .../Marvell/Applications/SpiTool/SpiFlashCmd.c     |  36 +--
 .../Marvell/Applications/SpiTool/SpiFlashCmd.inf   |   1 -
 Platform/Marvell/Armada/Armada70x0.dsc             |   5 -
 Platform/Marvell/Documentation/PortingGuide.txt    |  18 --
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 267 +++++++++++++++++----
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |   2 +
 .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |   7 -
 Platform/Marvell/Include/Protocol/Spi.h            |  37 +++
 Platform/Marvell/Include/Protocol/SpiFlash.h       |   4 +-
 Platform/Marvell/Marvell.dec                       |   6 -
 12 files changed, 271 insertions(+), 138 deletions(-)

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index 0951734..b0e94bc 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -94,32 +94,17 @@ SpiFlashProbe (
   )
 {
   EFI_STATUS       Status;
-  UINT32           IdBuffer, Id, RefId;
-
-  Id = PcdGet32 (PcdSpiFlashId);
-
-  IdBuffer = CMD_READ_ID & 0xff;
 
   // Read SPI flash ID
-  SpiFlashProtocol->ReadId (Slave, sizeof (UINT32), (UINT8 *)&IdBuffer);
-
-  // Swap and extract 3 bytes of the ID
-  RefId = SwapBytes32 (IdBuffer) >> 8;
-
-  if (RefId == 0) {
-    Print (L"%s: No SPI flash detected");
-    return EFI_DEVICE_ERROR;
-  } else if (RefId != Id) {
-    Print (L"%s: Unsupported SPI flash detected with ID=%2x\n", CMD_NAME_STRING, RefId);
-    return EFI_DEVICE_ERROR;
+  Status = SpiFlashProtocol->ReadId (Slave);
+  if (EFI_ERROR (Status)) {
+    return SHELL_ABORTED;
   }
 
-  Print (L"%s: Detected supported SPI flash with ID=%3x\n", CMD_NAME_STRING, RefId);
-
   Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
   if (EFI_ERROR(Status)) {
     Print (L"%s: Cannot initialize flash device\n", CMD_NAME_STRING);
-    return EFI_DEVICE_ERROR;
+    return SHELL_ABORTED;
   }
 
   return EFI_SUCCESS;
diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
index 92c3333..43cac42 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
@@ -64,9 +64,6 @@
   UefiLib
   UefiRuntimeServicesTableLib
 
-[Pcd]
- gMarvellTokenSpaceGuid.PcdSpiFlashId
-
 [Protocols]
  gMarvellSpiFlashProtocolGuid
  gMarvellSpiMasterProtocolGuid
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index ee14270..b5db8b5 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -166,37 +166,21 @@ FlashProbe (
   )
 {
   EFI_STATUS Status;
-  UINT8  IdBuffer[4];
-  UINT32 Id, RefId;
 
-  Id = PcdGet32 (PcdSpiFlashId);
-
-  IdBuffer[0] = CMD_READ_ID;
-
-  SpiFlashProtocol->ReadId (
-    Slave,
-    4,
-    IdBuffer
-    );
-
-  RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];
+  Status = SpiFlashProtocol->ReadId (Slave);
+  if (EFI_ERROR (Status)) {
+    return SHELL_ABORTED;
+  }
 
-  if (RefId == Id) {
-    Print (L"sf: Detected supported SPI flash with ID=%3x\n", RefId);
-    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
-    if (EFI_ERROR(Status)) {
-      Print (L"sf: Cannot initialize flash device\n");
-      return SHELL_ABORTED;
-    }
-    InitFlag = 0;
-    return EFI_SUCCESS;
-  } else if (RefId != 0) {
-    Print (L"sf: Unsupported SPI flash detected with ID=%2x\n", RefId);
+  Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
+  if (EFI_ERROR (Status)) {
+    Print (L"sf: Cannot initialize flash device\n");
     return SHELL_ABORTED;
   }
 
-  Print (L"sf: No SPI flash detected");
-  return SHELL_ABORTED;
+  InitFlag = 0;
+
+  return SHELL_SUCCESS;
 }
 
 SHELL_STATUS
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
index c1ab770..343d5b5 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
@@ -65,7 +65,6 @@
  FileHandleLib
 
 [Pcd]
- gMarvellTokenSpaceGuid.PcdSpiFlashId
  gMarvellTokenSpaceGuid.PcdSpiFlashCs
  gMarvellTokenSpaceGuid.PcdSpiFlashMode
 
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index df2ebdb..4633e32 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -93,11 +93,6 @@
   gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
   gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
 
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
-  gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
   gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
 
diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
index f637fee..3ac35a0 100644
--- a/Platform/Marvell/Documentation/PortingGuide.txt
+++ b/Platform/Marvell/Documentation/PortingGuide.txt
@@ -288,24 +288,6 @@ SpiFlash configuration
 ======================
 Folowing PCDs for spi flash driver configuration must be set properly:
 
-  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
-	(Size of SPI flash address in bytes (3 or 4) )
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
-	(Size of minimal erase block in bytes)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
-	(Size of SPI flash page)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
-	(Size of SPI flash sector, 65536 bytes by default)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashId
-	(Id of SPI flash)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
-	(Spi flash polling flag)
-
   - gMarvellTokenSpaceGuid.PcdSpiFlashMode
 	(Default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h))
 
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index f3fdba4..bfb8fa3 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -36,6 +36,136 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
 SPI_FLASH_INSTANCE  *mSpiFlashInstance;
 
+#define INFO(JedecId, ExtId, SecSize, NSectors, FlashFlags)  \
+  .Id = {                       \
+    ((JedecId) >> 16) & 0xff,   \
+    ((JedecId) >> 8) & 0xff,    \
+    (JedecId) & 0xff,           \
+    ((ExtId) >> 8) & 0xff,      \
+    (ExtId) & 0xff,             \
+    },                          \
+  .IdLen = (!(JedecId) ? 0 : (3 + ((ExtId) ? 2 : 0))),  \
+  .SectorSize = (SecSize),      \
+  .SectorCount = (NSectors),    \
+  .PageSize = 256,              \
+  .Flags = (FlashFlags),
+
+static SPI_FLASH_INFO SpiFlashIds[] = {
+  /* ATMEL */
+  {L"at45db011d",        INFO(0x1f2200, 0x0, 64 * 1024,     4, SECT_4K) },
+  {L"at45db021d",        INFO(0x1f2300, 0x0, 64 * 1024,     8, SECT_4K) },
+  {L"at45db041d",        INFO(0x1f2400, 0x0, 64 * 1024,     8, SECT_4K) },
+  {L"at45db081d",        INFO(0x1f2500, 0x0, 64 * 1024,    16, SECT_4K) },
+  {L"at45db161d",        INFO(0x1f2600, 0x0, 64 * 1024,    32, SECT_4K) },
+  {L"at45db321d",        INFO(0x1f2700, 0x0, 64 * 1024,    64, SECT_4K) },
+  {L"at45db641d",        INFO(0x1f2800, 0x0, 64 * 1024,   128, SECT_4K) },
+  {L"at25df321a",        INFO(0x1f4701, 0x0, 64 * 1024,    64, SECT_4K) },
+  {L"at25df321",         INFO(0x1f4700, 0x0, 64 * 1024,    64, SECT_4K) },
+  {L"at26df081a",        INFO(0x1f4501, 0x0, 64 * 1024,    16, SECT_4K) },
+  /* EON */
+  {L"en25q32b",          INFO(0x1c3016, 0x0, 64 * 1024,    64, 0) },
+  {L"en25q64",           INFO(0x1c3017, 0x0, 64 * 1024,   128, SECT_4K) },
+  {L"en25q128b",         INFO(0x1c3018, 0x0, 64 * 1024,   256, 0) },
+  {L"en25s64",           INFO(0x1c3817, 0x0, 64 * 1024,   128, 0) },
+  /* GIGADEVICE */
+  {L"gd25q64b",          INFO(0xc84017, 0x0, 64 * 1024,   128, SECT_4K) },
+  {L"gd25lq32",          INFO(0xc86016, 0x0, 64 * 1024,    64, SECT_4K) },
+  /* ISSI */
+  {L"is25lp032",         INFO(0x9d6016, 0x0, 64 * 1024,    64, 0) },
+  {L"is25lp064",         INFO(0x9d6017, 0x0, 64 * 1024,   128, 0) },
+  {L"is25lp128",         INFO(0x9d6018, 0x0, 64 * 1024,   256, 0) },
+  /* MACRONIX */
+  {L"mx25l2006e",        INFO(0xc22012, 0x0, 64 * 1024,     4, 0) },
+  {L"mx25l4005",         INFO(0xc22013, 0x0, 64 * 1024,     8, 0) },
+  {L"mx25l8005",         INFO(0xc22014, 0x0, 64 * 1024,    16, 0) },
+  {L"mx25l1605d",        INFO(0xc22015, 0x0, 64 * 1024,    32, 0) },
+  {L"mx25l3205d",        INFO(0xc22016, 0x0, 64 * 1024,    64, 0) },
+  {L"mx25l6405d",        INFO(0xc22017, 0x0, 64 * 1024,   128, 0) },
+  {L"mx25l12805",        INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"mx25l25635f",       INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP | ADDR_CYC_4) },
+  {L"mx25l51235f",       INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
+  {L"mx25l12855e",       INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"mx66u51235f",       INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
+  {L"mx66l1g45g",        INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
+  /* SPANSION */
+  {L"s25fl008a",         INFO(0x010213, 0x0, 64 * 1024,    16, 0) },
+  {L"s25fl016a",         INFO(0x010214, 0x0, 64 * 1024,    32, 0) },
+  {L"s25fl032a",         INFO(0x010215, 0x0, 64 * 1024,    64, 0) },
+  {L"s25fl064a",         INFO(0x010216, 0x0, 64 * 1024,   128, 0) },
+  {L"s25fl116k",         INFO(0x014015, 0x0, 64 * 1024,   128, 0) },
+  {L"s25fl164k",         INFO(0x014017, 0x0140,  64 * 1024,   128, 0) },
+  {L"s25fl128p_256k",    INFO(0x012018, 0x0300, 256 * 1024,    64, RD_FULL | WR_QPP) },
+  {L"s25fl128p_64k",     INFO(0x012018, 0x0301,  64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"s25fl032p",         INFO(0x010215, 0x4d00,  64 * 1024,    64, RD_FULL | WR_QPP) },
+  {L"s25fl064p",         INFO(0x010216, 0x4d00,  64 * 1024,   128, RD_FULL | WR_QPP) },
+  {L"s25fl128s_256k",    INFO(0x012018, 0x4d00, 256 * 1024,    64, RD_FULL | WR_QPP) },
+  {L"s25fl128s_64k",     INFO(0x012018, 0x4d01,  64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"s25fl256s_256k",    INFO(0x010219, 0x4d00, 256 * 1024,   128, RD_FULL | WR_QPP) },
+  {L"s25fl256s_64k",     INFO(0x010219, 0x4d01,  64 * 1024,   512, RD_FULL | WR_QPP) },
+  {L"s25fl512s_256k",    INFO(0x010220, 0x4d00, 256 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"s25fl512s_64k",     INFO(0x010220, 0x4d01,  64 * 1024,  1024, RD_FULL | WR_QPP) },
+  {L"s25fl512s_512k",    INFO(0x010220, 0x4f00, 256 * 1024,   256, RD_FULL | WR_QPP) },
+  /* STMICRO */
+  {L"m25p10",            INFO(0x202011, 0x0, 32 * 1024,     4, 0) },
+  {L"m25p20",            INFO(0x202012, 0x0, 64 * 1024,     4, 0) },
+  {L"m25p40",            INFO(0x202013, 0x0, 64 * 1024,     8, 0) },
+  {L"m25p80",            INFO(0x202014, 0x0, 64 * 1024,    16, 0) },
+  {L"m25p16",            INFO(0x202015, 0x0, 64 * 1024,    32, 0) },
+  {L"m25pE16",           INFO(0x208015, 0x1000, 64 * 1024, 32, 0) },
+  {L"m25pX16",           INFO(0x207115, 0x1000, 64 * 1024, 32, RD_QUAD | RD_DUAL) },
+  {L"m25p32",            INFO(0x202016, 0x0,  64 * 1024,    64, 0) },
+  {L"m25p64",            INFO(0x202017, 0x0,  64 * 1024,   128, 0) },
+  {L"m25p128",           INFO(0x202018, 0x0, 256 * 1024,    64, 0) },
+  {L"m25pX64",           INFO(0x207117, 0x0,  64 * 1024,   128, SECT_4K) },
+  {L"n25q016a",          INFO(0x20bb15, 0x0,  64 * 1024,    32, SECT_4K) },
+  {L"n25q32",            INFO(0x20ba16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q32a",           INFO(0x20bb16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q64",            INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q64a",           INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q128",           INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"n25q128a",          INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
+  {L"n25q256",           INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q256a",          INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
+  {L"n25q512",           INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
+  {L"n25q512a",          INFO(0x20bb20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
+  {L"n25q1024",          INFO(0x20ba21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K | ADDR_CYC_4) },
+  {L"n25q1024a",         INFO(0x20bb21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
+  {L"mt25qu02g",         INFO(0x20bb22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
+  {L"mt25ql02g",         INFO(0x20ba22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
+  /* SST */
+  {L"sst25vf040b",       INFO(0xbf258d, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
+  {L"sst25vf080b",       INFO(0xbf258e, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
+  {L"sst25vf016b",       INFO(0xbf2541, 0x0,  64 * 1024,    32, SECT_4K | SST_WR) },
+  {L"sst25vf032b",       INFO(0xbf254a, 0x0,  64 * 1024,    64, SECT_4K | SST_WR) },
+  {L"sst25vf064c",       INFO(0xbf254b, 0x0,  64 * 1024,   128, SECT_4K) },
+  {L"sst25wf512",        INFO(0xbf2501, 0x0,  64 * 1024,     1, SECT_4K | SST_WR) },
+  {L"sst25wf010",        INFO(0xbf2502, 0x0,  64 * 1024,     2, SECT_4K | SST_WR) },
+  {L"sst25wf020",        INFO(0xbf2503, 0x0,  64 * 1024,     4, SECT_4K | SST_WR) },
+  {L"sst25wf040",        INFO(0xbf2504, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
+  {L"sst25wf040b",       INFO(0x621613, 0x0,  64 * 1024,     8, SECT_4K) },
+  {L"sst25wf080",        INFO(0xbf2505, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
+  /* WINBOND */
+  {L"w25p80",            INFO(0xef2014, 0x0,  64 * 1024,    16, 0) },
+  {L"w25p16",            INFO(0xef2015, 0x0,  64 * 1024,    32, 0) },
+  {L"w25p32",            INFO(0xef2016, 0x0,  64 * 1024,    64, 0) },
+  {L"w25x40",            INFO(0xef3013, 0x0,  64 * 1024,     8, SECT_4K) },
+  {L"w25x16",            INFO(0xef3015, 0x0,  64 * 1024,    32, SECT_4K) },
+  {L"w25x32",            INFO(0xef3016, 0x0,  64 * 1024,    64, SECT_4K) },
+  {L"w25x64",            INFO(0xef3017, 0x0,  64 * 1024,   128, SECT_4K) },
+  {L"w25q80bl",          INFO(0xef4014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q16cl",          INFO(0xef4015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q32bv",          INFO(0xef4016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q64cv",          INFO(0xef4017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q128bv",         INFO(0xef4018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q256",           INFO(0xef4019, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q80bw",          INFO(0xef5014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q16dw",          INFO(0xef6015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q32dw",          INFO(0xef6016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q64dw",          INFO(0xef6017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
+  {L"w25q128fw",         INFO(0xef6018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
+  {},  /* Empty entry to terminate the list */
+};
+
 STATIC
 VOID
 SpiFlashFormatAddress (
@@ -104,13 +234,13 @@ MvSpiFlashWriteCommon (
   UINT8 CmdStatus = CMD_READ_STATUS;
   UINT8 State;
   UINT32 Counter = 0xFFFFF;
-  UINT8 poll_bit = STATUS_REG_POLL_WIP;
-  UINT8 check_status = 0x0;
+  UINT8 PollBit = STATUS_REG_POLL_WIP;
+  UINT8 CheckStatus = 0x0;
 
-  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
-  if (CmdStatus == CMD_FLAG_STATUS) {
-    poll_bit = STATUS_REG_POLL_PEC;
-    check_status = poll_bit;
+  if (Slave->Info->Flags & E_FSR) {
+    CmdStatus = CMD_FLAG_STATUS;
+    PollBit = STATUS_REG_POLL_PEC;
+    CheckStatus = STATUS_REG_POLL_PEC;
   }
 
   // Send command
@@ -127,7 +257,7 @@ MvSpiFlashWriteCommon (
     SpiMasterProtocol->Transfer (SpiMasterProtocol, Slave, 1, NULL, &State,
       0);
     Counter--;
-    if ((State & poll_bit) == check_status)
+    if ((State & PollBit) == CheckStatus)
       break;
   } while (Counter > 0);
   if (Counter == 0) {
@@ -181,8 +311,19 @@ MvSpiFlashErase (
   UINTN EraseSize;
   UINT8 Cmd[5];
 
-  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
-  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
+  if (Slave->Info->Flags & ADDR_CYC_4) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
+
+  if (Slave->Info->Flags & SECT_4K) {
+    Cmd[0] = CMD_ERASE_4K;
+    EraseSize = SPI_ERASE_SIZE_4K;
+  } else {
+    Cmd[0] = CMD_ERASE_64K;
+    EraseSize = Slave->Info->SectorSize;
+  }
 
   // Check input parameters
   if (Offset % EraseSize || Length % EraseSize) {
@@ -191,21 +332,6 @@ MvSpiFlashErase (
     return EFI_DEVICE_ERROR;
   }
 
-  switch (EraseSize) {
-  case SPI_ERASE_SIZE_4K:
-    Cmd[0] = CMD_ERASE_4K;
-    break;
-  case SPI_ERASE_SIZE_32K:
-    Cmd[0] = CMD_ERASE_32K;
-    break;
-  case SPI_ERASE_SIZE_64K:
-    Cmd[0] = CMD_ERASE_64K;
-    break;
-  default:
-    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
-    return EFI_INVALID_PARAMETER;
-  }
-
   while (Length) {
     EraseAddr = Offset;
 
@@ -239,7 +365,11 @@ MvSpiFlashRead (
   UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
   UINTN BankSel = 0;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
+  if (Slave->Info->Flags & ADDR_CYC_4) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
 
   Cmd[0] = CMD_READ_ARRAY_FAST;
 
@@ -282,8 +412,13 @@ MvSpiFlashWrite (
   UINT32 WriteAddr;
   UINT8 Cmd[5], AddrSize;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
-  PageSize = PcdGet32 (PcdSpiFlashPageSize);
+  if (Slave->Info->Flags & ADDR_CYC_4) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
+
+  PageSize = Slave->Info->PageSize;
 
   Cmd[0] = CMD_PAGE_PROGRAM;
 
@@ -370,7 +505,7 @@ MvSpiFlashUpdate (
   UINT64 SectorSize, ToUpdate, Scale = 1;
   UINT8 *TmpBuf, *End;
 
-  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
+  SectorSize = Slave->Info->SectorSize;
 
   End = Buf + ByteCount;
 
@@ -400,38 +535,66 @@ MvSpiFlashUpdate (
   return EFI_SUCCESS;
 }
 
+STATIC
+VOID
+MvPrintFlashInfo (
+  IN     SPI_FLASH_INFO *Info
+  )
+{
+  UINTN EraseSize;
+
+  if (Info->Flags & SECT_4K) {
+    EraseSize = SPI_ERASE_SIZE_4K;
+  } else {
+    EraseSize = Info->SectorSize;
+  }
+
+  DEBUG ((DEBUG_ERROR,
+    "Detected %s SPI flash with page size %d B, erase size %d KB, total %d MB\n",
+    Info->Name,
+    Info->PageSize,
+    EraseSize / 1024,
+    (Info->SectorSize * Info->SectorCount) / 1024 / 1024));
+}
+
 EFI_STATUS
 EFIAPI
 MvSpiFlashReadId (
-  IN     SPI_DEVICE *SpiDev,
-  IN     UINT32     DataByteCount,
-  IN OUT UINT8      *Buffer
+  IN     SPI_DEVICE *SpiDev
   )
 {
+  SPI_FLASH_INFO *Info;
   EFI_STATUS Status;
-  UINT8 *DataOut;
-
-  DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);
-  if (DataOut == NULL) {
-    DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
-    return EFI_OUT_OF_RESOURCES;
-  }
-  Status = SpiMasterProtocol->Transfer (SpiMasterProtocol, SpiDev,
-    DataByteCount, Buffer, DataOut, SPI_TRANSFER_BEGIN | SPI_TRANSFER_END);
-  if (EFI_ERROR(Status)) {
-    FreePool (DataOut);
-    DEBUG((DEBUG_ERROR, "SpiFlash: Spi transfer error\n"));
+  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
+  UINT8 Cmd;
+
+  Cmd = CMD_READ_ID;
+  Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,
+                                SpiDev,
+                                &Cmd,
+                                SPI_CMD_LEN,
+                                NULL,
+                                Id,
+                                SPI_FLASH_MAX_ID_LEN);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));
     return Status;
   }
 
-  // Bytes 1,2 and 3 contain SPI flash ID
-  Buffer[0] = DataOut[1];
-  Buffer[1] = DataOut[2];
-  Buffer[2] = DataOut[3];
+  Info = SpiFlashIds;
+  for (; Info->Name != NULL; Info++) {
+    if (Info->IdLen != 0) {
+      if (CompareMem (Info->Id, Id, Info->IdLen) == 0) {
+        SpiDev->Info = Info;
+        MvPrintFlashInfo (Info);
+        return EFI_SUCCESS;
+      }
+    }
+  }
 
-  FreePool (DataOut);
+  DEBUG ((DEBUG_ERROR, "ReadId: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n", Id[0], Id[1], Id[2]));
 
-  return EFI_SUCCESS;
+  return EFI_NOT_FOUND;
 }
 
 EFI_STATUS
@@ -445,7 +608,11 @@ MvSpiFlashInit (
   UINT8 Cmd, StatusRegister;
   UINT32 AddrSize;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
+  if (Slave->Info->Flags & ADDR_CYC_4) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
 
   if (AddrSize == 4) {
     // Set 4 byte address mode
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
index 646598a..b876966 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
@@ -62,6 +62,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_ERASE_64K                   0xd8
 #define CMD_4B_ADDR_ENABLE              0xb7
 
+#define SPI_CMD_LEN                     1
+
 #define STATUS_REG_POLL_WIP             (1 << 0)
 #define STATUS_REG_POLL_PEC             (1 << 7)
 
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
index 4519b02..f82c6e0 100644
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
@@ -53,13 +53,6 @@
   DebugLib
   MemoryAllocationLib
 
-[FixedPcd]
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
-  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
-
 [Protocols]
   gMarvellSpiMasterProtocolGuid
   gMarvellSpiFlashProtocolGuid
diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
index ae78a31..73f0d80 100644
--- a/Platform/Marvell/Include/Protocol/Spi.h
+++ b/Platform/Marvell/Include/Protocol/Spi.h
@@ -38,6 +38,42 @@ extern EFI_GUID gMarvellSpiMasterProtocolGuid;
 
 typedef struct _MARVELL_SPI_MASTER_PROTOCOL MARVELL_SPI_MASTER_PROTOCOL;
 
+#define SPI_FLASH_MAX_ID_LEN    6
+
+typedef struct {
+  /* Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) */
+  UINT16 *Name;
+
+  /*
+   * This array stores the ID bytes.
+   * The first three bytes are the JEDIC ID.
+   * JEDEC ID zero means "no ID" (mostly older chips).
+   */
+  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
+  UINT8 IdLen;
+
+  /*
+   * The size listed here is what works with SPINOR_OP_SE, which isn't
+   * necessarily called a "sector" by the vendor.
+   */
+  UINT32 SectorSize;
+  UINT32 SectorCount;
+
+  UINT16 PageSize;
+
+  UINT16 Flags;
+#define SECT_4K      1 << 0  /* CMD_ERASE_4K works uniformly */
+#define E_FSR        1 << 1  /* use flag status register for */
+#define SST_WR       1 << 2  /* use SST byte/word programming */
+#define WR_QPP       1 << 3  /* use Quad Page Program */
+#define RD_QUAD      1 << 4  /* use Quad Read */
+#define RD_DUAL      1 << 5  /* use Dual Read */
+#define RD_QUADIO    1 << 6  /* use Quad IO Read */
+#define RD_DUALIO    1 << 7  /* use Dual IO Read */
+#define RD_FULL      (RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
+#define ADDR_CYC_4   1 << 8  /* use 4 byte addressing format */
+} SPI_FLASH_INFO;
+
 typedef enum {
   SPI_MODE0, // CPOL = 0 & CPHA = 0
   SPI_MODE1, // CPOL = 0 & CPHA = 1
@@ -49,6 +85,7 @@ typedef struct {
   INTN Cs;
   INTN MaxFreq;
   SPI_MODE Mode;
+  SPI_FLASH_INFO *Info;
 } SPI_DEVICE;
 
 typedef
diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
index 743bb87..e12f55b 100644
--- a/Platform/Marvell/Include/Protocol/SpiFlash.h
+++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
@@ -61,9 +61,7 @@ EFI_STATUS
 typedef
 EFI_STATUS
 (EFIAPI *MV_SPI_FLASH_READ_ID) (
-  IN     SPI_DEVICE *SpiDev,
-  IN     UINT32     DataByteCount,
-  IN OUT UINT8      *Buffer
+  IN     SPI_DEVICE *SpiDev
   );
 
 typedef
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index fc00f1a..418d960 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -123,12 +123,6 @@
   gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052
   gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053
 
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0|UINT32|0x3000052
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
-  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
-  gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
   gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
 
-- 
1.8.3.1

[platforms: PATCH 11/11] Drivers/Spi/Devices/MvSpiFlash: Fix bank selection for Spansion

[Marcin Wojtas]

Spansion SPI flash devices use different command for bank
selection. Update it, basing on the first byte of flash ID.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 5 +++++
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index bfb8fa3..4c15513 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -280,6 +280,11 @@ SpiFlashCmdBankaddrWrite (
 {
   UINT8 Cmd = CMD_BANK_WRITE;
 
+  /* Update bank selection command for Spansion */
+  if (Slave->Info->Id[0] == 0x01) {
+    Cmd = CMD_BANKADDR_BRWR;
+  }
+
   MvSpiFlashWriteCommon (Slave, &Cmd, 1, &BankSel, 1);
 }
 
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
index b876966..d44c56f 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
@@ -57,6 +57,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_READ_ARRAY_FAST             0x0b
 #define CMD_PAGE_PROGRAM                0x02
 #define CMD_BANK_WRITE                  0xc5
+#define CMD_BANKADDR_BRWR               0x17
 #define CMD_ERASE_4K                    0x20
 #define CMD_ERASE_32K                   0x52
 #define CMD_ERASE_64K                   0xd8
-- 
1.8.3.1

Re: [platforms: PATCH 02/11] Drivers/Spi/MvSpiDxe: Log and return correct error

[Ard Biesheuvel]

On 1 September 2017 at 14:08, Marcin Wojtas <mw@semihalf.com> wrote:
> From: Piotr Król <piotr.krol@3mdeb.com>
>
> Make log information clear where it came from and return correct code to
> be interpreted by caller.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> index aab20fc..0872f61 100755
> --- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> +++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> @@ -240,7 +240,8 @@ MvSpiTransfer (
>      }
>
>      if (Iterator >= SPI_TIMEOUT) {
> -      DEBUG ((DEBUG_ERROR, "Timeout\n"));
> +      DEBUG ((DEBUG_ERROR, "EfiSpiTransfer: Timeout\n"));


Please don't use EFI prefixes for things that are not defined in the UEFI spec.

Also, you may want to use %a here, and pass __FUNCTION__ as the argument.

Re: [platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:13PM +0200, Marcin Wojtas wrote:
> From: Nir Erez <nerez@marvell.com>
> 
> This patch introduces following improvements to the PortingGuide
> * Replace split documentation with single file
> * Align format to Doxygen constraints

* Add Build description.
(Comment on that below.)

Since you are moving things around, I take this as license to
bikeshed:

I think we should start looking towards separating the development
board and SoC portions, like is done for most other platforms. After
all, the goal is to get additional platforms (not just the EVB).

Not saying this is something that needs to happen overnight, but it
would make sense to make a start by moving these docs to
Silicon/Marvell/Documentation as part of this patch.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Documentation/Build.txt           |  58 ++++
>  Platform/Marvell/Documentation/PortingGuide.txt    | 371 +++++++++++++++++++++
>  .../Marvell/Documentation/PortingGuide/ComPhy.txt  |  45 ---
>  .../Marvell/Documentation/PortingGuide/I2c.txt     |  20 --
>  .../Marvell/Documentation/PortingGuide/Mdio.txt    |   7 -
>  .../Marvell/Documentation/PortingGuide/Mpp.txt     |  48 ---
>  .../Documentation/PortingGuide/PciEmulation.txt    |  31 --
>  .../Marvell/Documentation/PortingGuide/Phy.txt     |  45 ---
>  .../Marvell/Documentation/PortingGuide/Pp2.txt     |  35 --
>  .../Marvell/Documentation/PortingGuide/Reset.txt   |   7 -
>  .../Marvell/Documentation/PortingGuide/Spi.txt     |  16 -
>  .../Documentation/PortingGuide/SpiFlash.txt        |  23 --
>  .../Marvell/Documentation/PortingGuide/Utmi.txt    |  35 --

Also, it would be nice if you could generate patches with 
  --stat=1000 --stat-graph-width=20
as documented in Laszlo's unkempt guide:
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
Makes the above summare _much_ easier to read.

>  13 files changed, 429 insertions(+), 312 deletions(-)
>  create mode 100644 Platform/Marvell/Documentation/Build.txt
>  create mode 100644 Platform/Marvell/Documentation/PortingGuide.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt
> 
> diff --git a/Platform/Marvell/Documentation/Build.txt b/Platform/Marvell/Documentation/Build.txt
> new file mode 100644
> index 0000000..1162e2e
> --- /dev/null
> +++ b/Platform/Marvell/Documentation/Build.txt
> @@ -0,0 +1,58 @@
> +UEFI Build Instructions
> +=======================
> +
> +For toolchain versions limitations please refer to edk2 wiki page:
> +https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC
> +
> +Fully supported are gcc4.5 - gcc4.9, so possible {toolchain_name} are:
> +	- GCC45
> +	- GCC46
> +	- GCC47
> +	- GCC48
> +	- GCC49
> +	- GCC5
> +
> +Supported {platform} are:
> +	- Armada70x0
> +
> +Supported {target} are
> +	- DEBUG
> +	- RELEASE
> +
> +Build procedure
> +---------------
> +1. Prerequisites:
> +
> +	Clone into edk2 repositories and apply Marvell patches (Please refer to
> +	Release notes for instructions).
> +
> +2. Prepare environment:
> +
> +	2.1 Several packages will be needed to fully set up an edk2 build environment:
> +
> +			# sudo apt-get install build-essential uuid-dev
> +			# sudo apt-get install lib32stdc++6 lib32z1
> +
> +	2.2 Set up EDK2 environment
> +
> +			# source edksetup.sh
> +
> +	2.3 Build base tools
> +
> +			# make -C BaseTools
> +
> +	2.4 Set {toolchain_name}_AARCH64_PREFIX to path to your cross compiler
> +
> +			# export {toolchain_name}_AARCH64_PREFIX=/path/to/toolchain
> +
> +		Example:
> +		--------
> +			# export GCC5_AARCH64_PREFIX=/opt/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-
> +
> +3. Build EDK2 for selected {platform}:
> +
> +			# build -a AARCH64 -t {toolchain_name} -b {target} -p OpenPlatformPkg/Platforms/Marvell/Armada/{platform}.dsc
> +
> +	Example for building edk2 for Armada70x0 platform with GCC5 for DEBUG:
> +
> +			# build -a AARCH64 -t GCC5 -b DEBUG -p OpenPlatformPkg/Platforms/Marvell/Armada/Armada70x0.dsc

The above all documents OpenPlatformPkg, so is now dated.
My preference would be for this to become a Readme.md under
Platform/Marvell, if you feel additional documentation to the
top-level Readme.md to be necessary.

Regardless, it is unrelated to the rest of changes here, so would make
sense as a separate patch.

> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> new file mode 100644
> index 0000000..8c3579e
> --- /dev/null
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -0,0 +1,371 @@
> +UEFI Porting Guide
> +==================
> +
> +This document provides instructions for adding support for new Marvell Armada
> +board. For the sake of simplicity new Marvell board will be called "new_board".
> +
> +1. Create configuration files for new target
> +	1.1 Create FDF file for new board
> +
> +	 - Copy and rename PathToYourOpp/Platforms/Marvell/Armada/Armada70x0.fdf to
> +	   PathToYourOpp/Platforms/Marvell/Armada/new_board.fdf

OpenPlatformPkg.

> +	 - Change the first no-comment line:
> +	   [FD.Armada70x0_EFI] to [FD.{new_board}_EFI]
> +
> +	1.2 Create DSC file for new board
> +
> +	 - Add new_board.dsc file to PathToYourOpp/Platforms/Marvell/Armada directory

OpenPlatformPkg.

> +	 - Insert following [Defines] section to new_board.dsc:
> +
> +			[Defines]
> +			  PLATFORM_NAME                  = {new_board}
> +			  PLATFORM_GUID                  = {newly_generated_GUID}
> +			  PLATFORM_VERSION               = 0.1
> +			  DSC_SPECIFICATION              = 0x00010019
> +			  OUTPUT_DIRECTORY               = {output_directory}
> +			  SUPPORTED_ARCHITECTURES        = AARCH64
> +			  BUILD_TARGETS                  = DEBUG|RELEASE
> +			  SKUID_IDENTIFIER               = DEFAULT
> +			  FLASH_DEFINITION               = {path_to_fdf_file}
> +
> +	 - Add "!include Armada.dsc.inc" entry to new_board.dsc
> +
> +2. Driver support
> + - According to content of files from PathToYourOpp/Documentation/Marvell/PortingGuide

OpenPlatformPkg.

> +   insert PCD entries into new_board.dsc for every needed interface (as listed below).
> +
> +3. Compilation
> + - Refer to PathToYourOpp/Documentation/Marvell/Build.txt. Remember to change

OpenPlatformPkg. (Also referencing the Build.txt.)

> +   {platform} to new_board in order to point build system to newly created DSC file.
> +
> +4. Output file
> + - Output files (and among others FD file, which may be used by ATF) are
> +   generated under directory pointed by "OUTPUT_DIRECTORY" entry (see point 1.2).
> +
> +
> +COMPHY configuration
> +====================
> +In order to configure ComPhy library, following PCDs are available:
> +
> +  - gMarvellTokenSpaceGuid.PcdComPhyDevices
> +
> +This array indicates, which ones of the ComPhy chips defined in
> +MVHW_COMPHY_DESC template will be configured.
> +
> +Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
> +important, but configuration will be set for first PcdComPhyChipCount chips).
> +
> +Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
> +settings for this chip. Their format is unicode string, containing settings
> +for up to 10 lanes. Setting for each one is separated with semicolon.
> +These PCDs together describe outputs of PHY integrated in simple cihp.
> +Below is example for the first chip (Chip0).
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
> +	(Unicode string indicating PHY types. Currently supported are:
> +
> +		{ L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
> +		  L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
> +		  L"SGMII1", L"SGMII2", L"SGMII3", L"QSGMII",
> +		  L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
> +		  L"XAUI0", L"XAUI1", L"XAUI2", L"XAUI3", L"RXAUI0",
> +		  L"RXAUI1", L"KR" } )
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
> +	(Indicates PHY speeds in MHz. Currently supported are:
> +		{ 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 1031 }  )
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
> +	(Indicates lane polarity invert)
> +
> +Example
> +-------
> +
> +		  #ComPhy
> +		  gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
> +		  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
> +		  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
> +
> +
> +PHY Driver configuration
> +========================
> +MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
> +Currently only 1518 series PHYs are supported. Following PCDs are required:
> +
> +  - gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
> +	(list of values corresponding to PHY_CONNECTION enum)
> +  - gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
> +	(boolean - if true, driver waits for autonegotiation on startup)
> +  - gMarvellTokenSpaceGuid.PcdPhyDeviceIds
> +	(list of values corresponding to MV_PHY_DEVICE_ID enum)
> +
> +PHY_CONNECTION enum type is defined as follows:
> +
> +		typedef enum {
> +			0    PHY_CONNECTION_RGMII,
> +			1    PHY_CONNECTION_RGMII_ID,
> +			2    PHY_CONNECTION_RGMII_TXID,
> +			3    PHY_CONNECTION_RGMII_RXID,
> +			4    PHY_CONNECTION_SGMII,
> +			5    PHY_CONNECTION_RTBI,
> +			6    PHY_CONNECTION_XAUI,
> +			7    PHY_CONNECTION_RXAUI
> +		} PHY_CONNECTION;
> +
> +MV_PHY_DEVICE_ID:
> +
> +		typedef enum {
> +			0    MV_PHY_DEVICE_1512,
> +		} MV_PHY_DEVICE_ID;
> +
> +It should be extended when adding support for other PHY models.
> +Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
> +
> + gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
> +
> +with disabled autonegotiation:
> +
> + gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
> +
> +assuming, that PHY models are 1512:
> +
> + gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
> +
> +
> +MDIO configuration
> +==================
> +MDIO driver provides access to network PHYs' registers via EFI_MDIO_READ and
> +EFI_MDIO_WRITE functions (EFI_MDIO_PROTOCOL). Following PCD is required:
> +
> +  - gMarvellTokenSpaceGuid.PcdMdioBaseAddress
> +	(base address of SMI management register)
> +
> +
> +I2C configuration
> +=================
> +In order to enable driver on a new platform, following steps need to be taken:
> + - add following line to .dsc file:
> +   OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> + - add following line to .fdf file:
> +   INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> + - add PCDs with relevant values to .dsc file:
> +	- gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57 }
> +		(addresses of I2C slave devices on bus)
> +	- gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0 }
> +		(buses to which accoring slaves are attached)
> +	- gMarvellTokenSpaceGuid.PcdI2cBusCount|2
> +		(number of SoC's I2C buses)
> +	- gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
> +		(base addresses of I2C controller buses)
> +	- gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
> +		(I2C host controller clock frequency)
> +	- gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
> +		(baud rate used in I2C transmission)
> +
> +
> +PciEmulation configuration
> +==========================
> +Installation of various NonDiscoverable devices via PciEmulation driver is performed

I think calling this PciEmulation is no longer accurate.

> +via set of PCDs. Following are available:
> +
> +  - gMarvellTokenSpaceGuid.PcdPciEXhci
> +	(Indicates, which Xhci devices are used)
> +
> +  - gMarvellTokenSpaceGuid.PcdPciEAhci
> +	(Indicates, which Ahci devices are used)
> +
> +  - gMarvellTokenSpaceGuid.PcdPciESdhci
> +	(Indicates, which Sdhci devices are used)
> +
> +All above PCD's correspond to hardware description in a dedicated structure:
> +
> +STATIC PCI_E_PLATFORM_DESC A70x0PlatDescTemplate
> +
> +in Platforms/Marvell/PciEmulation/PciEmulation.c file. It comprises device
> +count, base addresses, register region size and DMA-coherency type.
> +
> +Example
> +-------
> +
> +Assuming we want to enable second XHCI port and one SDHCI port on Armada
> +70x0 board, following needs to be declared:
> +
> +		gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x0 0x1 }
> +		gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1 }
> +
> +
> +SATA configuration
> +==================
> +There is one additional PCD for AHCI:
> +
> +  - gMarvellTokenSpaceGuid.PcdSataBaseAddress
> +	(Base address of SATA controller register space - used in SATA ComPhy init
> +	 sequence)
> +
> +
> +Pp2Dxe configuration
> +====================
> +Pp2Dxe is driver supporting PP2 NIC on Marvell platforms. Following PCDs
> +are required to operate:
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2Controllers
> +	(Array with used controllers
> +	 Set to 0x1 for enabled, 0x0 for disabled)
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2Port2Controller
> +	(Array specifying, to which controller the port belongs to)
> +
> +  - gMarvellTokenSpaceGuid.PcdPhySmiAddresses
> +	(Addresses of PHY devices)
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2PortIds
> +	(Identificators of PP2 ports)
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2GopIndexes
> +	(Indexes used in GOP operation)
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
> +	(Set to 0x1 for always-up interface, 0x0 otherwise)
> +
> +  - gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
> +	(Values corresponding to PHY_SPEED enum.
> +	 PHY_SPEED is defined as follows:
> +
> +		  typedef enum {
> +			  0  NO_SPEED,
> +			  1  SPEED_10,
> +			  2  SPEED_100,
> +			  3  SPEED_1000,
> +			  4  SPEED_2500,
> +			  5  SPEED_10000
> +		  } PHY_SPEED;
> +
> +
> +UTMI PHY configuration
> +======================
> +In order to configure UTMI, following PCDs are available:
> +
> +  - gMarvellTokenSpaceGuid.PcdUtmiPhyCount
> +	(Indicates how many UTMI PHYs are available on platform)
> +
> +Next four PCDs are in unicode string format containing settings for all devices
> +separated with semicolon.
> +
> +  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
> +	(Indicates base address of the UTMI unit)
> +
> +  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
> +	(Indicates address of USB Configuration register)
> +
> +  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
> +	(Indicates address of external UTMI configuration)
> +
> +  - gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
> +	(Indicates type of the connected USB port)
> +
> +Example
> +-------
> +
> +		# UtmiPhy
> +		  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
> +		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
> +		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
> +		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
> +		  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
> +
> +
> +SPI driver configuration
> +========================
> +Following PCDs are available for configuration of spi driver:
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiClockFrequency
> +	(Frequency (in Hz) of SPI clock)
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
> +	(Max SCLK line frequency (in Hz) (max transfer frequency) )
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiDefaultMode
> +	(default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h) )
> +
> +
> +SpiFlash configuration
> +======================
> +Folowing PCDs for spi flash driver configuration must be set properly:
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
> +	(Size of SPI flash address in bytes (3 or 4) )
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
> +	(Size of minimal erase block in bytes)
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
> +	(Size of SPI flash page)
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashId
> +	(Id of SPI flash)
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> +	(Spi flash polling flag)
> +
> +
> +MPP configuration
> +=================
> +Multi-Purpose Ports (MPP) are configurable through platform PCDs.
> +In order to set desired pin multiplexing, .dsc file needs to be modified.
> +(OpenPlatformPkg/Platforms/Marvell/Armada/{platform_name}.dsc - please refer to

OpenPlatformPkg.

> +Documentation/Build.txt for currently supported {platftorm_name} )

And Build.txt.

/
    Leif

> +Following PCDs are available:
> +
> +  - gMarvellTokenSpaceGuid.PcdMppChipCount
> +	(Indicates how many different chips are placed on board. So far up to 4 chips
> +	 are supported)
> +
> +Every MPP PCD has <Num> part where
> + <Num> stands for chip ID (order is not important, but configuration will be
> + set for first PcdMppChipCount chips).
> +
> +Below is example for the first chip (Chip0).
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag
> +	(Indicates that register order is reversed. (Needs to be used only for AP806-Z1) )
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress
> +	(This is base address for MPP configuration register)
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0MppPinCount
> +	(Defines how many MPP pins are available)
> +
> +  - gMarvellTokenSpaceGuid.PcdChip0MppSel0
> +  - gMarvellTokenSpaceGuid.PcdChip0MppSel1
> +  - gMarvellTokenSpaceGuid.PcdChip0MppSel2
> +	(This registers defines functions of 10 pins in ascending order)
> +
> +Examples
> +--------
> +
> +		# APN806-A0 MPP SET
> +		 gMarvellTokenSpaceGuid.PcdChip0MppReverseFlag|FALSE
> +		 gMarvellTokenSpaceGuid.PcdChip0MppBaseAddress|0xF06F4000
> +		 gMarvellTokenSpaceGuid.PcdChip0MppRegCount|3
> +		 gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x0 }
> +		 gMarvellTokenSpaceGuid.PcdChip0MppSel1|{ 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 }
> +
> +Set pin 6 and 7 to 0xa function:
> +		 gMarvellTokenSpaceGuid.PcdChip0MppSel0|{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xa, 0xa, 0x0, 0x0 }
> +
> +
> +MarvellResetSystemLib configuration
> +===================================
> +This simple library allows to mask given bits in given reg at UEFI 'reset'
> +command call. These variables are configurable through PCDs:
> +
> +  - gMarvellTokenSpaceGuid.PcdResetRegAddress
> +  - gMarvellTokenSpaceGuid.PcdResetRegMask
> +
> +
> +Ramdisk configuration
> +=====================
> +There is one PCD available for Ramdisk configuration
> +
> +  - gMarvellTokenSpaceGuid.PcdRamDiskSize
> +	(Defines size of Ramdisk)

Re: [platforms: PATCH 03/11] Drivers/Spi/MvSpiDxe: Fix write bug

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:15PM +0200, Marcin Wojtas wrote:
> From: Joe Zhou <shjzhou@marvell.com>
> 
> This patch prevents possible NULL pointer dereference
> during SPI transfers.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Joe Zhou <shjzhou@marvell.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> index 0872f61..6ddfcf6 100755
> --- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> +++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> @@ -226,9 +226,8 @@ MvSpiTransfer (
>      // Wait for memory ready
>      for (Iterator = 0; Iterator < SPI_TIMEOUT; Iterator++) {
>        if (MmioRead32 (SpiRegBase + SPI_INT_CAUSE_REG)) {
> -        *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
> -
>          if (DataInPtr != NULL) {
> +          *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
>            DataInPtr++;
>          }
>          if (DataOutPtr != NULL) {
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 04/11] Applications/SpiTool: Enable configurable CS and SCLK mode

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:16PM +0200, Marcin Wojtas wrote:
> Until now transfer SCLK mode and CS were fixed, when using
> shell 'sf' command. This patch enables their configuration.
> Update porting guide accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c   | 6 +++++-
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf | 2 ++
>  Platform/Marvell/Documentation/PortingGuide.txt       | 9 +++++----
>  Platform/Marvell/Marvell.dec                          | 2 ++
>  4 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> index 184e3d7..b6dc54f 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -218,6 +218,7 @@ EFI_STATUS              Status;
>    CONST CHAR16          *LengthStr = NULL, *FileStr = NULL;
>    BOOLEAN               AddrFlag = FALSE, LengthFlag = TRUE, FileFlag = FALSE;
>    UINT8                 Flag = 0, CheckFlag = 0;
> +  UINT8                 Mode, Cs;
>  
>    Status = gBS->LocateProtocol (
>      &gMarvellSpiFlashProtocolGuid,
> @@ -283,8 +284,11 @@ EFI_STATUS              Status;
>      }
>    }
>  
> +  Mode = PcdGet32 (PcdSpiFlashMode);
> +  Cs = PcdGet32 (PcdSpiFlashCs);
> +
>    // Setup new spi device
> -  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, 0, 0);
> +  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, Cs, Mode);
>      if (Slave == NULL) {
>        Print(L"sf: Cannot allocate SPI device!\n");
>        return SHELL_ABORTED;
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> index 41b7b7c..c1ab770 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> @@ -66,6 +66,8 @@
>  
>  [Pcd]
>   gMarvellTokenSpaceGuid.PcdSpiFlashId
> + gMarvellTokenSpaceGuid.PcdSpiFlashCs
> + gMarvellTokenSpaceGuid.PcdSpiFlashMode

Alphabetically sorted, please.

>  
>  [Protocols]
>   gMarvellSpiFlashProtocolGuid
> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> index 8c3579e..3b79bd2 100644
> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -284,10 +284,6 @@ Following PCDs are available for configuration of spi driver:
>    - gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
>  	(Max SCLK line frequency (in Hz) (max transfer frequency) )
>  
> -  - gMarvellTokenSpaceGuid.PcdSpiDefaultMode
> -	(default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h) )
> -
> -
>  SpiFlash configuration
>  ======================
>  Folowing PCDs for spi flash driver configuration must be set properly:
> @@ -307,6 +303,11 @@ Folowing PCDs for spi flash driver configuration must be set properly:
>    - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
>  	(Spi flash polling flag)
>  
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashMode
> +	(Default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h))
> +
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashCs
> +	(Chip select used for communication with the Flash)
>  
>  MPP configuration
>  =================
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index 4e2dd6d..869e376 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -128,6 +128,8 @@
>    gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
>    gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
>    gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
> +  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
> +  gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
>  
>  #ComPhy
>    gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x0 }|VOID*|0x30000098
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 05/11] Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:17PM +0200, Marcin Wojtas wrote:
> This patch makes use of recently added SPI configuration
> PCDs and sets CS with SCLK mode on Armada 7040 DB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Armada/Armada70x0.dsc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index f519196..df2ebdb 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -98,6 +98,8 @@
>    gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
>    gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
>    gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
> +  gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
> +  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
>  
>    #ComPhy
>    gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 06/11] Applications/SpiTool: Fix bug in error test

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:18PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Fix a misplaced closing parenthesis.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> index b6dc54f..e6e1007 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -378,7 +378,7 @@ EFI_STATUS              Status;
>        FilePath = (CHAR16 *) FileStr;
>        Status = ShellIsFile (FilePath);
>        // When read file into flash, file doesn't have to exist
> -      if (EFI_ERROR(Status && !(Flag & READ_FILE))) {
> +      if (EFI_ERROR (Status) && !(Flag & READ_FILE)) {
>          Print (L"sf: Wrong FilePath parameter!\n");
>          return SHELL_ABORTED;
>        }
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Fix casting and related issues to make this code build for 32-bit ARM.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> index edb6986..0951734 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
>    EFI_STATUS           Status;
>    UINT64               OpenMode;
>    UINTN                *Buffer;
> +  UINT64               Size;
>  
>    // Parse string from commandline
>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
>        return EFI_DEVICE_ERROR;
>      }
>  
> -  Status = FileHandleGetSize (*FileHandle, FileSize);
> +  Status = FileHandleGetSize (*FileHandle, &Size);
>      if (EFI_ERROR (Status)) {
>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
>      }
>  
> +  *FileSize = (UINTN)Size;
> +

Rather than juggling around with temporary variables, why not make
FileSize in ShellCommandRunFUpdate() UINT64 and update
PrepareFirmwareImage() prototype accordingly?

/
     Leif

>    // Read Image header into buffer
>    Buffer = AllocateZeroPool (*FileSize);
>  
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 08/11] Applications/SpiTool: Fix 32-bit issues

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:20PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Fix casting and related issues to make this code build for 32-bit ARM.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> index e6e1007..ee14270 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -418,7 +418,7 @@ EFI_STATUS              Status;
>      }
>    }
>  
> -  Buffer = (UINT8 *) Address;
> +  Buffer = (UINT8 *)(UINTN)Address;
>    if (FileFlag) {
>      Buffer = FileBuffer;
>    }
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide

[Marcin Wojtas]

Hi Leif,

2017-09-01 16:36 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Sep 01, 2017 at 03:08:13PM +0200, Marcin Wojtas wrote:
>> From: Nir Erez <nerez@marvell.com>
>>
>> This patch introduces following improvements to the PortingGuide
>> * Replace split documentation with single file
>> * Align format to Doxygen constraints
>
> * Add Build description.
> (Comment on that below.)
>
> Since you are moving things around, I take this as license to
> bikeshed:
>
> I think we should start looking towards separating the development
> board and SoC portions, like is done for most other platforms. After
> all, the goal is to get additional platforms (not just the EVB).
>
> Not saying this is something that needs to happen overnight, but it
> would make sense to make a start by moving these docs to
> Silicon/Marvell/Documentation as part of this patch.

I can add it there, no problem. About Silicon vs Platform - I checked
the code and basically we'd have to move everything from
Platform/Marvell to Silicon/Marvell and leave just subdirectories with
board files (dsc, fdf + dtb):
Platform/Marvell/Armada70x0Db
Platform/Marvell/Armada80x0Db
Platform/Marvell/Armada80x0McBin
etc.
Is it what you mean?

I'm wondering when will be good moment for this - I'm rebasing 70 OPP
patches slowly in my extra time, so maybe after the SPI patchset? Or
better after we merge everything to Platform and then do the code
shifting? What is your feeling about it?

>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Documentation/Build.txt           |  58 ++++
>>  Platform/Marvell/Documentation/PortingGuide.txt    | 371 +++++++++++++++++++++
>>  .../Marvell/Documentation/PortingGuide/ComPhy.txt  |  45 ---
>>  .../Marvell/Documentation/PortingGuide/I2c.txt     |  20 --
>>  .../Marvell/Documentation/PortingGuide/Mdio.txt    |   7 -
>>  .../Marvell/Documentation/PortingGuide/Mpp.txt     |  48 ---
>>  .../Documentation/PortingGuide/PciEmulation.txt    |  31 --
>>  .../Marvell/Documentation/PortingGuide/Phy.txt     |  45 ---
>>  .../Marvell/Documentation/PortingGuide/Pp2.txt     |  35 --
>>  .../Marvell/Documentation/PortingGuide/Reset.txt   |   7 -
>>  .../Marvell/Documentation/PortingGuide/Spi.txt     |  16 -
>>  .../Documentation/PortingGuide/SpiFlash.txt        |  23 --
>>  .../Marvell/Documentation/PortingGuide/Utmi.txt    |  35 --
>
> Also, it would be nice if you could generate patches with
>   --stat=1000 --stat-graph-width=20
> as documented in Laszlo's unkempt guide:
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
> Makes the above summare _much_ easier to read.

No problem, will do.

>
>>  13 files changed, 429 insertions(+), 312 deletions(-)
>>  create mode 100644 Platform/Marvell/Documentation/Build.txt
>>  create mode 100644 Platform/Marvell/Documentation/PortingGuide.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
>>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt
>>
>> diff --git a/Platform/Marvell/Documentation/Build.txt b/Platform/Marvell/Documentation/Build.txt
>> new file mode 100644
>> index 0000000..1162e2e
>> --- /dev/null
>> +++ b/Platform/Marvell/Documentation/Build.txt
>> @@ -0,0 +1,58 @@
>> +UEFI Build Instructions
>> +=======================
>> +
>> +For toolchain versions limitations please refer to edk2 wiki page:
>> +https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC
>> +
>> +Fully supported are gcc4.5 - gcc4.9, so possible {toolchain_name} are:
>> +     - GCC45
>> +     - GCC46
>> +     - GCC47
>> +     - GCC48
>> +     - GCC49
>> +     - GCC5
>> +
>> +Supported {platform} are:
>> +     - Armada70x0
>> +
>> +Supported {target} are
>> +     - DEBUG
>> +     - RELEASE
>> +
>> +Build procedure
>> +---------------
>> +1. Prerequisites:
>> +
>> +     Clone into edk2 repositories and apply Marvell patches (Please refer to
>> +     Release notes for instructions).
>> +
>> +2. Prepare environment:
>> +
>> +     2.1 Several packages will be needed to fully set up an edk2 build environment:
>> +
>> +                     # sudo apt-get install build-essential uuid-dev
>> +                     # sudo apt-get install lib32stdc++6 lib32z1
>> +
>> +     2.2 Set up EDK2 environment
>> +
>> +                     # source edksetup.sh
>> +
>> +     2.3 Build base tools
>> +
>> +                     # make -C BaseTools
>> +
>> +     2.4 Set {toolchain_name}_AARCH64_PREFIX to path to your cross compiler
>> +
>> +                     # export {toolchain_name}_AARCH64_PREFIX=/path/to/toolchain
>> +
>> +             Example:
>> +             --------
>> +                     # export GCC5_AARCH64_PREFIX=/opt/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-
>> +
>> +3. Build EDK2 for selected {platform}:
>> +
>> +                     # build -a AARCH64 -t {toolchain_name} -b {target} -p OpenPlatformPkg/Platforms/Marvell/Armada/{platform}.dsc
>> +
>> +     Example for building edk2 for Armada70x0 platform with GCC5 for DEBUG:
>> +
>> +                     # build -a AARCH64 -t GCC5 -b DEBUG -p OpenPlatformPkg/Platforms/Marvell/Armada/Armada70x0.dsc
>
> The above all documents OpenPlatformPkg, so is now dated.
> My preference would be for this to become a Readme.md under
> Platform/Marvell, if you feel additional documentation to the
> top-level Readme.md to be necessary.

Right. How about only some minimal Readme.md as you suggest with the
list of available Marvell platforms and pointer to the main Readme
file?

>
> Regardless, it is unrelated to the rest of changes here, so would make
> sense as a separate patch.

Ok.

>
>> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
>> new file mode 100644
>> index 0000000..8c3579e
>> --- /dev/null
>> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
>> @@ -0,0 +1,371 @@
>> +UEFI Porting Guide
>> +==================
>> +
>> +This document provides instructions for adding support for new Marvell Armada
>> +board. For the sake of simplicity new Marvell board will be called "new_board".
>> +
>> +1. Create configuration files for new target
>> +     1.1 Create FDF file for new board
>> +
>> +      - Copy and rename PathToYourOpp/Platforms/Marvell/Armada/Armada70x0.fdf to
>> +        PathToYourOpp/Platforms/Marvell/Armada/new_board.fdf
>
> OpenPlatformPkg.

Sure, I will update all of them.

>
>> +      - Change the first no-comment line:
>> +        [FD.Armada70x0_EFI] to [FD.{new_board}_EFI]
>> +
>> +     1.2 Create DSC file for new board
>> +
>> +      - Add new_board.dsc file to PathToYourOpp/Platforms/Marvell/Armada directory
>
> OpenPlatformPkg.
>
>> +      - Insert following [Defines] section to new_board.dsc:
>> +
>> +                     [Defines]
>> +                       PLATFORM_NAME                  = {new_board}
>> +                       PLATFORM_GUID                  = {newly_generated_GUID}
>> +                       PLATFORM_VERSION               = 0.1
>> +                       DSC_SPECIFICATION              = 0x00010019
>> +                       OUTPUT_DIRECTORY               = {output_directory}
>> +                       SUPPORTED_ARCHITECTURES        = AARCH64
>> +                       BUILD_TARGETS                  = DEBUG|RELEASE
>> +                       SKUID_IDENTIFIER               = DEFAULT
>> +                       FLASH_DEFINITION               = {path_to_fdf_file}
>> +
>> +      - Add "!include Armada.dsc.inc" entry to new_board.dsc
>> +
>> +2. Driver support
>> + - According to content of files from PathToYourOpp/Documentation/Marvell/PortingGuide
>
> OpenPlatformPkg.
>
>> +   insert PCD entries into new_board.dsc for every needed interface (as listed below).
>> +
>> +3. Compilation
>> + - Refer to PathToYourOpp/Documentation/Marvell/Build.txt. Remember to change
>
> OpenPlatformPkg. (Also referencing the Build.txt.)
>
>> +   {platform} to new_board in order to point build system to newly created DSC file.
>> +
>> +4. Output file
>> + - Output files (and among others FD file, which may be used by ATF) are
>> +   generated under directory pointed by "OUTPUT_DIRECTORY" entry (see point 1.2).
>> +
>> +
>> +COMPHY configuration
>> +====================
>> +In order to configure ComPhy library, following PCDs are available:
>> +
>> +  - gMarvellTokenSpaceGuid.PcdComPhyDevices
>> +
>> +This array indicates, which ones of the ComPhy chips defined in
>> +MVHW_COMPHY_DESC template will be configured.
>> +
>> +Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
>> +important, but configuration will be set for first PcdComPhyChipCount chips).
>> +
>> +Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
>> +settings for this chip. Their format is unicode string, containing settings
>> +for up to 10 lanes. Setting for each one is separated with semicolon.
>> +These PCDs together describe outputs of PHY integrated in simple cihp.
>> +Below is example for the first chip (Chip0).
>> +
>> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
>> +     (Unicode string indicating PHY types. Currently supported are:
>> +
>> +             { L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
>> +               L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
>> +               L"SGMII1", L"SGMII2", L"SGMII3", L"QSGMII",
>> +               L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
>> +               L"XAUI0", L"XAUI1", L"XAUI2", L"XAUI3", L"RXAUI0",
>> +               L"RXAUI1", L"KR" } )
>> +
>> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
>> +     (Indicates PHY speeds in MHz. Currently supported are:
>> +             { 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 1031 }  )
>> +
>> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
>> +     (Indicates lane polarity invert)
>> +
>> +Example
>> +-------
>> +
>> +               #ComPhy
>> +               gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
>> +               gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
>> +               gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
>> +
>> +
>> +PHY Driver configuration
>> +========================
>> +MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
>> +Currently only 1518 series PHYs are supported. Following PCDs are required:
>> +
>> +  - gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
>> +     (list of values corresponding to PHY_CONNECTION enum)
>> +  - gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
>> +     (boolean - if true, driver waits for autonegotiation on startup)
>> +  - gMarvellTokenSpaceGuid.PcdPhyDeviceIds
>> +     (list of values corresponding to MV_PHY_DEVICE_ID enum)
>> +
>> +PHY_CONNECTION enum type is defined as follows:
>> +
>> +             typedef enum {
>> +                     0    PHY_CONNECTION_RGMII,
>> +                     1    PHY_CONNECTION_RGMII_ID,
>> +                     2    PHY_CONNECTION_RGMII_TXID,
>> +                     3    PHY_CONNECTION_RGMII_RXID,
>> +                     4    PHY_CONNECTION_SGMII,
>> +                     5    PHY_CONNECTION_RTBI,
>> +                     6    PHY_CONNECTION_XAUI,
>> +                     7    PHY_CONNECTION_RXAUI
>> +             } PHY_CONNECTION;
>> +
>> +MV_PHY_DEVICE_ID:
>> +
>> +             typedef enum {
>> +                     0    MV_PHY_DEVICE_1512,
>> +             } MV_PHY_DEVICE_ID;
>> +
>> +It should be extended when adding support for other PHY models.
>> +Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
>> +
>> + gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
>> +
>> +with disabled autonegotiation:
>> +
>> + gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
>> +
>> +assuming, that PHY models are 1512:
>> +
>> + gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
>> +
>> +
>> +MDIO configuration
>> +==================
>> +MDIO driver provides access to network PHYs' registers via EFI_MDIO_READ and
>> +EFI_MDIO_WRITE functions (EFI_MDIO_PROTOCOL). Following PCD is required:
>> +
>> +  - gMarvellTokenSpaceGuid.PcdMdioBaseAddress
>> +     (base address of SMI management register)
>> +
>> +
>> +I2C configuration
>> +=================
>> +In order to enable driver on a new platform, following steps need to be taken:
>> + - add following line to .dsc file:
>> +   OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
>> + - add following line to .fdf file:
>> +   INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
>> + - add PCDs with relevant values to .dsc file:
>> +     - gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57 }
>> +             (addresses of I2C slave devices on bus)
>> +     - gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0 }
>> +             (buses to which accoring slaves are attached)
>> +     - gMarvellTokenSpaceGuid.PcdI2cBusCount|2
>> +             (number of SoC's I2C buses)
>> +     - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
>> +             (base addresses of I2C controller buses)
>> +     - gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
>> +             (I2C host controller clock frequency)
>> +     - gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
>> +             (baud rate used in I2C transmission)
>> +
>> +
>> +PciEmulation configuration
>> +==========================
>> +Installation of various NonDiscoverable devices via PciEmulation driver is performed
>
> I think calling this PciEmulation is no longer accurate.

Our driver in the tree is called this way, what do you suggest instead?

Thanks,
Marcin

Re: [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues

[Ard Biesheuvel]

On 1 September 2017 at 15:54, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Fix casting and related issues to make this code build for 32-bit ARM.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index edb6986..0951734 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
>>    EFI_STATUS           Status;
>>    UINT64               OpenMode;
>>    UINTN                *Buffer;
>> +  UINT64               Size;
>>
>>    // Parse string from commandline
>>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
>> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
>>        return EFI_DEVICE_ERROR;
>>      }
>>
>> -  Status = FileHandleGetSize (*FileHandle, FileSize);
>> +  Status = FileHandleGetSize (*FileHandle, &Size);
>>      if (EFI_ERROR (Status)) {
>>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
>>      }
>>
>> +  *FileSize = (UINTN)Size;
>> +
>
> Rather than juggling around with temporary variables, why not make
> FileSize in ShellCommandRunFUpdate() UINT64 and update
> PrepareFirmwareImage() prototype accordingly?
>

I don't remember /exactly/, but I think it breaks in another place then.

Re: [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:21PM +0200, Marcin Wojtas wrote:
> Although, hitherto support allowed for using configurable EraseSize,
> the erase command was fixed to CMD_ERASE_64K. Also it was
> assumed that EraseSize equals SectorSize, which is not true
> for some flash devices.

Another thing I immediately wish I had never learned.
Thanks :/

> Fix both issues by adding new PCD
> (gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
> this parameter properly in MvSpiFlashUpdate routine instead
> of the EraseSize. Also erase command is adjusted to the settings.
> Update PortingGuide accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Documentation/PortingGuide.txt    |  3 +++
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
>  Platform/Marvell/Marvell.dec                       |  1 +
>  5 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> index 3b79bd2..f637fee 100644
> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -297,6 +297,9 @@ Folowing PCDs for spi flash driver configuration must be set properly:
>    - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>  	(Size of SPI flash page)
>  
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> +	(Size of SPI flash sector, 65536 bytes by default)
> +
>    - gMarvellTokenSpaceGuid.PcdSpiFlashId
>  	(Id of SPI flash)
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index 9a04493..f3fdba4 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -191,7 +191,21 @@ MvSpiFlashErase (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  Cmd[0] = CMD_ERASE_64K;
> +  switch (EraseSize) {
> +  case SPI_ERASE_SIZE_4K:
> +    Cmd[0] = CMD_ERASE_4K;
> +    break;
> +  case SPI_ERASE_SIZE_32K:
> +    Cmd[0] = CMD_ERASE_32K;
> +    break;
> +  case SPI_ERASE_SIZE_64K:
> +    Cmd[0] = CMD_ERASE_64K;
> +    break;
> +  default:
> +    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    while (Length) {
>      EraseAddr = Offset;
>  
> @@ -353,14 +367,14 @@ MvSpiFlashUpdate (
>    )
>  {
>    EFI_STATUS Status;
> -  UINT64 EraseSize, ToUpdate, Scale = 1;
> +  UINT64 SectorSize, ToUpdate, Scale = 1;
>    UINT8 *TmpBuf, *End;
>  
> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> +  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
>  
>    End = Buf + ByteCount;
>  
> -  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
> +  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
>    if (TmpBuf == NULL) {
>      DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
>      return EFI_OUT_OF_RESOURCES;
> @@ -370,9 +384,9 @@ MvSpiFlashUpdate (
>      Scale = (End - Buf) / 100;
>  
>    for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
> -    ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
> +    ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
>      Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
> -    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize);
> +    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize);
>  
>      if (EFI_ERROR (Status)) {
>        DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> index 3889643..646598a 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> @@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define CMD_READ_ARRAY_FAST             0x0b
>  #define CMD_PAGE_PROGRAM                0x02
>  #define CMD_BANK_WRITE                  0xc5
> +#define CMD_ERASE_4K                    0x20
> +#define CMD_ERASE_32K                   0x52
>  #define CMD_ERASE_64K                   0xd8
>  #define CMD_4B_ADDR_ENABLE              0xb7
>  
> @@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define SPI_TRANSFER_BEGIN              0x01  // Assert CS before transfer
>  #define SPI_TRANSFER_END                0x02  // Deassert CS after transfers
>  
> +#define SPI_ERASE_SIZE_4K               4096
> +#define SPI_ERASE_SIZE_32K              32768
> +#define SPI_ERASE_SIZE_64K              65536
> +

Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?

/
    Leif

>  #define SPI_FLASH_16MB_BOUN             0x1000000
>  
>  typedef enum {
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> index d035d47..4519b02 100644
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> @@ -58,6 +58,7 @@
>    gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
>    gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>    gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> +  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
>  
>  [Protocols]
>    gMarvellSpiMasterProtocolGuid
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index 869e376..fc00f1a 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -127,6 +127,7 @@
>    gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
>    gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
>    gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
> +  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
>    gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter

[Marcin Wojtas]

2017-09-01 17:21 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Sep 01, 2017 at 03:08:21PM +0200, Marcin Wojtas wrote:
>> Although, hitherto support allowed for using configurable EraseSize,
>> the erase command was fixed to CMD_ERASE_64K. Also it was
>> assumed that EraseSize equals SectorSize, which is not true
>> for some flash devices.
>
> Another thing I immediately wish I had never learned.
> Thanks :/

Do you mean the code or flash quirks? :)

>
>> Fix both issues by adding new PCD
>> (gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
>> this parameter properly in MvSpiFlashUpdate routine instead
>> of the EraseSize. Also erase command is adjusted to the settings.
>> Update PortingGuide accordingly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Documentation/PortingGuide.txt    |  3 +++
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
>>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
>>  Platform/Marvell/Marvell.dec                       |  1 +
>>  5 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
>> index 3b79bd2..f637fee 100644
>> --- a/Platform/Marvell/Documentation/PortingGuide.txt
>> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
>> @@ -297,6 +297,9 @@ Folowing PCDs for spi flash driver configuration must be set properly:
>>    - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>>       (Size of SPI flash page)
>>
>> +  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
>> +     (Size of SPI flash sector, 65536 bytes by default)
>> +
>>    - gMarvellTokenSpaceGuid.PcdSpiFlashId
>>       (Id of SPI flash)
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> index 9a04493..f3fdba4 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> @@ -191,7 +191,21 @@ MvSpiFlashErase (
>>      return EFI_DEVICE_ERROR;
>>    }
>>
>> -  Cmd[0] = CMD_ERASE_64K;
>> +  switch (EraseSize) {
>> +  case SPI_ERASE_SIZE_4K:
>> +    Cmd[0] = CMD_ERASE_4K;
>> +    break;
>> +  case SPI_ERASE_SIZE_32K:
>> +    Cmd[0] = CMD_ERASE_32K;
>> +    break;
>> +  case SPI_ERASE_SIZE_64K:
>> +    Cmd[0] = CMD_ERASE_64K;
>> +    break;
>> +  default:
>> +    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>>    while (Length) {
>>      EraseAddr = Offset;
>>
>> @@ -353,14 +367,14 @@ MvSpiFlashUpdate (
>>    )
>>  {
>>    EFI_STATUS Status;
>> -  UINT64 EraseSize, ToUpdate, Scale = 1;
>> +  UINT64 SectorSize, ToUpdate, Scale = 1;
>>    UINT8 *TmpBuf, *End;
>>
>> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
>> +  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
>>
>>    End = Buf + ByteCount;
>>
>> -  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
>> +  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
>>    if (TmpBuf == NULL) {
>>      DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
>>      return EFI_OUT_OF_RESOURCES;
>> @@ -370,9 +384,9 @@ MvSpiFlashUpdate (
>>      Scale = (End - Buf) / 100;
>>
>>    for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
>> -    ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
>> +    ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
>>      Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
>> -    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize);
>> +    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize);
>>
>>      if (EFI_ERROR (Status)) {
>>        DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> index 3889643..646598a 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> @@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #define CMD_READ_ARRAY_FAST             0x0b
>>  #define CMD_PAGE_PROGRAM                0x02
>>  #define CMD_BANK_WRITE                  0xc5
>> +#define CMD_ERASE_4K                    0x20
>> +#define CMD_ERASE_32K                   0x52
>>  #define CMD_ERASE_64K                   0xd8
>>  #define CMD_4B_ADDR_ENABLE              0xb7
>>
>> @@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #define SPI_TRANSFER_BEGIN              0x01  // Assert CS before transfer
>>  #define SPI_TRANSFER_END                0x02  // Deassert CS after transfers
>>
>> +#define SPI_ERASE_SIZE_4K               4096
>> +#define SPI_ERASE_SIZE_32K              32768
>> +#define SPI_ERASE_SIZE_64K              65536
>> +
>
> Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?
>

Sure, will do.

Marcin

Re: [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:22PM +0200, Marcin Wojtas wrote:
> Hitherto mechanism of fixing SPI flash model in the PCDs,
> occured to be very inefficient and problematic. Enable
> dynamic detection by reworking MvSpiFlashReadId() command,
> which now reads the Id and goes through newly added table
> with JEDEC compliant devices and their description.
> 
> On the occasion fix the ReadId process by using master's
> ReadWrite routine instead of pure Transfer - no longer
> swapping and byte shifting is needed. Simplify code by
> using local array instead of dynamic allocation. Also
> reduced number of ReadId arguments allowed for cleaning
> Fupdate and Sf shell commands probe routines.
> 
> Additionally, use SpiFlashInfo fields instead of PCDs,
> and configure needed settings in MvSpiFlashInit.
> 
> Update PortingGuide documentation accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

So, this looks _really_ good, but I'm seeing three separate patches in
here really:
- Style fixups (yay). I love them, but squashing them in make the
  functional changes impossible to spot in the noise.
- SPI Flash Id table. Love it!
  But this is a completely generic feature. Medium-term, this should
  be living in EDK2. For now, can you break it out into a separate
  library that can be used by other platforms (and will be easier to
  move)?
- Functional improvements to existing driver - great! (but can't
  really review before the above two have been broken out).

/
    Leif

> ---
>  .../Marvell/Applications/FirmwareUpdate/FUpdate.c  |  23 +-
>  .../Applications/FirmwareUpdate/FUpdate.inf        |   3 -
>  .../Marvell/Applications/SpiTool/SpiFlashCmd.c     |  36 +--
>  .../Marvell/Applications/SpiTool/SpiFlashCmd.inf   |   1 -
>  Platform/Marvell/Armada/Armada70x0.dsc             |   5 -
>  Platform/Marvell/Documentation/PortingGuide.txt    |  18 --
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 267 +++++++++++++++++----
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |   2 +
>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |   7 -
>  Platform/Marvell/Include/Protocol/Spi.h            |  37 +++
>  Platform/Marvell/Include/Protocol/SpiFlash.h       |   4 +-
>  Platform/Marvell/Marvell.dec                       |   6 -
>  12 files changed, 271 insertions(+), 138 deletions(-)
> 
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> index 0951734..b0e94bc 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> @@ -94,32 +94,17 @@ SpiFlashProbe (
>    )
>  {
>    EFI_STATUS       Status;
> -  UINT32           IdBuffer, Id, RefId;
> -
> -  Id = PcdGet32 (PcdSpiFlashId);
> -
> -  IdBuffer = CMD_READ_ID & 0xff;
>  
>    // Read SPI flash ID
> -  SpiFlashProtocol->ReadId (Slave, sizeof (UINT32), (UINT8 *)&IdBuffer);
> -
> -  // Swap and extract 3 bytes of the ID
> -  RefId = SwapBytes32 (IdBuffer) >> 8;
> -
> -  if (RefId == 0) {
> -    Print (L"%s: No SPI flash detected");
> -    return EFI_DEVICE_ERROR;
> -  } else if (RefId != Id) {
> -    Print (L"%s: Unsupported SPI flash detected with ID=%2x\n", CMD_NAME_STRING, RefId);
> -    return EFI_DEVICE_ERROR;
> +  Status = SpiFlashProtocol->ReadId (Slave);
> +  if (EFI_ERROR (Status)) {
> +    return SHELL_ABORTED;
>    }
>  
> -  Print (L"%s: Detected supported SPI flash with ID=%3x\n", CMD_NAME_STRING, RefId);
> -
>    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
>    if (EFI_ERROR(Status)) {
>      Print (L"%s: Cannot initialize flash device\n", CMD_NAME_STRING);
> -    return EFI_DEVICE_ERROR;
> +    return SHELL_ABORTED;
>    }
>  
>    return EFI_SUCCESS;
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> index 92c3333..43cac42 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> @@ -64,9 +64,6 @@
>    UefiLib
>    UefiRuntimeServicesTableLib
>  
> -[Pcd]
> - gMarvellTokenSpaceGuid.PcdSpiFlashId
> -
>  [Protocols]
>   gMarvellSpiFlashProtocolGuid
>   gMarvellSpiMasterProtocolGuid
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> index ee14270..b5db8b5 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -166,37 +166,21 @@ FlashProbe (
>    )
>  {
>    EFI_STATUS Status;
> -  UINT8  IdBuffer[4];
> -  UINT32 Id, RefId;
>  
> -  Id = PcdGet32 (PcdSpiFlashId);
> -
> -  IdBuffer[0] = CMD_READ_ID;
> -
> -  SpiFlashProtocol->ReadId (
> -    Slave,
> -    4,
> -    IdBuffer
> -    );
> -
> -  RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];
> +  Status = SpiFlashProtocol->ReadId (Slave);
> +  if (EFI_ERROR (Status)) {
> +    return SHELL_ABORTED;
> +  }
>  
> -  if (RefId == Id) {
> -    Print (L"sf: Detected supported SPI flash with ID=%3x\n", RefId);
> -    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
> -    if (EFI_ERROR(Status)) {
> -      Print (L"sf: Cannot initialize flash device\n");
> -      return SHELL_ABORTED;
> -    }
> -    InitFlag = 0;
> -    return EFI_SUCCESS;
> -  } else if (RefId != 0) {
> -    Print (L"sf: Unsupported SPI flash detected with ID=%2x\n", RefId);
> +  Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
> +  if (EFI_ERROR (Status)) {
> +    Print (L"sf: Cannot initialize flash device\n");
>      return SHELL_ABORTED;
>    }
>  
> -  Print (L"sf: No SPI flash detected");
> -  return SHELL_ABORTED;
> +  InitFlag = 0;
> +
> +  return SHELL_SUCCESS;
>  }
>  
>  SHELL_STATUS
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> index c1ab770..343d5b5 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> @@ -65,7 +65,6 @@
>   FileHandleLib
>  
>  [Pcd]
> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>   gMarvellTokenSpaceGuid.PcdSpiFlashCs
>   gMarvellTokenSpaceGuid.PcdSpiFlashMode
>  
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index df2ebdb..4633e32 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -93,11 +93,6 @@
>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>  
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
> -  gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
>  
> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> index f637fee..3ac35a0 100644
> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -288,24 +288,6 @@ SpiFlash configuration
>  ======================
>  Folowing PCDs for spi flash driver configuration must be set properly:
>  
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
> -	(Size of SPI flash address in bytes (3 or 4) )
> -
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
> -	(Size of minimal erase block in bytes)
> -
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
> -	(Size of SPI flash page)
> -
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> -	(Size of SPI flash sector, 65536 bytes by default)
> -
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashId
> -	(Id of SPI flash)
> -
> -  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> -	(Spi flash polling flag)
> -
>    - gMarvellTokenSpaceGuid.PcdSpiFlashMode
>  	(Default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h))
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index f3fdba4..bfb8fa3 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -36,6 +36,136 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
>  SPI_FLASH_INSTANCE  *mSpiFlashInstance;
>  
> +#define INFO(JedecId, ExtId, SecSize, NSectors, FlashFlags)  \
> +  .Id = {                       \
> +    ((JedecId) >> 16) & 0xff,   \
> +    ((JedecId) >> 8) & 0xff,    \
> +    (JedecId) & 0xff,           \
> +    ((ExtId) >> 8) & 0xff,      \
> +    (ExtId) & 0xff,             \
> +    },                          \
> +  .IdLen = (!(JedecId) ? 0 : (3 + ((ExtId) ? 2 : 0))),  \
> +  .SectorSize = (SecSize),      \
> +  .SectorCount = (NSectors),    \
> +  .PageSize = 256,              \
> +  .Flags = (FlashFlags),
> +
> +static SPI_FLASH_INFO SpiFlashIds[] = {
> +  /* ATMEL */
> +  {L"at45db011d",        INFO(0x1f2200, 0x0, 64 * 1024,     4, SECT_4K) },
> +  {L"at45db021d",        INFO(0x1f2300, 0x0, 64 * 1024,     8, SECT_4K) },
> +  {L"at45db041d",        INFO(0x1f2400, 0x0, 64 * 1024,     8, SECT_4K) },
> +  {L"at45db081d",        INFO(0x1f2500, 0x0, 64 * 1024,    16, SECT_4K) },
> +  {L"at45db161d",        INFO(0x1f2600, 0x0, 64 * 1024,    32, SECT_4K) },
> +  {L"at45db321d",        INFO(0x1f2700, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at45db641d",        INFO(0x1f2800, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"at25df321a",        INFO(0x1f4701, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at25df321",         INFO(0x1f4700, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at26df081a",        INFO(0x1f4501, 0x0, 64 * 1024,    16, SECT_4K) },
> +  /* EON */
> +  {L"en25q32b",          INFO(0x1c3016, 0x0, 64 * 1024,    64, 0) },
> +  {L"en25q64",           INFO(0x1c3017, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"en25q128b",         INFO(0x1c3018, 0x0, 64 * 1024,   256, 0) },
> +  {L"en25s64",           INFO(0x1c3817, 0x0, 64 * 1024,   128, 0) },
> +  /* GIGADEVICE */
> +  {L"gd25q64b",          INFO(0xc84017, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"gd25lq32",          INFO(0xc86016, 0x0, 64 * 1024,    64, SECT_4K) },
> +  /* ISSI */
> +  {L"is25lp032",         INFO(0x9d6016, 0x0, 64 * 1024,    64, 0) },
> +  {L"is25lp064",         INFO(0x9d6017, 0x0, 64 * 1024,   128, 0) },
> +  {L"is25lp128",         INFO(0x9d6018, 0x0, 64 * 1024,   256, 0) },
> +  /* MACRONIX */
> +  {L"mx25l2006e",        INFO(0xc22012, 0x0, 64 * 1024,     4, 0) },
> +  {L"mx25l4005",         INFO(0xc22013, 0x0, 64 * 1024,     8, 0) },
> +  {L"mx25l8005",         INFO(0xc22014, 0x0, 64 * 1024,    16, 0) },
> +  {L"mx25l1605d",        INFO(0xc22015, 0x0, 64 * 1024,    32, 0) },
> +  {L"mx25l3205d",        INFO(0xc22016, 0x0, 64 * 1024,    64, 0) },
> +  {L"mx25l6405d",        INFO(0xc22017, 0x0, 64 * 1024,   128, 0) },
> +  {L"mx25l12805",        INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"mx25l25635f",       INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP | ADDR_CYC_4) },
> +  {L"mx25l51235f",       INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"mx25l12855e",       INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"mx66u51235f",       INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"mx66l1g45g",        INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
> +  /* SPANSION */
> +  {L"s25fl008a",         INFO(0x010213, 0x0, 64 * 1024,    16, 0) },
> +  {L"s25fl016a",         INFO(0x010214, 0x0, 64 * 1024,    32, 0) },
> +  {L"s25fl032a",         INFO(0x010215, 0x0, 64 * 1024,    64, 0) },
> +  {L"s25fl064a",         INFO(0x010216, 0x0, 64 * 1024,   128, 0) },
> +  {L"s25fl116k",         INFO(0x014015, 0x0, 64 * 1024,   128, 0) },
> +  {L"s25fl164k",         INFO(0x014017, 0x0140,  64 * 1024,   128, 0) },
> +  {L"s25fl128p_256k",    INFO(0x012018, 0x0300, 256 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl128p_64k",     INFO(0x012018, 0x0301,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl032p",         INFO(0x010215, 0x4d00,  64 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl064p",         INFO(0x010216, 0x4d00,  64 * 1024,   128, RD_FULL | WR_QPP) },
> +  {L"s25fl128s_256k",    INFO(0x012018, 0x4d00, 256 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl128s_64k",     INFO(0x012018, 0x4d01,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl256s_256k",    INFO(0x010219, 0x4d00, 256 * 1024,   128, RD_FULL | WR_QPP) },
> +  {L"s25fl256s_64k",     INFO(0x010219, 0x4d01,  64 * 1024,   512, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_256k",    INFO(0x010220, 0x4d00, 256 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_64k",     INFO(0x010220, 0x4d01,  64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_512k",    INFO(0x010220, 0x4f00, 256 * 1024,   256, RD_FULL | WR_QPP) },
> +  /* STMICRO */
> +  {L"m25p10",            INFO(0x202011, 0x0, 32 * 1024,     4, 0) },
> +  {L"m25p20",            INFO(0x202012, 0x0, 64 * 1024,     4, 0) },
> +  {L"m25p40",            INFO(0x202013, 0x0, 64 * 1024,     8, 0) },
> +  {L"m25p80",            INFO(0x202014, 0x0, 64 * 1024,    16, 0) },
> +  {L"m25p16",            INFO(0x202015, 0x0, 64 * 1024,    32, 0) },
> +  {L"m25pE16",           INFO(0x208015, 0x1000, 64 * 1024, 32, 0) },
> +  {L"m25pX16",           INFO(0x207115, 0x1000, 64 * 1024, 32, RD_QUAD | RD_DUAL) },
> +  {L"m25p32",            INFO(0x202016, 0x0,  64 * 1024,    64, 0) },
> +  {L"m25p64",            INFO(0x202017, 0x0,  64 * 1024,   128, 0) },
> +  {L"m25p128",           INFO(0x202018, 0x0, 256 * 1024,    64, 0) },
> +  {L"m25pX64",           INFO(0x207117, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"n25q016a",          INFO(0x20bb15, 0x0,  64 * 1024,    32, SECT_4K) },
> +  {L"n25q32",            INFO(0x20ba16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q32a",           INFO(0x20bb16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q64",            INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q64a",           INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q128",           INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"n25q128a",          INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"n25q256",           INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q256a",          INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q512",           INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"n25q512a",          INFO(0x20bb20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"n25q1024",          INFO(0x20ba21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K | ADDR_CYC_4) },
> +  {L"n25q1024a",         INFO(0x20bb21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"mt25qu02g",         INFO(0x20bb22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"mt25ql02g",         INFO(0x20ba22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  /* SST */
> +  {L"sst25vf040b",       INFO(0xbf258d, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
> +  {L"sst25vf080b",       INFO(0xbf258e, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
> +  {L"sst25vf016b",       INFO(0xbf2541, 0x0,  64 * 1024,    32, SECT_4K | SST_WR) },
> +  {L"sst25vf032b",       INFO(0xbf254a, 0x0,  64 * 1024,    64, SECT_4K | SST_WR) },
> +  {L"sst25vf064c",       INFO(0xbf254b, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"sst25wf512",        INFO(0xbf2501, 0x0,  64 * 1024,     1, SECT_4K | SST_WR) },
> +  {L"sst25wf010",        INFO(0xbf2502, 0x0,  64 * 1024,     2, SECT_4K | SST_WR) },
> +  {L"sst25wf020",        INFO(0xbf2503, 0x0,  64 * 1024,     4, SECT_4K | SST_WR) },
> +  {L"sst25wf040",        INFO(0xbf2504, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
> +  {L"sst25wf040b",       INFO(0x621613, 0x0,  64 * 1024,     8, SECT_4K) },
> +  {L"sst25wf080",        INFO(0xbf2505, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
> +  /* WINBOND */
> +  {L"w25p80",            INFO(0xef2014, 0x0,  64 * 1024,    16, 0) },
> +  {L"w25p16",            INFO(0xef2015, 0x0,  64 * 1024,    32, 0) },
> +  {L"w25p32",            INFO(0xef2016, 0x0,  64 * 1024,    64, 0) },
> +  {L"w25x40",            INFO(0xef3013, 0x0,  64 * 1024,     8, SECT_4K) },
> +  {L"w25x16",            INFO(0xef3015, 0x0,  64 * 1024,    32, SECT_4K) },
> +  {L"w25x32",            INFO(0xef3016, 0x0,  64 * 1024,    64, SECT_4K) },
> +  {L"w25x64",            INFO(0xef3017, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"w25q80bl",          INFO(0xef4014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q16cl",          INFO(0xef4015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q32bv",          INFO(0xef4016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q64cv",          INFO(0xef4017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q128bv",         INFO(0xef4018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q256",           INFO(0xef4019, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q80bw",          INFO(0xef5014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q16dw",          INFO(0xef6015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q32dw",          INFO(0xef6016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q64dw",          INFO(0xef6017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q128fw",         INFO(0xef6018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
> +  {},  /* Empty entry to terminate the list */
> +};
> +
>  STATIC
>  VOID
>  SpiFlashFormatAddress (
> @@ -104,13 +234,13 @@ MvSpiFlashWriteCommon (
>    UINT8 CmdStatus = CMD_READ_STATUS;
>    UINT8 State;
>    UINT32 Counter = 0xFFFFF;
> -  UINT8 poll_bit = STATUS_REG_POLL_WIP;
> -  UINT8 check_status = 0x0;
> +  UINT8 PollBit = STATUS_REG_POLL_WIP;
> +  UINT8 CheckStatus = 0x0;
>  
> -  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
> -  if (CmdStatus == CMD_FLAG_STATUS) {
> -    poll_bit = STATUS_REG_POLL_PEC;
> -    check_status = poll_bit;
> +  if (Slave->Info->Flags & E_FSR) {
> +    CmdStatus = CMD_FLAG_STATUS;
> +    PollBit = STATUS_REG_POLL_PEC;
> +    CheckStatus = STATUS_REG_POLL_PEC;
>    }
>  
>    // Send command
> @@ -127,7 +257,7 @@ MvSpiFlashWriteCommon (
>      SpiMasterProtocol->Transfer (SpiMasterProtocol, Slave, 1, NULL, &State,
>        0);
>      Counter--;
> -    if ((State & poll_bit) == check_status)
> +    if ((State & PollBit) == CheckStatus)
>        break;
>    } while (Counter > 0);
>    if (Counter == 0) {
> @@ -181,8 +311,19 @@ MvSpiFlashErase (
>    UINTN EraseSize;
>    UINT8 Cmd[5];
>  
> -  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
> +
> +  if (Slave->Info->Flags & SECT_4K) {
> +    Cmd[0] = CMD_ERASE_4K;
> +    EraseSize = SPI_ERASE_SIZE_4K;
> +  } else {
> +    Cmd[0] = CMD_ERASE_64K;
> +    EraseSize = Slave->Info->SectorSize;
> +  }
>  
>    // Check input parameters
>    if (Offset % EraseSize || Length % EraseSize) {
> @@ -191,21 +332,6 @@ MvSpiFlashErase (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  switch (EraseSize) {
> -  case SPI_ERASE_SIZE_4K:
> -    Cmd[0] = CMD_ERASE_4K;
> -    break;
> -  case SPI_ERASE_SIZE_32K:
> -    Cmd[0] = CMD_ERASE_32K;
> -    break;
> -  case SPI_ERASE_SIZE_64K:
> -    Cmd[0] = CMD_ERASE_64K;
> -    break;
> -  default:
> -    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
>    while (Length) {
>      EraseAddr = Offset;
>  
> @@ -239,7 +365,11 @@ MvSpiFlashRead (
>    UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>    UINTN BankSel = 0;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
>  
>    Cmd[0] = CMD_READ_ARRAY_FAST;
>  
> @@ -282,8 +412,13 @@ MvSpiFlashWrite (
>    UINT32 WriteAddr;
>    UINT8 Cmd[5], AddrSize;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> -  PageSize = PcdGet32 (PcdSpiFlashPageSize);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
> +
> +  PageSize = Slave->Info->PageSize;
>  
>    Cmd[0] = CMD_PAGE_PROGRAM;
>  
> @@ -370,7 +505,7 @@ MvSpiFlashUpdate (
>    UINT64 SectorSize, ToUpdate, Scale = 1;
>    UINT8 *TmpBuf, *End;
>  
> -  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
> +  SectorSize = Slave->Info->SectorSize;
>  
>    End = Buf + ByteCount;
>  
> @@ -400,38 +535,66 @@ MvSpiFlashUpdate (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC
> +VOID
> +MvPrintFlashInfo (
> +  IN     SPI_FLASH_INFO *Info
> +  )
> +{
> +  UINTN EraseSize;
> +
> +  if (Info->Flags & SECT_4K) {
> +    EraseSize = SPI_ERASE_SIZE_4K;
> +  } else {
> +    EraseSize = Info->SectorSize;
> +  }
> +
> +  DEBUG ((DEBUG_ERROR,
> +    "Detected %s SPI flash with page size %d B, erase size %d KB, total %d MB\n",
> +    Info->Name,
> +    Info->PageSize,
> +    EraseSize / 1024,
> +    (Info->SectorSize * Info->SectorCount) / 1024 / 1024));
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  MvSpiFlashReadId (
> -  IN     SPI_DEVICE *SpiDev,
> -  IN     UINT32     DataByteCount,
> -  IN OUT UINT8      *Buffer
> +  IN     SPI_DEVICE *SpiDev
>    )
>  {
> +  SPI_FLASH_INFO *Info;
>    EFI_STATUS Status;
> -  UINT8 *DataOut;
> -
> -  DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);
> -  if (DataOut == NULL) {
> -    DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -  Status = SpiMasterProtocol->Transfer (SpiMasterProtocol, SpiDev,
> -    DataByteCount, Buffer, DataOut, SPI_TRANSFER_BEGIN | SPI_TRANSFER_END);
> -  if (EFI_ERROR(Status)) {
> -    FreePool (DataOut);
> -    DEBUG((DEBUG_ERROR, "SpiFlash: Spi transfer error\n"));
> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
> +  UINT8 Cmd;
> +
> +  Cmd = CMD_READ_ID;
> +  Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,
> +                                SpiDev,
> +                                &Cmd,
> +                                SPI_CMD_LEN,
> +                                NULL,
> +                                Id,
> +                                SPI_FLASH_MAX_ID_LEN);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));
>      return Status;
>    }
>  
> -  // Bytes 1,2 and 3 contain SPI flash ID
> -  Buffer[0] = DataOut[1];
> -  Buffer[1] = DataOut[2];
> -  Buffer[2] = DataOut[3];
> +  Info = SpiFlashIds;
> +  for (; Info->Name != NULL; Info++) {
> +    if (Info->IdLen != 0) {
> +      if (CompareMem (Info->Id, Id, Info->IdLen) == 0) {
> +        SpiDev->Info = Info;
> +        MvPrintFlashInfo (Info);
> +        return EFI_SUCCESS;
> +      }
> +    }
> +  }
>  
> -  FreePool (DataOut);
> +  DEBUG ((DEBUG_ERROR, "ReadId: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n", Id[0], Id[1], Id[2]));
>  
> -  return EFI_SUCCESS;
> +  return EFI_NOT_FOUND;
>  }
>  
>  EFI_STATUS
> @@ -445,7 +608,11 @@ MvSpiFlashInit (
>    UINT8 Cmd, StatusRegister;
>    UINT32 AddrSize;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
>  
>    if (AddrSize == 4) {
>      // Set 4 byte address mode
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> index 646598a..b876966 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> @@ -62,6 +62,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define CMD_ERASE_64K                   0xd8
>  #define CMD_4B_ADDR_ENABLE              0xb7
>  
> +#define SPI_CMD_LEN                     1
> +
>  #define STATUS_REG_POLL_WIP             (1 << 0)
>  #define STATUS_REG_POLL_PEC             (1 << 7)
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> index 4519b02..f82c6e0 100644
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> @@ -53,13 +53,6 @@
>    DebugLib
>    MemoryAllocationLib
>  
> -[FixedPcd]
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> -  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> -
>  [Protocols]
>    gMarvellSpiMasterProtocolGuid
>    gMarvellSpiFlashProtocolGuid
> diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
> index ae78a31..73f0d80 100644
> --- a/Platform/Marvell/Include/Protocol/Spi.h
> +++ b/Platform/Marvell/Include/Protocol/Spi.h
> @@ -38,6 +38,42 @@ extern EFI_GUID gMarvellSpiMasterProtocolGuid;
>  
>  typedef struct _MARVELL_SPI_MASTER_PROTOCOL MARVELL_SPI_MASTER_PROTOCOL;
>  
> +#define SPI_FLASH_MAX_ID_LEN    6
> +
> +typedef struct {
> +  /* Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) */
> +  UINT16 *Name;
> +
> +  /*
> +   * This array stores the ID bytes.
> +   * The first three bytes are the JEDIC ID.
> +   * JEDEC ID zero means "no ID" (mostly older chips).
> +   */
> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
> +  UINT8 IdLen;
> +
> +  /*
> +   * The size listed here is what works with SPINOR_OP_SE, which isn't
> +   * necessarily called a "sector" by the vendor.
> +   */
> +  UINT32 SectorSize;
> +  UINT32 SectorCount;
> +
> +  UINT16 PageSize;
> +
> +  UINT16 Flags;
> +#define SECT_4K      1 << 0  /* CMD_ERASE_4K works uniformly */
> +#define E_FSR        1 << 1  /* use flag status register for */
> +#define SST_WR       1 << 2  /* use SST byte/word programming */
> +#define WR_QPP       1 << 3  /* use Quad Page Program */
> +#define RD_QUAD      1 << 4  /* use Quad Read */
> +#define RD_DUAL      1 << 5  /* use Dual Read */
> +#define RD_QUADIO    1 << 6  /* use Quad IO Read */
> +#define RD_DUALIO    1 << 7  /* use Dual IO Read */
> +#define RD_FULL      (RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
> +#define ADDR_CYC_4   1 << 8  /* use 4 byte addressing format */

Oh, and parentheses around all of these defines.

/
    Leif

> +} SPI_FLASH_INFO;
> +
>  typedef enum {
>    SPI_MODE0, // CPOL = 0 & CPHA = 0
>    SPI_MODE1, // CPOL = 0 & CPHA = 1
> @@ -49,6 +85,7 @@ typedef struct {
>    INTN Cs;
>    INTN MaxFreq;
>    SPI_MODE Mode;
> +  SPI_FLASH_INFO *Info;
>  } SPI_DEVICE;
>  
>  typedef
> diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
> index 743bb87..e12f55b 100644
> --- a/Platform/Marvell/Include/Protocol/SpiFlash.h
> +++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
> @@ -61,9 +61,7 @@ EFI_STATUS
>  typedef
>  EFI_STATUS
>  (EFIAPI *MV_SPI_FLASH_READ_ID) (
> -  IN     SPI_DEVICE *SpiDev,
> -  IN     UINT32     DataByteCount,
> -  IN OUT UINT8      *Buffer
> +  IN     SPI_DEVICE *SpiDev
>    );
>  
>  typedef
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index fc00f1a..418d960 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -123,12 +123,6 @@
>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052
>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053
>  
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0|UINT32|0x3000052
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
> -  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
> -  gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
>  
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 11/11] Drivers/Spi/Devices/MvSpiFlash: Fix bank selection for Spansion

[Leif Lindholm]

On Fri, Sep 01, 2017 at 03:08:23PM +0200, Marcin Wojtas wrote:
> Spansion SPI flash devices use different command for bank
> selection. Update it, basing on the first byte of flash ID.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 5 +++++
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index bfb8fa3..4c15513 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -280,6 +280,11 @@ SpiFlashCmdBankaddrWrite (
>  {
>    UINT8 Cmd = CMD_BANK_WRITE;
>  
> +  /* Update bank selection command for Spansion */
> +  if (Slave->Info->Id[0] == 0x01) {

Need a #define for that 0x01.

/
    Leif

> +    Cmd = CMD_BANKADDR_BRWR;
> +  }
> +
>    MvSpiFlashWriteCommon (Slave, &Cmd, 1, &BankSel, 1);
>  }
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> index b876966..d44c56f 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> @@ -57,6 +57,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define CMD_READ_ARRAY_FAST             0x0b
>  #define CMD_PAGE_PROGRAM                0x02
>  #define CMD_BANK_WRITE                  0xc5
> +#define CMD_BANKADDR_BRWR               0x17
>  #define CMD_ERASE_4K                    0x20
>  #define CMD_ERASE_32K                   0x52
>  #define CMD_ERASE_64K                   0xd8
> -- 
> 1.8.3.1
> 

Re: [platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide

[Leif Lindholm]

On Fri, Sep 01, 2017 at 05:08:39PM +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> 2017-09-01 16:36 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Fri, Sep 01, 2017 at 03:08:13PM +0200, Marcin Wojtas wrote:
> >> From: Nir Erez <nerez@marvell.com>
> >>
> >> This patch introduces following improvements to the PortingGuide
> >> * Replace split documentation with single file
> >> * Align format to Doxygen constraints
> >
> > * Add Build description.
> > (Comment on that below.)
> >
> > Since you are moving things around, I take this as license to
> > bikeshed:
> >
> > I think we should start looking towards separating the development
> > board and SoC portions, like is done for most other platforms. After
> > all, the goal is to get additional platforms (not just the EVB).
> >
> > Not saying this is something that needs to happen overnight, but it
> > would make sense to make a start by moving these docs to
> > Silicon/Marvell/Documentation as part of this patch.
> 
> I can add it there, no problem. About Silicon vs Platform - I checked
> the code and basically we'd have to move everything from
> Platform/Marvell to Silicon/Marvell and leave just subdirectories with
> board files (dsc, fdf + dtb):
> Platform/Marvell/Armada70x0Db
> Platform/Marvell/Armada80x0Db
> Platform/Marvell/Armada80x0McBin
> etc.
> Is it what you mean?

Yeah, exactly. (Although depending on point if view, maybe
Marvell/Armada80x0McBin should be SolidRun/MacchiatoBin.)
I'm hoping there could be a lot of sharing of common bits of files in
generic .dsc.inc/.fdf.inc - both here and under Silicon/Marvell.

> I'm wondering when will be good moment for this - I'm rebasing 70 OPP
> patches slowly in my extra time, so maybe after the SPI patchset? Or
> better after we merge everything to Platform and then do the code
> shifting? What is your feeling about it?

Well, if I was going to take a hard line on this, that was an option I
had during the initial import to OpenPlatformPkg. Since I didn't, this
is very much a "I would be happy to see this happen" kind of thing.

So it's sort of up to you.

But I'll throw in that I am planning to go into hiding for a couple of
days next week to try and once and for all create some generic config
fragments that can be reused across all platforms instead of copying
the same bits along everywhere. So it would make sense to wait with
any larger interventions until that work is complete.

/
    Leif

> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Platform/Marvell/Documentation/Build.txt           |  58 ++++
> >>  Platform/Marvell/Documentation/PortingGuide.txt    | 371 +++++++++++++++++++++
> >>  .../Marvell/Documentation/PortingGuide/ComPhy.txt  |  45 ---
> >>  .../Marvell/Documentation/PortingGuide/I2c.txt     |  20 --
> >>  .../Marvell/Documentation/PortingGuide/Mdio.txt    |   7 -
> >>  .../Marvell/Documentation/PortingGuide/Mpp.txt     |  48 ---
> >>  .../Documentation/PortingGuide/PciEmulation.txt    |  31 --
> >>  .../Marvell/Documentation/PortingGuide/Phy.txt     |  45 ---
> >>  .../Marvell/Documentation/PortingGuide/Pp2.txt     |  35 --
> >>  .../Marvell/Documentation/PortingGuide/Reset.txt   |   7 -
> >>  .../Marvell/Documentation/PortingGuide/Spi.txt     |  16 -
> >>  .../Documentation/PortingGuide/SpiFlash.txt        |  23 --
> >>  .../Marvell/Documentation/PortingGuide/Utmi.txt    |  35 --
> >
> > Also, it would be nice if you could generate patches with
> >   --stat=1000 --stat-graph-width=20
> > as documented in Laszlo's unkempt guide:
> > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
> > Makes the above summare _much_ easier to read.
> 
> No problem, will do.
> 
> >
> >>  13 files changed, 429 insertions(+), 312 deletions(-)
> >>  create mode 100644 Platform/Marvell/Documentation/Build.txt
> >>  create mode 100644 Platform/Marvell/Documentation/PortingGuide.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
> >>  delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt
> >>
> >> diff --git a/Platform/Marvell/Documentation/Build.txt b/Platform/Marvell/Documentation/Build.txt
> >> new file mode 100644
> >> index 0000000..1162e2e
> >> --- /dev/null
> >> +++ b/Platform/Marvell/Documentation/Build.txt
> >> @@ -0,0 +1,58 @@
> >> +UEFI Build Instructions
> >> +=======================
> >> +
> >> +For toolchain versions limitations please refer to edk2 wiki page:
> >> +https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC
> >> +
> >> +Fully supported are gcc4.5 - gcc4.9, so possible {toolchain_name} are:
> >> +     - GCC45
> >> +     - GCC46
> >> +     - GCC47
> >> +     - GCC48
> >> +     - GCC49
> >> +     - GCC5
> >> +
> >> +Supported {platform} are:
> >> +     - Armada70x0
> >> +
> >> +Supported {target} are
> >> +     - DEBUG
> >> +     - RELEASE
> >> +
> >> +Build procedure
> >> +---------------
> >> +1. Prerequisites:
> >> +
> >> +     Clone into edk2 repositories and apply Marvell patches (Please refer to
> >> +     Release notes for instructions).
> >> +
> >> +2. Prepare environment:
> >> +
> >> +     2.1 Several packages will be needed to fully set up an edk2 build environment:
> >> +
> >> +                     # sudo apt-get install build-essential uuid-dev
> >> +                     # sudo apt-get install lib32stdc++6 lib32z1
> >> +
> >> +     2.2 Set up EDK2 environment
> >> +
> >> +                     # source edksetup.sh
> >> +
> >> +     2.3 Build base tools
> >> +
> >> +                     # make -C BaseTools
> >> +
> >> +     2.4 Set {toolchain_name}_AARCH64_PREFIX to path to your cross compiler
> >> +
> >> +                     # export {toolchain_name}_AARCH64_PREFIX=/path/to/toolchain
> >> +
> >> +             Example:
> >> +             --------
> >> +                     # export GCC5_AARCH64_PREFIX=/opt/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-
> >> +
> >> +3. Build EDK2 for selected {platform}:
> >> +
> >> +                     # build -a AARCH64 -t {toolchain_name} -b {target} -p OpenPlatformPkg/Platforms/Marvell/Armada/{platform}.dsc
> >> +
> >> +     Example for building edk2 for Armada70x0 platform with GCC5 for DEBUG:
> >> +
> >> +                     # build -a AARCH64 -t GCC5 -b DEBUG -p OpenPlatformPkg/Platforms/Marvell/Armada/Armada70x0.dsc
> >
> > The above all documents OpenPlatformPkg, so is now dated.
> > My preference would be for this to become a Readme.md under
> > Platform/Marvell, if you feel additional documentation to the
> > top-level Readme.md to be necessary.
> 
> Right. How about only some minimal Readme.md as you suggest with the
> list of available Marvell platforms and pointer to the main Readme
> file?
> 
> >
> > Regardless, it is unrelated to the rest of changes here, so would make
> > sense as a separate patch.
> 
> Ok.
> 
> >
> >> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> >> new file mode 100644
> >> index 0000000..8c3579e
> >> --- /dev/null
> >> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> >> @@ -0,0 +1,371 @@
> >> +UEFI Porting Guide
> >> +==================
> >> +
> >> +This document provides instructions for adding support for new Marvell Armada
> >> +board. For the sake of simplicity new Marvell board will be called "new_board".
> >> +
> >> +1. Create configuration files for new target
> >> +     1.1 Create FDF file for new board
> >> +
> >> +      - Copy and rename PathToYourOpp/Platforms/Marvell/Armada/Armada70x0.fdf to
> >> +        PathToYourOpp/Platforms/Marvell/Armada/new_board.fdf
> >
> > OpenPlatformPkg.
> 
> Sure, I will update all of them.
> 
> >
> >> +      - Change the first no-comment line:
> >> +        [FD.Armada70x0_EFI] to [FD.{new_board}_EFI]
> >> +
> >> +     1.2 Create DSC file for new board
> >> +
> >> +      - Add new_board.dsc file to PathToYourOpp/Platforms/Marvell/Armada directory
> >
> > OpenPlatformPkg.
> >
> >> +      - Insert following [Defines] section to new_board.dsc:
> >> +
> >> +                     [Defines]
> >> +                       PLATFORM_NAME                  = {new_board}
> >> +                       PLATFORM_GUID                  = {newly_generated_GUID}
> >> +                       PLATFORM_VERSION               = 0.1
> >> +                       DSC_SPECIFICATION              = 0x00010019
> >> +                       OUTPUT_DIRECTORY               = {output_directory}
> >> +                       SUPPORTED_ARCHITECTURES        = AARCH64
> >> +                       BUILD_TARGETS                  = DEBUG|RELEASE
> >> +                       SKUID_IDENTIFIER               = DEFAULT
> >> +                       FLASH_DEFINITION               = {path_to_fdf_file}
> >> +
> >> +      - Add "!include Armada.dsc.inc" entry to new_board.dsc
> >> +
> >> +2. Driver support
> >> + - According to content of files from PathToYourOpp/Documentation/Marvell/PortingGuide
> >
> > OpenPlatformPkg.
> >
> >> +   insert PCD entries into new_board.dsc for every needed interface (as listed below).
> >> +
> >> +3. Compilation
> >> + - Refer to PathToYourOpp/Documentation/Marvell/Build.txt. Remember to change
> >
> > OpenPlatformPkg. (Also referencing the Build.txt.)
> >
> >> +   {platform} to new_board in order to point build system to newly created DSC file.
> >> +
> >> +4. Output file
> >> + - Output files (and among others FD file, which may be used by ATF) are
> >> +   generated under directory pointed by "OUTPUT_DIRECTORY" entry (see point 1.2).
> >> +
> >> +
> >> +COMPHY configuration
> >> +====================
> >> +In order to configure ComPhy library, following PCDs are available:
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdComPhyDevices
> >> +
> >> +This array indicates, which ones of the ComPhy chips defined in
> >> +MVHW_COMPHY_DESC template will be configured.
> >> +
> >> +Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
> >> +important, but configuration will be set for first PcdComPhyChipCount chips).
> >> +
> >> +Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
> >> +settings for this chip. Their format is unicode string, containing settings
> >> +for up to 10 lanes. Setting for each one is separated with semicolon.
> >> +These PCDs together describe outputs of PHY integrated in simple cihp.
> >> +Below is example for the first chip (Chip0).
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
> >> +     (Unicode string indicating PHY types. Currently supported are:
> >> +
> >> +             { L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
> >> +               L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
> >> +               L"SGMII1", L"SGMII2", L"SGMII3", L"QSGMII",
> >> +               L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
> >> +               L"XAUI0", L"XAUI1", L"XAUI2", L"XAUI3", L"RXAUI0",
> >> +               L"RXAUI1", L"KR" } )
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
> >> +     (Indicates PHY speeds in MHz. Currently supported are:
> >> +             { 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 1031 }  )
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
> >> +     (Indicates lane polarity invert)
> >> +
> >> +Example
> >> +-------
> >> +
> >> +               #ComPhy
> >> +               gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
> >> +               gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
> >> +               gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
> >> +
> >> +
> >> +PHY Driver configuration
> >> +========================
> >> +MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
> >> +Currently only 1518 series PHYs are supported. Following PCDs are required:
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
> >> +     (list of values corresponding to PHY_CONNECTION enum)
> >> +  - gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
> >> +     (boolean - if true, driver waits for autonegotiation on startup)
> >> +  - gMarvellTokenSpaceGuid.PcdPhyDeviceIds
> >> +     (list of values corresponding to MV_PHY_DEVICE_ID enum)
> >> +
> >> +PHY_CONNECTION enum type is defined as follows:
> >> +
> >> +             typedef enum {
> >> +                     0    PHY_CONNECTION_RGMII,
> >> +                     1    PHY_CONNECTION_RGMII_ID,
> >> +                     2    PHY_CONNECTION_RGMII_TXID,
> >> +                     3    PHY_CONNECTION_RGMII_RXID,
> >> +                     4    PHY_CONNECTION_SGMII,
> >> +                     5    PHY_CONNECTION_RTBI,
> >> +                     6    PHY_CONNECTION_XAUI,
> >> +                     7    PHY_CONNECTION_RXAUI
> >> +             } PHY_CONNECTION;
> >> +
> >> +MV_PHY_DEVICE_ID:
> >> +
> >> +             typedef enum {
> >> +                     0    MV_PHY_DEVICE_1512,
> >> +             } MV_PHY_DEVICE_ID;
> >> +
> >> +It should be extended when adding support for other PHY models.
> >> +Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
> >> +
> >> + gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
> >> +
> >> +with disabled autonegotiation:
> >> +
> >> + gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
> >> +
> >> +assuming, that PHY models are 1512:
> >> +
> >> + gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
> >> +
> >> +
> >> +MDIO configuration
> >> +==================
> >> +MDIO driver provides access to network PHYs' registers via EFI_MDIO_READ and
> >> +EFI_MDIO_WRITE functions (EFI_MDIO_PROTOCOL). Following PCD is required:
> >> +
> >> +  - gMarvellTokenSpaceGuid.PcdMdioBaseAddress
> >> +     (base address of SMI management register)
> >> +
> >> +
> >> +I2C configuration
> >> +=================
> >> +In order to enable driver on a new platform, following steps need to be taken:
> >> + - add following line to .dsc file:
> >> +   OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> >> + - add following line to .fdf file:
> >> +   INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> >> + - add PCDs with relevant values to .dsc file:
> >> +     - gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57 }
> >> +             (addresses of I2C slave devices on bus)
> >> +     - gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0 }
> >> +             (buses to which accoring slaves are attached)
> >> +     - gMarvellTokenSpaceGuid.PcdI2cBusCount|2
> >> +             (number of SoC's I2C buses)
> >> +     - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
> >> +             (base addresses of I2C controller buses)
> >> +     - gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
> >> +             (I2C host controller clock frequency)
> >> +     - gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
> >> +             (baud rate used in I2C transmission)
> >> +
> >> +
> >> +PciEmulation configuration
> >> +==========================
> >> +Installation of various NonDiscoverable devices via PciEmulation driver is performed
> >
> > I think calling this PciEmulation is no longer accurate.
> 
> Our driver in the tree is called this way, what do you suggest instead?
> 
> Thanks,
> Marcin

Re: [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues

[Leif Lindholm]

On Fri, Sep 01, 2017 at 04:16:24PM +0100, Ard Biesheuvel wrote:
> On 1 September 2017 at 15:54, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> Fix casting and related issues to make this code build for 32-bit ARM.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> index edb6986..0951734 100644
> >> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
> >>    EFI_STATUS           Status;
> >>    UINT64               OpenMode;
> >>    UINTN                *Buffer;
> >> +  UINT64               Size;
> >>
> >>    // Parse string from commandline
> >>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
> >> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
> >>        return EFI_DEVICE_ERROR;
> >>      }
> >>
> >> -  Status = FileHandleGetSize (*FileHandle, FileSize);
> >> +  Status = FileHandleGetSize (*FileHandle, &Size);
> >>      if (EFI_ERROR (Status)) {
> >>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
> >>      }
> >>
> >> +  *FileSize = (UINTN)Size;
> >> +
> >
> > Rather than juggling around with temporary variables, why not make
> > FileSize in ShellCommandRunFUpdate() UINT64 and update
> > PrepareFirmwareImage() prototype accordingly?
> 
> I don't remember /exactly/, but I think it breaks in another place then.

The only thing I see that _could_ be affected is
    Status = SpiFlashProtocol->Update (Slave, 0, FileSize, (UINT8 *)FileBuffer);

And I would prefer a (UINTN) cast there over temporary variable
shuffling in a subfunction.

/
    Leif

Re: [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter

[Leif Lindholm]

On Fri, Sep 01, 2017 at 05:25:00PM +0200, Marcin Wojtas wrote:
> 2017-09-01 17:21 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Fri, Sep 01, 2017 at 03:08:21PM +0200, Marcin Wojtas wrote:
> >> Although, hitherto support allowed for using configurable EraseSize,
> >> the erase command was fixed to CMD_ERASE_64K. Also it was
> >> assumed that EraseSize equals SectorSize, which is not true
> >> for some flash devices.
> >
> > Another thing I immediately wish I had never learned.
> > Thanks :/
> 
> Do you mean the code or flash quirks? :)

The flash quirks...

/
    Leif

> >> Fix both issues by adding new PCD
> >> (gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
> >> this parameter properly in MvSpiFlashUpdate routine instead
> >> of the EraseSize. Also erase command is adjusted to the settings.
> >> Update PortingGuide accordingly.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Platform/Marvell/Documentation/PortingGuide.txt    |  3 +++
> >>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
> >>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
> >>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
> >>  Platform/Marvell/Marvell.dec                       |  1 +
> >>  5 files changed, 31 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> >> index 3b79bd2..f637fee 100644
> >> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> >> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> >> @@ -297,6 +297,9 @@ Folowing PCDs for spi flash driver configuration must be set properly:
> >>    - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
> >>       (Size of SPI flash page)
> >>
> >> +  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> >> +     (Size of SPI flash sector, 65536 bytes by default)
> >> +
> >>    - gMarvellTokenSpaceGuid.PcdSpiFlashId
> >>       (Id of SPI flash)
> >>
> >> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> >> index 9a04493..f3fdba4 100755
> >> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> >> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> >> @@ -191,7 +191,21 @@ MvSpiFlashErase (
> >>      return EFI_DEVICE_ERROR;
> >>    }
> >>
> >> -  Cmd[0] = CMD_ERASE_64K;
> >> +  switch (EraseSize) {
> >> +  case SPI_ERASE_SIZE_4K:
> >> +    Cmd[0] = CMD_ERASE_4K;
> >> +    break;
> >> +  case SPI_ERASE_SIZE_32K:
> >> +    Cmd[0] = CMD_ERASE_32K;
> >> +    break;
> >> +  case SPI_ERASE_SIZE_64K:
> >> +    Cmd[0] = CMD_ERASE_64K;
> >> +    break;
> >> +  default:
> >> +    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >>    while (Length) {
> >>      EraseAddr = Offset;
> >>
> >> @@ -353,14 +367,14 @@ MvSpiFlashUpdate (
> >>    )
> >>  {
> >>    EFI_STATUS Status;
> >> -  UINT64 EraseSize, ToUpdate, Scale = 1;
> >> +  UINT64 SectorSize, ToUpdate, Scale = 1;
> >>    UINT8 *TmpBuf, *End;
> >>
> >> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> >> +  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
> >>
> >>    End = Buf + ByteCount;
> >>
> >> -  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
> >> +  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
> >>    if (TmpBuf == NULL) {
> >>      DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
> >>      return EFI_OUT_OF_RESOURCES;
> >> @@ -370,9 +384,9 @@ MvSpiFlashUpdate (
> >>      Scale = (End - Buf) / 100;
> >>
> >>    for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
> >> -    ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
> >> +    ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
> >>      Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
> >> -    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize);
> >> +    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize);
> >>
> >>      if (EFI_ERROR (Status)) {
> >>        DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
> >> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> >> index 3889643..646598a 100755
> >> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> >> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> >> @@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >>  #define CMD_READ_ARRAY_FAST             0x0b
> >>  #define CMD_PAGE_PROGRAM                0x02
> >>  #define CMD_BANK_WRITE                  0xc5
> >> +#define CMD_ERASE_4K                    0x20
> >> +#define CMD_ERASE_32K                   0x52
> >>  #define CMD_ERASE_64K                   0xd8
> >>  #define CMD_4B_ADDR_ENABLE              0xb7
> >>
> >> @@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >>  #define SPI_TRANSFER_BEGIN              0x01  // Assert CS before transfer
> >>  #define SPI_TRANSFER_END                0x02  // Deassert CS after transfers
> >>
> >> +#define SPI_ERASE_SIZE_4K               4096
> >> +#define SPI_ERASE_SIZE_32K              32768
> >> +#define SPI_ERASE_SIZE_64K              65536
> >> +
> >
> > Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?
> >
> 
> Sure, will do.
> 
> Marcin

Re: [platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide

[Marcin Wojtas]

2017-09-01 17:45 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Sep 01, 2017 at 05:08:39PM +0200, Marcin Wojtas wrote:
>> Hi Leif,
>>
>> 2017-09-01 16:36 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> > On Fri, Sep 01, 2017 at 03:08:13PM +0200, Marcin Wojtas wrote:
>> >> From: Nir Erez <nerez@marvell.com>
>> >>
>> >> This patch introduces following improvements to the PortingGuide
>> >> * Replace split documentation with single file
>> >> * Align format to Doxygen constraints
>> >
>> > * Add Build description.
>> > (Comment on that below.)
>> >
>> > Since you are moving things around, I take this as license to
>> > bikeshed:
>> >
>> > I think we should start looking towards separating the development
>> > board and SoC portions, like is done for most other platforms. After
>> > all, the goal is to get additional platforms (not just the EVB).
>> >
>> > Not saying this is something that needs to happen overnight, but it
>> > would make sense to make a start by moving these docs to
>> > Silicon/Marvell/Documentation as part of this patch.
>>
>> I can add it there, no problem. About Silicon vs Platform - I checked
>> the code and basically we'd have to move everything from
>> Platform/Marvell to Silicon/Marvell and leave just subdirectories with
>> board files (dsc, fdf + dtb):
>> Platform/Marvell/Armada70x0Db
>> Platform/Marvell/Armada80x0Db
>> Platform/Marvell/Armada80x0McBin
>> etc.
>> Is it what you mean?
>
> Yeah, exactly. (Although depending on point if view, maybe
> Marvell/Armada80x0McBin should be SolidRun/MacchiatoBin.)

Personally, I'd keep all Marvell boards under Platform/Marvell, so
that not to make a mess, but we can discuss it later of course.

> I'm hoping there could be a lot of sharing of common bits of files in
> generic .dsc.inc/.fdf.inc - both here and under Silicon/Marvell.

On our working branch, there is single dsc.inc and .fdf for all boards
and I think such shareability can be maintained further.

>
>> I'm wondering when will be good moment for this - I'm rebasing 70 OPP
>> patches slowly in my extra time, so maybe after the SPI patchset? Or
>> better after we merge everything to Platform and then do the code
>> shifting? What is your feeling about it?
>
> Well, if I was going to take a hard line on this, that was an option I
> had during the initial import to OpenPlatformPkg. Since I didn't, this
> is very much a "I would be happy to see this happen" kind of thing.
>
> So it's sort of up to you.
>
> But I'll throw in that I am planning to go into hiding for a couple of
> days next week to try and once and for all create some generic config
> fragments that can be reused across all platforms instead of copying
> the same bits along everywhere. So it would make sense to wait with
> any larger interventions until that work is complete.
>

Ok. I have a lot of stuff queued, before adding the new A8040 boards,
so I can proceed with fixes/enhancements, that won't be problematic to
halt in the middle and do the shift.

Thanks,
Marcin

Re: [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection

[Marcin Wojtas]

2017-09-01 17:33 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Sep 01, 2017 at 03:08:22PM +0200, Marcin Wojtas wrote:
>> Hitherto mechanism of fixing SPI flash model in the PCDs,
>> occured to be very inefficient and problematic. Enable
>> dynamic detection by reworking MvSpiFlashReadId() command,
>> which now reads the Id and goes through newly added table
>> with JEDEC compliant devices and their description.
>>
>> On the occasion fix the ReadId process by using master's
>> ReadWrite routine instead of pure Transfer - no longer
>> swapping and byte shifting is needed. Simplify code by
>> using local array instead of dynamic allocation. Also
>> reduced number of ReadId arguments allowed for cleaning
>> Fupdate and Sf shell commands probe routines.
>>
>> Additionally, use SpiFlashInfo fields instead of PCDs,
>> and configure needed settings in MvSpiFlashInit.
>>
>> Update PortingGuide documentation accordingly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> So, this looks _really_ good, but I'm seeing three separate patches in
> here really:
> - Style fixups (yay). I love them, but squashing them in make the
>   functional changes impossible to spot in the noise.
> - SPI Flash Id table. Love it!
>   But this is a completely generic feature. Medium-term, this should
>   be living in EDK2. For now, can you break it out into a separate
>   library that can be used by other platforms (and will be easier to
>   move)?
> - Functional improvements to existing driver - great! (but can't
>   really review before the above two have been broken out).
>

I'll check and do the style fixup/functional improvement split.

About flash ids in a separate library - we already have
SpiFlashProtocol->ReadId callback. In fact despite Mv prefix the
MvSpiFlash.c driver and its associated SPI_FLASH_PROTOCOL are
completely generic for now and SoC controller agnostic. It works in
very similar way as U-Boot driver (drivers/mtd/spi) and Linux one
(drivers/mtd/spi-nor/spi-nor.c). Due to lack of similar solution, how
about moving it to EDK2? Do you think there's chance to promote such
solution?

Marcin

> /
>     Leif
>
>> ---
>>  .../Marvell/Applications/FirmwareUpdate/FUpdate.c  |  23 +-
>>  .../Applications/FirmwareUpdate/FUpdate.inf        |   3 -
>>  .../Marvell/Applications/SpiTool/SpiFlashCmd.c     |  36 +--
>>  .../Marvell/Applications/SpiTool/SpiFlashCmd.inf   |   1 -
>>  Platform/Marvell/Armada/Armada70x0.dsc             |   5 -
>>  Platform/Marvell/Documentation/PortingGuide.txt    |  18 --
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 267 +++++++++++++++++----
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |   2 +
>>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |   7 -
>>  Platform/Marvell/Include/Protocol/Spi.h            |  37 +++
>>  Platform/Marvell/Include/Protocol/SpiFlash.h       |   4 +-
>>  Platform/Marvell/Marvell.dec                       |   6 -
>>  12 files changed, 271 insertions(+), 138 deletions(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index 0951734..b0e94bc 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -94,32 +94,17 @@ SpiFlashProbe (
>>    )
>>  {
>>    EFI_STATUS       Status;
>> -  UINT32           IdBuffer, Id, RefId;
>> -
>> -  Id = PcdGet32 (PcdSpiFlashId);
>> -
>> -  IdBuffer = CMD_READ_ID & 0xff;
>>
>>    // Read SPI flash ID
>> -  SpiFlashProtocol->ReadId (Slave, sizeof (UINT32), (UINT8 *)&IdBuffer);
>> -
>> -  // Swap and extract 3 bytes of the ID
>> -  RefId = SwapBytes32 (IdBuffer) >> 8;
>> -
>> -  if (RefId == 0) {
>> -    Print (L"%s: No SPI flash detected");
>> -    return EFI_DEVICE_ERROR;
>> -  } else if (RefId != Id) {
>> -    Print (L"%s: Unsupported SPI flash detected with ID=%2x\n", CMD_NAME_STRING, RefId);
>> -    return EFI_DEVICE_ERROR;
>> +  Status = SpiFlashProtocol->ReadId (Slave);
>> +  if (EFI_ERROR (Status)) {
>> +    return SHELL_ABORTED;
>>    }
>>
>> -  Print (L"%s: Detected supported SPI flash with ID=%3x\n", CMD_NAME_STRING, RefId);
>> -
>>    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
>>    if (EFI_ERROR(Status)) {
>>      Print (L"%s: Cannot initialize flash device\n", CMD_NAME_STRING);
>> -    return EFI_DEVICE_ERROR;
>> +    return SHELL_ABORTED;
>>    }
>>
>>    return EFI_SUCCESS;
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> index 92c3333..43cac42 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> @@ -64,9 +64,6 @@
>>    UefiLib
>>    UefiRuntimeServicesTableLib
>>
>> -[Pcd]
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>> -
>>  [Protocols]
>>   gMarvellSpiFlashProtocolGuid
>>   gMarvellSpiMasterProtocolGuid
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> index ee14270..b5db8b5 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> @@ -166,37 +166,21 @@ FlashProbe (
>>    )
>>  {
>>    EFI_STATUS Status;
>> -  UINT8  IdBuffer[4];
>> -  UINT32 Id, RefId;
>>
>> -  Id = PcdGet32 (PcdSpiFlashId);
>> -
>> -  IdBuffer[0] = CMD_READ_ID;
>> -
>> -  SpiFlashProtocol->ReadId (
>> -    Slave,
>> -    4,
>> -    IdBuffer
>> -    );
>> -
>> -  RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];
>> +  Status = SpiFlashProtocol->ReadId (Slave);
>> +  if (EFI_ERROR (Status)) {
>> +    return SHELL_ABORTED;
>> +  }
>>
>> -  if (RefId == Id) {
>> -    Print (L"sf: Detected supported SPI flash with ID=%3x\n", RefId);
>> -    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
>> -    if (EFI_ERROR(Status)) {
>> -      Print (L"sf: Cannot initialize flash device\n");
>> -      return SHELL_ABORTED;
>> -    }
>> -    InitFlag = 0;
>> -    return EFI_SUCCESS;
>> -  } else if (RefId != 0) {
>> -    Print (L"sf: Unsupported SPI flash detected with ID=%2x\n", RefId);
>> +  Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);
>> +  if (EFI_ERROR (Status)) {
>> +    Print (L"sf: Cannot initialize flash device\n");
>>      return SHELL_ABORTED;
>>    }
>>
>> -  Print (L"sf: No SPI flash detected");
>> -  return SHELL_ABORTED;
>> +  InitFlag = 0;
>> +
>> +  return SHELL_SUCCESS;
>>  }
>>
>>  SHELL_STATUS
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> index c1ab770..343d5b5 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> @@ -65,7 +65,6 @@
>>   FileHandleLib
>>
>>  [Pcd]
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>>   gMarvellTokenSpaceGuid.PcdSpiFlashCs
>>   gMarvellTokenSpaceGuid.PcdSpiFlashMode
>>
>> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
>> index df2ebdb..4633e32 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -93,11 +93,6 @@
>>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>>
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
>>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
>>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
>>
>> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
>> index f637fee..3ac35a0 100644
>> --- a/Platform/Marvell/Documentation/PortingGuide.txt
>> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
>> @@ -288,24 +288,6 @@ SpiFlash configuration
>>  ======================
>>  Folowing PCDs for spi flash driver configuration must be set properly:
>>
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
>> -     (Size of SPI flash address in bytes (3 or 4) )
>> -
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
>> -     (Size of minimal erase block in bytes)
>> -
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>> -     (Size of SPI flash page)
>> -
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
>> -     (Size of SPI flash sector, 65536 bytes by default)
>> -
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashId
>> -     (Id of SPI flash)
>> -
>> -  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
>> -     (Spi flash polling flag)
>> -
>>    - gMarvellTokenSpaceGuid.PcdSpiFlashMode
>>       (Default SCLK mode (see SPI_MODE enum in file OpenPlatformPkg/Drivers/Spi/MvSpi.h))
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> index f3fdba4..bfb8fa3 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> @@ -36,6 +36,136 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
>>  SPI_FLASH_INSTANCE  *mSpiFlashInstance;
>>
>> +#define INFO(JedecId, ExtId, SecSize, NSectors, FlashFlags)  \
>> +  .Id = {                       \
>> +    ((JedecId) >> 16) & 0xff,   \
>> +    ((JedecId) >> 8) & 0xff,    \
>> +    (JedecId) & 0xff,           \
>> +    ((ExtId) >> 8) & 0xff,      \
>> +    (ExtId) & 0xff,             \
>> +    },                          \
>> +  .IdLen = (!(JedecId) ? 0 : (3 + ((ExtId) ? 2 : 0))),  \
>> +  .SectorSize = (SecSize),      \
>> +  .SectorCount = (NSectors),    \
>> +  .PageSize = 256,              \
>> +  .Flags = (FlashFlags),
>> +
>> +static SPI_FLASH_INFO SpiFlashIds[] = {
>> +  /* ATMEL */
>> +  {L"at45db011d",        INFO(0x1f2200, 0x0, 64 * 1024,     4, SECT_4K) },
>> +  {L"at45db021d",        INFO(0x1f2300, 0x0, 64 * 1024,     8, SECT_4K) },
>> +  {L"at45db041d",        INFO(0x1f2400, 0x0, 64 * 1024,     8, SECT_4K) },
>> +  {L"at45db081d",        INFO(0x1f2500, 0x0, 64 * 1024,    16, SECT_4K) },
>> +  {L"at45db161d",        INFO(0x1f2600, 0x0, 64 * 1024,    32, SECT_4K) },
>> +  {L"at45db321d",        INFO(0x1f2700, 0x0, 64 * 1024,    64, SECT_4K) },
>> +  {L"at45db641d",        INFO(0x1f2800, 0x0, 64 * 1024,   128, SECT_4K) },
>> +  {L"at25df321a",        INFO(0x1f4701, 0x0, 64 * 1024,    64, SECT_4K) },
>> +  {L"at25df321",         INFO(0x1f4700, 0x0, 64 * 1024,    64, SECT_4K) },
>> +  {L"at26df081a",        INFO(0x1f4501, 0x0, 64 * 1024,    16, SECT_4K) },
>> +  /* EON */
>> +  {L"en25q32b",          INFO(0x1c3016, 0x0, 64 * 1024,    64, 0) },
>> +  {L"en25q64",           INFO(0x1c3017, 0x0, 64 * 1024,   128, SECT_4K) },
>> +  {L"en25q128b",         INFO(0x1c3018, 0x0, 64 * 1024,   256, 0) },
>> +  {L"en25s64",           INFO(0x1c3817, 0x0, 64 * 1024,   128, 0) },
>> +  /* GIGADEVICE */
>> +  {L"gd25q64b",          INFO(0xc84017, 0x0, 64 * 1024,   128, SECT_4K) },
>> +  {L"gd25lq32",          INFO(0xc86016, 0x0, 64 * 1024,    64, SECT_4K) },
>> +  /* ISSI */
>> +  {L"is25lp032",         INFO(0x9d6016, 0x0, 64 * 1024,    64, 0) },
>> +  {L"is25lp064",         INFO(0x9d6017, 0x0, 64 * 1024,   128, 0) },
>> +  {L"is25lp128",         INFO(0x9d6018, 0x0, 64 * 1024,   256, 0) },
>> +  /* MACRONIX */
>> +  {L"mx25l2006e",        INFO(0xc22012, 0x0, 64 * 1024,     4, 0) },
>> +  {L"mx25l4005",         INFO(0xc22013, 0x0, 64 * 1024,     8, 0) },
>> +  {L"mx25l8005",         INFO(0xc22014, 0x0, 64 * 1024,    16, 0) },
>> +  {L"mx25l1605d",        INFO(0xc22015, 0x0, 64 * 1024,    32, 0) },
>> +  {L"mx25l3205d",        INFO(0xc22016, 0x0, 64 * 1024,    64, 0) },
>> +  {L"mx25l6405d",        INFO(0xc22017, 0x0, 64 * 1024,   128, 0) },
>> +  {L"mx25l12805",        INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"mx25l25635f",       INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP | ADDR_CYC_4) },
>> +  {L"mx25l51235f",       INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
>> +  {L"mx25l12855e",       INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"mx66u51235f",       INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
>> +  {L"mx66l1g45g",        INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
>> +  /* SPANSION */
>> +  {L"s25fl008a",         INFO(0x010213, 0x0, 64 * 1024,    16, 0) },
>> +  {L"s25fl016a",         INFO(0x010214, 0x0, 64 * 1024,    32, 0) },
>> +  {L"s25fl032a",         INFO(0x010215, 0x0, 64 * 1024,    64, 0) },
>> +  {L"s25fl064a",         INFO(0x010216, 0x0, 64 * 1024,   128, 0) },
>> +  {L"s25fl116k",         INFO(0x014015, 0x0, 64 * 1024,   128, 0) },
>> +  {L"s25fl164k",         INFO(0x014017, 0x0140,  64 * 1024,   128, 0) },
>> +  {L"s25fl128p_256k",    INFO(0x012018, 0x0300, 256 * 1024,    64, RD_FULL | WR_QPP) },
>> +  {L"s25fl128p_64k",     INFO(0x012018, 0x0301,  64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"s25fl032p",         INFO(0x010215, 0x4d00,  64 * 1024,    64, RD_FULL | WR_QPP) },
>> +  {L"s25fl064p",         INFO(0x010216, 0x4d00,  64 * 1024,   128, RD_FULL | WR_QPP) },
>> +  {L"s25fl128s_256k",    INFO(0x012018, 0x4d00, 256 * 1024,    64, RD_FULL | WR_QPP) },
>> +  {L"s25fl128s_64k",     INFO(0x012018, 0x4d01,  64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"s25fl256s_256k",    INFO(0x010219, 0x4d00, 256 * 1024,   128, RD_FULL | WR_QPP) },
>> +  {L"s25fl256s_64k",     INFO(0x010219, 0x4d01,  64 * 1024,   512, RD_FULL | WR_QPP) },
>> +  {L"s25fl512s_256k",    INFO(0x010220, 0x4d00, 256 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"s25fl512s_64k",     INFO(0x010220, 0x4d01,  64 * 1024,  1024, RD_FULL | WR_QPP) },
>> +  {L"s25fl512s_512k",    INFO(0x010220, 0x4f00, 256 * 1024,   256, RD_FULL | WR_QPP) },
>> +  /* STMICRO */
>> +  {L"m25p10",            INFO(0x202011, 0x0, 32 * 1024,     4, 0) },
>> +  {L"m25p20",            INFO(0x202012, 0x0, 64 * 1024,     4, 0) },
>> +  {L"m25p40",            INFO(0x202013, 0x0, 64 * 1024,     8, 0) },
>> +  {L"m25p80",            INFO(0x202014, 0x0, 64 * 1024,    16, 0) },
>> +  {L"m25p16",            INFO(0x202015, 0x0, 64 * 1024,    32, 0) },
>> +  {L"m25pE16",           INFO(0x208015, 0x1000, 64 * 1024, 32, 0) },
>> +  {L"m25pX16",           INFO(0x207115, 0x1000, 64 * 1024, 32, RD_QUAD | RD_DUAL) },
>> +  {L"m25p32",            INFO(0x202016, 0x0,  64 * 1024,    64, 0) },
>> +  {L"m25p64",            INFO(0x202017, 0x0,  64 * 1024,   128, 0) },
>> +  {L"m25p128",           INFO(0x202018, 0x0, 256 * 1024,    64, 0) },
>> +  {L"m25pX64",           INFO(0x207117, 0x0,  64 * 1024,   128, SECT_4K) },
>> +  {L"n25q016a",          INFO(0x20bb15, 0x0,  64 * 1024,    32, SECT_4K) },
>> +  {L"n25q32",            INFO(0x20ba16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q32a",           INFO(0x20bb16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q64",            INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q64a",           INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q128",           INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"n25q128a",          INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
>> +  {L"n25q256",           INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q256a",          INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"n25q512",           INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
>> +  {L"n25q512a",          INFO(0x20bb20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
>> +  {L"n25q1024",          INFO(0x20ba21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K | ADDR_CYC_4) },
>> +  {L"n25q1024a",         INFO(0x20bb21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
>> +  {L"mt25qu02g",         INFO(0x20bb22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
>> +  {L"mt25ql02g",         INFO(0x20ba22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
>> +  /* SST */
>> +  {L"sst25vf040b",       INFO(0xbf258d, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
>> +  {L"sst25vf080b",       INFO(0xbf258e, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
>> +  {L"sst25vf016b",       INFO(0xbf2541, 0x0,  64 * 1024,    32, SECT_4K | SST_WR) },
>> +  {L"sst25vf032b",       INFO(0xbf254a, 0x0,  64 * 1024,    64, SECT_4K | SST_WR) },
>> +  {L"sst25vf064c",       INFO(0xbf254b, 0x0,  64 * 1024,   128, SECT_4K) },
>> +  {L"sst25wf512",        INFO(0xbf2501, 0x0,  64 * 1024,     1, SECT_4K | SST_WR) },
>> +  {L"sst25wf010",        INFO(0xbf2502, 0x0,  64 * 1024,     2, SECT_4K | SST_WR) },
>> +  {L"sst25wf020",        INFO(0xbf2503, 0x0,  64 * 1024,     4, SECT_4K | SST_WR) },
>> +  {L"sst25wf040",        INFO(0xbf2504, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
>> +  {L"sst25wf040b",       INFO(0x621613, 0x0,  64 * 1024,     8, SECT_4K) },
>> +  {L"sst25wf080",        INFO(0xbf2505, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
>> +  /* WINBOND */
>> +  {L"w25p80",            INFO(0xef2014, 0x0,  64 * 1024,    16, 0) },
>> +  {L"w25p16",            INFO(0xef2015, 0x0,  64 * 1024,    32, 0) },
>> +  {L"w25p32",            INFO(0xef2016, 0x0,  64 * 1024,    64, 0) },
>> +  {L"w25x40",            INFO(0xef3013, 0x0,  64 * 1024,     8, SECT_4K) },
>> +  {L"w25x16",            INFO(0xef3015, 0x0,  64 * 1024,    32, SECT_4K) },
>> +  {L"w25x32",            INFO(0xef3016, 0x0,  64 * 1024,    64, SECT_4K) },
>> +  {L"w25x64",            INFO(0xef3017, 0x0,  64 * 1024,   128, SECT_4K) },
>> +  {L"w25q80bl",          INFO(0xef4014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q16cl",          INFO(0xef4015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q32bv",          INFO(0xef4016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q64cv",          INFO(0xef4017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q128bv",         INFO(0xef4018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q256",           INFO(0xef4019, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q80bw",          INFO(0xef5014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q16dw",          INFO(0xef6015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q32dw",          INFO(0xef6016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q64dw",          INFO(0xef6017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
>> +  {L"w25q128fw",         INFO(0xef6018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
>> +  {},  /* Empty entry to terminate the list */
>> +};
>> +
>>  STATIC
>>  VOID
>>  SpiFlashFormatAddress (
>> @@ -104,13 +234,13 @@ MvSpiFlashWriteCommon (
>>    UINT8 CmdStatus = CMD_READ_STATUS;
>>    UINT8 State;
>>    UINT32 Counter = 0xFFFFF;
>> -  UINT8 poll_bit = STATUS_REG_POLL_WIP;
>> -  UINT8 check_status = 0x0;
>> +  UINT8 PollBit = STATUS_REG_POLL_WIP;
>> +  UINT8 CheckStatus = 0x0;
>>
>> -  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
>> -  if (CmdStatus == CMD_FLAG_STATUS) {
>> -    poll_bit = STATUS_REG_POLL_PEC;
>> -    check_status = poll_bit;
>> +  if (Slave->Info->Flags & E_FSR) {
>> +    CmdStatus = CMD_FLAG_STATUS;
>> +    PollBit = STATUS_REG_POLL_PEC;
>> +    CheckStatus = STATUS_REG_POLL_PEC;
>>    }
>>
>>    // Send command
>> @@ -127,7 +257,7 @@ MvSpiFlashWriteCommon (
>>      SpiMasterProtocol->Transfer (SpiMasterProtocol, Slave, 1, NULL, &State,
>>        0);
>>      Counter--;
>> -    if ((State & poll_bit) == check_status)
>> +    if ((State & PollBit) == CheckStatus)
>>        break;
>>    } while (Counter > 0);
>>    if (Counter == 0) {
>> @@ -181,8 +311,19 @@ MvSpiFlashErase (
>>    UINTN EraseSize;
>>    UINT8 Cmd[5];
>>
>> -  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
>> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
>> +  if (Slave->Info->Flags & ADDR_CYC_4) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>> +
>> +  if (Slave->Info->Flags & SECT_4K) {
>> +    Cmd[0] = CMD_ERASE_4K;
>> +    EraseSize = SPI_ERASE_SIZE_4K;
>> +  } else {
>> +    Cmd[0] = CMD_ERASE_64K;
>> +    EraseSize = Slave->Info->SectorSize;
>> +  }
>>
>>    // Check input parameters
>>    if (Offset % EraseSize || Length % EraseSize) {
>> @@ -191,21 +332,6 @@ MvSpiFlashErase (
>>      return EFI_DEVICE_ERROR;
>>    }
>>
>> -  switch (EraseSize) {
>> -  case SPI_ERASE_SIZE_4K:
>> -    Cmd[0] = CMD_ERASE_4K;
>> -    break;
>> -  case SPI_ERASE_SIZE_32K:
>> -    Cmd[0] = CMD_ERASE_32K;
>> -    break;
>> -  case SPI_ERASE_SIZE_64K:
>> -    Cmd[0] = CMD_ERASE_64K;
>> -    break;
>> -  default:
>> -    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
>> -    return EFI_INVALID_PARAMETER;
>> -  }
>> -
>>    while (Length) {
>>      EraseAddr = Offset;
>>
>> @@ -239,7 +365,11 @@ MvSpiFlashRead (
>>    UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>>    UINTN BankSel = 0;
>>
>> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> +  if (Slave->Info->Flags & ADDR_CYC_4) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>>
>>    Cmd[0] = CMD_READ_ARRAY_FAST;
>>
>> @@ -282,8 +412,13 @@ MvSpiFlashWrite (
>>    UINT32 WriteAddr;
>>    UINT8 Cmd[5], AddrSize;
>>
>> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> -  PageSize = PcdGet32 (PcdSpiFlashPageSize);
>> +  if (Slave->Info->Flags & ADDR_CYC_4) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>> +
>> +  PageSize = Slave->Info->PageSize;
>>
>>    Cmd[0] = CMD_PAGE_PROGRAM;
>>
>> @@ -370,7 +505,7 @@ MvSpiFlashUpdate (
>>    UINT64 SectorSize, ToUpdate, Scale = 1;
>>    UINT8 *TmpBuf, *End;
>>
>> -  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
>> +  SectorSize = Slave->Info->SectorSize;
>>
>>    End = Buf + ByteCount;
>>
>> @@ -400,38 +535,66 @@ MvSpiFlashUpdate (
>>    return EFI_SUCCESS;
>>  }
>>
>> +STATIC
>> +VOID
>> +MvPrintFlashInfo (
>> +  IN     SPI_FLASH_INFO *Info
>> +  )
>> +{
>> +  UINTN EraseSize;
>> +
>> +  if (Info->Flags & SECT_4K) {
>> +    EraseSize = SPI_ERASE_SIZE_4K;
>> +  } else {
>> +    EraseSize = Info->SectorSize;
>> +  }
>> +
>> +  DEBUG ((DEBUG_ERROR,
>> +    "Detected %s SPI flash with page size %d B, erase size %d KB, total %d MB\n",
>> +    Info->Name,
>> +    Info->PageSize,
>> +    EraseSize / 1024,
>> +    (Info->SectorSize * Info->SectorCount) / 1024 / 1024));
>> +}
>> +
>>  EFI_STATUS
>>  EFIAPI
>>  MvSpiFlashReadId (
>> -  IN     SPI_DEVICE *SpiDev,
>> -  IN     UINT32     DataByteCount,
>> -  IN OUT UINT8      *Buffer
>> +  IN     SPI_DEVICE *SpiDev
>>    )
>>  {
>> +  SPI_FLASH_INFO *Info;
>>    EFI_STATUS Status;
>> -  UINT8 *DataOut;
>> -
>> -  DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);
>> -  if (DataOut == NULL) {
>> -    DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
>> -    return EFI_OUT_OF_RESOURCES;
>> -  }
>> -  Status = SpiMasterProtocol->Transfer (SpiMasterProtocol, SpiDev,
>> -    DataByteCount, Buffer, DataOut, SPI_TRANSFER_BEGIN | SPI_TRANSFER_END);
>> -  if (EFI_ERROR(Status)) {
>> -    FreePool (DataOut);
>> -    DEBUG((DEBUG_ERROR, "SpiFlash: Spi transfer error\n"));
>> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
>> +  UINT8 Cmd;
>> +
>> +  Cmd = CMD_READ_ID;
>> +  Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,
>> +                                SpiDev,
>> +                                &Cmd,
>> +                                SPI_CMD_LEN,
>> +                                NULL,
>> +                                Id,
>> +                                SPI_FLASH_MAX_ID_LEN);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));
>>      return Status;
>>    }
>>
>> -  // Bytes 1,2 and 3 contain SPI flash ID
>> -  Buffer[0] = DataOut[1];
>> -  Buffer[1] = DataOut[2];
>> -  Buffer[2] = DataOut[3];
>> +  Info = SpiFlashIds;
>> +  for (; Info->Name != NULL; Info++) {
>> +    if (Info->IdLen != 0) {
>> +      if (CompareMem (Info->Id, Id, Info->IdLen) == 0) {
>> +        SpiDev->Info = Info;
>> +        MvPrintFlashInfo (Info);
>> +        return EFI_SUCCESS;
>> +      }
>> +    }
>> +  }
>>
>> -  FreePool (DataOut);
>> +  DEBUG ((DEBUG_ERROR, "ReadId: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n", Id[0], Id[1], Id[2]));
>>
>> -  return EFI_SUCCESS;
>> +  return EFI_NOT_FOUND;
>>  }
>>
>>  EFI_STATUS
>> @@ -445,7 +608,11 @@ MvSpiFlashInit (
>>    UINT8 Cmd, StatusRegister;
>>    UINT32 AddrSize;
>>
>> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> +  if (Slave->Info->Flags & ADDR_CYC_4) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>>
>>    if (AddrSize == 4) {
>>      // Set 4 byte address mode
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> index 646598a..b876966 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> @@ -62,6 +62,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #define CMD_ERASE_64K                   0xd8
>>  #define CMD_4B_ADDR_ENABLE              0xb7
>>
>> +#define SPI_CMD_LEN                     1
>> +
>>  #define STATUS_REG_POLL_WIP             (1 << 0)
>>  #define STATUS_REG_POLL_PEC             (1 << 7)
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
>> index 4519b02..f82c6e0 100644
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
>> @@ -53,13 +53,6 @@
>>    DebugLib
>>    MemoryAllocationLib
>>
>> -[FixedPcd]
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
>> -
>>  [Protocols]
>>    gMarvellSpiMasterProtocolGuid
>>    gMarvellSpiFlashProtocolGuid
>> diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
>> index ae78a31..73f0d80 100644
>> --- a/Platform/Marvell/Include/Protocol/Spi.h
>> +++ b/Platform/Marvell/Include/Protocol/Spi.h
>> @@ -38,6 +38,42 @@ extern EFI_GUID gMarvellSpiMasterProtocolGuid;
>>
>>  typedef struct _MARVELL_SPI_MASTER_PROTOCOL MARVELL_SPI_MASTER_PROTOCOL;
>>
>> +#define SPI_FLASH_MAX_ID_LEN    6
>> +
>> +typedef struct {
>> +  /* Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) */
>> +  UINT16 *Name;
>> +
>> +  /*
>> +   * This array stores the ID bytes.
>> +   * The first three bytes are the JEDIC ID.
>> +   * JEDEC ID zero means "no ID" (mostly older chips).
>> +   */
>> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
>> +  UINT8 IdLen;
>> +
>> +  /*
>> +   * The size listed here is what works with SPINOR_OP_SE, which isn't
>> +   * necessarily called a "sector" by the vendor.
>> +   */
>> +  UINT32 SectorSize;
>> +  UINT32 SectorCount;
>> +
>> +  UINT16 PageSize;
>> +
>> +  UINT16 Flags;
>> +#define SECT_4K      1 << 0  /* CMD_ERASE_4K works uniformly */
>> +#define E_FSR        1 << 1  /* use flag status register for */
>> +#define SST_WR       1 << 2  /* use SST byte/word programming */
>> +#define WR_QPP       1 << 3  /* use Quad Page Program */
>> +#define RD_QUAD      1 << 4  /* use Quad Read */
>> +#define RD_DUAL      1 << 5  /* use Dual Read */
>> +#define RD_QUADIO    1 << 6  /* use Quad IO Read */
>> +#define RD_DUALIO    1 << 7  /* use Dual IO Read */
>> +#define RD_FULL      (RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
>> +#define ADDR_CYC_4   1 << 8  /* use 4 byte addressing format */
>
> Oh, and parentheses around all of these defines.
>
> /
>     Leif
>
>> +} SPI_FLASH_INFO;
>> +
>>  typedef enum {
>>    SPI_MODE0, // CPOL = 0 & CPHA = 0
>>    SPI_MODE1, // CPOL = 0 & CPHA = 1
>> @@ -49,6 +85,7 @@ typedef struct {
>>    INTN Cs;
>>    INTN MaxFreq;
>>    SPI_MODE Mode;
>> +  SPI_FLASH_INFO *Info;
>>  } SPI_DEVICE;
>>
>>  typedef
>> diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
>> index 743bb87..e12f55b 100644
>> --- a/Platform/Marvell/Include/Protocol/SpiFlash.h
>> +++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
>> @@ -61,9 +61,7 @@ EFI_STATUS
>>  typedef
>>  EFI_STATUS
>>  (EFIAPI *MV_SPI_FLASH_READ_ID) (
>> -  IN     SPI_DEVICE *SpiDev,
>> -  IN     UINT32     DataByteCount,
>> -  IN OUT UINT8      *Buffer
>> +  IN     SPI_DEVICE *SpiDev
>>    );
>>
>>  typedef
>> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
>> index fc00f1a..418d960 100644
>> --- a/Platform/Marvell/Marvell.dec
>> +++ b/Platform/Marvell/Marvell.dec
>> @@ -123,12 +123,6 @@
>>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052
>>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053
>>
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0|UINT32|0x3000052
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
>>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
>>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
>>
>> --
>> 1.8.3.1
>>

Re: [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection

[Leif Lindholm]

On Fri, Sep 01, 2017 at 07:20:06PM +0200, Marcin Wojtas wrote:
> 2017-09-01 17:33 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Fri, Sep 01, 2017 at 03:08:22PM +0200, Marcin Wojtas wrote:
> >> Hitherto mechanism of fixing SPI flash model in the PCDs,
> >> occured to be very inefficient and problematic. Enable
> >> dynamic detection by reworking MvSpiFlashReadId() command,
> >> which now reads the Id and goes through newly added table
> >> with JEDEC compliant devices and their description.
> >>
> >> On the occasion fix the ReadId process by using master's
> >> ReadWrite routine instead of pure Transfer - no longer
> >> swapping and byte shifting is needed. Simplify code by
> >> using local array instead of dynamic allocation. Also
> >> reduced number of ReadId arguments allowed for cleaning
> >> Fupdate and Sf shell commands probe routines.
> >>
> >> Additionally, use SpiFlashInfo fields instead of PCDs,
> >> and configure needed settings in MvSpiFlashInit.
> >>
> >> Update PortingGuide documentation accordingly.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >
> > So, this looks _really_ good, but I'm seeing three separate patches in
> > here really:
> > - Style fixups (yay). I love them, but squashing them in make the
> >   functional changes impossible to spot in the noise.
> > - SPI Flash Id table. Love it!
> >   But this is a completely generic feature. Medium-term, this should
> >   be living in EDK2. For now, can you break it out into a separate
> >   library that can be used by other platforms (and will be easier to
> >   move)?
> > - Functional improvements to existing driver - great! (but can't
> >   really review before the above two have been broken out).
> >
> 
> I'll check and do the style fixup/functional improvement split.

Thanks!

> About flash ids in a separate library - we already have
> SpiFlashProtocol->ReadId callback. In fact despite Mv prefix the
> MvSpiFlash.c driver and its associated SPI_FLASH_PROTOCOL are
> completely generic for now and SoC controller agnostic. It works in
> very similar way as U-Boot driver (drivers/mtd/spi) and Linux one
> (drivers/mtd/spi-nor/spi-nor.c). Due to lack of similar solution, how
> about moving it to EDK2? Do you think there's chance to promote such
> solution?

Oh, certainly.
The only question would be whether you would be willing/able to make
that happen for this series. Having the identification portion as a
separate library may not be a bad idea regardless ... and it's a
decent halfway house.

/
    Leif