On 03/06/2017 08:10 AM, Ley Foon Tan wrote:
On Sab, 2017-02-25 at 22:35 +0100, Marek Vasut wrote:
On 02/22/2017 10:47 AM, Ley Foon Tan wrote:

Add clock driver support for Arria 10 and update Gen5 clock driver.

Signed-off-by: Tien Fong Chee <tien.fong.c...@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon....@intel.com>
---

[...]

diff --git a/arch/arm/mach-socfpga/clock_manager_arria10.c
b/arch/arm/mach-socfpga/clock_manager_arria10.c
new file mode 100644
index 0000000..4fa841f
--- /dev/null
+++ b/arch/arm/mach-socfpga/clock_manager_arria10.c
@@ -0,0 +1,1104 @@
+/*
+ * Copyright (C) 2016-2017 Intel Corporation
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/clock_manager.h>
+#include <fdtdec.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+u32 eosc1_hz;
+u32 cb_intosc_hz;
+u32 f2s_free_hz;
+u32 cm_l4_main_clk_hz;
+u32 cm_l4_sp_clk_hz;
+u32 cm_l4_mp_clk_hz;
+u32 cm_l4_sys_free_clk_hz;
Shouldn't these values be static ? (if needed at all) ?
They are used in clock_manager.c clock info printing. But, I can
restructure that part and keep all these static.

Yeah, either that or use accessors.



+struct strtopu32 {
+       const char *str;
+       u32 *p;
+};
What is this ^ and is this needed ? Probably not ...
Will remove.
[...]

[...]

-static unsigned int cm_get_main_vco_clk_hz(void)
+unsigned int cm_get_main_vco_clk_hz(void)
 {
-       u32 reg, clock;
+       u32 src_hz, numer, denom, vco;

        /* get the main VCO clock */
-       reg = readl(&clock_manager_base->main_pll.vco);
-       clock = cm_get_osc_clk_hz(1);
-       clock /= ((reg & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
-                 CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET) + 1;
-       clock *= ((reg & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
-                 CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET) + 1;
+       vco = readl(&clock_manager_base->main_pll.vco);
How many of the changes here are relevant ?

The changes to this file are total chaos and this really does need
some
splitting, it's unreviewable.
This file mainly to rename of variables and minor fixes on some clock
calculation.
I will separate these changes from this patch and move to after "[PATCH
01/20] arm: socfpga: restructure clock manager driver".

Good, thanks.



-       return clock;
+       numer = ((vco & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
+               CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET);
+       denom = ((vco & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
+                CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET);
+
+       src_hz = cm_get_osc_clk_hz(1);
+
+       vco = src_hz;
+       vco /= (1 + denom);
+       vco *= (1 + numer);
+
+       return vco;
 }
[...]

Thanks

Regards
Ley Foon


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to