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:c0c::235; helo=mail-wr0-x235.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x235.google.com (mail-wr0-x235.google.com [IPv6:2a00:1450:400c:c0c::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 D4874210F1579 for ; Mon, 18 Jun 2018 10:09:21 -0700 (PDT) Received: by mail-wr0-x235.google.com with SMTP id v13-v6so17601866wrp.13 for ; Mon, 18 Jun 2018 10:09:21 -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=Jhbnh/sF52xuR484G6wj+YvXYj7kr94mjHzZaxOwjFc=; b=Zt7KsZmJe8IPpNfhzEAlwOYFox0HOWMtt6BqrBP3Vq7OKZd5Aipa9aKnLcBi7L+fXs o5PHhzq4pSHd/ORtUusHzsA1yIhR/LugZ7osIIKHBHFrWsp4zP32HxtezsZXOBezi98T MuLajs+SCJ0+onVdltYz3IRvNwKpZ6M/pRQGs= 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=Jhbnh/sF52xuR484G6wj+YvXYj7kr94mjHzZaxOwjFc=; b=NXoAnXPA3fX9v1io+9kVxasNbvFinxHG2TFZnmJ9bWgmBzR0PZmtDqQXEUqCSGNhBY EfC1q2KqC0cucysl5zYTFfWz0iZwRLpwlozm98ERI1IEyiP4SGjZnSJVU4IfrZIbXvoO xgOm7QePt1eDqwZXT/eU0c8GcPnferNeJxZsz3wXjnPhh/ihHms2nmnwCreos1iWy3qH Ijr6Yg1fDm4ROvob2UZAYuiPK+Hfv0ZlikecIKAA6l3PqDeF3+F5CZriM9NZ71rjJu2h 74CY+1cFuldFbo+D+7ZAlkBFIHSwYK9dv9T6TvUogjdMqDp5tLM2anVRqH/WCXjVX3pc h9hw== X-Gm-Message-State: APt69E3mqBiEvxWAzLrrRh2m9UxHWNI6alJjolOct/fhtyJv4Z0ORJkk p+E2SpWwUZG0oPHXUVkXZBDN0w== X-Google-Smtp-Source: ADUXVKKYxq+KpdDVdvJ98D0wekhu2NeIMbukTBIraSWU0zskklr+sRFNhriy4z2YIjEvcJlkTtKpRA== X-Received: by 2002:adf:dd8c:: with SMTP id x12-v6mr12128056wrl.212.1529341760157; Mon, 18 Jun 2018 10:09:20 -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 y30-v6sm18967530wrd.70.2018.06.18.10.09.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 18 Jun 2018 10:09:18 -0700 (PDT) Date: Mon, 18 Jun 2018 18:09:17 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Hua Jing , semihalf-dabros-jan , Grzegorz Jaszczyk Message-ID: <20180618170917.skga4b6odfi2pmuy@bivouac.eciton.net> References: <1529266325-18371-1-git-send-email-mw@semihalf.com> <1529266325-18371-2-git-send-email-mw@semihalf.com> <20180618154411.u7pv4om3z4yucvvc@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH v2 01/25] Marvell/Library: Introduce ArmadaSoCDescLib class X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 17:09:22 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jun 18, 2018 at 05:58:42PM +0200, Marcin Wojtas wrote: > Hi Leif, > > 2018-06-18 17:44 GMT+02:00 Leif Lindholm : > > On Sun, Jun 17, 2018 at 10:11:41PM +0200, Marcin Wojtas wrote: > >> From: jinghua > >> > >> ArmadaSoCDescLib is a per SoC family library, which provides SoC > >> description, like register base of some hardware module controller, > >> COMPHY/I2C/NETWORK etc., which right now is hardcoded in MvHwDescLib.h. > >> There will be a new protocol, which gets SoC description from this > >> library, and provides board description based on enable/disable > >> values of each hardware module controller in dsc file. > >> > >> As a first example implement obtaining UTMI controllers information. > >> Remaining interfaces will be added in follow-up commits. > >> This patch introduces new library callback (ArmadaSoCDescUtmiGet ()), > >> which dynamically allocates and fills MV_SOC_UTMI_DESC structure, > >> SoC description of UTMI PHYs. A new PCD is introduced (PcdMaxCpCount) > >> which stores maximal amount of CP110 blocks in the SoC family. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: jinghua > > > > Several of these pre-existing Signed-off-by remain in this set. I did > > only comment it on 2/25, but it remains there as well. > > As I understood, the problem was 'Reviewed-by' from internal review - > all those were dropped. So far when the commit was authored by someone > else (at least in OpenPlatformPkg times) we used to leave the > authorship and authors 'Signed-off-by' - in such case I had to add my > own Signed-off-by, as I was sending this to the lists. We may have been poor at communicating (and if so I apologise) and potentially on spotting this before, but we did mean both. You are the one contributing this to the project, so we need your sign-off that the code is actually under the license you say it is and that you are permitted to contribute it. Including Jinghua's Signed-off-by on initial submission is effectively you saying "Jinghua says this is fine to distribute under this license", which is nice and all but untraceable and hence of no legal value. Jinghua's effort get recognised by the Author tag because you've kept them in the From: field. Now, if you and Jinghua were playing tag team and sending different sets that got merged together, then sure. We treat Signed-off-by (nearly) exactly like Linux. We may not be as stringent on explicitly mentioning ourselves as signing off on changes we may make on pushing. I see a few patches in the tree where you and Ard have been working together in public, and then the multiple signed-off-by make sense. You will also note that the bulk import I did to edk2-platforms does not come with any other Signed-off-by than my own. It does come with a commit message pointing to where I got everything from. Best Regards, Leif > > Please send a v3 with this addressed across all patches. > > But please wait until I've had a chance to go through this set. > > When you do so, this one has > > Reviewed-by: Leif Lindholm > > > > I will comment explicitly on all such patches in this set, apart from > > the one where I already gave Reviewed-by (but please delete all other > > signed-off-bys than your own from those as well). > > I will update according to your wish and the new policy. > > Best regards, > Marcin > > > / > > Leif > > > >> Signed-off-by: Marcin Wojtas > >> --- > >> Silicon/Marvell/Marvell.dec | 4 ++ > >> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf | 37 +++++++++++ > >> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 35 +++++++++++ > >> Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 33 ++++++++++ > >> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 65 ++++++++++++++++++++ > >> 5 files changed, 174 insertions(+) > >> create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf > >> create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > >> create mode 100644 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > >> create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > >> > >> diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > >> index be74b4e..2a92eff 100644 > >> --- a/Silicon/Marvell/Marvell.dec > >> +++ b/Silicon/Marvell/Marvell.dec > >> @@ -60,6 +60,7 @@ > >> gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > >> > >> [LibraryClasses] > >> + ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h > >> SampleAtResetLib|Include/Library/SampleAtResetLib.h > >> > >> [Protocols] > >> @@ -68,6 +69,9 @@ > >> gMarvellPlatformInitCompleteProtocolGuid = { 0x465b8cf7, 0x016f, 0x4ba6, { 0xbe, 0x6b, 0x28, 0x0e, 0x3a, 0x7d, 0x38, 0x6f } } > >> > >> [PcdsFixedAtBuild.common] > >> +#Board description > >> + gMarvellTokenSpaceGuid.PcdMaxCpCount|0x2|UINT8|0x30000072 > >> + > >> #MPP > >> gMarvellTokenSpaceGuid.PcdMppChipCount|0|UINT32|0x30000001 > >> > >> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf > >> new file mode 100644 > >> index 0000000..2b73b73 > >> --- /dev/null > >> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf > >> @@ -0,0 +1,37 @@ > >> +## @file > >> +# > >> +# 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. > >> +# > >> +## > >> + > >> +[Defines] > >> + INF_VERSION = 0x0001001A > >> + BASE_NAME = Armada7k8kDescLib > >> + FILE_GUID = c64f0048-4ca3-4573-b0a6-c2e9e6457285 > >> + MODULE_TYPE = BASE > >> + VERSION_STRING = 1.0 > >> + LIBRARY_CLASS = ArmadaSoCDescLib > >> + > >> +[Sources] > >> + Armada7k8kSoCDescLib.c > >> + > >> +[Packages] > >> + MdeModulePkg/MdeModulePkg.dec > >> + MdePkg/MdePkg.dec > >> + Silicon/Marvell/Marvell.dec > >> + > >> +[LibraryClasses] > >> + DebugLib > >> + IoLib > >> + PcdLib > >> + > >> +[FixedPcd] > >> + gMarvellTokenSpaceGuid.PcdMaxCpCount > >> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > >> new file mode 100644 > >> index 0000000..c5711b0 > >> --- /dev/null > >> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > >> @@ -0,0 +1,35 @@ > >> +/** > >> +* > >> +* 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: > >> +* AP - Application Processor hardware block (Armada 7k8k incorporates AP806) > >> +* CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110) > >> +**/ > >> + > >> +#ifndef __ARMADA7K8K_SOC_DESC_LIB_H__ > >> +#define __ARMADA7K8K_SOC_DESC_LIB_H__ > >> + > >> +// > >> +// Common macros > >> +// > >> +#define MV_SOC_CP_BASE(Cp) (0xF2000000 + ((Cp) * 0x2000000)) > >> + > >> +// > >> +// Platform description of UTMI PHY's > >> +// > >> +#define MV_SOC_UTMI_PER_CP_COUNT 2 > >> +#define MV_SOC_UTMI_ID(Utmi) (Utmi) > >> +#define MV_SOC_UTMI_BASE(Utmi) (0x580000 + ((Utmi) * 0x1000)) > >> +#define MV_SOC_UTMI_CFG_BASE 0x440440 > >> +#define MV_SOC_UTMI_USB_CFG_BASE 0x440420 > >> + > >> +#endif > >> diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > >> new file mode 100644 > >> index 0000000..0d45684 > >> --- /dev/null > >> +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > >> @@ -0,0 +1,33 @@ > >> +/** > >> +* > >> +* 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. > >> +* > >> +**/ > >> +#ifndef __ARMADA_SOC_DESC_LIB_H__ > >> +#define __ARMADA_SOC_DESC_LIB_H__ > >> + > >> +// > >> +// UTMI PHY devices SoC description > >> +// > >> +typedef struct { > >> + UINT8 UtmiPhyId; > >> + UINTN UtmiBaseAddress; > >> + UINTN UtmiConfigAddress; > >> + UINTN UsbConfigAddress; > >> +} MV_SOC_UTMI_DESC; > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +ArmadaSoCDescUtmiGet ( > >> + IN OUT MV_SOC_UTMI_DESC **UtmiDesc, > >> + IN OUT UINTN *DescCount > >> + ); > >> +#endif /* __ARMADA_SOC_DESC_LIB_H__ */ > >> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > >> new file mode 100644 > >> index 0000000..63fb224 > >> --- /dev/null > >> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > >> @@ -0,0 +1,65 @@ > >> +/** > >> +* > >> +* 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: > >> +* AP - Application Processor hardware block (Armada 7k8k incorporates AP806) > >> +* CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110) > >> +**/ > >> + > >> +#include > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#include > >> + > >> +#include "Armada7k8kSoCDescLib.h" > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +ArmadaSoCDescUtmiGet ( > >> + IN OUT MV_SOC_UTMI_DESC **UtmiDesc, > >> + IN OUT UINTN *DescCount > >> + ) > >> +{ > >> + MV_SOC_UTMI_DESC *Desc; > >> + UINTN CpCount, CpIndex, Index, UtmiIndex; > >> + > >> + CpCount = FixedPcdGet8 (PcdMaxCpCount); > >> + > >> + *DescCount = CpCount * MV_SOC_UTMI_PER_CP_COUNT; > >> + Desc = AllocateZeroPool (*DescCount * sizeof (MV_SOC_UTMI_DESC)); > >> + if (Desc == NULL) { > >> + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__)); > >> + return EFI_OUT_OF_RESOURCES; > >> + } > >> + > >> + *UtmiDesc = Desc; > >> + > >> + UtmiIndex = 0; > >> + for (CpIndex = 0; CpIndex < CpCount; CpIndex++) { > >> + for (Index = 0; Index < MV_SOC_UTMI_PER_CP_COUNT; Index++) { > >> + Desc->UtmiPhyId = MV_SOC_UTMI_ID (UtmiIndex); > >> + Desc->UtmiBaseAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_BASE (Index); > >> + Desc->UtmiConfigAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_CFG_BASE; > >> + Desc->UsbConfigAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_UTMI_USB_CFG_BASE; > >> + Desc++; > >> + UtmiIndex++; > >> + } > >> + } > >> + > >> + return EFI_SUCCESS; > >> +} > >> -- > >> 2.7.4 > >>