[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.

Please ignore all my replies above. Will drop this patch.
System Manager is read only in EL2 and read/write in EL3.
No change required since it only read back from System Manager’s registers.