Re: [platforms: PATCH 4/6] Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 12 July 2018 at 09:39, Marcin Wojtas <mw@semihalf.com> wrote:
> This patch introduces new library callback (ArmadaSoCDescIcuGet ()),
> which dynamically allocates and fills MV_SOC_ICU_DESC structure with
> the SoC description of ICU (Interrupt Consolidation Unit).
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 12 ++++++
>  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h                             | 30 +++++++++++++++
>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 39 ++++++++++++++++++++
>  3 files changed, 81 insertions(+)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
> index 3072883..c14b985 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
> @@ -44,6 +44,18 @@
>  #define MV_SOC_I2C_BASE(I2c)             (0x701000 + ((I2c) * 0x100))
>
>  //
> +// Platform description of ICU (Interrupt Consolidation Unit) controllers
> +//
> +#define ICU_GIC_MAPPING_OFFSET           0
> +#define ICU_NSR_SET_SPI_BASE             0xf03f0040
> +#define ICU_NSR_CLEAR_SPI_BASE           0xf03f0048
> +#define ICU_SEI_SET_SPI_BASE             0xf03f0230
> +#define ICU_SEI_CLEAR_SPI_BASE           0xf03f0230
> +#define ICU_REI_SET_SPI_BASE             0xf03f0270
> +#define ICU_REI_CLEAR_SPI_BASE           0xf03f0270
> +#define ICU_GROUP_UNSUPPORTED            0x0
> +
> +//
>  // Platform description of MDIO controllers
>  //
>  #define MV_SOC_MDIO_BASE(Cp)             (MV_SOC_CP_BASE (Cp) + 0x12A200)
> diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> index 56efdbe..4d2a85f 100644
> --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> @@ -58,6 +58,36 @@ ArmadaSoCDescI2cGet (
>    );
>
>  //
> +// ICU (Interrupt Consolidation Unit)
> +//
> +typedef enum {
> +  ICU_GROUP_NSR  = 0,
> +  ICU_GROUP_SR   = 1,
> +  ICU_GROUP_LPI  = 2,
> +  ICU_GROUP_VLPI = 3,
> +  ICU_GROUP_SEI  = 4,
> +  ICU_GROUP_REI  = 5,

Are these identifiers defined externally anywhere? If not, it is
better to use IcuGroupXxx, which is more idiomatic in UEFI/EDK2.

> +  ICU_GROUP_MAX,
> +} ICU_GROUP;
> +
> +typedef struct {
> +  ICU_GROUP Group;
> +  UINTN     SetSpiAddr;
> +  UINTN     ClrSpiAddr;
> +} ICU_MSI;
> +
> +typedef struct {
> +  UINTN    IcuSpiBase;
> +  ICU_MSI  IcuMsi[ICU_GROUP_MAX];
> +} MV_SOC_ICU_DESC;
> +
> +EFI_STATUS
> +EFIAPI
> +ArmadaSoCDescIcuGet (
> +  IN OUT MV_SOC_ICU_DESC  **IcuDesc
> +  );
> +
> +//
>  // MDIO
>  //
>  typedef struct {
> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> index c7c9c13..8383206 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> @@ -103,6 +103,45 @@ ArmadaSoCDescI2cGet (
>    return EFI_SUCCESS;
>  }
>
> +//
> +// Allocate the MSI address per interrupt Group,
> +// unsupported Groups get NULL address.
> +//
> +STATIC
> +MV_SOC_ICU_DESC mA7k8kIcuDescTemplate = {
> +  ICU_GIC_MAPPING_OFFSET,
> +  {
> +    /* Non secure interrupts */
> +    {ICU_GROUP_NSR,  ICU_NSR_SET_SPI_BASE,  ICU_NSR_CLEAR_SPI_BASE},

Please put a space after { and before }

> +    /* Secure interrupts */
> +    {ICU_GROUP_SR,   ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
> +    /* LPI interrupts */
> +    {ICU_GROUP_LPI,  ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
> +    /* Virtual LPI interrupts */
> +    {ICU_GROUP_VLPI, ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
> +    /* System error interrupts */
> +    {ICU_GROUP_SEI,  ICU_SEI_SET_SPI_BASE,  ICU_SEI_CLEAR_SPI_BASE},
> +    /* RAM error interrupts */
> +    {ICU_GROUP_REI,  ICU_REI_SET_SPI_BASE,  ICU_REI_CLEAR_SPI_BASE},
> +  }
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +ArmadaSoCDescIcuGet (
> +  IN OUT MV_SOC_ICU_DESC  **IcuDesc
> +  )
> +{
> +  *IcuDesc = AllocateCopyPool (sizeof (mA7k8kIcuDescTemplate),
> +               &mA7k8kIcuDescTemplate);
> +  if (*IcuDesc == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  ArmadaSoCDescMdioGet (
> --
> 2.7.4
>