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