[PATCH v2 11/21] arm: socfpga: Secure register access for clock manager (SoC 64bits)

[Marek Vasut]

On 2/20/20 3:32 AM, Ang, Chee Hong wrote:
>> On 2/19/20 1:25 PM, chee.hong.ang at intel.com wrote:
>>> From: Chee Hong Ang <chee.hong.ang at intel.com>
>>>
>>> Allow clock manager driver to access the System Manager's Boot Scratch
>>> Register 0 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/clock_manager_agilex.c | 5 +++--
>>>  arch/arm/mach-socfpga/clock_manager_s10.c    | 5 +++--
>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/clock_manager_agilex.c
>>> b/arch/arm/mach-socfpga/clock_manager_agilex.c
>>> index 4ee2b7b..e5a0998 100644
>>> --- a/arch/arm/mach-socfpga/clock_manager_agilex.c
>>> +++ b/arch/arm/mach-socfpga/clock_manager_agilex.c
>>> @@ -12,6 +12,7 @@
>>>  #include <asm/arch/system_manager.h>
>>>  #include <asm/io.h>
>>>  #include <dt-bindings/clock/agilex-clock.h>
>>> +#include <asm/arch/secure_reg_helper.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -65,8 +66,8 @@ unsigned int cm_get_l4_sys_free_clk_hz(void)
>>>
>>>  u32 cm_get_qspi_controller_clk_hz(void)
>>>  {
>>> -	return readl(socfpga_get_sysmgr_addr() +
>>> -		     SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>>> +	return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
>>> +
>> SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>>>  }
>>>
>>>  void cm_print_clock_quick_summary(void)
>>> diff --git a/arch/arm/mach-socfpga/clock_manager_s10.c
>>> b/arch/arm/mach-socfpga/clock_manager_s10.c
>>> index 05e4212..02578cc 100644
>>> --- a/arch/arm/mach-socfpga/clock_manager_s10.c
>>> +++ b/arch/arm/mach-socfpga/clock_manager_s10.c
>>> @@ -9,6 +9,7 @@
>>>  #include <asm/arch/clock_manager.h>
>>>  #include <asm/arch/handoff_s10.h>
>>>  #include <asm/arch/system_manager.h>
>>> +#include <asm/arch/secure_reg_helper.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -385,8 +386,8 @@ unsigned int cm_get_l4_sp_clk_hz(void)
>>>
>>>  unsigned int cm_get_qspi_controller_clk_hz(void)
>>>  {
>>> -	return readl(socfpga_get_sysmgr_addr() +
>>> -		     SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>>> +	return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
>>> +
>> SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>>>  }
>>>
>>>  unsigned int cm_get_spi_controller_clk_hz(void)
>>
>> Shouldn't the IO accessors already provide the necessary abstraction ?
> This function accesses the system manager registers, therefore it is required
> to call the secure register read function to make sure it still can access
> the system manager register if it's running EL2 (non-secure).

But shouldn't the standard IO accessors handle that transparently ?
What does Linux do ?