On 6 April 2016 at 21:06, Laszlo Ersek <ler...@redhat.com> wrote:
> On 04/06/16 18:14, Ard Biesheuvel wrote:
>> This introduces the FdtClientProtocol, which will be used to expose the
>> device tree provided by the host to other DXE drivers.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> ---
>> ArmVirtPkg/ArmVirtPkg.dec | 3 +
>> ArmVirtPkg/Include/Protocol/FdtClient.h | 89 ++++++++++++++++++++
>> 2 files changed, 92 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
>> index b6ff63677837..fa908253b320 100644
>> --- a/ArmVirtPkg/ArmVirtPkg.dec
>> +++ b/ArmVirtPkg/ArmVirtPkg.dec
>> @@ -34,6 +34,9 @@ [Guids.common]
>> gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E,
>> 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>> gEarlyPL011BaseAddressGuid = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80,
>> 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>>
>> +[Protocols]
>> + gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2,
>> 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>> +
>> [PcdsFixedAtBuild, PcdsPatchableInModule]
>> #
>> # This is the physical address where the device tree is expected to be
>> stored
>> diff --git a/ArmVirtPkg/Include/Protocol/FdtClient.h
>> b/ArmVirtPkg/Include/Protocol/FdtClient.h
>> new file mode 100644
>> index 000000000000..b7cf8191b5ab
>> --- /dev/null
>> +++ b/ArmVirtPkg/Include/Protocol/FdtClient.h
>> @@ -0,0 +1,89 @@
>> +/** @file
>
> (1) Please steal the disclaimer from the beginning of
> "OvmfPkg/Include/Protocol/VirtioDevice.h", and tack it here.
>
OK
>> +
>> + Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>> +
>> + This program and the accompanying materials
>> + are licensed and made available under the terms and conditions of the BSD
>> License
>> + which accompanies this distribution. The full text of the license may be
>> found at
>> + http://opensource.org/licenses/bsd-license.php
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> +
>> +**/
>> +
>> +#ifndef __FDT_CLIENT_PROTOCOL_H__
>> +#define __FDT_CLIENT_PROTOCOL_H__
>> +
>
> (2) Usually the word PROTOCOL is not added to the include guard. (I'm
> sure you can find some counter-examples, but still :))
>
OK
> (3) Please add a macro here that is suitable for initializing GUID
> objects with static storage duration.
>
> (An "extern EFI_GUID ..." at the end of the file may not be necessary,
> if you are against it -- we seem to have discussed that BaseTools
> generates that declaration. If you don't feel strongly about it, I'd
> prefer to stick with the current tradition. I don't insist.)
>
Actually, I think the conclusion was that it is required in order for
INF versions below 0x00010005 to be able to use this header. So I will
add it.
> Regarding the rest of the patch, it looks sane, but of course all such
> interfaces emerge from the actual callers' needs, so I will try to look
> at the protocol's implementation, and how it is used. Until then, the
> rest looks okay to me.
>
> Thanks!
> Laszlo
>
>> +//
>> +// Protocol interface structure
>> +//
>> +typedef struct _FDT_CLIENT_PROTOCOL FDT_CLIENT_PROTOCOL;
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_GET_NODE_PROPERTY) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + IN INT32 Node,
>> + IN CONST CHAR8 *PropertyName,
>> + OUT CONST VOID **Prop,
>> + OUT UINTN *PropSize OPTIONAL
>> + );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_SET_NODE_PROPERTY) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + IN INT32 Node,
>> + IN CONST CHAR8 *PropertyName,
>> + IN CONST VOID *Prop,
>> + IN UINTN PropSize
>> + );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_FIND_COMPATIBLE_NODE) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + IN CONST CHAR8 *CompatibleString,
>> + OUT INT32 *Node
>> + );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_FIND_COMPATIBLE_NODE_PROPERTY) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + IN CONST CHAR8 *CompatibleString,
>> + IN CONST CHAR8 *PropertyName,
>> + OUT CONST VOID **Prop,
>> + OUT UINTN *PropSize OPTIONAL
>> + );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_FIND_COMPATIBLE_NODE_REG) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + IN CONST CHAR8 *CompatibleString,
>> + OUT CONST VOID **Reg,
>> + OUT UINTN *RegElemSize,
>> + OUT UINTN *RegSize
>> + );
>> +
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *FDT_CLIENT_GET_CHOSEN_NODE) (
>> + IN FDT_CLIENT_PROTOCOL *This,
>> + OUT INT32 *Node
>> + );
>> +
>> +struct _FDT_CLIENT_PROTOCOL {
>> + FDT_CLIENT_GET_NODE_PROPERTY GetNodeProperty;
>> + FDT_CLIENT_SET_NODE_PROPERTY SetNodeProperty;
>> +
>> + FDT_CLIENT_FIND_COMPATIBLE_NODE FindCompatibleNode;
>> + FDT_CLIENT_FIND_COMPATIBLE_NODE_PROPERTY FindCompatibleNodeProperty;
>> + FDT_CLIENT_FIND_COMPATIBLE_NODE_REG FindCompatibleNodeReg;
>> +
>> + FDT_CLIENT_GET_CHOSEN_NODE GetChosenNode;
>> +};
>> +
>> +#endif
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
