[edk2-devel] [PATCH v4 3/9] OvmfPkg/PciHostBridgeLib: Extract InitRootBridge/UninitRootBridge
Jiahui Cen via groups.io
cenjiahui=huawei.com at groups.io
Wed Jan 13 06:00:24 UTC 2021
Hi Laszlo,
On 2021/1/13 9:28, Laszlo Ersek wrote:
> 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.
>
Right. I will fix them.
> (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().)
>
For the original PciHostBridgeLib, should I fix the dependency
in a new patch? Or just in this patch?
Thanks,
Jiahui
>
> 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 (#70195): https://edk2.groups.io/g/devel/message/70195
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list