[Linaro-uefi,20/26] D03/FdtUpdateLib: Update refclk in DT

Read reference clock from ARCH timer frequency and set it into DT.

Change-Id: I7a305b78512bcbaaab4ed3adc2675800c2462f46
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
---
 .../D03/Library/FdtUpdateLib/FdtUpdateLib.c        | 64 ++++++++++++++++++++++
 .../D03/Library/FdtUpdateLib/FdtUpdateLib.inf      |  8 ++-
 2 files changed, 69 insertions(+), 3 deletions(-)

On Thu, Oct 27, 2016 at 11:15:23AM +0800, Heyi Guo wrote:
> Read reference clock from ARCH timer frequency and set it into DT.
> 
> Change-Id: I7a305b78512bcbaaab4ed3adc2675800c2462f46

Please try to get rid of these change ids for v2.

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> ---
>  .../D03/Library/FdtUpdateLib/FdtUpdateLib.c        | 64 ++++++++++++++++++++++
>  .../D03/Library/FdtUpdateLib/FdtUpdateLib.inf      |  8 ++-
>  2 files changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.c b/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.c
> index b8b9503..f9db938 100755
> --- a/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.c
> +++ b/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.c
> @@ -19,6 +19,7 @@
>  #include <Library/IoLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> +#include <Library/ArmArchTimer.h>

Don't re-sort this whole list as part of this set, but please insert
this one as alphabetically sorted as possible.

>  #include <Library/FdtUpdateLib.h>
>  #include <PlatformArch.h>
>  #include <Library/PcdLib.h>
> @@ -183,6 +184,67 @@ DelPhyhandleUpdateMacAddress(IN VOID* Fdt)
>      return Status;
>  }
>  
> +STATIC
> +EFI_STATUS
> +UpdateRefClk (IN VOID* Fdt)
> +{
> +    INTN                node;

This uses 4-space indentation rather than 2-space indentation.
Please change for all parts of this patch.

> +    INTN                Error;
> +    struct              fdt_property *m_prop;
> +    int                 m_oldlen;
> +    UINTN               ArchTimerFreq = 0;
> +    UINT32              Data;
> +    CONST CHAR8         *Property = "clock-frequency";
> +
> +    ArmArchTimerReadReg (CntFrq, &ArchTimerFreq);
> +    if (!ArchTimerFreq)
> +    {

Move that { onto the if line. (And please apply to other instances in
patch too.)

> +        DEBUG ((EFI_D_ERROR, "[%a]:[%dL] Get timer frequency failed!\n", __FUNCTION__, __LINE__));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    node = fdt_subnode_offset(Fdt, 0, "soc");
> +    if (node < 0)
> +    {
> +        DEBUG ((EFI_D_ERROR, "can not find soc root node\n"));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    node = fdt_subnode_offset(Fdt, node, "refclk");
> +    if (node < 0)
> +    {
> +        DEBUG ((EFI_D_ERROR, "can not find refclk node\n"));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    m_prop = fdt_get_property_w(Fdt, node, Property, &m_oldlen);
> +    if(!m_prop)
> +    {
> +        DEBUG ((EFI_D_ERROR, "[%a]:[%dL] Can't find property %a\n", __FUNCTION__, __LINE__, Property));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    Error = fdt_delprop(Fdt, node, Property);
> +    if (Error)
> +    {
> +        DEBUG ((EFI_D_ERROR, "ERROR: fdt_delprop() %a: %a\n", Property, fdt_strerror (Error)));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    // UINT32 is enough for refclk data length
> +    Data = (UINT32) ArchTimerFreq;
> +    Data = cpu_to_fdt32 (Data);
> +    Error = fdt_setprop(Fdt, node, Property, &Data, sizeof(Data));
> +    if (Error)
> +    {
> +        DEBUG ((EFI_D_ERROR, "ERROR:fdt_setprop() %a: %a\n", Property, fdt_strerror (Error)));
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    DEBUG ((EFI_D_INFO, "Update refclk successfully.\n"));
> +    return EFI_SUCCESS;
> +}
> +
>  INTN
>  GetMemoryNode(VOID* Fdt)
>  {
> @@ -401,6 +463,8 @@ EFI_STATUS EFIFdtUpdate(UINTN FdtFileAddr)
>          Status = EFI_SUCCESS;
>      }
>  
> +    // Ignore the return status and continue running
> +    (VOID) UpdateRefClk (Fdt);

Why add so many error checks in the definition of the function, only
to ignore them all where it is actually used?

>  
>      Status = UpdateMemoryNode(Fdt);
>      if (EFI_ERROR (Status))
> diff --git a/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.inf b/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.inf
> index b885eae..816f6f5 100755
> --- a/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.inf
> +++ b/Platforms/Hisilicon/D03/Library/FdtUpdateLib/FdtUpdateLib.inf
> @@ -29,13 +29,15 @@
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  ArmPkg/ArmPkg.dec

Please insert first in this block.

>    EmbeddedPkg/EmbeddedPkg.dec
>    OpenPlatformPkg/Chips/Hisilicon/HisiPkg.dec
>  
>  [LibraryClasses]
> - FdtLib
> - PlatformSysCtrlLib
> - OemMiscLib
> +  ArmLib
> +  FdtLib
> +  PlatformSysCtrlLib
> +  OemMiscLib

No unrelated indentation changes, please.

>  
>  [Protocols]
>   gHisiBoardNicProtocolGuid
> -- 
> 1.9.1
>