Re: [PATCH edk2-platforms v4 2/9] Platform/ARM/Sgi: add NOR flash platform library implementation

[Thomas Abraham <thomas.abraham@arm.com>]

Hi Ard,

On Mon, May 21, 2018 at 2:23 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 21 May 2018 at 10:25, Thomas Abraham <thomas.abraham@arm.com> wrote:
>> From: Vishwanatha HG <vishwanatha.hg@arm.com>
>>
>> Add a initial NOR flash driver platform wrapper as part of the platform
>> library. Access to NOR flash devices connected to CS0 and CS1 is enabled
>> in this initial implementation.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Vishwanatha HG <vishwanatha.hg@arm.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@arm.com>
>> ---
>>  .../ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c   | 66 ++++++++++++++++++++++
>>  .../ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf | 34 +++++++++++
>>  2 files changed, 100 insertions(+)
>>  create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c
>>  create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf
>>
>> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c
>> new file mode 100644
>> index 0000000..9b4981f
>> --- /dev/null
>> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.c
>> @@ -0,0 +1,66 @@
>> +/** @file
>> +
>> +  Copyright (c) 2018, ARM Ltd. All rights reserved.
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> + **/
>> +
>> +#include <PiDxe.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/NorFlashPlatformLib.h>
>> +#include <SgiPlatform.h>
>> +
>> +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
>> +  {
>> +    SGI_EXP_SMC_CS0_BASE,
>> +    SGI_EXP_SMC_CS0_BASE,
>> +    SIZE_256KB * 256,
>> +    SIZE_256KB,
>> +    {0xEBF0B9DF, 0x17d0, 0x4812, { 0xA9, 0x59, 0xCF, 0xD7, 0x92, 0xEE, 0x31, 0x13} }
>> +  },
>> +  {
>> +    SGI_EXP_SMC_CS1_BASE,
>> +    SGI_EXP_SMC_CS1_BASE,
>> +    SIZE_256KB * 256,
>> +    SIZE_256KB,
>> +    { 0x1F15DA3C, 0x37FF, 0x4070, { 0xB4, 0x71, 0xBB, 0x4A, 0xF1, 0x2A, 0x72, 0x4A } }
>
> Please use consistent whitespace in these GUID definitions.
>
>> +  },
>> +};
>> +
>> +EFI_STATUS
>> +NorFlashPlatformInitialization (
>> +  VOID
>> +  )
>> +{
>> +  UINT64 SysRegFlash;
>> +
>> +  SysRegFlash = SGI_EXP_SYSPH_SYSTEM_REGISTERS + SGI_SYSPH_SYS_REG_FLASH;
>> +  if ((MmioRead32 (SysRegFlash) & SGI_SYSPH_SYS_REG_FLASH_RWEN) == 0) {
>> +    MmioWrite32 (SysRegFlash, 1);
>
> SGI_SYSPH_SYS_REG_FLASH_RWEN?

The SysReg register has a bit in it that has to be set to allow
read/write access to the NOR flash. So this macros represents that
bit.

>
> Should you use MmioOr32() here? (I can't tell, I don't have the docs)

Tested with MmioOr32() and that works as well. So updated the code.

Thanks,
Thomas.

<snip>