Hello Leif,
On 9/12/23 11:35, Leif Lindholm wrote:
Hi Jeff,
On Mon, Sep 11, 2023 at 23:48:57 +0000, Jeff Brasen wrote:
From: Vidya Sagar <vid...@nvidia.com>
Add helper functions to generate AML Resource Data describing I/O
ranges of four words long. API AmlCodeGenRdQWordIo () is exposed.
Reviewed-by: Shanker Donthineni <sdonthin...@nvidia.com>
The above isn't really applicable to upstream.
Although I feel less strongly about that than
Signed-off-by: Vidya Sagar <vid...@nvidia.com>
The DCO is a statement that you have performed basic legal due
diligence on the provenance of the change. I'm uncomfortable with
people making such statements on behalf of others.
If this is being upstreamed from a downstream repository, such that
the review trail is available there, then both of these could be fine.
But I think it would be useful to include a link to the patch in that
repository in the commit message in that case.
One technical, but not necessarily for this set (it just made me spot
it), note below.
Signed-off-by: Jeff Brasen <jbra...@nvidia.com>
---
.../Include/Library/AmlLib/AmlLib.h | 67 ++++++++++++++
.../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 90 +++++++++++++++++++
2 files changed, 157 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 9210c5091548..8e24cecdd77b 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -683,6 +683,73 @@ AmlCodeGenRdWordBusNumber (
OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL
);
+/** Code generation for the "QWordIO ()" ASL function.
+
+ The Resource Data effectively created is a QWord Address Space Resource
+ Data. Cf ACPI 6.4:
+ - s6.4.3.5.1 "QWord Address Space Descriptor".
+ - s19.6.109 "QWordIO".
+
+ The created resource data node can be:
+ - appended to the list of resource data elements of the NameOpNode.
+ In such case NameOpNode must be defined by a the "Name ()" ASL statement
+ and initially contain a "ResourceTemplate ()".
+ - returned through the NewRdNode parameter.
+
+ See ACPI 6.4 spec, s19.6.109 for more.
+
+ @param [in] IsResourceConsumer ResourceUsage parameter.
+ @param [in] IsMinFixed Minimum address is fixed.
+ @param [in] IsMaxFixed Maximum address is fixed.
+ @param [in] IsPosDecode Decode parameter
+ @param [in] IsaRanges Possible values are:
+ 0-Reserved
+ 1-NonISAOnly
+ 2-ISAOnly
+ 3-EntireRange
This is an existing antipattern which this patch (rightly) adheres to
when adding an additional variant of an existing API. But this also
pushes the count to three functions in the same file where we're doing
enum-but-in-doxygen and then keep magic values in the code.
I think someone should rewrite this as an enum and get rid of the
magic values in the callers.
An additional antipattern is that because the doxygen stanza becomes
exessively bulky, it leaves out actually documenting the parameter at
all.
But as I said, that's not the fault of this set, and does not need to
be fixed by it.
Ok yes, I'll make the modifications where required,
Thanks for the review,
Regards,
Pierre
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108525): https://edk2.groups.io/g/devel/message/108525
Mute This Topic: https://groups.io/mt/101305535/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
