Re: [platforms: PATCH v2 01/25] Marvell/Library: Introduce ArmadaSoCDescLib class

[Marcin Wojtas <mw@semihalf.com>]

Hi Leif,

2018-06-18 17:44 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Sun, Jun 17, 2018 at 10:11:41PM +0200, Marcin Wojtas wrote:
>> From: jinghua <jinghua@marvell.com>
>>
>> ArmadaSoCDescLib is a per SoC family library, which provides SoC
>> description, like register base of some hardware module controller,
>> COMPHY/I2C/NETWORK etc., which right now is hardcoded in MvHwDescLib.h.
>> There will be a new protocol, which gets SoC description from this
>> library, and provides board description based on enable/disable
>> values of each hardware module controller in dsc file.
>>
>> As a first example implement obtaining UTMI controllers information.
>> Remaining interfaces will be added in follow-up commits.
>> This patch introduces new library callback (ArmadaSoCDescUtmiGet ()),
>> which dynamically allocates and fills MV_SOC_UTMI_DESC structure,
>> SoC description of UTMI PHYs. A new PCD is introduced (PcdMaxCpCount)
>> which stores maximal amount of CP110 blocks in the SoC family.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: jinghua <jinghua@marvell.com>
>
> Several of these pre-existing Signed-off-by remain in this set. I did
> only comment it on 2/25, but it remains there as well.

As I understood, the problem was 'Reviewed-by' from internal review -
all those were dropped. So far when the commit was authored by someone
else (at least in OpenPlatformPkg times) we used to leave the
authorship and authors 'Signed-off-by' - in such case I had to add my
own Signed-off-by, as I was sending this to the lists.

>
> Please send a v3 with this addressed across all patches.
> But please wait until I've had a chance to go through this set.
> When you do so, this one has
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> I will comment explicitly on all such patches in this set, apart from
> the one where I already gave Reviewed-by (but please delete all other
> signed-off-bys than your own from those as well).
>

I will update  according to your wish and the new policy.

Best regards,
Marcin

> /
>     Leif
>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Silicon/Marvell/Marvell.dec                                                      |  4 ++
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf | 37 +++++++++++
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h   | 35 +++++++++++
>>  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h                               | 33 ++++++++++
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c   | 65 ++++++++++++++++++++
>>  5 files changed, 174 insertions(+)
>>  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf
>>  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
>>  create mode 100644 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
>>  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>>
>> diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
>> index be74b4e..2a92eff 100644
>> --- a/Silicon/Marvell/Marvell.dec
>> +++ b/Silicon/Marvell/Marvell.dec
>> @@ -60,6 +60,7 @@
>>    gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } }
>>
>>  [LibraryClasses]
>> +  ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h
>>    SampleAtResetLib|Include/Library/SampleAtResetLib.h
>>
>>  [Protocols]
>> @@ -68,6 +69,9 @@
>>    gMarvellPlatformInitCompleteProtocolGuid = { 0x465b8cf7, 0x016f, 0x4ba6, { 0xbe, 0x6b, 0x28, 0x0e, 0x3a, 0x7d, 0x38, 0x6f } }
>>
>>  [PcdsFixedAtBuild.common]
>> +#Board description
>> +  gMarvellTokenSpaceGuid.PcdMaxCpCount|0x2|UINT8|0x30000072
>> +
>>  #MPP
>>    gMarvellTokenSpaceGuid.PcdMppChipCount|0|UINT32|0x30000001
>>
>> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf
>> new file mode 100644
>> index 0000000..2b73b73
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf
>> @@ -0,0 +1,37 @@
>> +## @file
>> +#
>> +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates<BR>
>> +#
>> +#  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.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x0001001A
>> +  BASE_NAME                      = Armada7k8kDescLib
>> +  FILE_GUID                      = c64f0048-4ca3-4573-b0a6-c2e9e6457285
>> +  MODULE_TYPE                    = BASE
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = ArmadaSoCDescLib
>> +
>> +[Sources]
>> +  Armada7k8kSoCDescLib.c
>> +
>> +[Packages]
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  Silicon/Marvell/Marvell.dec
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> +  IoLib
>> +  PcdLib
>> +
>> +[FixedPcd]
>> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
>> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
>> new file mode 100644
>> index 0000000..c5711b0
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
>> @@ -0,0 +1,35 @@
>> +/**
>> +*
>> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
>> +*
>> +*  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.
>> +*
>> +*  Glossary - abbreviations used in Marvell SampleAtReset library implementation:
>> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
>> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
>> +**/
>> +
>> +#ifndef __ARMADA7K8K_SOC_DESC_LIB_H__
>> +#define __ARMADA7K8K_SOC_DESC_LIB_H__
>> +
>> +//
>> +// Common macros
>> +//
>> +#define MV_SOC_CP_BASE(Cp)               (0xF2000000 + ((Cp) * 0x2000000))
>> +
>> +//
>> +// Platform description of UTMI PHY's
>> +//
>> +#define MV_SOC_UTMI_PER_CP_COUNT         2
>> +#define MV_SOC_UTMI_ID(Utmi)             (Utmi)
>> +#define MV_SOC_UTMI_BASE(Utmi)           (0x580000 + ((Utmi) * 0x1000))
>> +#define MV_SOC_UTMI_CFG_BASE             0x440440
>> +#define MV_SOC_UTMI_USB_CFG_BASE         0x440420
>> +
>> +#endif
>> diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
>> new file mode 100644
>> index 0000000..0d45684
>> --- /dev/null
>> +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
>> @@ -0,0 +1,33 @@
>> +/**
>> +*
>> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates
>> +*
>> +*  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.
>> +*
>> +**/
>> +#ifndef __ARMADA_SOC_DESC_LIB_H__
>> +#define __ARMADA_SOC_DESC_LIB_H__
>> +
>> +//
>> +// UTMI PHY devices SoC description
>> +//
>> +typedef struct {
>> +  UINT8 UtmiPhyId;
>> +  UINTN UtmiBaseAddress;
>> +  UINTN UtmiConfigAddress;
>> +  UINTN UsbConfigAddress;
>> +} MV_SOC_UTMI_DESC;
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +ArmadaSoCDescUtmiGet (
>> +  IN OUT MV_SOC_UTMI_DESC  **UtmiDesc,
>> +  IN OUT UINTN              *DescCount
>> +  );
>> +#endif /* __ARMADA_SOC_DESC_LIB_H__ */
>> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>> new file mode 100644
>> index 0000000..63fb224
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>> @@ -0,0 +1,65 @@
>> +/**
>> +*
>> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
>> +*
>> +*  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.
>> +*
>> +*  Glossary - abbreviations used in Marvell SampleAtReset library implementation:
>> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
>> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
>> +**/
>> +
>> +#include <Uefi.h>
>> +
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +
>> +#include <Protocol/BoardDesc.h>
>> +
>> +#include "Armada7k8kSoCDescLib.h"
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +ArmadaSoCDescUtmiGet (
>> +  IN OUT MV_SOC_UTMI_DESC  **UtmiDesc,
>> +  IN OUT UINTN              *DescCount
>> +  )
>> +{
>> +  MV_SOC_UTMI_DESC *Desc;
>> +  UINTN CpCount, CpIndex, Index, UtmiIndex;
>> +
>> +  CpCount = FixedPcdGet8 (PcdMaxCpCount);
>> +
>> +  *DescCount = CpCount * MV_SOC_UTMI_PER_CP_COUNT;
>> +  Desc = AllocateZeroPool (*DescCount * sizeof (MV_SOC_UTMI_DESC));
>> +  if (Desc == NULL) {
>> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  *UtmiDesc = Desc;
>> +
>> +  UtmiIndex = 0;
>> +  for (CpIndex = 0; CpIndex < CpCount; CpIndex++) {
>> +    for (Index = 0; Index < MV_SOC_UTMI_PER_CP_COUNT; Index++) {
>> +      Desc->UtmiPhyId = MV_SOC_UTMI_ID (UtmiIndex);
>> +      Desc->UtmiBaseAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_BASE (Index);
>> +      Desc->UtmiConfigAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_CFG_BASE;
>> +      Desc->UsbConfigAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_USB_CFG_BASE;
>> +      Desc++;
>> +      UtmiIndex++;
>> +    }
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> --
>> 2.7.4
>>