> > On 2/22/20 11:05 AM, Ang, Chee Hong wrote: > > >>> From: Chee Hong Ang <chee.hong....@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....@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.