On 2023-11-10 09:11, Pierre Gondois wrote:
Hello Leif,

On 11/2/23 11:20, Pierre Gondois wrote:
Hello Leif,
Thanks for the review,

On 10/26/23 12:05, Leif Lindholm wrote:
On Wed, Oct 25, 2023 at 13:25:30 +0200, pierre.gond...@arm.com wrote:
From: Pierre Gondois <pierre.gond...@arm.com>

Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
versions of the protocol. The macro is neither used in edk2 nor
in edk2-platforms.

OK, so slight nitpick, but mainly because it parses a bit weirdly...
*Will* it be used after this series is merged, or is this an update
for completeness?

The 'fast channels' were added in the v2.0 SCMI specification. This patch-set
relies on this feature, so it is checked in:
     [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib
that the underlying SCP is at least at this version.

```
     // FastChannels were added in SCMI v2.0 spec.
     if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
       DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
       return EFI_UNSUPPORTED;
     }
```


Signed-off-by: Pierre Gondois <pierre.gond...@arm.com>
---
   ArmPkg/Include/Library/ArmLib.h                     |  1 +
   .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 ++++++++-----
   2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 0169dbc1092c..7b2b2238fed9 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -780,6 +780,7 @@ EFIAPI
   ArmHasVhe (
     VOID
     );
+
   #endif // MDE_CPU_AARCH64
   #ifdef MDE_CPU_ARM
diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index 7e548e4765c2..8e8e05d5a5f6 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -1,12 +1,12 @@
   /** @file
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
     SPDX-License-Identifier: BSD-2-Clause-Patent
-  System Control and Management Interface V1.0
-    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-    DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface, latest version:

I see this as a pattern throughout the series.
But this statement will at some point become untrue; this
implementation is written against a specific version. I think this
version shold be reflected in the comment. (And that applies
throughout the series.)

+  - https://developer.arm.com/documentation/den0056/latest/

But I think the above is the most useful link.

I was referring to this point I'm not sure I understood.

Oh, right.

I was referring to "latest version" being a moving target.
If I go to https://developer.arm.com/documentation/den0056/latest/, that currently means "version 3.2". At some point in the future, that number will change, but this code won't automatically get updated.

So I'd prefer something like
"System Control and Management Interface v3.2,
Latest version of the specification can be downloaded from
https://developer.arm.com/documentation/den0056/latest/";

If that makes more sense?

Regards,

Leif

Regards,
Pierre


I am not sure I understand completely. Do you mean that the SCMI
structures/interfaces defined in:
     ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
and that were written against the SCMI v1.0 specification should
not be used as such for other SCMI specification version ?
I.e. the same process as for the AcpiXX.h files
(MdePkg/Include/IndustryStandard/Acpi65.h) should be used ?

Or do you mean that the _CPC object generation implies that the
SCP should comply to the v2.0 version at least and this should be
reflected in the commit messages ?

Regards,
Pierre


/
      Leif

+
   **/
   #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
@@ -14,7 +14,10 @@
   #include <Protocol/ArmScmi.h>
-#define PERFORMANCE_PROTOCOL_VERSION  0x10000
+/// Arm Scmi performance protocol versions.
+#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
+#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
+#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
   #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
     0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
--
2.25.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111038): https://edk2.groups.io/g/devel/message/111038
Mute This Topic: https://groups.io/mt/102175810/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to