From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c06::22c; helo=mail-io0-x22c.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22c.google.com (mail-io0-x22c.google.com [IPv6:2607:f8b0:4001:c06::22c]) (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 E43FB21EA15B6 for ; Fri, 6 Oct 2017 12:18:50 -0700 (PDT) Received: by mail-io0-x22c.google.com with SMTP id l15so16956635iol.8 for ; Fri, 06 Oct 2017 12:22:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=m9Q/h5YOBTSjx2mVJ59gy9sEufFnm25uLfZWdku+BUw=; b=cr7fNbZTelvBmsKx3R5ahEdYFUMy6SykvEOOoT0Qss6A8fsweYQ4NQ0yh8dVgDo6ly FjggF5HrlcFddX37pEv4D5HlJ59QsHYTIHQNqSzQ7u+WDm0uyTVQ/d58hv0yK5xSSnIP m4cfGsUJ68cv5QdMfRssfgKxWBOC5fjC20er6vA3dpkvks76INiEh0TgRn/8SLdpFHvG OGoChRw56Yl0plEENZN2r9020r47ExBQU6MwByIS7CBjovqTmu+jcSQQM7DbauFAfH2x 4nRpjx0gs3dbMGEaSXVxv6ildSL/zcr+UFJHakwWvu1ONzpmfnWkhSSfQPLkEc62Y1rQ M1Sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=m9Q/h5YOBTSjx2mVJ59gy9sEufFnm25uLfZWdku+BUw=; b=j7m5FG4ya7rmruFkmXYtrKMv/u8fBomA3oylWdEsxg4Hl/f7vBB7ut6sbM7T/dG+m6 K3ZwaLMJYoShy7FKWklVgIQ5hdmIyuHr9IyBxmovzTVXTjxF1RmmC758KDN41yTWxXmt eekh4y7DWq3PX3pKFqVYEJj6C5wK9kl+A5Q3oia/rWgdlAWLJCBH8iJ6dDSTOcm7U/um 7/ahqA2q6/9Oa5Ns+8UXTTmjvAz/KHDWkJDQ6fRgXovWXzjLKYLNf+vCUA70xXz4OXiC 601Uq67E5Jyy6F7Sypbz/uKEF2/QUEgYaSYSumO55fqM4Q4dhmS5DjHbF+/ynHbI/t7e QLYQ== X-Gm-Message-State: AMCzsaURQjMtTSL2v15+U+Kl0DZjiwOYl08Bh8Q18rCiUW3LiT6lbJQC mM4v6tIWVypSttIrmYRI/23bdkE61oZd/ezHtRueTA== X-Google-Smtp-Source: AOwi7QAhZtO7HaJJ6aMC8qMbe0F5BYANzqUB0mMQWCfjldFRND7wF4+joYvFMzKXxd76g/rpnYL1fo9/RAidthucxmI= X-Received: by 10.107.184.194 with SMTP id i185mr3741901iof.155.1507317734189; Fri, 06 Oct 2017 12:22:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.157.141 with HTTP; Fri, 6 Oct 2017 12:22:13 -0700 (PDT) In-Reply-To: <20171006155215.g63kakcrvhx7ocra@bivouac.eciton.net> References: <1507276278-3608-1-git-send-email-mw@semihalf.com> <1507276278-3608-2-git-send-email-mw@semihalf.com> <20171006155215.g63kakcrvhx7ocra@bivouac.eciton.net> From: Marcin Wojtas Date: Fri, 6 Oct 2017 21:22:13 +0200 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , semihalf-dabros-jan Subject: Re: [platforms: PATCH 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing 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 19:18:51 -0000 Content-Type: text/plain; charset="UTF-8" Hi Leif, 2017-10-06 17:52 GMT+02:00 Leif Lindholm : > On Fri, Oct 06, 2017 at 09:51:14AM +0200, Marcin Wojtas wrote: >> Simplify obtaining lane data, using arrays with direct enum values, >> rather than strings. This is another step to completely remove >> ParsePcdLib. >> >> This patch replaces string-based description of ComPhy lanes >> on Armada 70x0 DB with the enum values of type and speed. >> PortingGuide is updated accordingly. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Marcin Wojtas >> --- >> Platform/Marvell/Armada/Armada70x0.dsc | 11 +++- >> Platform/Marvell/Library/ComPhyLib/ComPhyLib.c | 65 ++++---------------- >> Platform/Marvell/Library/ComPhyLib/ComPhyLib.h | 25 +++----- >> Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf | 1 - >> Silicon/Marvell/Documentation/PortingGuide.txt | 62 ++++++++++++++----- >> 5 files changed, 77 insertions(+), 87 deletions(-) >> >> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc >> index 467dfa3..7b03744 100644 >> --- a/Platform/Marvell/Armada/Armada70x0.dsc >> +++ b/Platform/Marvell/Armada/Armada70x0.dsc >> @@ -100,8 +100,15 @@ >> >> #ComPhy >> gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 } >> - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2" >> - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000" >> + # ComPhy0 >> + # 0: SGMII1 1.25 Gbps >> + # 1: USB3_HOST0 5 Gbps >> + # 2: SFI 10.31 Gbps >> + # 3: SATA1 5 Gbps >> + # 4: USB3_HOST1 5 Gbps >> + # 5: PCIE2 5 Gbps >> + gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{ 0xA, 0xE, 0x17, 0x6, 0xF, 0x3 } >> + gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ 0x1, 0x6, 0xA, 0x6, 0x6, 0x6 } > > So, I'm really pleased with this patchset, but these two lines above > are its weak spot. I know, this is why I placed acomment above... > > I am not sure about the best way to deal with this, but some way of > getting human readable strings above would be ideal (even if it blows > the line length out of the water). > > Could you create a dsc.inc (or .inc.dsc as suggested somewhere > recently) that holds only a [Defines] section declaring things like > COMPHY_SGMII1 = 0xA > COMPHY_1_25G = 0x1 > > And so on. (Basically all the ones defined in the document below.) > ? Of course I can do it. Two doubts however: 1. Wouldn't line with Pcd swell to too many signs? gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{COMPHY_SGMII1, COMPHY_USB3_HOST0, COMPHY_SFI, COMPHY_SATA1, COMPHY_USB3_HOST1, COMPHY_PCIE2} Alternatively we can shorten this to: gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{SGMII1, USB3_HOST0, SFI, SATA1, USB3_HOST1, PCIE2} Which one do you prefer? 2. If I define stuff e.g. in the .dsc [Defines] section - will they be visible in all modules, i.e. would I be able to remove the definitions from the ComPhy header? If yes, I guess the longer version above will have to be used... Best regards, Marcin