[edk2,01/21] ArmVirtPkg: introduce FdtClientProtocol

Message ID 1459959319-19293-2-git-send-email-ard.biesheuvel@linaro.org
State Superseded
Headers show

Commit Message

Ard Biesheuvel April 6, 2016, 4:14 p.m.
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.biesheuvel@linaro.org>

---
 ArmVirtPkg/ArmVirtPkg.dec               |  3 +
 ArmVirtPkg/Include/Protocol/FdtClient.h | 89 ++++++++++++++++++++
 2 files changed, 92 insertions(+)

-- 
2.5.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek April 6, 2016, 7:06 p.m. | #1
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.biesheuvel@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.

> +

> +  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 :))

(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.)

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
Ard Biesheuvel April 7, 2016, 7:14 a.m. | #2
On 6 April 2016 at 21:06, Laszlo Ersek <lersek@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.biesheuvel@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

Patch

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
+
+  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__
+
+//
+// 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