From: Marcin Wojtas <mw@semihalf.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
nadavh@marvell.com, Neta Zur Hershkovits <neta@marvell.com>,
Kostya Porotchkin <kostap@marvell.com>,
Hua Jing <jinghua@marvell.com>,
semihalf-dabros-jan <jsd@semihalf.com>
Subject: Re: [platforms: PATCH 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing
Date: Fri, 6 Oct 2017 21:22:13 +0200 [thread overview]
Message-ID: <CAPv3WKe7jDX5ZM96iMRw7BidnBYB0DSiSiL8JO8awAsbqScQnQ@mail.gmail.com> (raw)
In-Reply-To: <20171006155215.g63kakcrvhx7ocra@bivouac.eciton.net>
Hi Leif,
2017-10-06 17:52 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> 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 <mw@semihalf.com>
>> ---
>> 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
next prev parent reply other threads:[~2017-10-06 19:18 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-06 7:51 [platforms: PATCH 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
2017-10-06 7:51 ` [platforms: PATCH 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing Marcin Wojtas
2017-10-06 15:52 ` Leif Lindholm
2017-10-06 19:22 ` Marcin Wojtas [this message]
2017-10-06 19:51 ` Leif Lindholm
2017-10-06 20:08 ` Marcin Wojtas
2017-10-07 14:17 ` Marcin Wojtas
2017-10-07 14:57 ` Leif Lindholm
2017-10-06 7:51 ` [platforms: PATCH 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib Marcin Wojtas
2017-10-06 15:56 ` Leif Lindholm
2017-10-06 19:24 ` Marcin Wojtas
2017-10-06 7:51 ` [platforms: PATCH 3/5] Marvell/Library: UtmiLib: " Marcin Wojtas
2017-10-06 15:57 ` Leif Lindholm
2017-10-06 19:26 ` Marcin Wojtas
2017-10-06 19:57 ` Leif Lindholm
2017-10-06 7:51 ` [platforms: PATCH 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling Marcin Wojtas
2017-10-06 16:02 ` Leif Lindholm
2017-10-06 19:30 ` Marcin Wojtas
2017-10-06 7:51 ` [platforms: PATCH 5/5] Platform/Marvell/Armada: Remove ParsePcdLib Marcin Wojtas
2017-10-06 16:02 ` Leif Lindholm
2017-10-06 14:29 ` [platforms: PATCH 0/5] Armada 7k/8k - ParsePcdLib removal Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPv3WKe7jDX5ZM96iMRw7BidnBYB0DSiSiL8JO8awAsbqScQnQ@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox