From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4864:20::143; helo=mail-it1-x143.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D90E721A07A82 for ; Tue, 13 Nov 2018 22:05:15 -0800 (PST) Received: by mail-it1-x143.google.com with SMTP id o19so13006038itg.5 for ; Tue, 13 Nov 2018 22:05:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=AOcbMoImrGYLEsiGGrjqRKq010igiGtyyqCLaUJte0U=; b=Lefuo11w0kEsxIeB60IzsO2gkzFkwXpoqrNBLK/eXDse8V0yN9qlQH+b3IYvzPe9oQ 1ej0Xqplv1xJvtyPYoeNzI0noT7a97wQUvDXaMSfGiicddqVSEpJiH/mUG/rKJGlZlKD M59nh/9Vr+oehhIss88qoYAKl5SZOfUShHCHDeM7yXHnUbrzkQeyuoWU/QUDehQU/zi1 OnN+1z/ZUaNYhA1FDfVdxcuypC4ref72IU2nSDA8PDabaGXPB1zEb1vUVj+APx7mWm1X hEdg0H1XVYuc6FolOBLO4sFvxo8XqczZfqXdfPpzeRPXCer36du68TEwi9fZBRZitARf na9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=AOcbMoImrGYLEsiGGrjqRKq010igiGtyyqCLaUJte0U=; b=bAqm9FPILY1d38a4IrLJa/rqcJvCIdZDtob+8nsvSTUhocHOknOFo97fasBRYokmQT JC2BA0j14AidqJbrT/4tYwmWd/4RvhenpPHsM1gDNFVL7w5nOG7onWltWhdRIdOaSD1J e0U8VIF5PPJfa8ncLTTfRFCnARI7W0t/S9iGn14FNn8dTogoqPxzcPZ1CEbzlvF7sKf8 PsdwKAwcfYDEC2Q+VcG6vACYmIGtFzth2xslKDyqWoiGxoPkuHvcKNIc/teLAk/+fLuS l/UMCl9+iGovR43CAh5E2nY1PPbaEDRMpPQhQ/eAicUX//Kz6rL8jOl29O9uQjVPf1cW 7POw== X-Gm-Message-State: AGRZ1gKwEFiHldBaL07iJS93DgJrI09cft3egZ9zyDPhwxUS0cnbZ9mX aujmTPEdRF3tN5natMnnskqxcfsuIZ9mWTb82XMHrA== X-Google-Smtp-Source: AFSGD/XGgU0UTU6KREHDMGaSCo0E/YiKpuhOOXKIqLATL4EJSydc+Uae2pBaeP3nJKMd9KHneXZ5TqH5dPI2S2CNZi4= X-Received: by 2002:a05:660c:681:: with SMTP id n1mr816324itk.37.1542175514973; Tue, 13 Nov 2018 22:05:14 -0800 (PST) MIME-Version: 1.0 References: <1540000661-1956-1-git-send-email-mw@semihalf.com> <1540000661-1956-2-git-send-email-mw@semihalf.com> <20181114011044.inajftqy7qmfq7n3@bivouac.eciton.net> In-Reply-To: <20181114011044.inajftqy7qmfq7n3@bivouac.eciton.net> From: Marcin Wojtas Date: Wed, 14 Nov 2018 07:05:01 +0100 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, "jsd@semihalf.com" , Grzegorz Jaszczyk , Kostya Porotchkin Subject: Re: [platforms: PATCH 01/12] Marvell/Library: ArmadaSoCDescLib: Add GPIO information X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Nov 2018 06:05:16 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Leif, =C5=9Br., 14 lis 2018 o 02:10 Leif Lindholm napi= sa=C5=82(a): > > On Sat, Oct 20, 2018 at 03:57:30AM +0200, Marcin Wojtas wrote: > > This patch introduces new library callback (ArmadaSoCDescGpioGet ()), > > which dynamically allocates and fills MV_SOC_GPIO_DESC structure with > > the SoC description of GPIO controllers. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCD= escLib.h | 10 +++++ > > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h = | 15 ++++++++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCD= escLib.c | 39 ++++++++++++++++++++ > > 3 files changed, 64 insertions(+) > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Ar= mada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDesc= Lib/Armada7k8kSoCDescLib.h > > index c14b985..85dd67c 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8= kSoCDescLib.h > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8= kSoCDescLib.h > > @@ -22,6 +22,7 @@ > > // Common macros > > // > > #define MV_SOC_CP_BASE(Cp) (0xF2000000 + ((Cp) * 0x20000= 00)) > > +#define MV_SOC_AP_COUNT 1 > > I think all of my comments on this patch can be summarised as "what is > an AP in this context"? > > The term either needs explicit documenting, or expansion in the macro > names such that documentation is not required. Isn't a glossary on top of this file: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/8ff9b1= 3675a401588d3cc999aef9891838047b18/Silicon/Marvell/Armada7k8k/Library/Armad= a7k8kSoCDescLib/Armada7k8kSoCDescLib.h#L13 and C file: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/8ff9b1= 3675a401588d3cc999aef9891838047b18/Silicon/Marvell/Armada7k8k/Library/Armad= a7k8kSoCDescLib/Armada7k8kSoCDescLib.c#L13 sufficient? Thanks, Marcin > > > > > // > > // Platform description of AHCI controllers > > @@ -38,6 +39,15 @@ > > #define MV_SOC_COMPHY_MUX_BITS 4 > > > > // > > +// Platform description of GPIO controllers > > +// > > +#define MV_SOC_AP_GPIO_BASE 0xF06F5040 > > +#define MV_SOC_AP_GPIO_PIN_COUNT 20 > > +#define MV_SOC_GPIO_PER_CP_COUNT 2 > > +#define MV_SOC_CP_GPIO_BASE(Gpio) (0x440100 + ((Gpio) * 0x40)) > > +#define MV_SOC_CP_GPIO_PIN_COUNT(Gpio) ((Gpio) =3D=3D 0 ? 32 : 31) > > + > > +// > > // Platform description of I2C controllers > > // > > #define MV_SOC_I2C_PER_CP_COUNT 2 > > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silic= on/Marvell/Include/Library/ArmadaSoCDescLib.h > > index cdfb51b..f3d4f80 100644 > > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > @@ -46,6 +46,21 @@ ArmadaSoCDescCpBaseGet ( > > ); > > > > // > > +// GPIO devices description template definition > > +// > > +typedef struct { > > + UINTN GpioBaseAddress; > > + UINTN GpioPinCount; > > +} MV_SOC_GPIO_DESC; > > + > > +EFI_STATUS > > +EFIAPI > > +ArmadaSoCDescGpioGet ( > > + IN OUT MV_SOC_GPIO_DESC **GpioDesc, > > + IN OUT UINTN *DescCount > > + ); > > + > > +// > > // I2C > > // > > typedef struct { > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Ar= mada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDesc= Lib/Armada7k8kSoCDescLib.c > > index 6902fda..7db4ec7 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8= kSoCDescLib.c > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8= kSoCDescLib.c > > @@ -74,6 +74,45 @@ ArmadaSoCDescCpBaseGet ( > > > > EFI_STATUS > > EFIAPI > > +ArmadaSoCDescGpioGet ( > > + IN OUT MV_SOC_GPIO_DESC **GpioDesc, > > + IN OUT UINTN *DescCount > > + ) > > +{ > > + MV_SOC_GPIO_DESC *Desc; > > + UINTN CpCount, CpIndex, Index; > > + > > + CpCount =3D FixedPcdGet8 (PcdMaxCpCount); > > + > > + *DescCount =3D CpCount * MV_SOC_GPIO_PER_CP_COUNT + MV_SOC_AP_COUNT; > > + Desc =3D AllocateZeroPool (*DescCount * sizeof (MV_SOC_GPIO_DESC)); > > + if (Desc =3D=3D NULL) { > > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)= ); > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + *GpioDesc =3D Desc; > > + > > + /* AP GPIO controller */ > > + Desc->GpioBaseAddress =3D MV_SOC_AP_GPIO_BASE; > > + Desc->GpioPinCount =3D MV_SOC_AP_GPIO_PIN_COUNT; > > + Desc++; > > + > > + /* CP GPIO controllers */ > > + for (CpIndex =3D 0; CpIndex < CpCount; CpIndex++) { > > + for (Index =3D 0; Index < MV_SOC_GPIO_PER_CP_COUNT; Index++) { > > + Desc->GpioBaseAddress =3D MV_SOC_CP_BASE (CpIndex) + > > + MV_SOC_CP_GPIO_BASE (Index); > > + Desc->GpioPinCount =3D MV_SOC_CP_GPIO_PIN_COUNT (Index); > > + Desc++; > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > ArmadaSoCDescI2cGet ( > > IN OUT MV_SOC_I2C_DESC **I2cDesc, > > IN OUT UINTN *DescCount > > -- > > 2.7.4 > >