[edk2-devel] [PATCH v4 3/9] OvmfPkg/PciHostBridgeLib: Extract InitRootBridge/UninitRootBridge

[Laszlo Ersek]

On 01/12/21 10:45, Jiahui Cen via groups.io wrote:
> Extract InitRootBridge/UninitRootBridge to PciHostBridgeUtilityLib as
> common utility functions. No change of functionality.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
> 
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Cc: Anthony Perard <anthony.perard at citrix.com>
> Cc: Julien Grall <julien at xen.org>
> Signed-off-by: Jiahui Cen <cenjiahui at huawei.com>
> Signed-off-by: Yubo Miao <miaoyubo at huawei.com>
> ---
>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf               |   2 -
>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf |   7 +
>  OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h                   |  75 ++++++++++
>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h                    |  56 -------
>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c                 | 158 +-------------------
>  OvmfPkg/Library/PciHostBridgeLib/XenSupport.c                       |   3 +-
>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c   | 155 +++++++++++++++++++
>  7 files changed, 243 insertions(+), 213 deletions(-)

(1) Renaming OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH to
PCI_ROOT_BRIDGE_DEVICE_PATH is not only unnecessary, it carries a risk
of conflicting with something that PciHostBridgeLib in MdeModulePkg
might introduce in the future. Please undo this rename.

(2) Renaming "mRootBridgeDevicePathTemplate" to "mRootBridgeDevicePath"
is not necessary. Although this rename is not risky, it makes the review
(before/after comparison) of this patch more difficult than needed.
Please undo this rename. The word "template" is correct BTW, because we
modify the device path after we allocate & copy it from the template --
see UID.

(3) Unfortunately, the original PciHostBridgeLib instance fails to list
its PcdLib dependency, both between the #include directives, and in the
INF file. I'm not asking you to fix that up, but please do spell out the
PcdLib #include and INF file depencency in the new
PciHostBridgeUtilityLib instance. (See the PcdGet16() call in
PciHostBridgeUtilityInitRootBridge().)


With the above small warts fixed, I'm ready to give R-b.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70178): https://edk2.groups.io/g/devel/message/70178
Mute This Topic: https://groups.io/mt/79619794/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-