Re: [platforms PATCH 00/25] Armada herdware description rework

[Marcin Wojtas <mw@semihalf.com>]

Hi Leif,

2018-06-13 0:48 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> So, I've made all the comments I was going to be making on this set.
>
> Patches 3,5-7,13,20-21 are fine as long as you drop pre-existing
> Signed-off-by and Reviewed-by, so if you get rid of those you can add
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> when reposting.
>
> (If any patch apart from the above didn't receive feedback I've
> fumbled and please let me know.)
>

Thanks a lot for your big review effort. I will go over each patch and
resend after correction.

Best regards,
Marcin

> Regards,
>
> Leif
>
> On Fri, Jun 08, 2018 at 05:33:58PM +0200, Marcin Wojtas wrote:
>> Hi,
>>
>> This big patchset reworks handling of the SoC and Boards' description
>> to the final version. Hitherto mechanism of hardcoded structures
>> and parsing PCDs inside the drivers was unfortunate - it didn't
>> allow to support different SoC families with different number
>> of controllers or base addresses.
>>
>> The main concept is introducing a hardware description layer
>> with the new protocol, which is responsible for providing
>> data to the consumer platforms drivers. Additionally a new
>> SoC and Board description libraries allow to move information
>> from overly used PCDs to C code in an organized manner:
>>
>> ArmadaSoCDescLib + ArmadaBoardDescLib
>> (per SoC family)   (per Board, in next steps more of description
>>        |            of ComPhy, Mpp and others can go there)
>>        |                   |
>>        |                   |
>>        |-> MV_BOARD_DESC <-|
>>                 |
>>                 |
>>                 |
>>         Driver/Library (e.g. ComPhy, I2c, Pp2Dxe)
>>
>> Please don't be discouraged by big amount of patches,
>> they are sort of repeatable: update protocol, libraries and
>> the consumer drivers/libraries until the MvHwDescLib.h
>> header could be completely removed. More details can
>> be found in the commit logs.
>>
>> The patches are available in the github:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/boarddesc-upstream-r20180608
>>
>> I'm looking forward to review and any comments/remarks.
>>
>> Best regards,
>> Marcin
>>
>> Marcin Wojtas (21):
>>   Marvell/Library: Introduce ArmadaBoardDescLib class
>>   Marvell/Library: UtmiPhyLib: Switch to use MARVELL_BOARD_DESC protocol
>>   Marvell/Library: RealTimeClockLib: Simplify obtaining base address
>>   Marvell/Armada7k8k: Extend ArmadaSoCDescLib with PP2 information
>>   Marvell/Drivers: MvBoardDesc: Extend protocol with PP2 support
>>   Marvell/Drivers: Pp2Dxe: Switch to use MARVELL_BOARD_DESC protocol
>>   Marvell/Armada7k8k: Extend ArmadaSoCDescLib with AHCI/SDMMC/XHCI
>>   Marvell/Drivers: MvBoardDesc: Extend protocol with AHCI/SDMMC/XHCI
>>   Marvell/Drivers: NonDiscoverable: Switch to use MARVELL_BOARD_DESC
>>   Marvell/Library: ComPhyLib: Get AHCI data with MARVELL_BOARD_DESC
>>   Marvell/Armada7k8k: Extend ArmadaSoCDescLib with ComPhy information
>>   Marvell/Drivers: MvBoardDesc: Extend protocol with COMPHY support
>>   Marvell/Library: ComPhyLib: Switch library to use MARVELL_BOARD_DESC
>>   Marvell/Armada7k8k: Extend ArmadaSoCDescLib with MDIO information
>>   Marvell/Drivers: MvBoardDesc: Extend protocol with MDIO support
>>   Marvell/Drivers: MvMdioDxe: Enable 64bit addressing
>>   Marvell/Drivers: MvMdioDxe: Switch driver to use MARVELL_BOARD_DESC
>>   Marvell/Armada7k8k: Extend ArmadaSoCDescLib with I2C information
>>   Marvell/Drivers: MvBoardDesc: Extend protocol with I2C support
>>   Marvell/Drivers: MvI2cDxe: Switch driver to use MARVELL_BOARD_DESC
>>   Marvell/Drivers: MvPhyDxe: Remove MvHwDescLib.h dependency
>>
>> jinghua (4):
>>   Marvell/Library: Introduce ArmadaSoCDescLib class
>>   Marvell: Introduce MARVELL_BOARD_DESC_PROTOCOL
>>   Marvell/Drivers: MvBoardDescDxe: Introduce board description driver
>>   Marvell/Armada7k8k: Enable board description driver compilation
>>
>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc                                   |   2 +-
>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf                                   |   1 +
>>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                                    |   2 +
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf               |   2 -
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c   | 338 ++++++++++++
>>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf |  37 ++
>>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c           |  29 +-
>>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf         |   2 +-
>>  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c                               | 573 ++++++++++++++++++++
>>  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h                               |  59 ++
>>  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf                             |  70 +++
>>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c                                  |  37 +-
>>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf                                |   1 +
>>  Silicon/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c                                |  41 +-
>>  Silicon/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf                              |   1 +
>>  Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c                                  |   3 +-
>>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c                                      |  43 +-
>>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf                                    |   2 +-
>>  Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c                  | 100 ++--
>>  Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf                |   6 +-
>>  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h                             |  99 ++++
>>  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h                               | 146 +++++
>>  Silicon/Marvell/Include/Library/MvHwDescLib.h                                    | 290 ----------
>>  Silicon/Marvell/Include/Library/UtmiPhyLib.h                                     |   2 +
>>  Silicon/Marvell/Include/Protocol/BoardDesc.h                                     | 118 ++++
>>  Silicon/Marvell/Include/Protocol/Mdio.h                                          |   4 +-
>>  Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c                                  |  50 +-
>>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c                                    |  74 ++-
>>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h                                    |   4 +
>>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf                                  |   6 +-
>>  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c                                  |  65 +--
>>  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h                                  |   5 +
>>  Silicon/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf                                |   9 +-
>>  Silicon/Marvell/Marvell.dec                                                      |   8 +-
>>  34 files changed, 1694 insertions(+), 535 deletions(-)
>>  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>>  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf
>>  create mode 100644 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
>>  create mode 100644 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.h
>>  create mode 100644 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
>>  create mode 100644 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
>>  create mode 100644 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
>>  delete mode 100644 Silicon/Marvell/Include/Library/MvHwDescLib.h
>>  create mode 100644 Silicon/Marvell/Include/Protocol/BoardDesc.h
>>
>> --
>> 2.7.4
>>