On Fri, 21 Dec 2018 at 17:48, Girish Pathak <[email protected]> wrote: > > Apart from some minor indentation errors in function parameter declarations > in ArmScmiClock2Protocol.h , this looks fine. >
Thanks Reviewed-by: Ard Biesheuvel <[email protected]> I've cleaned it up a bit and pushed it as 9bba10eb4336..559a07d84e5a > > -----Original Message----- > From: Ard Biesheuvel <[email protected]> > Sent: 21 December 2018 14:55 > To: Jeff Brasen <[email protected]>; Girish Pathak <[email protected]> > Cc: [email protected]; Leif Lindholm <[email protected]> > Subject: Re: [PATCH v3] ArmPkg/ArmScmiDxe: Add clock enable function > > On Fri, 14 Dec 2018 at 00:48, Jeff Brasen <[email protected]> wrote: > > > > Add function to allow enabling and disabling of the clock using the > > SCMI interface. Add gArmScmiClock2ProtocolGuid to distinguish > > platforms that support new API from those that just have the older protocol. > > > > SCMI_CLOCK2_PROTOCOL also adds a version parameter to allow for future > > changes. It is placed after the functions that are present in the > > existing protocol to allow SCMI_CLOCK2_PROTOCOL to be cast to > > SCMI_CLOCK_PROTOCOL so that only a single implementation of those > > function are needed. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jeff Brasen <[email protected]> > > Cc: Ard Biesheuvel <[email protected]> > > Cc: Leif Lindholm <[email protected]> > > Cc: Grish Pathak <[email protected]> > > I'd like to hear from Girish as well, since he contributed this code > originally. > > > --- > > ArmPkg/ArmPkg.dec | 1 + > > .../ArmScmiDxe/ArmScmiClockProtocolPrivate.h | 7 + > > ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf | 1 + > > ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c | 62 +++++++ > > ArmPkg/Include/Protocol/ArmScmiClock2Protocol.h | 198 > > +++++++++++++++++++++ > > 5 files changed, 269 insertions(+) > > create mode 100644 ArmPkg/Include/Protocol/ArmScmiClock2Protocol.h > > > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index > > d99eb67..2f5e5b3 100644 > > --- a/ArmPkg/ArmPkg.dec > > +++ b/ArmPkg/ArmPkg.dec > > @@ -59,6 +59,7 @@ > > ## Arm System Control and Management Interface(SCMI) Clock management > > protocol > > ## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h > > gArmScmiClockProtocolGuid = { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9, > > 0x9f, 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } } > > + gArmScmiClock2ProtocolGuid = { 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, > > + 0x34, 0x6c, 0x99, 0xfc, 0x05, 0xef, 0xcf } } > > > > ## Arm System Control and Management Interface(SCMI) Clock management > > protocol > > ## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h > > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > > b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > > index 0d1ec6f..c135bac 100644 > > --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > > +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > > @@ -59,6 +59,13 @@ typedef struct { > > CLOCK_RATE_DWORD Rate; > > } CLOCK_RATE_SET_ATTRIBUTES; > > > > + > > +// Message parameters for CLOCK_CONFIG_SET command. > > +typedef struct { > > + UINT32 ClockId; > > + UINT32 Attributes; > > +} CLOCK_CONFIG_SET_ATTRIBUTES; > > + > > // if ClockAttr Bit[0] is set then clock device is enabled. > > #define CLOCK_ENABLE_MASK 0x1 > > #define CLOCK_ENABLED(ClockAttr) ((ClockAttr & CLOCK_ENABLE_MASK) == > > 1) diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf > > b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf > > index 05ce9c0..9b29b9f 100644 > > --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf > > +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf > > @@ -46,6 +46,7 @@ > > [Protocols] > > gArmScmiBaseProtocolGuid > > gArmScmiClockProtocolGuid > > + gArmScmiClock2ProtocolGuid > > gArmScmiPerformanceProtocolGuid > > > > [Depex] > > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > > b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > > index 64d2afa..c7f27a3 100644 > > --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > > +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > > @@ -19,6 +19,7 @@ > > #include <Library/DebugLib.h> > > #include <Library/UefiBootServicesTableLib.h> > > #include <Protocol/ArmScmiClockProtocol.h> > > +#include <Protocol/ArmScmiClock2Protocol.h> > > > > #include "ArmScmiClockProtocolPrivate.h" > > #include "ScmiPrivate.h" > > @@ -388,6 +389,53 @@ ClockRateSet ( > > return Status; > > } > > > > +/** Enable/Disable specified clock. > > + > > + @param[in] This A Pointer to SCMI_CLOCK_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + @param[in] Enable TRUE to enable, FALSE to disable. > > + > > + @retval EFI_SUCCESS Clock enable/disable successful. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +STATIC > > +EFI_STATUS > > +ClockEnable ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + IN BOOLEAN Enable > > + ) > > +{ > > + EFI_STATUS Status; > > + CLOCK_CONFIG_SET_ATTRIBUTES *ClockConfigSetAttributes; > > + SCMI_COMMAND Cmd; > > + UINT32 PayloadLength; > > + > > + Status = ScmiCommandGetPayload > > + ((UINT32**)&ClockConfigSetAttributes); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + // Fill arguments for clock protocol command. > > + ClockConfigSetAttributes->ClockId = ClockId; > > + ClockConfigSetAttributes->Attributes = Enable ? BIT0 : 0; > > + > > + Cmd.ProtocolId = SCMI_PROTOCOL_ID_CLOCK; Cmd.MessageId = > > + SCMI_MESSAGE_ID_CLOCK_CONFIG_SET; > > + > > + PayloadLength = sizeof (CLOCK_CONFIG_SET_ATTRIBUTES); > > + > > + // Execute and wait for response on a SCMI channel. > > + Status = ScmiCommandExecute ( > > + &Cmd, > > + &PayloadLength, > > + NULL > > + ); > > + > > + return Status; > > +} > > + > > // Instance of the SCMI clock management protocol. > > STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol = { > > ClockGetVersion, > > @@ -398,6 +446,18 @@ STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol = { > > ClockRateSet > > }; > > > > +// Instance of the SCMI clock management protocol. > > +STATIC CONST SCMI_CLOCK2_PROTOCOL ScmiClock2Protocol = { > > + (SCMI_CLOCK2_GET_VERSION)ClockGetVersion, > > + (SCMI_CLOCK2_GET_TOTAL_CLOCKS)ClockGetTotalClocks, > > + (SCMI_CLOCK2_GET_CLOCK_ATTRIBUTES)ClockGetClockAttributes, > > + (SCMI_CLOCK2_DESCRIBE_RATES)ClockDescribeRates, > > + (SCMI_CLOCK2_RATE_GET)ClockRateGet, > > + (SCMI_CLOCK2_RATE_SET)ClockRateSet, > > + SCMI_CLOCK2_PROTOCOL_VERSION, > > + ClockEnable > > + }; > > + > > /** Initialize clock management protocol and install protocol on a given > > handle. > > > > @param[in] Handle Handle to install clock management > > protocol. > > @@ -413,6 +473,8 @@ ScmiClockProtocolInit ( > > Handle, > > &gArmScmiClockProtocolGuid, > > &ScmiClockProtocol, > > + &gArmScmiClock2ProtocolGuid, > > + &ScmiClock2Protocol, > > NULL > > ); > > } > > diff --git a/ArmPkg/Include/Protocol/ArmScmiClock2Protocol.h > > b/ArmPkg/Include/Protocol/ArmScmiClock2Protocol.h > > new file mode 100644 > > index 0000000..f2c9670 > > --- /dev/null > > +++ b/ArmPkg/Include/Protocol/ArmScmiClock2Protocol.h > > @@ -0,0 +1,198 @@ > > +/** @file > > + > > + Copyright (c) 2017-2018, Arm Limited. All rights reserved. > > + > > + 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. > > + > > + System Control and Management Interface V1.0 > > + http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/ > > + DEN0056A_System_Control_and_Management_Interface.pdf > > +**/ > > + > > +#ifndef ARM_SCMI_CLOCK2_PROTOCOL_H_ > > +#define ARM_SCMI_CLOCK2_PROTOCOL_H_ > > + > > +#include <Protocol/ArmScmi.h> > > +#include <Protocol/ArmScmiClockProtocol.h> > > + > > +#define ARM_SCMI_CLOCK2_PROTOCOL_GUID { \ > > + 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, 0x34, 0x6c, 0x99, 0xfc, 0x05, > > +0xef, 0xcf } \ > > + } > > + > > +extern EFI_GUID gArmScmiClock2ProtocolGuid; > > + > > +#define SCMI_CLOCK2_PROTOCOL_VERSION 1 > > + > > +typedef struct _SCMI_CLOCK2_PROTOCOL SCMI_CLOCK2_PROTOCOL; > > + > > +// Protocol Interface functions. > > + > > +/** Return version of the clock management protocol supported by SCP > > firmware. > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + > > + @param[out] Version Version of the supported SCMI Clock management > > protocol. > > + > > + @retval EFI_SUCCESS The version is returned. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_GET_VERSION) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + OUT UINT32 *Version > > + ); > > + > > +/** Return total number of clock devices supported by the clock management > > + protocol. > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + > > + @param[out] TotalClocks Total number of clocks supported. > > + > > + @retval EFI_SUCCESS Total number of clocks supported is returned. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_GET_TOTAL_CLOCKS) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + OUT UINT32 *TotalClocks > > + ); > > + > > +/** Return attributes of a clock device. > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + > > + @param[out] Enabled If TRUE, the clock device is enabled. > > + @param[out] ClockAsciiName A NULL terminated ASCII string with the clock > > + name, of up to 16 bytes. > > + > > + @retval EFI_SUCCESS Clock device attributes are returned. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_GET_CLOCK_ATTRIBUTES) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + OUT BOOLEAN *Enabled, > > + OUT CHAR8 *ClockAsciiName > > + ); > > + > > +/** Return list of rates supported by a given clock device. > > + > > + @param[in] This A pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + > > + @param[out] Format SCMI_CLOCK_RATE_FORMAT_DISCRETE: Clock device > > + supports range of clock rates which are > > non-linear. > > + > > + SCMI_CLOCK_RATE_FORMAT_LINEAR: Clock device > > supports > > + range of linear clock rates from Min to Max in > > steps. > > + > > + @param[out] TotalRates Total number of rates. > > + > > + @param[in,out] RateArraySize Size of the RateArray. > > + > > + @param[out] RateArray List of clock rates. > > + > > + @retval EFI_SUCCESS List of clock rates are returned. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval EFI_BUFFER_TOO_SMALL RateArraySize is too small for the result. > > + It has been updated to the size needed. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_DESCRIBE_RATES) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + OUT SCMI_CLOCK_RATE_FORMAT *Format, > > + OUT UINT32 *TotalRates, > > + IN OUT UINT32 *RateArraySize, > > + OUT SCMI_CLOCK_RATE *RateArray > > + ); > > + > > +/** Get clock rate. > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + > > + @param[out] Rate Clock rate. > > + > > + @retval EFI_SUCCESS Clock rate is returned. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_RATE_GET) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + OUT UINT64 *Rate > > + ); > > + > > +/** Set clock rate. > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + @param[in] Rate Clock rate. > > + > > + @retval EFI_SUCCESS Clock rate set success. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_RATE_SET) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + IN UINT64 Rate > > + ); > > + > > +/** Enable/Disable specified clock. > > + Function is only available under gArmScmiClock2ProtocolGuid > > + > > + @param[in] This A Pointer to SCMI_CLOCK2_PROTOCOL Instance. > > + @param[in] ClockId Identifier for the clock device. > > + @param[in] Enable TRUE to enable, FALSE to disable. > > + > > + @retval EFI_SUCCESS Clock enable/disable successful. > > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > > + @retval !(EFI_SUCCESS) Other errors. > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *SCMI_CLOCK2_ENABLE) ( > > + IN SCMI_CLOCK2_PROTOCOL *This, > > + IN UINT32 ClockId, > > + IN BOOLEAN Enable > > + ); > > + > > +typedef struct _SCMI_CLOCK2_PROTOCOL { > > + SCMI_CLOCK2_GET_VERSION GetVersion; > > + SCMI_CLOCK2_GET_TOTAL_CLOCKS GetTotalClocks; > > + SCMI_CLOCK2_GET_CLOCK_ATTRIBUTES GetClockAttributes; > > + SCMI_CLOCK2_DESCRIBE_RATES DescribeRates; > > + SCMI_CLOCK2_RATE_GET RateGet; > > + SCMI_CLOCK2_RATE_SET RateSet; > > + > > + //Extention to original ClockProtocol, added here so > > +SCMI_CLOCK2_PROTOCOL > > + //can be cast to SCMI_CLOCK_PROTOCOL > > + UINTN Version; //For future expandability of protocol > > + SCMI_CLOCK2_ENABLE Enable; > > +} SCMI_CLOCK2_PROTOCOL; > > + > > +#endif /* ARM_SCMI_CLOCK2_PROTOCOL_H_ */ > > + > > -- > > 2.7.4 > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
