From: "PierreGondois" <pierre.gondois@arm.com>
To: devel@edk2.groups.io, jbrasen@nvidia.com
Cc: ardb+tianocore@kernel.org, Sami.Mujawar@arm.com, Alexei.Fedorov@arm.com
Subject: Re: [edk2-devel] [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator
Date: Mon, 12 Sep 2022 12:37:56 +0200 [thread overview]
Message-ID: <ecea6631-d03b-7945-1e64-14e77e97744e@arm.com> (raw)
In-Reply-To: <6bca55eda775212fa16aa8a45e82bb981f1ee8df.1662563529.git.jbrasen@nvidia.com>
Hello Jeff,
Please find some remarks inline:
On 9/7/22 17:15, Jeff Brasen via groups.io wrote:
> Add code to use a token attached to GICC to generate _CPC object on cpus.
>
>
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>
> ---
>
> .../SsdtCpuTopologyGenerator.c | 211 +++++++++++++++++-
>
> 1 file changed, 206 insertions(+), 5 deletions(-)
>
>
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> index 8561f48e1f..ba1f1bd436 100644
>
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
[snip]
>
> @@ -584,6 +770,13 @@ CreateAmlCpuFromProcHierarchy (
>
> ASSERT_EFI_ERROR (Status);
>
I think a 'return Status;' needs to be added here.
> }
>
>
>
> + // If a CPC info is associated with the
>
> + // GicCinfo, create an _CPC method returning them.
>
> + if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
>
> + Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
>
> + ASSERT_EFI_ERROR (Status);
>
> + }
>
> +
>
> return Status;
>
> }
>
>
>
> @@ -934,10 +1127,11 @@ CreateTopologyFromGicC (
>
> IN AML_OBJECT_NODE_HANDLE ScopeNode
>
> )
>
> {
>
> - EFI_STATUS Status;
>
> - CM_ARM_GICC_INFO *GicCInfo;
>
> - UINT32 GicCInfoCount;
>
> - UINT32 Index;
>
> + EFI_STATUS Status;
>
> + CM_ARM_GICC_INFO *GicCInfo;
>
> + UINT32 GicCInfoCount;
>
> + UINT32 Index;
>
> + AML_OBJECT_NODE_HANDLE CpuNode;
>
>
>
> ASSERT (Generator != NULL);
>
> ASSERT (CfgMgrProtocol != NULL);
>
> @@ -961,12 +1155,19 @@ CreateTopologyFromGicC (
>
> ScopeNode,
>
> &GicCInfo[Index],
>
> Index,
>
> - NULL
>
> + &CpuNode
>
> );
>
> if (EFI_ERROR (Status)) {
>
> ASSERT (0);
>
> break;
>
> }
>
> +
>
> + // If a CPC info is associated with the
>
> + // GicCinfo, create an _CPC method returning them.
>
> + if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
>
> + Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, &GicCInfo[Index], CpuNode);
>
> + ASSERT_EFI_ERROR (Status);
Shouln't we consider this as an error and break ? As:
if (EFI_ERROR (Status)) {
ASSERT_EFI_ERROR (Status);
break;
}
>
> + }
>
> } // for
>
>
>
> return Status;
>
prev parent reply other threads:[~2022-09-12 10:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-07 15:15 [PATCH 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
2022-09-12 10:34 ` [edk2-devel] " PierreGondois
2022-09-07 15:15 ` [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
2022-09-12 10:36 ` [edk2-devel] " PierreGondois
2022-09-07 15:15 ` [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
2022-09-12 10:37 ` PierreGondois [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ecea6631-d03b-7945-1e64-14e77e97744e@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox