public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Marcin Wojtas <mw@semihalf.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	nadavh@marvell.com, Hanna Hawa <hannah@marvell.com>,
	"jsd@semihalf.com" <jsd@semihalf.com>,
	Grzegorz Jaszczyk <jaz@semihalf.com>
Subject: Re: [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib
Date: Thu, 12 Jul 2018 12:12:38 +0100	[thread overview]
Message-ID: <20180712111238.w3ysz6iamo2pq25l@bivouac.eciton.net> (raw)
In-Reply-To: <CAPv3WKdb8dX-UjV-ytji82vCQt95RRnWkM41ry4-NAH0tmmYGA@mail.gmail.com>

On Thu, Jul 12, 2018 at 12:59:16PM +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> 
> 
> 2018-07-12 12:35 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Thu, Jul 12, 2018 at 09:40:00AM +0200, Marcin Wojtas wrote:
> >> ICU (Interrupt Consolidation Unit) is a mechanism,
> >> that allows to send-message based interrupts from the
> >> CP110 unit (South Bridge) to the Application Processor
> >> hardware block. After dispatching the interrupts in the
> >> GIC are generated.
> >>
> >> This patch adds a basic version of the library, that
> >> allows to configure a static mapping between CP110
> >> interfaces and GICv2 of the Armada7k8k SoC family.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 ++++++++++++++++++++
> >>  3 files changed, 399 insertions(+)
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
> >>
> >> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >> new file mode 100644
> >> index 0000000..0010141
> >> --- /dev/null
> >> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >> @@ -0,0 +1,38 @@
> >> +## @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                      = IcuLib
> >> +  FILE_GUID                      = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
> >> +  MODULE_TYPE                    = BASE
> >> +  VERSION_STRING                 = 1.0
> >> +  LIBRARY_CLASS                  = ArmadaIcuLib
> >> +
> >> +[Sources]
> >> +  IcuLib.c
> >> +
> >> +[Packages]
> >> +  MdeModulePkg/MdeModulePkg.dec
> >> +  MdePkg/MdePkg.dec
> >> +  Silicon/Marvell/Marvell.dec
> >> +
> >> +[LibraryClasses]
> >> +  ArmadaSoCDescLib
> >> +  DebugLib
> >> +  IoLib
> >> +  PcdLib
> >> +
> >> +[FixedPcd]
> >> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
> >> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> >> new file mode 100644
> >> index 0000000..4bf2298
> >> --- /dev/null
> >> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> >> @@ -0,0 +1,46 @@
> >> +/**
> >> +*
> >> +*  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:
> >> +*  ICU - Interrupt Consolidation Unit
> >> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
> >> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> >> +*
> >> +**/
> >> +
> >> +#include <Uefi.h>
> >> +
> >> +#include <Library/ArmadaIcuLib.h>
> >> +#include <Library/ArmadaSoCDescLib.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>
> >> +
> >> +#define ICU_REG_BASE(Cp)        ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E0000
> >
> > Macro missing parentheses.
> >
> >> +
> >> +#define ICU_SET_SPI_AL(x)       (0x10 + (0x10 * x))
> >> +#define ICU_SET_SPI_AH(x)       (0x14 + (0x10 * x))
> >> +#define ICU_CLR_SPI_AL(x)       (0x18 + (0x10 * x))
> >> +#define ICU_CLR_SPI_AH(x)       (0x1c + (0x10 * x))
> >
> > Can that 0x10 be given a descriptive #define?
> >
> 
> 0x10 is just a part of register offset calculation formula. If you
> wish to replace it with macro, how about:
> #define ICU_GROUP_REGISTER_BASE_SHIFT    0x10
> ?

Not SHIFT (which suggests << or >> to me), but OFFSET would work.

> >> +#define ICU_INT_CFG(x)          (0x100 + 4 * x)
> >
> > Does this 4 signify sizeof (UINT32) - i.e. does it get the offset into
> > a series of 32-bit registers? If so, please use that instead.
> 
> Ok.
> 
> > Also, please use parentheses around multiplication consistently
> > (either not at all or everywhere).
> >
> 
> In previous series (board description) you recommended using the
> parentheses, so I'll follow this guideline consistently.

There were parentheses around (0x10 * x) but not around 4 * x.
This was the reason for my comment.
Since it's relegated to a non-complicated compiler macro, I don't
really care whether they're there or not - but I want them consistent.

> >> +
> >> +#define ICU_INT_ENABLE_OFFSET    24
> >> +#define ICU_IS_EDGE_OFFSET       28
> >> +#define ICU_GROUP_OFFSET         29
> >> +
> >> +#define ICU_MAX_SUPPORTED_UNITS  2
> >> +#define ICU_MAX_IRQS_PER_CP      64
> >> +
> >> +#define MAX_ICU_IRQS             207
> >> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.c b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> >> new file mode 100644
> >> index 0000000..4ac98aa
> >> --- /dev/null
> >> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> >> @@ -0,0 +1,315 @@
> >> +/**
> >> +*
> >> +*  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:
> >> +*  ICU - Interrupt Consolidation Unit
> >> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
> >> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> >> +*
> >> +**/
> >> +
> >> +#include "IcuLib.h"
> >> +
> >> +EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL;
> >
> > STATIC?
> > and mExitBootServicesEvent?
> > And no need to initialise it.
> >
> 
> 2x Ok.
> 
> > (Yes, I see you've looked at existing templates, and I'm sure we'll
> > get around to cleaning those up at some point.)
> >
> > Digging into the origin of this antipattern briefly made me question
> > my knowledge of the C language, until I realised EFI_EVENT is a
> > typedef for VOID *.
> >
> >> +
> >> +STATIC CONST ICU_IRQ IrqMapNonSecure[] = {
> >> +  {22,   0, Level}, /* PCIx4 INT A interrupt */
> >> +  {23,   1, Level}, /* PCIx1 INT A interrupt */
> >> +  {24,   2, Level}, /* PCIx1 INT A interrupt */
> >> +  {27,   3, Level}, /* SD/MMC */
> >> +  {33,   4, Level}, /* PPv2 DBG AXI monitor */
> >> +  {34,   4, Level}, /* HB1      AXI monitor */
> >> +  {35,   4, Level}, /* AP       AXI monitor */
> >> +  {36,   4, Level}, /* PPv2     AXI monitor */
> >> +  {39,   5, Level}, /* PPv2 Irq */
> >> +  {40,   6, Level}, /* PPv2 Irq */
> >> +  {41,   7, Level}, /* PPv2 Irq */
> >> +  {43,   8, Level}, /* PPv2 Irq */
> >> +  {44,   9, Level}, /* PPv2 Irq */
> >> +  {45,  10, Level}, /* PPv2 Irq */
> >> +  {47,  11, Level}, /* PPv2 Irq */
> >> +  {48,  12, Level}, /* PPv2 Irq */
> >> +  {49,  13, Level}, /* PPv2 Irq */
> >> +  {51,  14, Level}, /* PPv2 Irq */
> >> +  {52,  15, Level}, /* PPv2 Irq */
> >> +  {53,  16, Level}, /* PPv2 Irq */
> >> +  {55,  17, Level}, /* PPv2 Irq */
> >> +  {56,  18, Level}, /* PPv2 Irq */
> >> +  {57,  19, Level}, /* PPv2 Irq */
> >> +  {59,  20, Level}, /* PPv2 Irq */
> >> +  {60,  21, Level}, /* PPv2 Irq */
> >> +  {61,  22, Level}, /* PPv2 Irq */
> >> +  {63,  23, Level}, /* PPv2 Irq */
> >> +  {64,  24, Level}, /* PPv2 Irq */
> >> +  {65,  25, Level}, /* PPv2 Irq */
> >> +  {67,  26, Level}, /* PPv2 Irq */
> >> +  {68,  27, Level}, /* PPv2 Irq */
> >> +  {69,  28, Level}, /* PPv2 Irq */
> >> +  {71,  29, Level}, /* PPv2 Irq */
> >> +  {72,  30, Level}, /* PPv2 Irq */
> >> +  {73,  31, Level}, /* PPv2 Irq */
> >> +  {78,  32, Level}, /* MG Irq */
> >> +  {79,  33, Level}, /* GPIO 56-63 */
> >> +  {80,  34, Level}, /* GPIO 48-55 */
> >> +  {81,  35, Level}, /* GPIO 40-47 */
> >> +  {82,  36, Level}, /* GPIO 32-39 */
> >> +  {83,  37, Level}, /* GPIO 24-31 */
> >> +  {84,  38, Level}, /* GPIO 16-23 */
> >> +  {85,  39, Level}, /* GPIO  8-15 */
> >> +  {86,  40, Level}, /* GPIO  0-7  */
> >> +  {88,  41, Level}, /* EIP-197 ring-0 */
> >> +  {89,  42, Level}, /* EIP-197 ring-1 */
> >> +  {90,  43, Level}, /* EIP-197 ring-2 */
> >> +  {91,  44, Level}, /* EIP-197 ring-3 */
> >> +  {92,  45, Level}, /* EIP-197 int */
> >> +  {95,  46, Level}, /* EIP-150 Irq */
> >> +  {102, 47, Level}, /* USB3 Device Irq */
> >> +  {105, 48, Level}, /* USB3 Host-1 Irq */
> >> +  {106, 49, Level}, /* USB3 Host-0 Irq */
> >> +  {107, 50, Level}, /* SATA Host-1 Irq */
> >> +  {109, 50, Level}, /* SATA Host-0 Irq */
> >> +  {115, 52, Level}, /* NAND Irq */
> >> +  {117, 53, Level}, /* SPI-1 Irq */
> >> +  {118, 54, Level}, /* SPI-0 Irq */
> >> +  {120, 55, Level}, /* I2C 0 Irq */
> >> +  {121, 56, Level}, /* I2C 1 Irq */
> >> +  {122, 57, Level}, /* UART 0 Irq */
> >> +  {123, 58, Level}, /* UART 1 Irq */
> >> +  {124, 59, Level}, /* UART 2 Irq */
> >> +  {125, 60, Level}, /* UART 3 Irq */
> >> +  {127, 61, Level}, /* GOP-3 Irq */
> >> +  {128, 62, Level}, /* GOP-2 Irq */
> >> +  {129, 63, Level}, /* GOP-0 Irq */
> >> +};
> >> +
> >> +/*
> >> + * SEI - System Error Interrupts
> >> + * Note: SPI ID 0-20 are reserved for North-Bridge
> >> + */
> >> +STATIC ICU_IRQ IrqMapSei[] = {
> >> +  {11,  21, Level}, /* SEI error CP-2-CP */
> >> +  {15,  22, Level}, /* PIDI-64 SOC */
> >> +  {16,  23, Level}, /* D2D error Irq */
> >> +  {17,  24, Level}, /* D2D Irq */
> >> +  {18,  25, Level}, /* NAND error */
> >> +  {19,  26, Level}, /* PCIx4 error */
> >> +  {20,  27, Level}, /* PCIx1_0 error */
> >> +  {21,  28, Level}, /* PCIx1_1 error */
> >> +  {25,  29, Level}, /* SDIO reg error */
> >> +  {75,  30, Level}, /* IOB error */
> >> +  {94,  31, Level}, /* EIP150 error */
> >> +  {97,  32, Level}, /* XOR-1 system error */
> >> +  {99,  33, Level}, /* XOR-0 system error */
> >> +  {108, 34, Level}, /* SATA-1 error */
> >> +  {110, 35, Level}, /* SATA-0 error */
> >> +  {114, 36, Level}, /* TDM-MC error */
> >> +  {116, 37, Level}, /* DFX server Irq */
> >> +  {117, 38, Level}, /* Device bus error */
> >> +  {147, 39, Level}, /* Audio error */
> >> +  {171, 40, Level}, /* PIDI Sync error */
> >> +};
> >> +
> >> +/* REI - RAM Error Interrupts */
> >> +STATIC CONST ICU_IRQ IrqMapRei[] = {
> >> +  {12,  0, Level}, /* REI error CP-2-CP */
> >> +  {26,  1, Level}, /* SDIO memory error */
> >> +  {87,  2, Level}, /* EIP-197 ECC error */
> >> +  {93,  3, Edge},  /* EIP-150 RAM error */
> >> +  {96,  4, Level}, /* XOR-1 memory Irq */
> >> +  {98,  5, Level}, /* XOR-0 memory Irq */
> >> +  {100, 6, Edge},  /* USB3 device tx parity */
> >> +  {101, 7, Edge},  /* USB3 device rq parity */
> >> +  {103, 8, Edge},  /* USB3H-1 RAM error */
> >> +  {104, 9, Edge},  /* USB3H-0 RAM error */
> >> +};
> >> +
> >> +STATIC CONST ICU_CONFIG IcuConfigDefault = {
> >
> > The combination of CONST and Default confuses me slightly, as it
> > mildly implies there can be other config objects floating around -
> > whereas this just appears to refer to what the hardware is configured
> > to on every boot.
> > .
> > Would IcuInitialConfig work as well?
> 
> Ok, will rename.
> 
> >
> >> +  .NonSecure =  { IrqMapNonSecure, ARRAY_SIZE (IrqMapNonSecure) },
> >> +  .Sei =        { IrqMapSei, ARRAY_SIZE (IrqMapSei) },
> >> +  .Rei =        { IrqMapRei, ARRAY_SIZE (IrqMapRei) },
> >> +};
> >> +
> >> +STATIC
> >> +VOID
> >> +IcuClearIrq (
> >> +  IN UINTN IcuBase,
> >> +  IN UINTN Nr
> >> +)
> >> +{
> >> +  MmioWrite32 (IcuBase + ICU_INT_CFG (Nr), 0);
> >> +}
> >> +
> >> +STATIC
> >> +VOID
> >> +IcuSetIrq (
> >> +  IN UINTN           IcuBase,
> >> +  IN CONST ICU_IRQ  *Irq,
> >> +  IN UINTN           SpiBase,
> >> +  IN ICU_GROUP       Group
> >> +  )
> >> +{
> >> +  UINT32 IcuInt;
> >> +
> >> +  IcuInt  = (Irq->SpiId + SpiBase) | (1 << ICU_INT_ENABLE_OFFSET);
> >> +  IcuInt |= Irq->IrqType << ICU_IS_EDGE_OFFSET;
> >> +  IcuInt |= Group << ICU_GROUP_OFFSET;
> >> +
> >> +  MmioWrite32 (IcuBase + ICU_INT_CFG (Irq->IcuId), IcuInt);
> >> +}
> >> +
> >> +STATIC
> >> +VOID
> >> +IcuConfigure (
> >> +  IN UINTN             CpIndex,
> >> +  IN MV_SOC_ICU_DESC  *IcuDesc,
> >> +  IN CONST ICU_CONFIG *Config
> >> +  )
> >> +{
> >> +  UINTN IcuBase, Index, SpiOffset, SpiBase;
> >> +  CONST ICU_IRQ *Irq;
> >> +  ICU_MSI *Msi;
> >> +
> >> +  /* Get ICU registers base address */
> >> +  IcuBase = ICU_REG_BASE (CpIndex);
> >> +  /* Get the base of the GIC SPI ID in the MSI message */
> >> +  SpiBase = IcuDesc->IcuSpiBase;
> >> +  /* Get multiple CP110 instances SPI ID shift */
> >> +  SpiOffset = CpIndex * ICU_MAX_IRQS_PER_CP;
> >> +  /* Get MSI addresses per interrupt group */
> >> +  Msi = IcuDesc->IcuMsi;
> >> +
> >> +  /* Set the address for SET_SPI and CLR_SPI registers in AP */
> >> +  for (Index = 0; Index < ICU_GROUP_MAX; Index++, Msi++) {
> >> +    MmioWrite32 (IcuBase + ICU_SET_SPI_AL (Msi->Group), Msi->SetSpiAddr & 0xFFFFFFFF);
> >> +    MmioWrite32 (IcuBase + ICU_SET_SPI_AH (Msi->Group), Msi->SetSpiAddr >> 32);
> >> +    MmioWrite32 (IcuBase + ICU_CLR_SPI_AL (Msi->Group), Msi->ClrSpiAddr & 0xFFFFFFFF);
> >> +    MmioWrite32 (IcuBase + ICU_CLR_SPI_AH (Msi->Group), Msi->ClrSpiAddr >> 32);
> >
> > Hmm, this common operation (split 64 into 2x32) feels like something
> > EDK2 would have a macro for, but I can't find one. I.e. POINTER_HIGH32
> > and POINTER_LOW32.
> > I see it would be useful in other platforms' PCI code as well.
> >
> > Anyway - the above 4 lines are all too long.
> >
> 
> Lines are now 86 - 79 - 86 - 79 signs long - I didn't break them for
> the sake of readability, but I'll do it then.

Right, sorry - two of them are fine.

I don't complain about line lengths where they don't hide information,
but these ones end up getting cut off in the middle of a mask.

/
    Leif

> > /
> >     Leif
> >
> >> +  }
> >> +
> >> +  /* Mask all ICU interrupts */
> >> +  for (Index = 0; Index < MAX_ICU_IRQS; Index++) {
> >> +    IcuClearIrq (IcuBase, Index);
> >> +  }
> >> +
> >> +  /* Configure the ICU interrupt lines */
> >> +  Irq = Config->NonSecure.Map;
> >> +  for (Index = 0; Index < Config->NonSecure.Size; Index++, Irq++) {
> >> +    IcuSetIrq (IcuBase, Irq, SpiBase + SpiOffset, ICU_GROUP_NSR);
> >> +  }
> >> +
> >> +  Irq = Config->Sei.Map;
> >> +  for (Index = 0; Index < Config->Sei.Size; Index++, Irq++) {
> >> +    IcuSetIrq (IcuBase, Irq, SpiBase, ICU_GROUP_SEI);
> >> +  }
> >> +
> >> +  Irq = Config->Rei.Map;
> >> +  for (Index = 0; Index < Config->Rei.Size; Index++, Irq++) {
> >> +    IcuSetIrq (IcuBase, Irq, SpiBase, ICU_GROUP_REI);
> >> +  }
> >> +}
> >> +
> >> +STATIC
> >> +VOID
> >> +IcuClearGicSpi (
> >> +  IN UINTN             CpIndex,
> >> +  IN MV_SOC_ICU_DESC  *IcuDesc
> >> +  )
> >> +{
> >> +  CONST ICU_CONFIG *Config;
> >> +  UINTN Index, SpiOffset, SpiBase;
> >> +  CONST ICU_IRQ *Irq;
> >> +  ICU_MSI *Msi;
> >> +
> >> +  Config = &IcuConfigDefault;
> >> +
> >> +  /* Get the base of the GIC SPI ID in the MSI message */
> >> +  SpiBase = IcuDesc->IcuSpiBase;
> >> +  /* Get multiple CP110 instances SPI ID shift */
> >> +  SpiOffset = CpIndex * ICU_MAX_IRQS_PER_CP;
> >> +  /* Get MSI addresses per interrupt group */
> >> +  Msi = IcuDesc->IcuMsi;
> >> +
> >> +  /* Clear ICU-generated GIC SPI interrupts */
> >> +  Irq = Config->NonSecure.Map;
> >> +  for (Index = 0; Index < Config->NonSecure.Size; Index++, Irq++) {
> >> +    MmioWrite32 (Msi->ClrSpiAddr, Irq->SpiId + SpiBase + SpiOffset);
> >> +  }
> >> +}
> >> +
> >> +VOID
> >> +EFIAPI
> >> +IcuCleanUp (
> >> +  IN EFI_EVENT  Event,
> >> +  IN VOID      *Context
> >> +  )
> >> +{
> >> +  MV_SOC_ICU_DESC *IcuDesc;
> >> +  UINTN CpCount, CpIndex;
> >> +
> >> +  IcuDesc = Context;
> >> +
> >> +  CpCount = FixedPcdGet8 (PcdMaxCpCount);
> >> +  if (CpCount > ICU_MAX_SUPPORTED_UNITS) {
> >> +    CpCount = ICU_MAX_SUPPORTED_UNITS;
> >> +  }
> >> +
> >> +  for (CpIndex = 0; CpIndex < CpCount; CpIndex++) {
> >> +    IcuClearGicSpi (CpIndex, IcuDesc);
> >> +  }
> >> +}
> >> +
> >> +EFI_STATUS
> >> +EFIAPI
> >> +ArmadaIcuInitialize (
> >> +  )
> >> +{
> >> +  MV_SOC_ICU_DESC *IcuDesc;
> >> +  UINTN CpCount, CpIndex;
> >> +  EFI_STATUS Status;
> >> +
> >> +  /*
> >> +   * Due to limited amount of interrupt lanes, only 2 units can be
> >> +   * wired to the GIC.
> >> +   */
> >> +  CpCount = FixedPcdGet8 (PcdMaxCpCount);
> >> +  if (CpCount > ICU_MAX_SUPPORTED_UNITS) {
> >> +    DEBUG ((DEBUG_ERROR,
> >> +      "%a: Default ICU to GIC mapping is available for maximum %d CP110 units",
> >> +      ICU_MAX_SUPPORTED_UNITS,
> >> +      __FUNCTION__));
> >> +    CpCount = ICU_MAX_SUPPORTED_UNITS;
> >> +  }
> >> +
> >> +  /* Obtain SoC description of the ICU */
> >> +  Status = ArmadaSoCDescIcuGet (&IcuDesc);
> >> +  if (EFI_ERROR (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >> +  /* Configure default ICU to GIC interrupt mapping for each CP110 */
> >> +  for (CpIndex = 0; CpIndex < CpCount; CpIndex++) {
> >> +    IcuConfigure (CpIndex, IcuDesc, &IcuConfigDefault);
> >> +  }
> >> +
> >> +  /*
> >> +   * In order to be immune to the OS capability of clearing ICU-generated
> >> +   * GIC interrupts, register ExitBootServices event, that will
> >> +   * make sure they remain disabled during OS boot.
> >> +   */
> >> +  Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> >> +                  TPL_NOTIFY,
> >> +                  IcuCleanUp,
> >> +                  IcuDesc,
> >> +                  &EfiExitBootServicesEvent);
> >> +
> >> +  return Status;
> >> +}
> >> --
> >> 2.7.4
> >>


  reply	other threads:[~2018-07-12 11:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-12  7:39 [platforms: PATCH 0/6] Armada7k8k ICU support Marcin Wojtas
2018-07-12  7:39 ` [platforms: PATCH 1/6] Marvell/Armada70x0Db: Set correct CP110 count Marcin Wojtas
2018-07-12  7:58   ` Ard Biesheuvel
2018-07-12  7:39 ` [platforms: PATCH 2/6] Marvell/Library: Introduce ArmadaIcuLib class Marcin Wojtas
2018-07-12 14:35   ` Ard Biesheuvel
2018-07-12  7:39 ` [platforms: PATCH 3/6] Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address Marcin Wojtas
2018-07-12 14:37   ` Ard Biesheuvel
2018-07-12  7:39 ` [platforms: PATCH 4/6] Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information Marcin Wojtas
2018-07-13  6:48   ` Ard Biesheuvel
2018-07-12  7:40 ` [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib Marcin Wojtas
2018-07-12  7:57   ` Ard Biesheuvel
2018-07-12  8:42     ` Marcin Wojtas
2018-07-12 10:35   ` Leif Lindholm
2018-07-12 10:59     ` Marcin Wojtas
2018-07-12 11:12       ` Leif Lindholm [this message]
2018-07-12  7:40 ` [platforms: PATCH 6/6] Marvell/Armada7k8k: Enable ICU configuration Marcin Wojtas
2018-07-13  6:49   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180712111238.w3ysz6iamo2pq25l@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox