[PATCH v2 13/21] arm: socfpga: Secure register access for reading PLL frequency

[Ang, Chee Hong]

> > On 2/22/20 11:05 AM, Ang, Chee Hong wrote:
> > >>> From: Chee Hong Ang <chee.hong.ang at intel.com>
> > >>>
> > >>> Allow reading external oscillator and FPGA clock's frequency from
> > >>> System Manager's Boot Scratch Register 1 and Boot Scratch Register
> > >>> 2 in non-secure mode (EL2) on SoC 64bits platform.
> > >>>
> > >>> Signed-off-by: Chee Hong Ang <chee.hong.ang at intel.com>
> > >>> ---
> > >>>  arch/arm/mach-socfpga/wrap_pll_config_s10.c | 9 +++++----
> > >>>  1 file changed, 5 insertions(+), 4 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm/mach-socfpga/wrap_pll_config_s10.c
> > >>> b/arch/arm/mach- socfpga/wrap_pll_config_s10.c index
> > >>> 3da8579..7bd92d0
> > >>> 100644
> > >>> --- a/arch/arm/mach-socfpga/wrap_pll_config_s10.c
> > >>> +++ b/arch/arm/mach-socfpga/wrap_pll_config_s10.c
> > >>> @@ -9,6 +9,7 @@
> > >>>  #include <asm/io.h>
> > >>>  #include <asm/arch/handoff_s10.h>  #include
> > >>> <asm/arch/system_manager.h>
> > >>> +#include <asm/arch/secure_reg_helper.h>
> > >>>
> > >>>  const struct cm_config * const cm_get_default_config(void)  { @@
> > >>> -39,8 +40,8 @@ const unsigned int cm_get_osc_clk_hz(void)
> > >>>  	writel(clock,
> > >>>  	       socfpga_get_sysmgr_addr() +
> > >>> SYSMGR_SOC64_BOOT_SCRATCH_COLD1);  #endif
> > >>> -	return readl(socfpga_get_sysmgr_addr() +
> > >>> -		     SYSMGR_SOC64_BOOT_SCRATCH_COLD1);
> > >>> +	return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> > >>> +
> > >>> SYSMGR_SOC64_BOOT_SCRATCH_COLD1);
> > >>>  }
> > >>>
> > >>>  const unsigned int cm_get_intosc_clk_hz(void) @@ -56,6 +57,6 @@
> > >>> const unsigned int cm_get_fpga_clk_hz(void)
> > >>>  	writel(clock,
> > >>>  	       socfpga_get_sysmgr_addr() +
> > >>> SYSMGR_SOC64_BOOT_SCRATCH_COLD2);  #endif
> > >>> -	return readl(socfpga_get_sysmgr_addr() +
> > >>> -		     SYSMGR_SOC64_BOOT_SCRATCH_COLD2);
> > >>> +	return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> > >>> +
> > >>> SYSMGR_SOC64_BOOT_SCRATCH_COLD2);
> > >>>  }
> > >>> --
> > >>> 2.7.4
> > >> This clock info could be directly read from the handoff table
> > >> (OCRAM) instead of the System Manager's boot scratch register (secure
> zone).
> > >> Please refer to my full explanation in my previous email reply:
> > >> [PATCH v2 11/21] arm: socfpga: Secure register access for clock
> > >> manager (SoC
> > >> 64bits)
> > > Simon raised a good security concern on this approach. I will drop
> > > this
> > approach.
> > > Will go for high-level APIs in ATF for clock queries:
> > > INTEL_SIP_SMC_CLK_GET_OSC
> > > INTEL_SIP_SMC_CLK_GET_FPGA
> >
> > Can't you have a clock driver read out the clock tree once and then
> > have all the drivers in U-Boot just get clock settings from the clock driver?
In 'wrap_pll_config_s10.c':
cm_get_osc_clk_hz()
cm_get_intosc_clk_hz()
cm_get_fpga_clk_hz()

All the clock settings returned by S10/Agilex clock manager drivers derived
from these 3 clock sources (listed above) then all other U-Boot drivers get the clock
settings from the clock driver. 

Can you help clarify what do you mean by "read out the clock tree once" ?

Anyway, there will be only one high-level API for reading the OSC/FPGA/QSPI clock
sources depending on which clock source chosen by caller.

Please ignore my reply below.
> Yes. This could be part of the clock driver (clock_manager_s10 / agilex.c) instead
> of scattering around in different places.