From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::235; helo=mail-wm0-x235.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x235.google.com (mail-wm0-x235.google.com [IPv6:2a00:1450:400c:c09::235]) (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 7E25821EA15D5 for ; Fri, 6 Oct 2017 08:52:51 -0700 (PDT) Received: by mail-wm0-x235.google.com with SMTP id u138so8386865wmu.5 for ; Fri, 06 Oct 2017 08:56:15 -0700 (PDT) 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=Oej3QMYO2BusLIUczllcfmXsvwEMRiHoTNpIW9aqP6M=; b=NCeumO3w/uBIwnHvNgYOuZlN/6+q8XUWfcYNqaKgYio5Fk5zEX/GDpHqREBSaOO2Sf D8SYAYFJ/31FFSFZRZ73XcszDO7f3SDxWc1kYNRrdQpbiHhZBBZS71xrtRLjj1g+UgeU Jp0PeDm+93RnalX1k6jexkqDGlBgey+cgbIEg= 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=Oej3QMYO2BusLIUczllcfmXsvwEMRiHoTNpIW9aqP6M=; b=Z8A9miKV5gSaNNJhR0XS6DdNkCIO1ukMmjwwT5DTNVTgvdrD+7c3nbSK83QeLpG5fg Auvk/3VH/9M/0rtQSDjiXQsZjAEXrQ6KWzuaBkasDlpdgGaShqTL85Ll+WxnZ9JB9zGR 5t4ryHVrHqVXUkX1VuT7KAazJ1nqlEMf9NgMEmjrA5xeRQSuSWmiLFPGwOCPy2acEHhW U+0etUfRv7xe1Aa927Iz64d2J3uIt0I/Wds7jvQRHZw8lWpCpxgIm7B5DbHb3ZM4Pd1I VsyD7HJBhrFXgRHMhqjTvI15KrLpT+GqTv1b8nNEZjOgAb1UrKjv3OVdDsj7SL3LL38q CptQ== X-Gm-Message-State: AMCzsaVRC1HyS/lYYpKLbazoOMz27oXDZmLJFbOpnmWFso95LFgLa5bo dNmwsxWDrPorAWF84p+xs0nprw== X-Google-Smtp-Source: AOwi7QDVmEtU0juopGEgICJpuykSdqkC7fBOH52qsqqPRu9k+XoPTy5ec61Vx06leirzm6FEMOeyvw== X-Received: by 10.28.209.2 with SMTP id i2mr2041299wmg.153.1507305373868; Fri, 06 Oct 2017 08:56:13 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id i136sm2441653wmd.23.2017.10.06.08.56.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 06 Oct 2017 08:56:12 -0700 (PDT) Date: Fri, 6 Oct 2017 16:56:11 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, neta@marvell.com, kostap@marvell.com, jinghua@marvell.com, jsd@semihalf.com Message-ID: <20171006155611.ethg4wwts3zwmhsi@bivouac.eciton.net> References: <1507276278-3608-1-git-send-email-mw@semihalf.com> <1507276278-3608-3-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1507276278-3608-3-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Oct 2017 15:52:51 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 06, 2017 at 09:51:15AM +0200, Marcin Wojtas wrote: > This patch introduces I2c description, using the new structures > and template in MvHwDescLib. This change enables more flexible > addition of multiple I2c controllers and also allows for > removal of string PCD parsing. Update Armada 70x0 DB description > and PortingGuide accordingly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Platform/Marvell/Armada/Armada70x0.dsc | 2 +- > Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 42 +++++++++++--------- > Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf | 3 +- > Platform/Marvell/Include/Library/MvHwDescLib.h | 25 ++++++++++++ > Platform/Marvell/Marvell.dec | 2 +- > Silicon/Marvell/Documentation/PortingGuide.txt | 4 +- > 6 files changed, 54 insertions(+), 24 deletions(-) > > diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc > index 7b03744..d9d126d 100644 > --- a/Platform/Marvell/Armada/Armada70x0.dsc > +++ b/Platform/Marvell/Armada/Armada70x0.dsc > @@ -78,7 +78,7 @@ > # I2C > gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60 } > gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0 } > - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100" > + gMarvellTokenSpaceGuid.PcdI2cControllers|{ 0x1, 0x1 } Similarly to the previous patch, the above is a bit opaque. I guess in this case, this is simply a boolean array saying whether a specific controller is enabled or not? If my interpretation is correct, could the Pcd be renamed PcdI2cControllersEnabled? / Leif > gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57 } > gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0 } > gMarvellTokenSpaceGuid.PcdI2cClockFrequency|250000000 > diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > index fa79ebc..ff8006e 100755 > --- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > +++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c > @@ -42,12 +42,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > #include > #include > #include > -#include > #include > +#include > #include > > #include "MvI2cDxe.h" > > +DECLARE_A7K8K_I2C_TEMPLATE; > + > STATIC MV_I2C_BAUD_RATE baud_rate; > > STATIC MV_I2C_DEVICE_PATH MvI2cDevicePathProtocol = { > @@ -172,35 +174,39 @@ MvI2cInitialise ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > + MVHW_I2C_DESC *Desc = &mA7k8kI2cDescTemplate; > + UINT8 *I2cDeviceTable, Index; > EFI_STATUS Status; > - UINT32 BusCount; > - EFI_PHYSICAL_ADDRESS I2cBaseAddresses[PcdGet32 (PcdI2cBusCount)]; > - INTN i; > > - BusCount = PcdGet32 (PcdI2cBusCount); > - if (BusCount == 0) > - return EFI_SUCCESS; > + /* Obtain table with enabled I2c devices */ > + I2cDeviceTable = (UINT8 *)PcdGetPtr (PcdI2cControllers); > + if (I2cDeviceTable == NULL) { > + DEBUG ((DEBUG_ERROR, "Missing PcdI2cControllers\n")); > + return EFI_INVALID_PARAMETER; > + } > > - Status = ParsePcdString ( > - (CHAR16 *) PcdGetPtr (PcdI2cBaseAddresses), > - BusCount, > - I2cBaseAddresses, > - NULL > - ); > - if (EFI_ERROR(Status)) > - return Status; > + if (PcdGetSize (PcdI2cControllers) > MVHW_MAX_I2C_DEVS) { > + DEBUG ((DEBUG_ERROR, "Wrong PcdI2cControllers format\n")); > + return EFI_INVALID_PARAMETER; > + } > + > + /* Initialize enabled chips */ > + for (Index = 0; Index < PcdGetSize (PcdI2cControllers); Index++) { > + if (!MVHW_DEV_ENABLED (I2c, Index)) { > + DEBUG ((DEBUG_ERROR, "Skip I2c chip %d\n", Index)); > + continue; > + } > > - for (i = 0; i < BusCount; i++) { > Status = MvI2cInitialiseController( > ImageHandle, > SystemTable, > - I2cBaseAddresses[i] > + Desc->I2cBaseAddresses[Index] > ); > if (EFI_ERROR(Status)) > return Status; > } > > - return Status; > + return EFI_SUCCESS; > } > > STATIC > diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf > index 16374ef..c453b4f 100755 > --- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf > +++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf > @@ -55,7 +55,6 @@ > UefiLib > UefiDriverEntryPoint > UefiBootServicesTableLib > - ParsePcdLib > > [Protocols] > gEfiI2cMasterProtocolGuid > @@ -66,7 +65,7 @@ > [Pcd] > gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses > gMarvellTokenSpaceGuid.PcdI2cSlaveBuses > - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses > + gMarvellTokenSpaceGuid.PcdI2cControllers > gMarvellTokenSpaceGuid.PcdI2cClockFrequency > gMarvellTokenSpaceGuid.PcdI2cBaudRate > gMarvellTokenSpaceGuid.PcdI2cBusCount > diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h > index 6a86865..e029b50 100644 > --- a/Platform/Marvell/Include/Library/MvHwDescLib.h > +++ b/Platform/Marvell/Include/Library/MvHwDescLib.h > @@ -60,6 +60,16 @@ typedef struct { > } MVHW_COMPHY_DESC; > > // > +// I2C devices description template definition > +// > +#define MVHW_MAX_I2C_DEVS 4 > + > +typedef struct { > + UINT8 I2cDevCount; > + UINTN I2cBaseAddresses[MVHW_MAX_I2C_DEVS]; > +} MVHW_I2C_DESC; > + > +// > // NonDiscoverable devices description template definition > // > #define MVHW_MAX_XHCI_DEVS 4 > @@ -130,6 +140,21 @@ MVHW_COMPHY_DESC mA7k8kComPhyDescTemplate = {\ > } > > // > +// Platform description of I2C devices > +// > +#define MVHW_CP0_I2C0_BASE 0xF2701000 > +#define MVHW_CP0_I2C1_BASE 0xF2701100 > +#define MVHW_CP1_I2C0_BASE 0xF4701000 > +#define MVHW_CP1_I2C1_BASE 0xF4701100 > + > +#define DECLARE_A7K8K_I2C_TEMPLATE \ > +STATIC \ > +MVHW_I2C_DESC mA7k8kI2cDescTemplate = {\ > + 4,\ > + { MVHW_CP0_I2C0_BASE, MVHW_CP0_I2C1_BASE, MVHW_CP1_I2C0_BASE, MVHW_CP1_I2C1_BASE }\ > +} > + > +// > // Platform description of NonDiscoverable devices > // > #define MVHW_CP0_XHCI0_BASE 0xF2500000 > diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec > index fc00f1a..25a4258 100644 > --- a/Platform/Marvell/Marvell.dec > +++ b/Platform/Marvell/Marvell.dec > @@ -113,7 +113,7 @@ > gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0 }|VOID*|0x3000184 > gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x0 }|VOID*|0x3000050 > gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0 }|VOID*|0x3000185 > - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|{ 0x0 }|VOID*|0x3000047 > + gMarvellTokenSpaceGuid.PcdI2cControllers|{ 0x0 }|VOID*|0x3000047 > gMarvellTokenSpaceGuid.PcdI2cClockFrequency|0|UINT32|0x3000048 > gMarvellTokenSpaceGuid.PcdI2cBaudRate|0|UINT32|0x3000049 > gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183 > diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt > index 47a667d..1a30c46 100644 > --- a/Silicon/Marvell/Documentation/PortingGuide.txt > +++ b/Silicon/Marvell/Documentation/PortingGuide.txt > @@ -188,8 +188,8 @@ In order to enable driver on a new platform, following steps need to be taken: > (buses to which accoring slaves are attached) > - gMarvellTokenSpaceGuid.PcdI2cBusCount|2 > (number of SoC's I2C buses) > - - gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100" > - (base addresses of I2C controller buses) > + - gMarvellTokenSpaceGuid.PcdI2cControllers|{ 0x1, 0x1 } > + (array with used controllers) > - gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000 > (I2C host controller clock frequency) > - gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000 > -- > 1.8.3.1 >