Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib

[Marcin Wojtas <mw@semihalf.com>]

2017-10-09 18:23 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Mon, Oct 09, 2017 at 06:06:53PM +0200, Marcin Wojtas wrote:
>> 2017-10-09 18:00 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> > On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
>> >> This patch introduces UTMI description, using the new structures
>> >> and template in MvHwDescLib. This change enables more flexible
>> >> addition of multiple CP with UTMI PHY's and also significantly
>> >> reduces amount of used PCD's for that purpose. Update PortingGuide
>> >> documentation accordingly.
>> >>
>> >> This patch replaces string-based description of Utmi on
>> >> Armada 70x0 DB with new, reduced format, which uses macros
>> >> in Armada.dsc.inc file for better readability.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> >> ---
>> >>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
>> >>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
>> >>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
>> >>  Platform/Marvell/Marvell.dec                       |   7 +-
>> >>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
>> >>  8 files changed, 148 insertions(+), 110 deletions(-)
>> >>
>
>> > This indentation does not appear to follow any of the patterns
>> > permitted by the coding style. Please address here and in the two
>> > instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).
>> >
>> > No need to resubmit the whole series - just the single patch.
>>
>> Sure, will send right away.
>
> Thx.

Rebased patches are available here:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/parsepcd-upstream-r20171009

>
>> >>  Example
>> >>  -------
>> >>
>> >>               # UtmiPhy
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
>> >> +               gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
>> >> +               gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
>> >
>> > Actually, looking at this bit made me realise the PortingGuide.txt
>> > uses tab characters and uses \n line endings.
>> >
>> > This is not caused by this set, so does not need to be addressed as
>> > part of this series, but if you could follow up with a patch adjusting
>> > the formating of documentation, I would be grateful.
>> >
>> > This is also made painfully clear when running CheckPatch.py.
>> >
>>
>> Well, yes. It's in Sphinx acceptable format
>
> Apart from a massive structure in Egypt, what is a Sphinx?
>
>> and it is generated into
>> Marvell documentation along with all other projects. If I align it to
>> edk2 coding style, I'd have to maintain 2 copies of this file... If
>> you insist, I'll change it, but I would be very grateful for accepting
>> this one exception, if possible:) Please let know your decision
>> (looking at my branch, there won't be much updates of this file).
>
> Well, I don't see how we can keep a file that makes PatchCheck.py
> barf whenever we touch it. If you want to keep this format, please
> send a proposal to exclude files of this type from said checks by
> PatchCheck.py to edk2-devel.
>
> Is there a default "Sphinx source" file extension that can be used to
> describe this format, rather than changing the rule for anything
> called ".txt"? (If not, should we make one?)
>

Ok, never mind - I'll change it.

Thanks,
Marcin