Re: [PATCH v3] ArmPkg/ArmScmiDxe: Add clock enable function

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 14 Dec 2018 at 00:48, Jeff Brasen <jbrasen@nvidia.com> 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 <jbrasen@nvidia.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Grish Pathak <girish.pathak@arm.com>

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
>