On 03/21/16 10:58, Ard Biesheuvel wrote:
> On 21 March 2016 at 10:44, Laszlo Ersek <[email protected]> wrote:
>> On 03/21/16 10:33, Ard Biesheuvel wrote:
>>> On 14 March 2016 at 13:52, Laszlo Ersek <[email protected]> wrote:
>>>> The explanation is in the patch titled
>>>>
>>>> OvmfPkg: introduce gRootBusesConnectedProtocolGuid
>>>>
>>>> At this point, this protocol doesn't do anything yet.
>>>>
>>>> Cc: Ard Biesheuvel <[email protected]>
>>>> Cc: Gerd Hoffmann <[email protected]>
>>>> Cc: Jordan Justen <[email protected]>
>>>> Cc: Marcel Apfelbaum <[email protected]>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Laszlo Ersek <[email protected]>
>>>> ---
>>>> ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 1 +
>>>> ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 11
>>>> +++++++++++
>>>> 2 files changed, 12 insertions(+)
>>>>
>>>> diff --git
>>>> a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>>> b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>>> index 79ba7b2afbf7..bc0eab100319 100644
>>>> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>>> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>>> @@ -74,7 +74,8 @@ [Guids]
>>>>
>>>> [Protocols]
>>>> gEfiDevicePathProtocolGuid
>>>> gEfiGraphicsOutputProtocolGuid
>>>> gEfiLoadedImageProtocolGuid
>>>> gEfiPciRootBridgeIoProtocolGuid
>>>> gEfiSimpleFileSystemProtocolGuid
>>>> + gRootBusesConnectedProtocolGuid
>>>> diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>>> b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>>> index b242a293a103..c8acfb1e86a6 100644
>>>> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>>> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>>> @@ -21,14 +21,15 @@
>>>> #include <Library/PlatformBdsLib.h>
>>>> #include <Library/QemuBootOrderLib.h>
>>>> #include <Protocol/DevicePath.h>
>>>> #include <Protocol/GraphicsOutput.h>
>>>> #include <Protocol/PciIo.h>
>>>> #include <Protocol/PciRootBridgeIo.h>
>>>> #include <Guid/EventGroup.h>
>>>> +#include <Protocol/RootBusesConnected.h>
>>>
>>> As noted in response to 1/5, if you only ever refer to the guid by its
>>> name 'gEfiPciRootBridgeIoProtocolGuid', I don't think this header is
>>> necessary.
>>
>> I think the argument you cited for 1/5 is not correct.
>>
>> Even if it was, it would break away from existing practice. So I'd like
>> to see a reference to that argument (and to the agreement that edk2 in
>> general will adopt that pattern going forward).
>>
>
> I don't care deeply about this, so I am not going to debate this any
> further. IIRC it was mentioned to me in a session between some ARM
> engineers, myself and Eugene Cohem, whom I think was the person
> mentioning this (I could be wrong, though)
>
> In general, including the file allows you to get away with using these
> GUIDs without declaring your dependency upon them in the .INF.
That's not correct. Including the header will give you only the
declaration; it won't give you the definition, and linking the module
will fail. The definition is in "AutoGen.c", and depends on listing the
protocol or the GUID in the INF file.
> As to
> your point regarding static initializers, I suppose that indeed still
> required a header file of the sort you are introducing here. So my
> point pertains to the extern symbol declaration only, which is
> arguably redundant under EDK2.
After checking an AutoGen.h file, I sort of agree.
"Sort of", because I also checked "Source/Python/AutoGen/GenC.py". It
has code like this:
if Info.AutoGenVersion >= 0x00010005:
CreateGuidDefinitionCode(Info, AutoGenC, AutoGenH)
CreateProtocolDefinitionCode(Info, AutoGenC, AutoGenH)
If we say that our INF files are universally >= 0x00010005, then I agree
there's no reason to include the extern declaration in the protocol
header file.
> But by all means, let's decouple this from the review of this series.
Thanks. Two recent Protocol/*.h additions were a48d0a3d72f9 and
78741ce91e12, those also added the extern declarations. I agree the
declarations shouldn't be duplicated, but then we should all start
following this new rule.
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
