Re: [Linaro-uefi] [RFC 00/20] Marvell Armada7040 SoC support

Hi Leif,

Possibly some of those variables shouldn't get this prefix. We will browse our code in terms of your comment.

Best Regards, Jan

2016-06-20 17:09 GMT+02:00 Leif Lindholm leif.lindholm@linaro.org:

So, let me start with a grovelling apology for how long this has taken. I'm mostly dug out of the hole I was in, so I promise to do better in the future.

I have one generic comment - as in something I've seen in a few patches and wanted to mention separately:

The EDK2 coding standards document[1] states that: "A global variable is intended to be accessed throughout the firmware, usually indirectly through a protocol pointer or similar mechanism." I've seen a few instances in these patches where file-global variables are given the 'g' prefix - is this a misinterpretation of the meaning of "global" variable in this context? Or are they intended to be made globally accessible by later patches?

[1] https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Dra...

Best Regards,

Leif

On Fri, Apr 29, 2016 at 07:25:50PM +0200, Marcin Wojtas wrote:

...

Hello,

Here's v4 version of the support. After thorough review of v3, this one comprise a lot of improvements. The major change is replacing support of obsolete SoC revisions with the newest one (A0). All the details can be found in a changelog below.

As usually the patchset is available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/opp-...

Any comments or remarks would be welcome.

Best regards, Marcin

Changelog: v3 -> v4

• Update license messages
  • Unify
  • Add "Based on" statement were needed
  • Wrap every message at 80 characters
  • Add Marvell copyrights
• Fix namings
  • All protocol/file names updated as suggested in review
• Clean-up
  • Remove unneded blank lines and spaces
  • Fix typos
• Add "Reviewed by" for currently accepted patches
• Add support for new platform - Armada70x0
  • Remove support for old platforms Apn806 and Armada7040_rz
  • Armada70x0 is based on Armada7040_rz with silicone fixes and improvements
  • No changes on drivers/libraries caused by this change
  • Update commit logs in whole tree
• MppLib
  • Fix issue with writing to uninitialized memory
• I2c and eeprom command
  • Remove dependency on BdsLibConnectAll call
  • I2c stack is connected only if eeprom command is executed and only in case it wasn't done before
  • gBS->ConnectController() used
• BdsLib
  • Remove import of BdsLib
  • After rebase on top of new edk2, extra BdsLibConnectAll call isn't necessary (actually for now no BdsLibConnectAll is needed at all for our platform support)
• sf command
  • Remove inclusion of relative path
  • Necessary macro now defined in globally exported file
• RamDisk driver
  • Drop patch with this feature
  • As long as there is no possibility to format RamDisk with FAT, driver is not useful on Armada70x0
• fupdate command
  • Fix issue with "bad checksum"

v2 -> v3

• Decrease binary size
• Fupdate command
  • Remove regression - replace ShellExecute with RunShellCommand
  • Improve error paths
• Add proper DEBUG library and change error print levels

v1 -> v2

• Fix "may be used uninitialized" variables issue, branch compiles both with DEBUG and RELEASE flags
• Remove I2C_FLAG_NORESTART dependency - replace it with local definition
• Remove Marvell UART Library:
  • DW8250 IP has programming interface compatible with NS-16550.
  Since in edk2 there is support for 16550, internal library is unnecessary.
  • /Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf is used instead
• RamDisk driver:
  • Remove old driver imported from EFI TOOLKIT 2.0
  • Replace it with new MdeModulePkg/Universal/Disk/RamDiskDxe from edk2
  • Lack of FAT formatting issue emerged
• Add explicit dependency on IoLib in Drivers/Spi/SpiDxe.inf
• Add poster's "Signed-off-by" registry to every commit message
• Move to 05.04.2016 HEAD of edk2/master
  • Update CpuExceptionHandlerLib within OPP.

Bartosz Szczepanek (3): Drivers/I2c: Create MvI2cDxe driver Drivers/I2c: Add MvEeprom driver Aplications/Eeprom: Add 'eeprom' command to shell

Jan Dąbroś (15): Platforms/Marvell: Create MppLib Platforms/Marvell: Armada: Enable MppLib on Armada70x0 platform Platforms/Marvell: Enable I2C driver on Armada70x0 platform Plaforms/Marvell: Enable EEPROM driver on Armada70x0 platform Platforms/Marvell: Enable 'eeprom' command on Armada70x0 platform Platforms/Marvell: Add MARVELL_SPI_MASTER_PROTOCOL Drivers/Spi: Add Spi master driver Platforms/Marvell: Enable Spi master driver for Armada70x0 platform Platforms/Marvell: Add MARVELL_SPI_FLASH_PROTOCOL Drivers/Spi: Implement Spi flash driver Platforms/Marvell: Enable Spi flash driver for Armada70x0 platform Applications/SpiTool: Add 'sf' command utility Platforms/Marvell: Enable 'sf' command on Armada70x0 platform Applications/FirmwareUpdate: Add 'fupdate' comand to shell Platforms/Marvell: Enable 'fupdate' command on Armada70x0 platform

Yehuda Yitschak (2): Platforms/Marvell: Add initial support for Armada70x0 SOC lib Platforms/Marvell: Add support for Armada70x0 platform

Applications/EepromCmd/EepromCmd.c | 383 ++++++++++++ Applications/EepromCmd/EepromCmd.inf | 71 +++ Applications/EepromCmd/EepromCmd.uni | Bin 0 -> 6816 bytes Applications/FirmwareUpdate/FUpdate.c | 447 ++++++++++++++ Applications/FirmwareUpdate/FUpdate.inf | 68 ++ Applications/FirmwareUpdate/FUpdate.uni | Bin 0 -> 5838 bytes Applications/SpiTool/SpiFlashCmd.c | 526 ++++++++++++++++ Applications/SpiTool/SpiFlashCmd.inf | 75 +++ Applications/SpiTool/SpiFlashCmd.uni | Bin 0 -> 7216 bytes Documentation/Marvell/Drivers/EepromDriver.txt | 96 +++ Documentation/Marvell/Drivers/I2cDriver.txt | 64 ++ Documentation/Marvell/Drivers/SpiDriver.txt | 116 ++++ Documentation/Marvell/PortingGuide/I2c.txt | 16 + Documentation/Marvell/PortingGuide/Mpp.txt | 48 ++ Documentation/Marvell/PortingGuide/Spi.txt | 15 + Documentation/Marvell/PortingGuide/SpiFlash.txt | 19 + Drivers/I2c/Devices/MvEeprom/MvEeprom.c | 293 +++++++++ Drivers/I2c/Devices/MvEeprom/MvEeprom.h | 90 +++ Drivers/I2c/Devices/MvEeprom/MvEeprom.inf | 69 +++ Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 683 +++++++++++++++++++++ Drivers/I2c/MvI2cDxe/MvI2cDxe.h | 258 ++++++++ Drivers/I2c/MvI2cDxe/MvI2cDxe.inf | 72 +++ Drivers/Spi/Devices/MvSpiFlash.c | 523 ++++++++++++++++ Drivers/Spi/Devices/MvSpiFlash.h | 130 ++++ Drivers/Spi/Devices/MvSpiFlash.inf | 66 ++ Drivers/Spi/MvSpiDxe.c | 378 ++++++++++++ Drivers/Spi/MvSpiDxe.h | 145 +++++ Drivers/Spi/MvSpiDxe.inf | 66 ++ Platforms/Marvell/Armada/Armada.dsc.inc | 459 ++++++++++++++ Platforms/Marvell/Armada/Armada70x0.dsc | 93 +++ Platforms/Marvell/Armada/Armada70x0.fdf | 273 ++++++++ .../Armada70x0Lib/AArch64/ArmPlatformHelper.S | 66 ++ .../Armada/Library/Armada70x0Lib/Armada70x0Lib.c | 155 +++++ .../Armada/Library/Armada70x0Lib/Armada70x0Lib.inf | 69 +++ .../Library/Armada70x0Lib/Armada70x0LibMem.c | 93 +++ Platforms/Marvell/Include/Library/MppLib.h | 42 ++ Platforms/Marvell/Include/Protocol/Eeprom.h | 60 ++ Platforms/Marvell/Include/Protocol/Spi.h | 105 ++++ Platforms/Marvell/Include/Protocol/SpiFlash.h | 113 ++++ Platforms/Marvell/Library/MppLib/MppLib.c | 161 +++++ Platforms/Marvell/Library/MppLib/MppLib.inf | 107 ++++ Platforms/Marvell/Marvell.dec | 132 ++++ 42 files changed, 6645 insertions(+) create mode 100644 Applications/EepromCmd/EepromCmd.c create mode 100644 Applications/EepromCmd/EepromCmd.inf create mode 100644 Applications/EepromCmd/EepromCmd.uni create mode 100644 Applications/FirmwareUpdate/FUpdate.c create mode 100644 Applications/FirmwareUpdate/FUpdate.inf create mode 100644 Applications/FirmwareUpdate/FUpdate.uni create mode 100644 Applications/SpiTool/SpiFlashCmd.c create mode 100644 Applications/SpiTool/SpiFlashCmd.inf create mode 100644 Applications/SpiTool/SpiFlashCmd.uni create mode 100644 Documentation/Marvell/Drivers/EepromDriver.txt create mode 100644 Documentation/Marvell/Drivers/I2cDriver.txt create mode 100644 Documentation/Marvell/Drivers/SpiDriver.txt create mode 100644 Documentation/Marvell/PortingGuide/I2c.txt create mode 100644 Documentation/Marvell/PortingGuide/Mpp.txt create mode 100644 Documentation/Marvell/PortingGuide/Spi.txt create mode 100644 Documentation/Marvell/PortingGuide/SpiFlash.txt create mode 100644 Drivers/I2c/Devices/MvEeprom/MvEeprom.c create mode 100644 Drivers/I2c/Devices/MvEeprom/MvEeprom.h create mode 100644 Drivers/I2c/Devices/MvEeprom/MvEeprom.inf create mode 100644 Drivers/I2c/MvI2cDxe/MvI2cDxe.c create mode 100644 Drivers/I2c/MvI2cDxe/MvI2cDxe.h create mode 100644 Drivers/I2c/MvI2cDxe/MvI2cDxe.inf create mode 100644 Drivers/Spi/Devices/MvSpiFlash.c create mode 100644 Drivers/Spi/Devices/MvSpiFlash.h create mode 100644 Drivers/Spi/Devices/MvSpiFlash.inf create mode 100644 Drivers/Spi/MvSpiDxe.c create mode 100644 Drivers/Spi/MvSpiDxe.h create mode 100644 Drivers/Spi/MvSpiDxe.inf create mode 100644 Platforms/Marvell/Armada/Armada.dsc.inc create mode 100644 Platforms/Marvell/Armada/Armada70x0.dsc create mode 100644 Platforms/Marvell/Armada/Armada70x0.fdf create mode 100644 Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S create mode 100644 Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.c create mode 100644 Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf create mode 100644 Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c create mode 100644 Platforms/Marvell/Include/Library/MppLib.h create mode 100644 Platforms/Marvell/Include/Protocol/Eeprom.h create mode 100644 Platforms/Marvell/Include/Protocol/Spi.h create mode 100644 Platforms/Marvell/Include/Protocol/SpiFlash.h create mode 100644 Platforms/Marvell/Library/MppLib/MppLib.c create mode 100644 Platforms/Marvell/Library/MppLib/MppLib.inf create mode 100644 Platforms/Marvell/Marvell.dec

-- 1.8.3.1