Re: [PATCH v3 16/16] ArmPkg: Introduce SCMI protocol

[Girish Pathak <Girish.Pathak@arm.com>]

Hi Leif,

> -----Original Message-----
> From: Leif Lindholm <leif.lindholm@linaro.org>
> Sent: 23 April 2018 17:22
> To: Girish Pathak <Girish.Pathak@arm.com>
> Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Matteo Carlini
> <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-
> Fitt@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v3 16/16] ArmPkg: Introduce SCMI protocol
> 
> On Mon, Apr 23, 2018 at 04:06:43PM +0000, Girish Pathak wrote:
> > > From: Leif Lindholm <leif.lindholm@linaro.org>
> > > Sent: 23 April 2018 12:32
> > > To: Girish Pathak <Girish.Pathak@arm.com>
> > > Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org; Matteo
> > > Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt
> > > <Stephanie.Hughes- Fitt@arm.com>; nd <nd@arm.com>
> > > Subject: Re: [PATCH v3 16/16] ArmPkg: Introduce SCMI protocol
> > >
> > > Hmm, I did find a few minor things below that I need to hear back on
> > > before pushing.
> >
> > Thanks for your comments, We can incorporate all the changes
> > suggested, shall I tidy this up and resubmit the patch series ?
> 
> Oh God, please don't.
> 
> If you're happy with all of my comments on previous patches, can you just
> say if you're OK with me folding in the below patch into 16/16 before
> pushing?:

Thanks for this, it's ok to fold it.

Regards,
Girish

> 
> From 50294f90db0c3e7b5b19bc2022b5c1d866d0b8e1 Mon Sep 17 00:00:00
> 2001
> From: Leif Lindholm <leif.lindholm@linaro.org>
> Date: Mon, 23 Apr 2018 12:36:06 +0100
> Subject: [PATCH] fixup scmi
> 
> ---
>  ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h | 2 +-
>  ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c           | 4 ++--
>  ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c     | 8 ++++----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> index 71245c1647..0d1ec6f5ad 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> @@ -30,7 +30,7 @@ typedef struct {
>  #define RATE_FORMAT_SHIFT           12
>  #define RATE_FORMAT_MASK            0x0001
>  #define RATE_FORMAT(RatesFlags)     ((RatesFlags >> RATE_FORMAT_SHIFT)
> \
> -                                      & RATE_FORMAT_MASK)
> +                                     & RATE_FORMAT_MASK)
> 
>  // Number of remaining rates after a call to the SCP, RatesFlag Bits[31:16]
>  #define NUM_REMAIN_RATES_SHIFT        16
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> index fe7edd2a8c..64d2afab72 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> @@ -83,7 +83,7 @@ ClockGetTotalClocks (
> 
>    Status = ScmiGetProtocolAttributes (SCMI_PROTOCOL_ID_CLOCK,
> &ReturnValues);
>    if (EFI_ERROR (Status)) {
> -   return Status;
> +    return Status;
>    }
> 
>    *TotalClocks = SCMI_CLOCK_PROTOCOL_TOTAL_CLKS (ReturnValues[0]);
> @@ -334,7 +334,7 @@ ClockRateGet (
>      return Status;
>    }
> 
> -  *Rate = ((UINT64)ClockRate->High << 32) | ClockRate->Low;
> +  *Rate = ConvertTo64Bit (ClockRate->Low, ClockRate->High);
> 
>    return EFI_SUCCESS;
>  }
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> index ac32442ad8..e012424a9b 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> @@ -15,9 +15,9 @@
>      DEN0056A_System_Control_and_Management_Interface.pdf
>  **/
> 
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Protocol/ArmScmiPerformanceProtocol.h>
> -#include <string.h>
> 
>  #include "ArmScmiPerformanceProtocolPrivate.h"
>  #include "ScmiPrivate.h"
> @@ -72,7 +72,7 @@ PerformanceGetAttributes (
>      return Status;
>    }
> 
> -  memcpy (
> +  CopyMem (
>      Attributes,
>      ReturnValues,
>      sizeof (SCMI_PERFORMANCE_PROTOCOL_ATTRIBUTES)
> @@ -127,7 +127,7 @@ PerformanceDomainAttributes (
>      return Status;
>    }
> 
> -  memcpy (
> +  CopyMem (
>      DomainAttributes,
>      ReturnValues,
>      sizeof (SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES)
> @@ -219,7 +219,7 @@ PerformanceDescribeLevels (
>      }
> 
>      for (LevelNo = 0; LevelNo < ReturnNumLevels; LevelNo++) {
> -       memcpy (
> +       CopyMem (
>           &LevelArray[LevelIndex++],
>           &Levels->PerfLevel[LevelNo],
>           sizeof (SCMI_PERFORMANCE_LEVEL)
> --
> 2.11.0