From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::442; helo=mail-wr1-x442.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) (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 67458211963C7 for ; Tue, 4 Dec 2018 07:42:57 -0800 (PST) Received: by mail-wr1-x442.google.com with SMTP id b14so2868209wru.12 for ; Tue, 04 Dec 2018 07:42:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=2pTs98nBW1tn5YdueU0XfMrqCagJ90Fi2lhP3useamQ=; b=cl8RO8Py0koW+nR/ajz4thWvMR6S9FYnxdBACvT2Y0vFuo+jtk3zjbYYWUxA4UUDkn HrAmqdw8z65zfz+kE/qdXo+o3MVGlDHqNjWmxapuvSUcdl1oJODCs8+3pMvz3IGaC4Zs U5jsZAoWV1JObYQOvKBScTYFdQXcB6REUCCCk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=2pTs98nBW1tn5YdueU0XfMrqCagJ90Fi2lhP3useamQ=; b=tcek0ukWmlHPtaHKADrDcSP4McpuihECW/fNdlhyAA/i8Trnj61pd6pm2EmYIcEvsE 5RgO3E44/SASRf/G80xoLL7LNxrTsFo7tK54NV0/x4ZPR07f8iUdilcmObA+MJuumaUg l3NR+r3/pWfapfuqzxs3KRCWtdOocxolzyLEscWR5oaoI2EhCB/Q+crnbRmqPmYDn3Km hSyU76ZJkcafZ4eww3kCNwhoDYJYlNaXU8fEcfS6/+5ln3+D3JSdtx1fFOsvsjRgu8Nf Cg9y+UcNn2UMctCrPanZSTuptwlGkPMhjkejhZ2ZzoPkkABJ143Z2F1wV4KCKWkq7zUj Ql8A== X-Gm-Message-State: AA+aEWatwik62eEJbN1GB8m8Bge1SkV+e6zPn6Iv7M6edTeQ7dCl1fnq Wktl8h/ioZP0XixviA4gz1BniqeskVI= X-Google-Smtp-Source: AFSGD/VDPxKA6GJNCap6PvEE3dMd9KwnBx0NGJ5hgSOFfWHtd+DFd/jCOXZz8HmWwJWxTMXOOXbp/g== X-Received: by 2002:adf:e78f:: with SMTP id n15mr19009487wrm.115.1543938175686; Tue, 04 Dec 2018 07:42:55 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id m4sm6236347wmc.3.2018.12.04.07.42.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 04 Dec 2018 07:42:54 -0800 (PST) Date: Tue, 4 Dec 2018 15:42:53 +0000 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, jsd@semihalf.com, jaz@semihalf.com, kostap@marvell.com Message-ID: <20181204154253.c2hp25av3iwgupwu@bivouac.eciton.net> References: <1540000661-1956-1-git-send-email-mw@semihalf.com> <1540000661-1956-7-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1540000661-1956-7-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 06/12] Marvell/Drivers: MvBoardDesc: Extend protocol with GPIO support 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: Tue, 04 Dec 2018 15:42:57 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, Oct 20, 2018 at 03:57:35AM +0200, Marcin Wojtas wrote: > Introduce new callback that can provide information > about GPIO SoC controllers, as well as on-board > I2C IO expanders. According ArmadaSoCDescLib > ArmadaBoardDescLib routines are used for > obtaining required data. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf | 1 + > Silicon/Marvell/Include/Protocol/BoardDesc.h | 8 ++++ > Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 48 ++++++++++++++++++++ > 3 files changed, 57 insertions(+) > > diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf > index 41f72d6..0b93948 100644 > --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf > +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf > @@ -47,6 +47,7 @@ > Silicon/Marvell/Marvell.dec > > [LibraryClasses] > + ArmadaBoardDescLib > ArmadaSoCDescLib > DebugLib > MemoryAllocationLib > diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h b/Silicon/Marvell/Include/Protocol/BoardDesc.h > index 1d57a16..e06d689 100644 > --- a/Silicon/Marvell/Include/Protocol/BoardDesc.h > +++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h > @@ -50,6 +50,13 @@ EFI_STATUS > > typedef > EFI_STATUS > +(EFIAPI *MV_BOARD_DESC_GPIO_GET) ( > + IN MARVELL_BOARD_DESC_PROTOCOL *This, > + IN OUT MV_BOARD_GPIO_DESC **GpioDesc > + ); > + > +typedef > +EFI_STATUS > (EFIAPI *MV_BOARD_DESC_I2C_GET) ( > IN MARVELL_BOARD_DESC_PROTOCOL *This, > IN OUT MV_BOARD_I2C_DESC **I2cDesc > @@ -106,6 +113,7 @@ VOID > struct _MARVELL_BOARD_DESC_PROTOCOL { > MV_BOARD_DESC_AHCI_GET BoardDescAhciGet; > MV_BOARD_DESC_COMPHY_GET BoardDescComPhyGet; > + MV_BOARD_DESC_GPIO_GET BoardDescGpioGet; > MV_BOARD_DESC_I2C_GET BoardDescI2cGet; > MV_BOARD_DESC_MDIO_GET BoardDescMdioGet; > MV_BOARD_DESC_PP2_GET BoardDescPp2Get; > diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c > index 39dc06c..948a6a0 100644 > --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c > +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c > @@ -100,6 +100,53 @@ MvBoardDescComPhyGet ( > > STATIC > EFI_STATUS > +MvBoardDescGpioGet ( > + IN MARVELL_BOARD_DESC_PROTOCOL *This, > + IN OUT MV_BOARD_GPIO_DESC **GpioDesc > + ) > +{ > + MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc; > + UINTN SoCGpioCount, I2cIoExpanderCount; > + STATIC MV_BOARD_GPIO_DESC *BoardDesc; > + MV_SOC_GPIO_DESC *SoCDesc; > + EFI_STATUS Status; > + > + if (BoardDesc != NULL) { > + *GpioDesc = BoardDesc; > + return EFI_SUCCESS; > + } For a completely opposite comment to the previous patch: I would much prefer for BoardDesc to be a globally visible variable with a g prefix and proper namespace. Even though it itself is not globally visible, the above maneuver makes it effectively so, but not semantically. It's not like there's a million call sites, so I would prefer if the check was there rather than in this function. > + > + /* Get SoC data about all available GPIO controllers */ > + Status = ArmadaSoCDescGpioGet (&SoCDesc, &SoCGpioCount); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + /* Get per-board information about all available I2C IO expanders */ > + Status = ArmadaBoardDescGpioGet (&I2cIoExpanderDesc, &I2cIoExpanderCount); Same I2C->GPIO comments apply here. > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + /* Allocate and fill board description */ > + BoardDesc = AllocateZeroPool (sizeof (MV_BOARD_GPIO_DESC)); > + if (BoardDesc == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)); > + return EFI_OUT_OF_RESOURCES; > + } > + > + BoardDesc->SoC = SoCDesc; > + BoardDesc->GpioDevCount = SoCGpioCount; > + BoardDesc->I2cIoExpanderDesc = I2cIoExpanderDesc; > + BoardDesc->I2cIoExpanderCount = I2cIoExpanderCount; > + > + *GpioDesc = BoardDesc; > + > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > MvBoardDescI2cGet ( > IN MARVELL_BOARD_DESC_PROTOCOL *This, > IN OUT MV_BOARD_I2C_DESC **I2cDesc > @@ -556,6 +603,7 @@ MvBoardDescInitProtocol ( > { > BoardDescProtocol->BoardDescAhciGet = MvBoardDescAhciGet; > BoardDescProtocol->BoardDescComPhyGet = MvBoardDescComPhyGet; > + BoardDescProtocol->BoardDescGpioGet = MvBoardDescGpioGet; > BoardDescProtocol->BoardDescI2cGet = MvBoardDescI2cGet; Even more so because of the two lines above :) / Leif > BoardDescProtocol->BoardDescMdioGet = MvBoardDescMdioGet; > BoardDescProtocol->BoardDescPp2Get = MvBoardDescPp2Get; > -- > 2.7.4 >