arm64: rk3399: enable SPL with ATF support
Subsystems (6)
cover arm64: rk3399: enable SPL with ATF support
RK3399 SPL is not enable bacause of the lack of ATF support in SPL, after port some source code from ATF, I manage to enable the support for ATF in SPL. This patch set depends on some patch for SPL support multi binary in FIT which is from Andre.
The patch of sdram controller is still not clean, because I want to get comments from upstream.I port this driver from coreboot, and I don't want to make much change on it. The rk3399 dram controller has many registers to config, I want to make them directly used by driver instead of parse and copy them one by one from dts.
Another part of comment is welcome for the implementation of spl_atf.c and the entry in spl.c
Kever Yang (3):
arm64: rk3399: add SPL support
arm64: rk3399: add ddr controller driver
spl: add support to booting with ATF
arch/arm/Kconfig | 1 + arch/arm/dts/rk3399.dtsi | 24 + arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 118 ++ arch/arm/include/asm/arch-rockchip/sdram_rk3399.h | 188 +++ arch/arm/mach-rockchip/Kconfig | 2 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/rk3399-board-spl.c | 157 ++ arch/arm/mach-rockchip/rk3399/Makefile | 1 + arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc | 1565 ++++++++++++++++++++ arch/arm/mach-rockchip/rk3399/sdram_rk3399.c | 1121 ++++++++++++++ common/spl/Kconfig | 14 + common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++ configs/evb-rk3399_defconfig | 17 + drivers/clk/rockchip/clk_rk3399.c | 42 +- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 106 -- include/atf_common.h | 295 ++++ include/configs/rk3399_common.h | 11 + include/dt-bindings/clock/rk3399-cru.h | 16 +- include/spl.h | 1 + 21 files changed, 3663 insertions(+), 113 deletions(-) create mode 100644 arch/arm/include/asm/arch-rockchip/sdram_rk3399.h create mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c create mode 100644 arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc create mode 100644 arch/arm/mach-rockchip/rk3399/sdram_rk3399.c create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.h
Signature · 1 line
1.9.1
RFC PATCH 1/3 arm64: rk3399: add SPL support
Add spl support for rk3399.
---
arch/arm/Kconfig | 1 + arch/arm/dts/rk3399.dtsi | 24 ++++ arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 118 ++++++++++++++++++ arch/arm/mach-rockchip/Kconfig | 2 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/rk3399-board-spl.c | 157 ++++++++++++++++++++++++ configs/evb-rk3399_defconfig | 17 +++ drivers/clk/rockchip/clk_rk3399.c | 42 ++++++- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 106 ---------------- include/configs/rk3399_common.h | 11 ++ include/dt-bindings/clock/rk3399-cru.h | 16 ++- 11 files changed, 382 insertions(+), 113 deletions(-) create mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c
arch/arm/Kconfig +1 -0
@@ -882,6 +882,7 @@ config ARCH_ROCKCHIP select DM select SPL_DM if SPL select SYS_MALLOC_F + select SPL_SEPARATE_BSS if SPL select SPL_SYS_MALLOC_SIMPLE if SPL select DM_GPIO select DM_I2C
arch/arm/dts/rk3399.dtsi +24 -0
@@ -183,6 +183,7 @@ }; sdhci: sdhci at fe330000 { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; reg = <0x0 0xfe330000 0x0 0x10000>; interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
@@ -416,6 +417,7 @@ }; pmugrf: syscon at ff320000 { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-pmugrf", "syscon", "simple-mfd"; reg = <0x0 0xff320000 0x0 0x1000>; #address-cells = <1>;
@@ -497,7 +499,26 @@ status = "disabled"; }; + dfi: dfi at ff630000 { + reg = <0x00 0xff630000 0x00 0x4000>; + compatible = "rockchip,rk3399-dfi"; + rockchip,pmu = <&pmugrf>; + clocks = <&cru PCLK_DDR_MON>; + clock-names = "pclk_ddr_mon"; + status = "disabled"; + }; + + dmc: dmc { + u-boot,dm-pre-reloc; + compatible = "rockchip,rk3399-dmc"; + devfreq-events = <&dfi>; + interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH 0>; + clocks = <&cru SCLK_DDRCLK>; + clock-names = "dmc_clk"; + }; + pmucru: pmu-clock-controller at ff750000 { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-pmucru"; reg = <0x0 0xff750000 0x0 0x1000>; #clock-cells = <1>;
@@ -507,6 +528,7 @@ }; cru: clock-controller at ff760000 { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-cru"; reg = <0x0 0xff760000 0x0 0x1000>; #clock-cells = <1>;
@@ -530,6 +552,7 @@ }; grf: syscon at ff770000 { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; reg = <0x0 0xff770000 0x0 0x10000>; #address-cells = <1>;
@@ -607,6 +630,7 @@ }; pinctrl: pinctrl { + u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-pinctrl"; rockchip,grf = <&grf>; rockchip,pmu = <&pmugrf>;
arch/arm/include/asm/arch-rockchip/grf_rk3399.h +118 -0
122 lines not on this page · raw message
arch/arm/mach-rockchip/Kconfig +2 -0
@@ -26,6 +26,8 @@ config ROCKCHIP_RK3288 config ROCKCHIP_RK3399 bool "Support Rockchip RK3399" select ARM64 + select SUPPORT_SPL + select SPL help The Rockchip RK3399 is a ARM-based SoC with a dual-core Cortex-A72 and quad-core Cortex-A53.
arch/arm/mach-rockchip/Makefile +1 -0
@@ -7,6 +7,7 @@ ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board-spl.o +obj-$(CONFIG_ROCKCHIP_RK3399) += rk3399-board-spl.o obj-$(CONFIG_ROCKCHIP_SPL_BACK_TO_BROM) += save_boot_param.o else obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board.o
arch/arm/mach-rockchip/rk3399-board-spl.c new file +157 -0
157 lines not on this page · raw message
configs/evb-rk3399_defconfig +17 -0
@@ -3,7 +3,16 @@ CONFIG_ARCH_ROCKCHIP=y CONFIG_ROCKCHIP_RK3399=y CONFIG_DEFAULT_DEVICE_TREE="rk3399-evb" CONFIG_FIT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_OF_LIBFDT=y +CONFIG_SPL_ATF_SUPPORT=y +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x200 +CONFIG_SPL_ATF_TEXT_BASE=0x00010000 # CONFIG_DISPLAY_CPUINFO is not set +CONFIG_SPL_STACK_R=y +CONFIG_SPL_STACK_R_ADDR=0x80000 +CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000 +CONFIG_SYS_MALLOC_F_LEN=0x4000 CONFIG_HUSH_PARSER=y CONFIG_CMD_BOOTZ=y # CONFIG_CMD_IMLS is not set
@@ -16,18 +25,26 @@ CONFIG_CMD_EXT2=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_PXE=y +CONFIG_SPL_OF_CONTROL=y +CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_REGMAP=y +CONFIG_SPL_REGMAP=y CONFIG_SYSCON=y +CONFIG_SPL_SYSCON=y CONFIG_CLK=y +CONFIG_SPL_CLK=y CONFIG_ROCKCHIP_GPIO=y CONFIG_ROCKCHIP_DWMMC=y CONFIG_ROCKCHIP_SDHCI=y CONFIG_PINCTRL=y +CONFIG_SPL_PINCTRL=y CONFIG_ROCKCHIP_RK3399_PINCTRL=y CONFIG_PWM_ROCKCHIP=y CONFIG_REGULATOR_PWM=y CONFIG_DM_REGULATOR_FIXED=y CONFIG_RAM=y +CONFIG_SPL_RAM=y CONFIG_DEBUG_UART=y CONFIG_DEBUG_UART_BASE=0xFF1A0000 CONFIG_DEBUG_UART_CLOCK=24000000
drivers/clk/rockchip/clk_rk3399.c +41 -1
60 lines not on this page · raw message
drivers/pinctrl/rockchip/pinctrl_rk3399.c +0 -106
112 lines not on this page · raw message
include/configs/rk3399_common.h +11 -0
@@ -17,12 +17,23 @@#define CONFIG_SYS_MALLOC_LEN (32 << 20)#define CONFIG_SYS_CBSIZE 1024#define CONFIG_SKIP_LOWLEVEL_INIT +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_DRIVERS_MISC_SUPPORT +#define CONFIG_SPL_LIBCOMMON_SUPPORT +#define CONFIG_SPL_LIBGENERIC_SUPPORT +#define CONFIG_SPL_SERIAL_SUPPORT #define CONFIG_SYS_NS16550_MEM32 #define CONFIG_SYS_TEXT_BASE 0x00200000#define CONFIG_SYS_INIT_SP_ADDR 0x00300000#define CONFIG_SYS_LOAD_ADDR 0x00800800 +#define CONFIG_SPL_STACK 0xff8effff +#define CONFIG_SPL_TEXT_BASE 0xff8c2008 +#define CONFIG_SPL_MAX_SIZE 0x30000 +/* BSS setup */ +#define CONFIG_SPL_BSS_START_ADDR 0xff8e0000 +#define CONFIG_SPL_BSS_MAX_SIZE 0x10000#define CONFIG_SYS_BOOTM_LEN (64 << 20)/* 64M */
include/dt-bindings/clock/rk3399-cru.h +10 -6
@@ -122,6 +122,10 @@#define SCLK_DPHY_RX0_CFG 165#define SCLK_RMII_SRC 166#define SCLK_PCIEPHY_REF100M 167 +#define SCLK_USBPHY0_480M_SRC 168 +#define SCLK_USBPHY1_480M_SRC 169 +#define SCLK_DDRCLK 170 +#define SCLK_TESTOUT2 171#define DCLK_VOP0 180#define DCLK_VOP1 181
@@ -589,13 +593,13 @@#define SRST_P_SPI0 214#define SRST_P_SPI1 215#define SRST_P_SPI2 216 -#define SRST_P_SPI3 217 -#define SRST_P_SPI4 218 +#define SRST_P_SPI4 217 +#define SRST_P_SPI5 218#define SRST_SPI0 219#define SRST_SPI1 220#define SRST_SPI2 221 -#define SRST_SPI3 222 -#define SRST_SPI4 223 +#define SRST_SPI4 222 +#define SRST_SPI5 223/* cru_softrst_con14 */ #define SRST_I2S0_8CH 224
@@ -717,8 +721,8 @@#define SRST_H_CM0S_NOC 3#define SRST_DBG_CM0S 4#define SRST_PO_CM0S 5 -#define SRST_P_SPI6 6 -#define SRST_SPI6 7 +#define SRST_P_SPI3 6 +#define SRST_SPI3 7#define SRST_P_TIMER_0_1 8#define SRST_P_TIMER_0 9#define SRST_P_TIMER_1 10
Signature · 1 line
1.9.1
RFC PATCH 1/3 arm64: rk3399: add SPL support
Hi Kever,
Quoted 5 lines · Kever Yang
Add spl support for rk3399.
Signed-off-by Kever Yang <[email protected]>---
Quoted 13 lines
arch/arm/Kconfig | 1 + arch/arm/dts/rk3399.dtsi | 24 ++++ arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 118 ++++++++++++++++++ arch/arm/mach-rockchip/Kconfig | 2 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/rk3399-board-spl.c | 157 ++++++++++++++++++++++++ configs/evb-rk3399_defconfig | 17 +++ drivers/clk/rockchip/clk_rk3399.c | 42 ++++++- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 106 ---------------- include/configs/rk3399_common.h | 11 ++ include/dt-bindings/clock/rk3399-cru.h | 16 ++- 11 files changed, 382 insertions(+), 113 deletions(-) create mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c
The code looks fine. But can you please split this into 3 patches?
- move grf register definitions to grf_rk3399.h
- add clock driver
- add SPL support
Also check your header #ifdef guards - they should not have SOC in them.
Regards,
Simon
RFC PATCH 2/3 arm64: rk3399: add ddr controller driver
In reply to Kever YangRK3399 support DDR3, LPDDR3, DDR4 sdram, this patch is porting from coreboot, support 4GB lpddr3 in this version.
---
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h | 188 +++ arch/arm/mach-rockchip/rk3399/Makefile | 1 + arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc | 1565 ++++++++++++++++++++ arch/arm/mach-rockchip/rk3399/sdram_rk3399.c | 1121 ++++++++++++++ 4 files changed, 2875 insertions(+) create mode 100644 arch/arm/include/asm/arch-rockchip/sdram_rk3399.h create mode 100644 arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc create mode 100644 arch/arm/mach-rockchip/rk3399/sdram_rk3399.c
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +188 -0
@@ -0,0 +1,188 @@ +/* +* Copyright (C) 2015 Rockchip Electronics Co., Ltd +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef __SOC_ROCKCHIP_RK3399_SDRAM_H__ +#define __SOC_ROCKCHIP_RK3399_SDRAM_H__ + +#include <stddef.h> + +enum { +DDR3 =3 , +LPDDR2 =5 , +LPDDR3 =6 , +LPDDR4 =7 , +UNUSED =0xFF +}; + +struct rk3399_ddr_pctl_regs { +u32 denali_ctl[332 ]; +}; + +struct rk3399_ddr_publ_regs { +u32 denali_phy[959 ]; +}; + +struct rk3399_ddr_pi_regs { +u32 denali_pi[200 ]; +}; +union noc_ddrtiminga0 { +u32 d32; +struct { +unsigned acttoact :6 ; +unsigned reserved0 :2 ; +unsigned rdtomiss :6 ; +unsigned reserved1 :2 ; +unsigned wrtomiss :6 ; +unsigned reserved2 :2 ; +unsigned readlatency :8 ; + } b; +}; + +union noc_ddrtimingb0 { +u32 d32; +struct { +unsigned rdtowr :5 ; +unsigned reserved0 :3 ; +unsigned wrtord :5 ; +unsigned reserved1 :3 ; +unsigned rrd :4 ; +unsigned reserved2 :4 ; +unsigned faw :6 ; +unsigned reserved3 :2 ; + } b; +}; + +union noc_ddrtimingc0 { +u32 d32; +struct { +unsigned burstpenalty :4 ; +unsigned reserved0 :4 ; +unsigned wrtomwr :6 ; +unsigned reserved1 :18 ; + } b; +}; + +union noc_devtodev0 { +u32 d32; +struct { +unsigned busrdtord :3 ; +unsigned reserved0 :1 ; +unsigned busrdtowr :3 ; +unsigned reserved1 :1 ; +unsigned buswrtord :3 ; +unsigned reserved2 :1 ; +unsigned buswrtowr :3 ; +unsigned reserved3 :17 ; + } b; +}; + +union noc_ddrmode { +u32 d32; +struct { +unsigned autoprecharge :1 ; +unsigned bypassfiltering :1 ; +unsigned fawbank :1 ; +unsigned burstsize :2 ; +unsigned mwrsize :2 ; +unsigned reserved2 :1 ; +unsigned forceorder :8 ; +unsigned forceorderstate :8 ; +unsigned reserved3 :8 ; + } b; +}; + +struct rk3399_msch_regs { +u32 coreid; +u32 revisionid; +u32 ddrconf; +u32 ddrsize; +union noc_ddrtiminga0 ddrtiminga0; +union noc_ddrtimingb0 ddrtimingb0; +union noc_ddrtimingc0 ddrtimingc0; +union noc_devtodev0 devtodev0; +u32 reserved0[(0x110 -0x20 )/4 ]; +union noc_ddrmode ddrmode; +u32 reserved1[(0x1000 -0x114 )/4 ]; +u32 agingx0; +}; + +struct rk3399_msch_timings { +union noc_ddrtiminga0 ddrtiminga0; +union noc_ddrtimingb0 ddrtimingb0; +union noc_ddrtimingc0 ddrtimingc0; +union noc_devtodev0 devtodev0; +union noc_ddrmode ddrmode; +u32 agingx0; +}; + +struct rk3399_ddr_cic_regs { +u32 cic_ctrl0; +u32 cic_ctrl1; +u32 cic_idle_th; +u32 cic_cg_wait_th; +u32 cic_status0; +u32 cic_status1; +u32 cic_ctrl2; +u32 cic_ctrl3; +u32 cic_ctrl4; +}; + +/* DENALI_CTL_00 */ +#define START (1) + +/* DENALI_CTL_68 */ +#define PWRUP_SREFRESH_EXIT (1 << 16) + +/* DENALI_CTL_274 */ +#define MEM_RST_VALID (1) + +struct rk3399_sdram_channel { +unsigned char rank; +/* col = 0, means this channel is invalid */ +unsigned char col; +/* 3:8bank, 2:4bank */ +unsigned char bk; +/* channel buswidth, 2:32bit, 1:16bit, 0:8bit */ +unsigned char bw; +/* die buswidth, 2:32bit, 1:16bit, 0:8bit */ +unsigned char dbw; +/* row_3_4 = 1: 6Gb or 12Gb die +* row_3_4 = 0: normal die, power of 2 +*/ +unsigned char row_3_4; +unsigned char cs0_row; +unsigned char cs1_row; +unsigned int ddrconfig; +struct rk3399_msch_timings noc_timings; +}; + +struct rk3399_sdram_params { +struct rk3399_sdram_channel ch[2 ]; +unsigned int ddr_freq; +unsigned char dramtype; +unsigned char num_channels; +unsigned char stride; +unsigned char odt; +/* align 8 byte */ +struct rk3399_ddr_pctl_regs pctl_regs; +/* align 8 byte */ +struct rk3399_ddr_pi_regs pi_regs; +/* align 8 byte */ +struct rk3399_ddr_publ_regs phy_regs; +/* used for align 8byte for next struct */ +unsigned int align_8; +}; + +#define PI_CA_TRAINING (1 << 0) +#define PI_WRITE_LEVELING (1 << 1) +#define PI_READ_GATE_TRAINING (1 << 2) +#define PI_READ_LEVELING (1 << 3) +#define PI_WDQ_LEVELING (1 << 4) +#define PI_FULL_TRAINING (0xff) + +size_t sdram_size_mb (void ); + +#endif
arch/arm/mach-rockchip/rk3399/Makefile +1 -0
@@ -7,3 +7,4 @@ obj-y += clk_rk3399.o obj-y += rk3399.o obj-y += syscon_rk3399.o +obj-y += sdram_rk3399.o
arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc new file +1565 -0
1,565 lines not on this page · raw message
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +1121 -0
1,121 lines not on this page · raw message
Signature · 1 line
1.9.1
RFC PATCH 2/3 arm64: rk3399: add ddr controller driver
Hi Kever,
Quoted 21 lines · Kever Yang
RK3399 support DDR3, LPDDR3, DDR4 sdram, this patch is porting from coreboot, support 4GB lpddr3 in this version.
Signed-off-by Kever Yang <[email protected]>---
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h | 188 +++ arch/arm/mach-rockchip/rk3399/Makefile | 1 + arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc | 1565 ++++++++++++++++++++ arch/arm/mach-rockchip/rk3399/sdram_rk3399.c | 1121 ++++++++++++++ 4 files changed, 2875 insertions(+) create mode 100644 arch/arm/include/asm/arch-rockchip/sdram_rk3399.h create mode 100644 arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc create mode 100644 arch/arm/mach-rockchip/rk3399/sdram_rk3399.carch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +0
-0
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 8
Quoted 8 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +8
-0@@ -0,0 +1,188 @@ +/* +* Copyright (C) 2015 Rockchip Electronics Co., Ltd +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef __SOC_ROCKCHIP_RK3399_SDRAM_H__ +#define __SOC_ROCKCHIP_RK3399_SDRAM_H__
Please use the same format as other headers (i.e. no SOC).
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 10
Quoted 2 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +2
-0+ +#include <stddef.h>
What is this for?
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 17
Quoted 7 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +7
-0+ +enum { +DDR3 =3 , +LPDDR2 =5 , +LPDDR3 =6 , +LPDDR4 =7 , +UNUSED =0xFF
Please can you use lower-case hex?
Quoted 14 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +14
-0+}; + +struct rk3399_ddr_pctl_regs { +u32 denali_ctl[332 ]; +}; + +struct rk3399_ddr_publ_regs { +u32 denali_phy[959 ]; +}; + +struct rk3399_ddr_pi_regs { +u32 denali_pi[200 ]; +}; +union noc_ddrtiminga0 {
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 41
Quoted 10 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +10
-0+u32 d32; +struct { +unsigned acttoact :6 ; +unsigned reserved0 :2 ; +unsigned rdtomiss :6 ; +unsigned reserved1 :2 ; +unsigned wrtomiss :6 ; +unsigned reserved2 :2 ; +unsigned readlatency :8 ; + } b;
Can we use a simple value here, and drop the bitfields?
Quoted 55 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +55
-0+}; + +union noc_ddrtimingb0 { +u32 d32; +struct { +unsigned rdtowr :5 ; +unsigned reserved0 :3 ; +unsigned wrtord :5 ; +unsigned reserved1 :3 ; +unsigned rrd :4 ; +unsigned reserved2 :4 ; +unsigned faw :6 ; +unsigned reserved3 :2 ; + } b; +}; + +union noc_ddrtimingc0 { +u32 d32; +struct { +unsigned burstpenalty :4 ; +unsigned reserved0 :4 ; +unsigned wrtomwr :6 ; +unsigned reserved1 :18 ; + } b; +}; + +union noc_devtodev0 { +u32 d32; +struct { +unsigned busrdtord :3 ; +unsigned reserved0 :1 ; +unsigned busrdtowr :3 ; +unsigned reserved1 :1 ; +unsigned buswrtord :3 ; +unsigned reserved2 :1 ; +unsigned buswrtowr :3 ; +unsigned reserved3 :17 ; + } b; +}; + +union noc_ddrmode { +u32 d32; +struct { +unsigned autoprecharge :1 ; +unsigned bypassfiltering :1 ; +unsigned fawbank :1 ; +unsigned burstsize :2 ; +unsigned mwrsize :2 ; +unsigned reserved2 :1 ; +unsigned forceorder :8 ; +unsigned forceorderstate :8 ; +unsigned reserved3 :8 ; + } b; +}; +
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 106
Quoted 10 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +10
-0+struct rk3399_msch_regs { +u32 coreid; +u32 revisionid; +u32 ddrconf; +u32 ddrsize; +union noc_ddrtiminga0 ddrtiminga0; +union noc_ddrtimingb0 ddrtimingb0; +union noc_ddrtimingc0 ddrtimingc0; +union noc_devtodev0 devtodev0; +u32 reserved0[(0x110 -0x20 )/4 ];
Please add spaces around operators:
u32 reserved0[(0x110 - 0x20) / 4];
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 112
Quoted 6 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +6
-0+union noc_ddrmode ddrmode; +u32 reserved1[(0x1000 -0x114 )/4 ]; +u32 agingx0; +}; + +struct rk3399_msch_timings {
These structures and members should have comments.
Quoted 12 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +12
-0+union noc_ddrtiminga0 ddrtiminga0; +union noc_ddrtimingb0 ddrtimingb0; +union noc_ddrtimingc0 ddrtimingc0; +union noc_devtodev0 devtodev0; +union noc_ddrmode ddrmode; +u32 agingx0; +}; + +struct rk3399_ddr_cic_regs { +u32 cic_ctrl0; +u32 cic_ctrl1; +u32 cic_idle_th;
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 134
Quoted 10 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +10
-0+u32 cic_cg_wait_th; +u32 cic_status0; +u32 cic_status1; +u32 cic_ctrl2; +u32 cic_ctrl3; +u32 cic_ctrl4; +}; + +/* DENALI_CTL_00 */ +#define START (1)
Drop brackets around simple constants
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 144
Quoted 10 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +10
-0+ +/* DENALI_CTL_68 */ +#define PWRUP_SREFRESH_EXIT (1 << 16) + +/* DENALI_CTL_274 */ +#define MEM_RST_VALID (1) + +struct rk3399_sdram_channel { +unsigned char rank; +/* col = 0, means this channel is invalid */
But what is col? Please can you expand these comments a bit?
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 152
Quoted 8 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +8
-0+unsigned char col; +/* 3:8bank, 2:4bank */ +unsigned char bk; +/* channel buswidth, 2:32bit, 1:16bit, 0:8bit */ +unsigned char bw; +/* die buswidth, 2:32bit, 1:16bit, 0:8bit */ +unsigned char dbw; +/* row_3_4 = 1: 6Gb or 12Gb die
Multi-line comment style:
/*
* ...
* ...
*/
Quoted 24 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +24
-0+* row_3_4 = 0: normal die, power of 2 +*/ +unsigned char row_3_4; +unsigned char cs0_row; +unsigned char cs1_row; +unsigned int ddrconfig; +struct rk3399_msch_timings noc_timings; +}; + +struct rk3399_sdram_params { +struct rk3399_sdram_channel ch[2 ]; +unsigned int ddr_freq; +unsigned char dramtype; +unsigned char num_channels; +unsigned char stride; +unsigned char odt; +/* align 8 byte */ +struct rk3399_ddr_pctl_regs pctl_regs; +/* align 8 byte */ +struct rk3399_ddr_pi_regs pi_regs; +/* align 8 byte */ +struct rk3399_ddr_publ_regs phy_regs; +/* used for align 8byte for next struct */ +unsigned int align_8;
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h line 186
Quoted 10 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +10
-0+}; + +#define PI_CA_TRAINING (1 << 0) +#define PI_WRITE_LEVELING (1 << 1) +#define PI_READ_GATE_TRAINING (1 << 2) +#define PI_READ_LEVELING (1 << 3) +#define PI_WDQ_LEVELING (1 << 4) +#define PI_FULL_TRAINING (0xff) + +size_t sdram_size_mb (void );
Function comment
arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc
Quoted 16 lines
arch/arm/include/asm/arch-rockchip/sdram_rk3399.h new file +2
-0+ +#endif arch/arm/mach-rockchip/rk3399/Makefile +1
-0@@ -7,3 +7,4 @@ obj-y += clk_rk3399.o obj-y += rk3399.o obj-y += syscon_rk3399.o +obj-y += sdram_rk3399.oarch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc new file +0
-0
Can you put this into the device tree as is done for rk3288?
Quoted 15 lines
arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc new file +14
-0@@ -0,0 +1,1565 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +
[...]
Quoted 7 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +1
-0@@ -0,0 +1,1121 @@ +/*
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 11
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+* (C) Copyright 2016 Rockchip Inc. +* +* SPDX-License-Identifier: GPL-2.0 +* +* Adapted from coreboot. +*/ +#include <common.h> +#include <dm.h> +#include <asm/io.h> +#include <clk.h>
clk.h should go after common.h
Quoted 30 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +30
-0+#include <asm/arch/sdram_rk3399.h> +#include <asm/arch/cru_rk3399.h> +#include <asm/arch/grf_rk3399.h> +#include <asm/arch/hardware.h> +#include <linux/err.h> + +DECLARE_GLOBAL_DATA_PTR ; + +#define DDRC0_BASE_ADDR 0xffa80000 +#define SERVER_MSCH0_BASE_ADDR 0xffa84000 +#define DDRC1_BASE_ADDR 0xffa88000 +#define SERVER_MSCH1_BASE_ADDR 0xffa8c000 + +#define DDR_PI_OFFSET 0x800 +#define DDR_PHY_OFFSET 0x2000 +#define DDRC0_PI_BASE_ADDR (DDRC0_BASE_ADDR + DDR_PI_OFFSET) +#define DDRC0_PHY_BASE_ADDR (DDRC0_BASE_ADDR + DDR_PHY_OFFSET) +#define DDRC1_PI_BASE_ADDR (DDRC1_BASE_ADDR + DDR_PI_OFFSET) +#define DDRC1_PHY_BASE_ADDR (DDRC1_BASE_ADDR + DDR_PHY_OFFSET) + +#define PMUCRU_BASE 0xff750000 +#define CRU_BASE 0xff760000 +#define PMUGRF_BASE 0xff320000 +#define PMUSGRF_BASE 0xff330000 +#define CIC_BASE_ADDR 0xff620000 + +static struct rk3399_pmucru *const pmucru_ptr = (void *)PMUCRU_BASE ; +static struct rk3399_cru *const cru_ptr = (void *)CRU_BASE ; +static struct rk3399_pmugrf_regs *const rk3399_pmugrf = (void *)PMUGRF_BASE ; +static struct rk3399_pmusgrf_regs *const rk3399_pmusgrf = (void *)PMUSGRF_BASE ;
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 51
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ +static struct rk3399_ddr_pctl_regs *const rk3399_ddr_pctl[2 ] = { + (void *)DDRC0_BASE_ADDR , (void *)DDRC1_BASE_ADDR }; +static struct rk3399_ddr_pi_regs *const rk3399_ddr_pi[2 ] = { + (void *)DDRC0_PI_BASE_ADDR , (void *)DDRC1_PI_BASE_ADDR }; +static struct rk3399_ddr_publ_regs *const rk3399_ddr_publ[2 ] = { + (void *)DDRC0_PHY_BASE_ADDR , (void *)DDRC1_PHY_BASE_ADDR }; +static struct rk3399_msch_regs *const rk3399_msch[2 ] = { + (void *)SERVER_MSCH0_BASE_ADDR , (void *)SERVER_MSCH1_BASE_ADDR }; +static struct rk3399_ddr_cic_regs *const rk3399_ddr_cic = (void *)CIC_BASE_ADDR ;
These should all come from the device tree, right?
Quoted 41 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +41
-0+ +/* +* sys_reg bitfield struct +* [31] row_3_4_ch1 +* [30] row_3_4_ch0 +* [29:28] chinfo +* [27] rank_ch1 +* [26:25] col_ch1 +* [24] bk_ch1 +* [23:22] cs0_row_ch1 +* [21:20] cs1_row_ch1 +* [19:18] bw_ch1 +* [17:16] dbw_ch1; +* [15:13] ddrtype +* [12] channelnum +* [11] rank_ch0 +* [10:9] col_ch0 +* [8] bk_ch0 +* [7:6] cs0_row_ch0 +* [5:4] cs1_row_ch0 +* [3:2] bw_ch0 +* [1:0] dbw_ch0 +*/ +#define SYS_REG_ENC_ROW_3_4 (n, ch) ((n) << (30 + (ch))) +#define SYS_REG_DEC_ROW_3_4 (n, ch) ((n >> (30 + ch)) & 0x1) +#define SYS_REG_ENC_CHINFO (ch) (1 << (28 + (ch))) +#define SYS_REG_ENC_DDRTYPE (n) ((n) << 13) +#define SYS_REG_ENC_NUM_CH (n) (((n) - 1) << 12) +#define SYS_REG_DEC_NUM_CH (n) (1 + ((n >> 12) & 0x1)) +#define SYS_REG_ENC_RANK (n, ch) (((n) - 1) << (11 + ((ch) * 16))) +#define SYS_REG_DEC_RANK (n, ch) (1 + ((n >> (11 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_COL (n, ch) (((n) - 9) << (9 + ((ch) * 16))) +#define SYS_REG_DEC_COL (n, ch) (9 + ((n >> (9 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BK (n, ch) (((n) == 3 ? 0 : 1) \ + << (8 + ((ch) * 16))) +#define SYS_REG_DEC_BK (n, ch) (3 - ((n >> (8 + 16 * ch)) & 0x1)) +#define SYS_REG_ENC_CS0_ROW (n, ch) (((n) - 13) << (6 + ((ch) * 16))) +#define SYS_REG_DEC_CS0_ROW (n, ch) (13 + ((n >> (6 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_CS1_ROW (n, ch) (((n) - 13) << (4 + ((ch) * 16))) +#define SYS_REG_DEC_CS1_ROW (n, ch) (13 + ((n >> (4 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_BW (n, ch) ((2 >> (n)) << (2 + ((ch) * 16)))
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 102
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+#define SYS_REG_DEC_BW (n, ch) (2 >> ((n >> (2 + 16 * ch)) & 0x3)) +#define SYS_REG_ENC_DBW (n, ch) ((2 >> (n)) << (0 + ((ch) * 16))) +#define SYS_REG_DEC_DBW (n, ch) (2 >> ((n >> (0 + 16 * ch)) & 0x3)) + +#define DDR_STRIDE (n) writel((0x1F << (10 + 16)) | (n << 10), \ + &rk3399_pmusgrf->soc_con4) + +#define PRESET_SGRF_HOLD (n) ((0x1 << (6+16)) | ((n) << 6)) +#define PRESET_GPIO0_HOLD (n) ((0x1 << (7+16)) | ((n) << 7)) +#define PRESET_GPIO1_HOLD (n) ((0x1 << (8+16)) | ((n) << 8))
Can we please have the shift/mask values defined, and use explicit shift/mask in the code, instead of these macros?
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 111
Quoted 9 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +9
-0+ +#define PHY_DRV_ODT_Hi_Z (0x0) +#define PHY_DRV_ODT_240 (0x1) +#define PHY_DRV_ODT_120 (0x8) +#define PHY_DRV_ODT_80 (0x9) +#define PHY_DRV_ODT_60 (0xc) +#define PHY_DRV_ODT_48 (0xd) +#define PHY_DRV_ODT_40 (0xe) +#define PHY_DRV_ODT_34_3 (0xf)
Drop brackets
Quoted 5 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +5
-0+ +#ifdef CONFIG_SPL_BUILD +static void copy_to_reg (u32 *dest,const u32 *src,u32 n) +{ +int i;
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 126
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ +for (i =0 ; i < n /sizeof (u32); i++) { +writel (*src, dest); + src++; + dest++; + } +} + +static void phy_dll_bypass_set (u32 channel, +struct rk3399_ddr_publ_regs *ddr_publ_regs,u32 freq)
Can you check patman/checkpatch for this? The alignment seems wrong.
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 134
Quoted 8 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +8
-0+{ +u32 *denali_phy = ddr_publ_regs->denali_phy; +if (freq <=125 *MHz) { +/* phy_sw_master_mode_X PHY_86/214/342/470 4bits offset_8 */ +setbits_le32 (&denali_phy[86 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[214 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[342 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[470 ], (0x3 <<2 ) <<8 );
These should really be defined as SHIFT and MASK values.
Quoted 21 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +21
-0+ +/* phy_adrctl_sw_master_mode PHY_547/675/803 4bits offset_16 */ +setbits_le32 (&denali_phy[547 ], (0x3 <<2 ) <<16 ); +setbits_le32 (&denali_phy[675 ], (0x3 <<2 ) <<16 ); +setbits_le32 (&denali_phy[803 ], (0x3 <<2 ) <<16 ); + }else { +/* phy_sw_master_mode_X PHY_86/214/342/470 4bits offset_8 */ +clrbits_le32 (&denali_phy[86 ], (0x3 <<2 ) <<8 ); +clrbits_le32 (&denali_phy[214 ], (0x3 <<2 ) <<8 ); +clrbits_le32 (&denali_phy[342 ], (0x3 <<2 ) <<8 ); +clrbits_le32 (&denali_phy[470 ], (0x3 <<2 ) <<8 ); + +/* phy_adrctl_sw_master_mode PHY_547/675/803 4bits offset_16 */ +clrbits_le32 (&denali_phy[547 ], (0x3 <<2 ) <<16 ); +clrbits_le32 (&denali_phy[675 ], (0x3 <<2 ) <<16 ); +clrbits_le32 (&denali_phy[803 ], (0x3 <<2 ) <<16 ); + } +} + +static void set_memory_map (u32 channel, +const struct rk3399_sdram_params *sdram_params)
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 165
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+{ +const struct rk3399_sdram_channel *sdram_ch = + &sdram_params->ch[channel]; +u32 *denali_ctl = rk3399_ddr_pctl[channel]->denali_ctl; +u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi; +u32 cs_map; +u32 reduc; +u32 row; + +if ((sdram_ch->ddrconfig <2 ) || (sdram_ch->ddrconfig ==4 ))
Can you add a comment as to what this code is doing?
Quoted 156 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +156
-0+ row =16 ; + else if (sdram_ch->ddrconfig == 3) + row =14 ; + else + row =15 ; + + cs_map = (sdram_ch->rank >1 ) ?3 :1 ; + reduc = (sdram_ch->bw ==2 ) ?0 :1 ; + +clrsetbits_le32 (&denali_ctl[191 ],0xF , (12 - sdram_ch->col)); +clrsetbits_le32 (&denali_ctl[190 ], (0x3 <<16 ) | (0x7 <<24 ), + ((3 - sdram_ch->bk) <<16 ) | + ((16 - row) <<24 )); + +clrsetbits_le32 (&denali_ctl[196 ],0x3 | (1 <<16 ), + cs_map | (reduc <<16 )); + +/* PI_199 PI_COL_DIFF:RW:0:4 */ +clrsetbits_le32 (&denali_pi[199 ],0xF , (12 - sdram_ch->col)); + +/* PI_155 PI_ROW_DIFF:RW:24:3 PI_BANK_DIFF:RW:16:2 */ +clrsetbits_le32 (&denali_pi[155 ], (0x3 <<16 ) | (0x7 <<24 ), + ((3 - sdram_ch->bk) <<16 ) | + ((16 - row) <<24 )); +/* PI_41 PI_CS_MAP:RW:24:4 */ +clrsetbits_le32 (&denali_pi[41 ],0xf <<24 , cs_map <<24 ); +if ((sdram_ch->rank ==1 ) && (sdram_params->dramtype ==DDR3 )) +writel (0x2EC7FFFF , &denali_pi[34 ]); +} + +static void set_ds_odt (u32 channel, +const struct rk3399_sdram_params *sdram_params) +{ +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy; + +u32 tsel_idle_en, tsel_wr_en, tsel_rd_en; +u32 tsel_idle_select_p, tsel_wr_select_p, tsel_rd_select_p; +u32 ca_tsel_wr_select_p, ca_tsel_wr_select_n; +u32 tsel_idle_select_n, tsel_wr_select_n, tsel_rd_select_n; +u32 reg_value; + +if (sdram_params->dramtype ==LPDDR4 ) { + tsel_rd_select_p = PHY_DRV_ODT_Hi_Z; + tsel_wr_select_p =PHY_DRV_ODT_40 ; + ca_tsel_wr_select_p =PHY_DRV_ODT_40 ; + tsel_idle_select_p = PHY_DRV_ODT_Hi_Z; + + tsel_rd_select_n =PHY_DRV_ODT_240 ; + tsel_wr_select_n =PHY_DRV_ODT_40 ; + ca_tsel_wr_select_n =PHY_DRV_ODT_40 ; + tsel_idle_select_n =PHY_DRV_ODT_240 ; + }else if (sdram_params->dramtype ==LPDDR3 ) { + tsel_rd_select_p =PHY_DRV_ODT_240 ; + tsel_wr_select_p =PHY_DRV_ODT_34_3 ; + ca_tsel_wr_select_p =PHY_DRV_ODT_48 ; + tsel_idle_select_p =PHY_DRV_ODT_240 ; + + tsel_rd_select_n = PHY_DRV_ODT_Hi_Z; + tsel_wr_select_n =PHY_DRV_ODT_34_3 ; + ca_tsel_wr_select_n =PHY_DRV_ODT_48 ; + tsel_idle_select_n = PHY_DRV_ODT_Hi_Z; + }else { + tsel_rd_select_p =PHY_DRV_ODT_240 ; + tsel_wr_select_p =PHY_DRV_ODT_34_3 ; + ca_tsel_wr_select_p =PHY_DRV_ODT_34_3 ; + tsel_idle_select_p =PHY_DRV_ODT_240 ; + + tsel_rd_select_n =PHY_DRV_ODT_240 ; + tsel_wr_select_n =PHY_DRV_ODT_34_3 ; + ca_tsel_wr_select_n =PHY_DRV_ODT_34_3 ; + tsel_idle_select_n =PHY_DRV_ODT_240 ; + } + +if (sdram_params->odt ==1 ) + tsel_rd_en =1 ; +else + tsel_rd_en =0 ; + + tsel_wr_en =0 ; + tsel_idle_en =0 ; + +/* +* phy_dq_tsel_select_X 24bits DENALI_PHY_6/134/262/390 offset_0 +* sets termination values for read/idle cycles and drive strength +* for write cycles for DQ/DM +*/ + reg_value = tsel_rd_select_n | (tsel_rd_select_p <<0x4 ) | + (tsel_wr_select_n <<8 ) | (tsel_wr_select_p <<12 ) | + (tsel_idle_select_n <<16 ) | (tsel_idle_select_p <<20 ); +clrsetbits_le32 (&denali_phy[6 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[134 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[262 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[390 ],0xffffff , reg_value); + +/* +* phy_dqs_tsel_select_X 24bits DENALI_PHY_7/135/263/391 offset_0 +* sets termination values for read/idle cycles and drive strength +* for write cycles for DQS +*/ +clrsetbits_le32 (&denali_phy[7 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[135 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[263 ],0xffffff , reg_value); +clrsetbits_le32 (&denali_phy[391 ],0xffffff , reg_value); + +/* phy_adr_tsel_select_ 8bits DENALI_PHY_544/672/800 offset_0 */ + reg_value = ca_tsel_wr_select_n | (ca_tsel_wr_select_p <<0x4 ); +clrsetbits_le32 (&denali_phy[544 ],0xff , reg_value); +clrsetbits_le32 (&denali_phy[672 ],0xff , reg_value); +clrsetbits_le32 (&denali_phy[800 ],0xff , reg_value); + +/* phy_pad_addr_drive 8bits DENALI_PHY_928 offset_0 */ +clrsetbits_le32 (&denali_phy[928 ],0xff , reg_value); + +/* phy_pad_rst_drive 8bits DENALI_PHY_937 offset_0 */ +clrsetbits_le32 (&denali_phy[937 ],0xff , reg_value); + +/* phy_pad_cke_drive 8bits DENALI_PHY_935 offset_0 */ +clrsetbits_le32 (&denali_phy[935 ],0xff , reg_value); + +/* phy_pad_cs_drive 8bits DENALI_PHY_939 offset_0 */ +clrsetbits_le32 (&denali_phy[939 ],0xff , reg_value); + +/* phy_pad_clk_drive 8bits DENALI_PHY_929 offset_0 */ +clrsetbits_le32 (&denali_phy[929 ],0xff , reg_value); + +/* phy_pad_fdbk_drive 23bit DENALI_PHY_924/925 */ +clrsetbits_le32 (&denali_phy[924 ],0xff , + tsel_wr_select_n | (tsel_wr_select_p <<4 )); +clrsetbits_le32 (&denali_phy[925 ],0xff , + tsel_rd_select_n | (tsel_rd_select_p <<4 )); + +/* phy_dq_tsel_enable_X 3bits DENALI_PHY_5/133/261/389 offset_16 */ + reg_value = (tsel_rd_en | (tsel_wr_en <<1 ) | (tsel_idle_en <<2 )) + <<16 ; +clrsetbits_le32 (&denali_phy[5 ],0x7 <<16 , reg_value); +clrsetbits_le32 (&denali_phy[133 ],0x7 <<16 , reg_value); +clrsetbits_le32 (&denali_phy[261 ],0x7 <<16 , reg_value); +clrsetbits_le32 (&denali_phy[389 ],0x7 <<16 , reg_value); + +/* phy_dqs_tsel_enable_X 3bits DENALI_PHY_6/134/262/390 offset_24 */ + reg_value = (tsel_rd_en | (tsel_wr_en <<1 ) | (tsel_idle_en <<2 )) + <<24 ; +clrsetbits_le32 (&denali_phy[6 ],0x7 <<24 , reg_value); +clrsetbits_le32 (&denali_phy[134 ],0x7 <<24 , reg_value); +clrsetbits_le32 (&denali_phy[262 ],0x7 <<24 , reg_value); +clrsetbits_le32 (&denali_phy[390 ],0x7 <<24 , reg_value); + +/* phy_adr_tsel_enable_ 1bit DENALI_PHY_518/646/774 offset_8 */ + reg_value = tsel_wr_en <<8 ; +clrsetbits_le32 (&denali_phy[518 ],0x1 <<8 , reg_value); +clrsetbits_le32 (&denali_phy[646 ],0x1 <<8 , reg_value); +clrsetbits_le32 (&denali_phy[774 ],0x1 <<8 , reg_value); + +/* phy_pad_addr_term tsel 1bit DENALI_PHY_933 offset_17 */ + reg_value = tsel_wr_en <<17 ; +clrsetbits_le32 (&denali_phy[933 ],0x1 <<17 , reg_value);
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 331
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+/* +* pad_rst/cke/cs/clk_term tsel 1bits +* DENALI_PHY_938/936/940/934 offset_17 +*/ +clrsetbits_le32 (&denali_phy[938 ],0x1 <<17 , reg_value); +clrsetbits_le32 (&denali_phy[936 ],0x1 <<17 , reg_value); +clrsetbits_le32 (&denali_phy[940 ],0x1 <<17 , reg_value); +clrsetbits_le32 (&denali_phy[934 ],0x1 <<17 , reg_value); + +/* phy_pad_fdbk_term 1bit DENALI_PHY_930 offset_17 */
These could all use SHIFT/MASK values
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 343
Quoted 12 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +12
-0+clrsetbits_le32 (&denali_phy[930 ],0x1 <<17 , reg_value); +} + +static void phy_io_config (u32 channel, +const struct rk3399_sdram_params *sdram_params) +{ +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy; +u32 vref_mode_dq =0 ; +u32 vref_value_dq =0 ; +u32 vref_mode_ac =0 ; +u32 vref_value_ac =0 ; +u32 mode_sel =0 ;
Don't init here unless you need to.
Quoted 19 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +19
-0+u32 reg_value; +u32 drv_value, odt_value; +u32 speed; + +/* vref setting */ +if (sdram_params->dramtype ==LPDDR4 ) { +/* LPDDR4 */ + vref_mode_dq =0x6 ; + vref_value_dq =0x1f ; + vref_mode_ac =0x6 ; + vref_value_ac =0x1f ; + }else if (sdram_params->dramtype ==LPDDR3 ) { +if (sdram_params->odt ==1 ) { + vref_mode_dq =0x5 ;/* LPDDR3 ODT */ + drv_value = (readl (&denali_phy[6 ]) >>12 ) &0xf ; + odt_value = (readl (&denali_phy[6 ]) >>4 ) &0xf ; +if (drv_value ==PHY_DRV_ODT_48 ) { +switch (odt_value) { +case PHY_DRV_ODT_240 :
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 372
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ vref_value_dq =0x16 ; +break ; +case PHY_DRV_ODT_120 : + vref_value_dq =0x26 ; +break ; +case PHY_DRV_ODT_60 : + vref_value_dq =0x36 ; +break ; +default : +error ("Halting: Invalid ODT value.\n" );
Can we return an error and use debug() ?
Quoted 115 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +115
-0+ } + }else if (drv_value ==PHY_DRV_ODT_40 ) { +switch (odt_value) { +case PHY_DRV_ODT_240 : + vref_value_dq =0x19 ; +break ; +case PHY_DRV_ODT_120 : + vref_value_dq =0x23 ; +break ; +case PHY_DRV_ODT_60 : + vref_value_dq =0x31 ; +break ; +default : +error ("Halting: Invalid ODT value.\n" ); + } + }else if (drv_value ==PHY_DRV_ODT_34_3 ) { +switch (odt_value) { +case PHY_DRV_ODT_240 : + vref_value_dq =0x17 ; +break ; +case PHY_DRV_ODT_120 : + vref_value_dq =0x20 ; +break ; +case PHY_DRV_ODT_60 : + vref_value_dq =0x2e ; +break ; +default : +error ("Halting: Invalid ODT value.\n" ); + } + }else { +error ("Halting: Invalid DRV value.\n" ); + } + }else { + vref_mode_dq =0x2 ;/* LPDDR3 */ + vref_value_dq =0x1f ; + } + vref_mode_ac =0x2 ; + vref_value_ac =0x1f ; + }else if (sdram_params->dramtype ==DDR3 ) { +/* DDR3L */ + vref_mode_dq =0x1 ; + vref_value_dq =0x1f ; + vref_mode_ac =0x1 ; + vref_value_ac =0x1f ; + } +else +error ("Halting: Unknown DRAM type.\n" ); + + reg_value = (vref_mode_dq <<9 ) | (0x1 <<8 ) | vref_value_dq; + +/* PHY_913 PHY_PAD_VREF_CTRL_DQ_0 12bits offset_8 */ +clrsetbits_le32 (&denali_phy[913 ],0xfff <<8 , reg_value <<8 ); +/* PHY_914 PHY_PAD_VREF_CTRL_DQ_1 12bits offset_0 */ +clrsetbits_le32 (&denali_phy[914 ],0xfff , reg_value); +/* PHY_914 PHY_PAD_VREF_CTRL_DQ_2 12bits offset_16 */ +clrsetbits_le32 (&denali_phy[914 ],0xfff <<16 , reg_value <<16 ); +/* PHY_915 PHY_PAD_VREF_CTRL_DQ_3 12bits offset_0 */ +clrsetbits_le32 (&denali_phy[915 ],0xfff , reg_value); + + reg_value = (vref_mode_ac <<9 ) | (0x1 <<8 ) | vref_value_ac; + +/* PHY_915 PHY_PAD_VREF_CTRL_AC 12bits offset_16 */ +clrsetbits_le32 (&denali_phy[915 ],0xfff <<16 , reg_value <<16 ); + +if (sdram_params->dramtype ==LPDDR4 ) + mode_sel =0x6 ; +else if (sdram_params->dramtype ==LPDDR3 ) + mode_sel =0x0 ; +else if (sdram_params->dramtype ==DDR3 ) + mode_sel =0x1 ; + +/* PHY_924 PHY_PAD_FDBK_DRIVE */ +clrsetbits_le32 (&denali_phy[924 ],0x7 <<15 , mode_sel <<15 ); +/* PHY_926 PHY_PAD_DATA_DRIVE */ +clrsetbits_le32 (&denali_phy[926 ],0x7 <<6 , mode_sel <<6 ); +/* PHY_927 PHY_PAD_DQS_DRIVE */ +clrsetbits_le32 (&denali_phy[927 ],0x7 <<6 , mode_sel <<6 ); +/* PHY_928 PHY_PAD_ADDR_DRIVE */ +clrsetbits_le32 (&denali_phy[928 ],0x7 <<14 , mode_sel <<14 ); +/* PHY_929 PHY_PAD_CLK_DRIVE */ +clrsetbits_le32 (&denali_phy[929 ],0x7 <<14 , mode_sel <<14 ); +/* PHY_935 PHY_PAD_CKE_DRIVE */ +clrsetbits_le32 (&denali_phy[935 ],0x7 <<14 , mode_sel <<14 ); +/* PHY_937 PHY_PAD_RST_DRIVE */ +clrsetbits_le32 (&denali_phy[937 ],0x7 <<14 , mode_sel <<14 ); +/* PHY_939 PHY_PAD_CS_DRIVE */ +clrsetbits_le32 (&denali_phy[939 ],0x7 <<14 , mode_sel <<14 ); + + +/* speed setting */ +if (sdram_params->ddr_freq <400 *MHz) + speed =0x0 ; +else if (sdram_params->ddr_freq <800 *MHz) + speed =0x1 ; +else if (sdram_params->ddr_freq <1200 *MHz) + speed =0x2 ; +else + speed =0x3 ; + +/* PHY_924 PHY_PAD_FDBK_DRIVE */ +clrsetbits_le32 (&denali_phy[924 ],0x3 <<21 , speed <<21 ); +/* PHY_926 PHY_PAD_DATA_DRIVE */ +clrsetbits_le32 (&denali_phy[926 ],0x3 <<9 , speed <<9 ); +/* PHY_927 PHY_PAD_DQS_DRIVE */ +clrsetbits_le32 (&denali_phy[927 ],0x3 <<9 , speed <<9 ); +/* PHY_928 PHY_PAD_ADDR_DRIVE */ +clrsetbits_le32 (&denali_phy[928 ],0x3 <<17 , speed <<17 ); +/* PHY_929 PHY_PAD_CLK_DRIVE */ +clrsetbits_le32 (&denali_phy[929 ],0x3 <<17 , speed <<17 ); +/* PHY_935 PHY_PAD_CKE_DRIVE */ +clrsetbits_le32 (&denali_phy[935 ],0x3 <<17 , speed <<17 ); +/* PHY_937 PHY_PAD_RST_DRIVE */ +clrsetbits_le32 (&denali_phy[937 ],0x3 <<17 , speed <<17 ); +/* PHY_939 PHY_PAD_CS_DRIVE */ +clrsetbits_le32 (&denali_phy[939 ],0x3 <<17 , speed <<17 );
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 497
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+} + +static int pctl_cfg (u32 channel, +const struct rk3399_sdram_params *sdram_params) +{ +u32 *denali_ctl = rk3399_ddr_pctl[channel]->denali_ctl; +u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi; +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy; +const u32 *params_ctl = sdram_params->pctl_regs.denali_ctl; +const u32 *params_phy = sdram_params->phy_regs.denali_phy;
I think all of these should go in a struct, and you can set up the values by reading from the DT.
Quoted 28 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +28
-0+u32 tmp, tmp1, tmp2; +u32 pwrup_srefresh_exit; + +/* +* work around controller bug: +* Do not program DRAM_CLASS until NO_PHY_IND_TRAIN_INT is programmed +*/ +copy_to_reg (&denali_ctl[1 ], ¶ms_ctl[1 ], +sizeof (struct rk3399_ddr_pctl_regs ) -4 ); +writel (params_ctl[0 ], &denali_ctl[0 ]); +copy_to_reg (denali_pi, &sdram_params->pi_regs.denali_pi[0 ], +sizeof (struct rk3399_ddr_pi_regs )); +/* rank count need to set for init */ +set_memory_map (channel, sdram_params); + +writel (sdram_params->phy_regs.denali_phy[910 ], &denali_phy[910 ]); +writel (sdram_params->phy_regs.denali_phy[911 ], &denali_phy[911 ]); +writel (sdram_params->phy_regs.denali_phy[912 ], &denali_phy[912 ]); + + pwrup_srefresh_exit =readl (&denali_ctl[68 ]) &PWRUP_SREFRESH_EXIT ; +clrbits_le32 (&denali_ctl[68 ],PWRUP_SREFRESH_EXIT ); + +/* PHY_DLL_RST_EN */ +clrsetbits_le32 (&denali_phy[957 ],0x3 <<24 ,1 <<24 ); + +setbits_le32 (&denali_pi[0 ],START ); +setbits_le32 (&denali_ctl[0 ],START ); +
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 535
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+while (1 ) { + tmp =readl (&denali_phy[920 ]); + tmp1 =readl (&denali_phy[921 ]); + tmp2 =readl (&denali_phy[922 ]); +if ((((tmp >>16 ) &0x1 ) ==0x1 ) && + (((tmp1 >>16 ) &0x1 ) ==0x1 ) && + (((tmp1 >>0 ) &0x1 ) ==0x1 ) && + (((tmp2 >>0 ) &0x1 ) ==0x1 )) +break ; + }
What is this loop doing? SHIFT/MASK defines might help.
Quoted 34 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +34
-0+ +copy_to_reg (&denali_phy[896 ], ¶ms_phy[896 ], (958 -895 ) *4 ); +copy_to_reg (&denali_phy[0 ], ¶ms_phy[0 ], (90 -0 +1 ) *4 ); +copy_to_reg (&denali_phy[128 ], ¶ms_phy[128 ], (218 -128 +1 ) *4 ); +copy_to_reg (&denali_phy[256 ], ¶ms_phy[256 ], (346 -256 +1 ) *4 ); +copy_to_reg (&denali_phy[384 ], ¶ms_phy[384 ], (474 -384 +1 ) *4 ); +copy_to_reg (&denali_phy[512 ], ¶ms_phy[512 ], (549 -512 +1 ) *4 ); +copy_to_reg (&denali_phy[640 ], ¶ms_phy[640 ], (677 -640 +1 ) *4 ); +copy_to_reg (&denali_phy[768 ], ¶ms_phy[768 ], (805 -768 +1 ) *4 ); +set_ds_odt (channel, sdram_params); + +/* +* phy_dqs_tsel_wr_timing_X 8bits DENALI_PHY_84/212/340/468 offset_8 +* dqs_tsel_wr_end[7:4] add Half cycle +*/ + tmp = (readl (&denali_phy[84 ]) >>8 ) &0xff ; +clrsetbits_le32 (&denali_phy[84 ],0xff <<8 , (tmp +0x10 ) <<8 ); + tmp = (readl (&denali_phy[212 ]) >>8 ) &0xff ; +clrsetbits_le32 (&denali_phy[212 ],0xff <<8 , (tmp +0x10 ) <<8 ); + tmp = (readl (&denali_phy[340 ]) >>8 ) &0xff ; +clrsetbits_le32 (&denali_phy[340 ],0xff <<8 , (tmp +0x10 ) <<8 ); + tmp = (readl (&denali_phy[468 ]) >>8 ) &0xff ; +clrsetbits_le32 (&denali_phy[468 ],0xff <<8 , (tmp +0x10 ) <<8 ); + +/* +* phy_dqs_tsel_wr_timing_X 8bits DENALI_PHY_83/211/339/467 offset_8 +* dq_tsel_wr_end[7:4] add Half cycle +*/ + tmp = (readl (&denali_phy[83 ]) >>16 ) &0xff ; +clrsetbits_le32 (&denali_phy[83 ],0xff <<16 , (tmp +0x10 ) <<16 ); + tmp = (readl (&denali_phy[211 ]) >>16 ) &0xff ; +clrsetbits_le32 (&denali_phy[211 ],0xff <<16 , (tmp +0x10 ) <<16 ); + tmp = (readl (&denali_phy[339 ]) >>16 ) &0xff ; +clrsetbits_le32 (&denali_phy[339 ],0xff <<16 , (tmp +0x10 ) <<16 );
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 579
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ tmp = (readl (&denali_phy[467 ]) >>16 ) &0xff ; +clrsetbits_le32 (&denali_phy[467 ],0xff <<16 , (tmp +0x10 ) <<16 ); + +phy_io_config (channel, sdram_params); + +/* PHY_DLL_RST_EN */ +clrsetbits_le32 (&denali_phy[957 ],0x3 <<24 ,0x2 <<24 ); + +/* +* FIXME:
TODO(email):
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 588
Quoted 9 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +9
-0+* need to care ERROR bit, +* if 100ms do not get right status, return err +*/ + tmp =0 ; +while (!(readl (&denali_ctl[203 ]) & (1 <<3 ))) { +mdelay (10 ); + tmp ++; +if (tmp >10 ) +return -1 ;
-EIO or something else?
Quoted 18 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +18
-0+ } + +clrsetbits_le32 (&denali_ctl[68 ],PWRUP_SREFRESH_EXIT , + pwrup_srefresh_exit); +return 0 ; +} + +static void select_per_cs_training_index (u32 channel,u32 rank) +{ +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy; + +/* PHY_84 PHY_PER_CS_TRAINING_EN_0 1bit offset_16 */ +if ((readl (&denali_phy[84 ])>>16 ) &1 ) { +/* +* PHY_8/136/264/392 +* phy_per_cs_training_index_X 1bit offset_24 +*/ +clrsetbits_le32 (&denali_phy[8 ],0x1 <<24 , rank <<24 );
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 616
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+clrsetbits_le32 (&denali_phy[136 ],0x1 <<24 , rank <<24 ); +clrsetbits_le32 (&denali_phy[264 ],0x1 <<24 , rank <<24 ); +clrsetbits_le32 (&denali_phy[392 ],0x1 <<24 , rank <<24 ); + } +} + +static void override_write_leveling_value (u32 channel) +{ +u32 *denali_ctl = rk3399_ddr_pctl[channel]->denali_ctl; +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy;
Again if you pass in a struct with all these values you can avoid assigning them here.
Quoted 55 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +55
-0+u32 byte; + +/* PHY_896 PHY_FREQ_SEL_MULTICAST_EN 1bit offset_0 */ +setbits_le32 (&denali_phy[896 ],1 ); + +/* +* PHY_8/136/264/392 +* phy_per_cs_training_multicast_en_X 1bit offset_16 +*/ +clrsetbits_le32 (&denali_phy[8 ],0x1 <<16 ,1 <<16 ); +clrsetbits_le32 (&denali_phy[136 ],0x1 <<16 ,1 <<16 ); +clrsetbits_le32 (&denali_phy[264 ],0x1 <<16 ,1 <<16 ); +clrsetbits_le32 (&denali_phy[392 ],0x1 <<16 ,1 <<16 ); + +for (byte =0 ; byte <4 ; byte++) +clrsetbits_le32 (&denali_phy[63 + (128 * byte)],0xffff <<16 , +0x200 <<16 ); + +/* PHY_896 PHY_FREQ_SEL_MULTICAST_EN 1bit offset_0 */ +clrbits_le32 (&denali_phy[896 ],1 ); + +/* CTL_200 ctrlupd_req 1bit offset_8 */ +clrsetbits_le32 (&denali_ctl[200 ],0x1 <<8 ,0x1 <<8 ); +} + +static int data_training (u32 channel, +const struct rk3399_sdram_params *sdram_params, +u32 training_flag) +{ +u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi; +u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy; +u32 i, tmp; +u32 obs_0, obs_1, obs_2, obs_3, obs_err =0 ; +u32 rank = sdram_params->ch[channel].rank; + +/* PHY_927 PHY_PAD_DQS_DRIVE RPULL offset_22 */ +setbits_le32 (&denali_phy[927 ], (1 <<22 )); + +if (training_flag ==PI_FULL_TRAINING ) { +if (sdram_params->dramtype ==LPDDR4 ) { + training_flag =PI_CA_TRAINING |PI_WRITE_LEVELING | +PI_READ_GATE_TRAINING | +PI_READ_LEVELING |PI_WDQ_LEVELING ; + }else if (sdram_params->dramtype ==LPDDR3 ) { + training_flag =PI_CA_TRAINING |PI_WRITE_LEVELING | +PI_READ_GATE_TRAINING ; + }else if (sdram_params->dramtype ==DDR3 ) { + training_flag =PI_WRITE_LEVELING | +PI_READ_GATE_TRAINING | +PI_READ_LEVELING ; + } + } + +/* ca training(LPDDR4,LPDDR3 support) */ +if ((training_flag &PI_CA_TRAINING ) ==PI_CA_TRAINING ) {
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 681
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+for (i =0 ; i < rank; i++) { +select_per_cs_training_index (channel, i); +/* PI_100 PI_CALVL_EN:RW:8:2 */ +clrsetbits_le32 (&denali_pi[100 ],0x3 <<8 ,0x2 <<8 ); +/* PI_92 PI_CALVL_REQ:WR:16:1,PI_CALVL_CS:RW:24:2 */ +clrsetbits_le32 (&denali_pi[92 ], + (0x1 <<16 ) | (0x3 <<24 ), + (0x1 <<16 ) | (i <<24 )); + +while (1 ) {
What is this loop doing? Can you add a comment?
Quoted 66 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +66
-0+/* PI_174 PI_INT_STATUS:RD:8:18 */ + tmp =readl (&denali_pi[174 ]) >>8 ; +/* +* check status obs +* PHY_532/660/789 phy_adr_calvl_obs1_:0:32 +*/ + obs_0 =readl (&denali_phy[532 ]); + obs_1 =readl (&denali_phy[660 ]); + obs_2 =readl (&denali_phy[788 ]); +if (((obs_0 >>30 ) &0x3 ) || + ((obs_1 >>30 ) &0x3 ) || + ((obs_2 >>30 ) &0x3 )) + obs_err =1 ; +if ((((tmp >>11 ) &0x1 ) ==0x1 ) && + (((tmp >>13 ) &0x1 ) ==0x1 ) && + (((tmp >>5 ) &0x1 ) ==0x0 ) && + (obs_err ==0 )) +break ; +else if ((((tmp >>5 ) &0x1 ) ==0x1 ) || + (obs_err ==1 )) +return -1 ; + } +/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */ +writel (0x00003f7c , (&denali_pi[175 ])); + } +clrbits_le32 (&denali_pi[100 ],0x3 <<8 ); + } + +/* write leveling(LPDDR4,LPDDR3,DDR3 support) */ +if ((training_flag &PI_WRITE_LEVELING ) ==PI_WRITE_LEVELING ) { +for (i =0 ; i < rank; i++) { +select_per_cs_training_index (channel, i); +/* PI_60 PI_WRLVL_EN:RW:8:2 */ +clrsetbits_le32 (&denali_pi[60 ],0x3 <<8 ,0x2 <<8 ); +/* PI_59 PI_WRLVL_REQ:WR:8:1,PI_WRLVL_CS:RW:16:2 */ +clrsetbits_le32 (&denali_pi[59 ], + (0x1 <<8 ) | (0x3 <<16 ), + (0x1 <<8 ) | (i <<16 )); + +while (1 ) { +/* PI_174 PI_INT_STATUS:RD:8:18 */ + tmp =readl (&denali_pi[174 ]) >>8 ; + +/* +* check status obs, if error maybe can not +* get leveling done PHY_40/168/296/424 +* phy_wrlvl_status_obs_X:0:13 +*/ + obs_0 =readl (&denali_phy[40 ]); + obs_1 =readl (&denali_phy[168 ]); + obs_2 =readl (&denali_phy[296 ]); + obs_3 =readl (&denali_phy[424 ]); +if (((obs_0 >>12 ) &0x1 ) || + ((obs_1 >>12 ) &0x1 ) || + ((obs_2 >>12 ) &0x1 ) || + ((obs_3 >>12 ) &0x1 )) + obs_err =1 ; +if ((((tmp >>10 ) &0x1 ) ==0x1 ) && + (((tmp >>13 ) &0x1 ) ==0x1 ) && + (((tmp >>4 ) &0x1 ) ==0x0 ) && + (obs_err ==0 )) +break ; +else if ((((tmp >>4 ) &0x1 ) ==0x1 ) || + (obs_err ==1 )) +return -1 ; + }
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 757
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */ +writel (0x00003f7c , (&denali_pi[175 ])); + } + +override_write_leveling_value (channel); +clrbits_le32 (&denali_pi[60 ],0x3 <<8 ); + } + +/* read gate training(LPDDR4,LPDDR3,DDR3 support) */ +if ((training_flag &PI_READ_GATE_TRAINING ) ==PI_READ_GATE_TRAINING ) {
This is a very long function. Can you put each stage in a separate function?
if ((training_flag & PI_CA_TRAINING) == PI_CA_TRAINING)
ca_training();
if ((training_flag & PI_READ_GATE_TRAINING) == PI_READ_GATE_TRAINING)
read_gate_training();
Or similar...
Quoted 47 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +47
-0+for (i =0 ; i < rank; i++) { +select_per_cs_training_index (channel, i); +/* PI_80 PI_RDLVL_GATE_EN:RW:24:2 */ +clrsetbits_le32 (&denali_pi[80 ],0x3 <<24 ,0x2 <<24 ); +/* +* PI_74 PI_RDLVL_GATE_REQ:WR:16:1 +* PI_RDLVL_CS:RW:24:2 +*/ +clrsetbits_le32 (&denali_pi[74 ], + (0x1 <<16 ) | (0x3 <<24 ), + (0x1 <<16 ) | (i <<24 )); + +while (1 ) { +/* PI_174 PI_INT_STATUS:RD:8:18 */ + tmp =readl (&denali_pi[174 ]) >>8 ; + +/* +* check status obs +* PHY_43/171/299/427 +* PHY_GTLVL_STATUS_OBS_x:16:8 +*/ + obs_0 =readl (&denali_phy[43 ]); + obs_1 =readl (&denali_phy[171 ]); + obs_2 =readl (&denali_phy[299 ]); + obs_3 =readl (&denali_phy[427 ]); +if (((obs_0 >> (16 +6 )) &0x3 ) || + ((obs_1 >> (16 +6 )) &0x3 ) || + ((obs_2 >> (16 +6 )) &0x3 ) || + ((obs_3 >> (16 +6 )) &0x3 )) + obs_err =1 ; +if ((((tmp >>9 ) &0x1 ) ==0x1 ) && + (((tmp >>13 ) &0x1 ) ==0x1 ) && + (((tmp >>3 ) &0x1 ) ==0x0 ) && + (obs_err ==0 )) +break ; +else if ((((tmp >>3 ) &0x1 ) ==0x1 ) || + (obs_err ==1 )) +return -1 ; + } +/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */ +writel (0x00003f7c , (&denali_pi[175 ])); + } +clrbits_le32 (&denali_pi[80 ],0x3 <<24 ); + } + +/* read leveling(LPDDR4,LPDDR3,DDR3 support) */ +if ((training_flag &PI_READ_LEVELING ) ==PI_READ_LEVELING ) {
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 814
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+for (i =0 ; i < rank; i++) { +select_per_cs_training_index (channel, i); +/* PI_80 PI_RDLVL_EN:RW:16:2 */ +clrsetbits_le32 (&denali_pi[80 ],0x3 <<16 ,0x2 <<16 ); +/* PI_74 PI_RDLVL_REQ:WR:8:1,PI_RDLVL_CS:RW:24:2 */ +clrsetbits_le32 (&denali_pi[74 ], + (0x1 <<8 ) | (0x3 <<24 ), + (0x1 <<8 ) | (i <<24 )); + +while (1 ) {
What is this loop doing? Is there any commonality compared to the other similar loops? Perhaps a function where you pass in the mask to check?
Quoted 79 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +79
-0+/* PI_174 PI_INT_STATUS:RD:8:18 */ + tmp =readl (&denali_pi[174 ]) >>8 ; + +/* +* make sure status obs not report error bit +* PHY_46/174/302/430 +* phy_rdlvl_status_obs_X:16:8 +*/ +if ((((tmp >>8 ) &0x1 ) ==0x1 ) && + (((tmp >>13 ) &0x1 ) ==0x1 ) && + (((tmp >>2 ) &0x1 ) ==0x0 )) +break ; +else if (((tmp >>2 ) &0x1 ) ==0x1 ) +return -1 ; + } +/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */ +writel (0x00003f7c , (&denali_pi[175 ])); + } +clrbits_le32 (&denali_pi[80 ],0x3 <<16 ); + } + +/* wdq leveling(LPDDR4 support) */ +if ((training_flag &PI_WDQ_LEVELING ) ==PI_WDQ_LEVELING ) { +for (i =0 ; i < rank; i++) { +select_per_cs_training_index (channel, i); +/* +* disable PI_WDQLVL_VREF_EN before wdq leveling? +* PI_181 PI_WDQLVL_VREF_EN:RW:8:1 +*/ +clrbits_le32 (&denali_pi[181 ],0x1 <<8 ); +/* PI_124 PI_WDQLVL_EN:RW:16:2 */ +clrsetbits_le32 (&denali_pi[124 ],0x3 <<16 ,0x2 <<16 ); +/* PI_121 PI_WDQLVL_REQ:WR:8:1,PI_WDQLVL_CS:RW:16:2 */ +clrsetbits_le32 (&denali_pi[121 ], + (0x1 <<8 ) | (0x3 <<16 ), + (0x1 <<8 ) | (i <<16 )); + +while (1 ) { +/* PI_174 PI_INT_STATUS:RD:8:18 */ + tmp =readl (&denali_pi[174 ]) >>8 ; +if ((((tmp >>12 ) &0x1 ) ==0x1 ) && + (((tmp >>13 ) &0x1 ) ==0x1 ) && + (((tmp >>6 ) &0x1 ) ==0x0 )) +break ; +else if (((tmp >>6 ) &0x1 ) ==0x1 ) +return -1 ; + } +/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */ +writel (0x00003f7c , (&denali_pi[175 ])); + } +clrbits_le32 (&denali_pi[124 ],0x3 <<16 ); + } + +/* PHY_927 PHY_PAD_DQS_DRIVE RPULL offset_22 */ +clrbits_le32 (&denali_phy[927 ], (1 <<22 )); + +return 0 ; +} + +static void set_ddrconfig (const struct rk3399_sdram_params *sdram_params, +unsigned char channel,u32 ddrconfig) +{ +/* only need to set ddrconfig */ +struct rk3399_msch_regs *ddr_msch_regs = rk3399_msch[channel]; +unsigned int cs0_cap =0 ; +unsigned int cs1_cap =0 ; + + cs0_cap = (1 << (sdram_params->ch[channel].cs0_row + + sdram_params->ch[channel].col + + sdram_params->ch[channel].bk + + sdram_params->ch[channel].bw -20 )); +if (sdram_params->ch[channel].rank >1 ) + cs1_cap = cs0_cap >> (sdram_params->ch[channel].cs0_row + - sdram_params->ch[channel].cs1_row); +if (sdram_params->ch[channel].row_3_4) { + cs0_cap = cs0_cap *3 /4 ; + cs1_cap = cs1_cap *3 /4 ; + } +
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 903
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+writel (ddrconfig | (ddrconfig <<8 ), &ddr_msch_regs->ddrconf); +writel (((cs0_cap /32 ) &0xff ) | (((cs1_cap /32 ) &0xff ) <<8 ), + &ddr_msch_regs->ddrsize); +} + +static void dram_all_config (const struct rk3399_sdram_params *sdram_params) +{ +u32 sys_reg =0 ; +unsigned int channel; +unsigned int use;
What is 'use' for? Is 'index' a better name?
Quoted 59 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +59
-0+ + sys_reg |=SYS_REG_ENC_DDRTYPE (sdram_params->dramtype); + sys_reg |=SYS_REG_ENC_NUM_CH (sdram_params->num_channels); +for (channel =0 , use =0 ; + (use < sdram_params->num_channels) && (channel <2 ); channel++) { +const struct rk3399_sdram_channel *info = + &sdram_params->ch[channel]; +struct rk3399_msch_regs *ddr_msch_regs; +const struct rk3399_msch_timings *noc_timing; + +if (sdram_params->ch[channel].col ==0 ) +continue ; + use++; + sys_reg |=SYS_REG_ENC_ROW_3_4 (info->row_3_4, channel); + sys_reg |=SYS_REG_ENC_CHINFO (channel); + sys_reg |=SYS_REG_ENC_RANK (info->rank, channel); + sys_reg |=SYS_REG_ENC_COL (info->col, channel); + sys_reg |=SYS_REG_ENC_BK (info->bk, channel); + sys_reg |=SYS_REG_ENC_CS0_ROW (info->cs0_row, channel); +if (sdram_params->ch[channel].rank >1 ) + sys_reg |=SYS_REG_ENC_CS1_ROW (info->cs1_row, channel); + sys_reg |=SYS_REG_ENC_BW (info->bw, channel); + sys_reg |=SYS_REG_ENC_DBW (info->dbw, channel); + + ddr_msch_regs = rk3399_msch[channel]; + noc_timing = &sdram_params->ch[channel].noc_timings; +writel (noc_timing->ddrtiminga0.d32, + &ddr_msch_regs->ddrtiminga0.d32); +writel (noc_timing->ddrtimingb0.d32, + &ddr_msch_regs->ddrtimingb0.d32); +writel (noc_timing->ddrtimingc0.d32, + &ddr_msch_regs->ddrtimingc0.d32); +writel (noc_timing->devtodev0.d32, + &ddr_msch_regs->devtodev0.d32); +writel (noc_timing->ddrmode.d32, + &ddr_msch_regs->ddrmode.d32); + +/* rank 1 memory clock disable (dfi_dram_clk_disable = 1) */ +if (sdram_params->ch[channel].rank ==1 ) +setbits_le32 (&rk3399_ddr_pctl[channel]->denali_ctl[276 ], +1 <<17 ); + } + +writel (sys_reg, &rk3399_pmugrf->os_reg2); +DDR_STRIDE (sdram_params->stride); + +/* reboot hold register set */ +writel (PRESET_SGRF_HOLD (0 ) |PRESET_GPIO0_HOLD (1 ) | +PRESET_GPIO1_HOLD (1 ), + &pmucru_ptr->pmucru_rstnhold_con[1 ]); +clrsetbits_le32 (&cru_ptr->glb_rst_con,0x3 ,0x3 ); +} + +static void switch_to_phy_index1 (const struct rk3399_sdram_params *sdram_params) +{ +u32 channel; +u32 *denali_phy; +u32 ch_count = sdram_params->num_channels; +int i =0 ;
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 972
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ +writel (RK_CLRSETBITS (0x03 <<4 |1 <<2 |1 , +1 <<4 |1 <<2 |1 ), + &rk3399_ddr_cic->cic_ctrl0); +while (!(readl (&rk3399_ddr_cic->cic_status0) & (1 <<2 ))) { +mdelay (10 ); + i++; +if (i >10 ) { +error ("index1 frequency change overtime, reset\n" ); +hang ();
Please have this return an error, so there is just one hang(), at the top-level caller. Also consider using debug() here.
Quoted 70 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +70
-0+ } + } + + i =0 ; +writel (RK_CLRSETBITS (1 <<1 ,1 <<1 ), &rk3399_ddr_cic->cic_ctrl0); +while (!(readl (&rk3399_ddr_cic->cic_status0) & (1 <<0 ))) { +mdelay (10 ); +if (i>10 ) { +error ("index1 frequency done overtime, reset\n" ); +hang (); + } + } + +for (channel =0 ; channel < ch_count; channel++) { + denali_phy = rk3399_ddr_publ[channel]->denali_phy; +clrsetbits_le32 (&denali_phy[896 ], (0x3 <<8 ) |1 ,1 <<8 ); +if (data_training (channel, sdram_params,PI_FULL_TRAINING )) { +error ("index1 training failed, reset\n" ); +hang (); + } + } +} +static struct rk3399_sdram_params sdram_configs[] = { +#include "sdram-lpddr3-4GB.inc" +}; + +void sdram_init (const struct rk3399_sdram_params *sdram_params) +{ +unsigned char dramtype = sdram_params->dramtype; +unsigned int ddr_freq = sdram_params->ddr_freq; +int channel; + +debug ("Starting SDRAM initialization...\n" ); + +if ((dramtype ==DDR3 && ddr_freq >800 *MHz) || + (dramtype ==LPDDR3 && ddr_freq >933 *MHz) || + (dramtype ==LPDDR4 && ddr_freq >800 *MHz)) +error ("SDRAM frequency is to high!" ); + + +for (channel =0 ; channel <2 ; channel++) { +phy_dll_bypass_set (channel, rk3399_ddr_publ[channel], ddr_freq); + +if (channel >= sdram_params->num_channels) +continue ; + +/* +* TODO: we need to find the root cause why this +* step may fail, before that, we just reset the +* system, and start again. +*/ +if (pctl_cfg (channel, sdram_params) !=0 ) { +printf ("pctl_cfg fail, reset\n" ); +hang (); + } + +/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */ +if (dramtype ==LPDDR3 ) +udelay (10 ); + +if (data_training (channel, sdram_params,PI_FULL_TRAINING )) { +printf ("SDRAM initialization failed, reset\n" ); +hang (); + } + +set_ddrconfig (sdram_params, channel, + sdram_params->ch[channel].ddrconfig); + } +dram_all_config (sdram_params); +switch_to_phy_index1 (sdram_params);
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 1052
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+ + debug("Finish SDRAM initialization...\n"); +} +#endif + +size_t sdram_size_mb (void ) +{ +u32 rank, col, bk, cs0_row, cs1_row, bw, row_3_4; +size_t chipsize_mb =0 ; +static size_t size_mb =0 ;
Do we need this? Perhaps we can just calculate it each time? It will only be called once or twice I suspect.
Quoted 44 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +44
-0+u32 ch; + +if (!size_mb) { +u32 sys_reg =readl (&rk3399_pmugrf->os_reg2); +u32 ch_num =SYS_REG_DEC_NUM_CH (sys_reg); + +for (ch =0 ; ch < ch_num; ch++) { + rank =SYS_REG_DEC_RANK (sys_reg, ch); + col =SYS_REG_DEC_COL (sys_reg, ch); + bk =SYS_REG_DEC_BK (sys_reg, ch); + cs0_row =SYS_REG_DEC_CS0_ROW (sys_reg, ch); + cs1_row =SYS_REG_DEC_CS1_ROW (sys_reg, ch); + bw =SYS_REG_DEC_BW (sys_reg, ch); + row_3_4 =SYS_REG_DEC_ROW_3_4 (sys_reg, ch); + + chipsize_mb = (1 << (cs0_row + col + bk + bw -20 )); + +if (rank >1 ) + chipsize_mb += chipsize_mb >> + (cs0_row - cs1_row); +if (row_3_4) + chipsize_mb = chipsize_mb *3 /4 ; + size_mb += chipsize_mb; + } + +/* +* we use the 0x00000000~0xf7ffffff space +* since 0xf8000000~0xffffffff is soc register space +* so we reserve it +*/ + size_mb =min_t (size_t, size_mb,0xf8000000 /(1 <<20 )); + } + +return size_mb; +} + +static int rk3399_dmc_probe (struct udevice *dev) +{ +#ifdef CONFIG_SPL_BUILD +struct rk3399_sdram_params *plat = sdram_configs; +int ret; +struct clk ddr_clk; + + ret =clk_get_by_index (dev,0 , &ddr_clk);
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 1106
Quoted 10 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +10
-0+if (ret){ +debug ("%s clk get failed %d\n" , __func__, ret); +return ret; + } + ret =clk_set_rate (&ddr_clk, plat->ddr_freq); +if (ret <0 ){ +debug ("%s clk set failed %d\n" , __func__, ret); +return ret; + } +sdram_init (plat);
Check return value here.
Quoted 16 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +15
-1+#endif +return 0 ; +} + +static const struct udevice_id rk3399_dmc_ids[] = { + { .compatible ="rockchip,rk3399-dmc" }, + { } +}; + +U_BOOT_DRIVER (dmc_rk3399) = { + .name ="rockchip_rk3399_dmc" , + .id =UCLASS_RAM , + .of_match = rk3399_dmc_ids, + .probe = rk3399_dmc_probe, +}; --
Quoted 2 lines
1.9.1
Regards,
Simon
RFC PATCH 2/3 arm64: rk3399: add ddr controller driver
Hi Simon,
Thanks for your review, I would like to take all the comments but
one below,
because I think the change only get a lot of work to do but have no any help on understand the source code, because each of shift/mask operation has comment
for it.
I have spend a lot of time on make different controller base
addresses from dts
and gather them into one structure, which we think it does not make any helpful on
source code itself but only met some bug on of-platdata for address decode.
There are hundreds of ctrl/pi/phy register in this dram controller,
I don't think it's a
good idea to add MASK/SHIFT MACRO definition for all of them since comments are already there.
I will send my patch V2 without mask/shift change, but updates by
your other comments.
Thanks,
- Kever
Quoted 10 lines · Simon Glass
Quoted 8 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c line 134
Quoted 8 lines
arch/arm/mach-rockchip/rk3399/sdram_rk3399.c new file +8
-0+{ +u32 *denali_phy = ddr_publ_regs->denali_phy; +if (freq <=125 *MHz) { +/* phy_sw_master_mode_X PHY_86/214/342/470 4bits offset_8 */ +setbits_le32 (&denali_phy[86 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[214 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[342 ], (0x3 <<2 ) <<8 ); +setbits_le32 (&denali_phy[470 ], (0x3 <<2 ) <<8 );These should really be defined as SHIFT and MASK values.
RFC PATCH 3/3 spl: add support to booting with ATF
In reply to Kever YangATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+) create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.h
common/spl/Kconfig +14 -0
@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at: https://github.com/ARM-software/arm-trusted-firmware + +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPL
common/spl/Makefile +1 -0
@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o
common/spl/spl.c +4 -0
@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);
common/spl/spl_atf.c new file +91 -0
91 lines not on this page · raw message
include/atf_common.h new file +295 -0
295 lines not on this page · raw message
include/spl.h +1 -0
@@ -251,4 +251,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);int spl_mmc_load_image (struct spl_image_info *spl_image,struct spl_boot_device *bootdev); +void bl31_entry (void );#endif
Signature · 1 line
1.9.1
RFC PATCH 3/3 spl: add support to booting with ATF
Quoted 4 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
Quoted 4 lines
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF.
Then SPL_ATF_TEXT_BASE address is not needed because it will be read from fit format.
Quoted 72 lines
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+) create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.hcommon/spl/Kconfig +14
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at: https://github.com/ARM-software/arm-trusted-firmware + +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +4
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +0
-0
common/spl/spl_atf.c line 7
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0@@ -0,0 +1,91 @@ +/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+
this is not based on gpl file that's why license should be different.
Quoted 36 lines
common/spl/spl_atf.c new file +36
-0+*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept +* aside to pass platform specific and trusted firmware related information +* to BL31. This memory is allocated by allocating memory to +* bl2_to_bl31_params_mem_t structure which is a superset of all the +* structure whose information is passed to BL31 +* NOTE: This function should be called only once and should be done +* before generating params to BL31 +******************************************************************************/ +struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ +memset (&bl31_params_mem,0 ,sizeof (struct bl2_to_bl31_params_mem_t )); + +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ +SET_PARAM_HEAD (bl2_to_bl31_params->bl31_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 );
common/spl/spl_atf.c line 53
Quoted 10 lines
common/spl/spl_atf.c new file +10
-0+ +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE + bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_ep_info,PARAM_EP , +VERSION_1 ,0 ); + bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); +#endif /* BL32_BASE */
Is this used?
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0+ +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */
common/spl/spl_atf.c line 70
Quoted 10 lines
common/spl/spl_atf.c new file +10
-0+ bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr =SPSR_64 (MODE_EL2 ,MODE_SP_ELX , +DISABLE_ALL_EXECPTIONS ); + + bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); + +
double lines.
Quoted 45 lines
common/spl/spl_atf.c new file +21
-0+return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; +void (*entry)(struct bl31_params_t *params,void *plat_params) =NULL ; + + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +18
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* +* Redistributions of source code must retain the above copyright notice, this +* list of conditions and the following disclaimer. +* +* Redistributions in binary form must reproduce the above copyright notice, +* this list of conditions and the following disclaimer in the documentation +* and/or other materials provided with the distribution. +* +* Neither the name of ARM nor the names of its contributors may be used +* to endorse or promote products derived from this software without specific +* prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
include/atf_common.h line 28
Quoted 10 lines
include/atf_common.h new file +10
-0+* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE.
This should be probably in SPDX format.
Quoted 273 lines
include/atf_common.h new file +267
-0+*/ + +#ifndef __BL_COMMON_H__ +#define __BL_COMMON_H__ + +#define SECURE 0x0 +#define NON_SECURE 0x1 +#define sec_state_is_valid (s) (((s) == SECURE) || ((s) == NON_SECURE)) + +#define UP 1 +#define DOWN 0 + +/******************************************************************************* +* Constants to identify the location of a memory region in a given memory +* layout. +******************************************************************************/ +#define TOP 0x1 +#define BOTTOM !TOP + +/******************************************************************************* +* Constants that allow assembler code to access members of and the +* 'entry_point_info' structure at their correct offsets. +******************************************************************************/ +#define ENTRY_POINT_INFO_PC_OFFSET 0x08 +#define ENTRY_POINT_INFO_ARGS_OFFSET 0x18 + +/* The following are used to set/get image attributes. */ +#define PARAM_EP_SECURITY_MASK (0x1) + +#define GET_SECURITY_STATE (x) (x & PARAM_EP_SECURITY_MASK) +#define SET_SECURITY_STATE (x, security) \ + ((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security)) + + +/* +* The following are used for image state attributes. +* Image can only be in one of the following state. +*/ +#define IMAGE_STATE_RESET 0 +#define IMAGE_STATE_COPIED 1 +#define IMAGE_STATE_COPYING 2 +#define IMAGE_STATE_AUTHENTICATED 3 +#define IMAGE_STATE_EXECUTED 4 +#define IMAGE_STATE_INTERRUPTED 5 + +#define EP_SECURE 0x0 +#define EP_NON_SECURE 0x1 + +#define EP_EE_MASK 0x2 +#define EP_EE_LITTLE 0x0 +#define EP_EE_BIG 0x2 +#define EP_GET_EE (x) (x & EP_EE_MASK) +#define EP_SET_EE (x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee)) + +#define EP_ST_MASK 0x4 +#define EP_ST_DISABLE 0x0 +#define EP_ST_ENABLE 0x4 +#define EP_GET_ST (x) (x & EP_ST_MASK) +#define EP_SET_ST (x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee)) + +#define EP_EXE_MASK 0x8 +#define NON_EXECUTABLE 0x0 +#define EXECUTABLE 0x8 +#define EP_GET_EXE (x) (x & EP_EXE_MASK) +#define EP_SET_EXE (x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee)) + +#define PARAM_EP 0x01 +#define PARAM_IMAGE_BINARY 0x02 +#define PARAM_BL31 0x03 + +#define VERSION_1 0x01 + +#define INVALID_IMAGE_ID (0xFFFFFFFF) + +#define SET_PARAM_HEAD (_p, _type, _ver, _attr) do { \ + (_p)->h.type = (uint8_t)(_type); \ + (_p)->h.version = (uint8_t)(_ver); \ + (_p)->h.size = (uint16_t)sizeof(*_p); \ + (_p)->h.attr = (uint32_t)(_attr) ; \ + } while (0) + +/* Following is used for populating structure members statically. */ +#define SET_STATIC_PARAM_HEAD (_p, _type, _ver, _p_type, _attr) \ + ._p.h.type = (uint8_t)(_type), \ + ._p.h.version = (uint8_t)(_ver), \ + ._p.h.size = (uint16_t)sizeof(_p_type), \ + ._p.h.attr = (uint32_t)(_attr) + +#define MODE_RW_SHIFT 0x4 +#define MODE_RW_MASK 0x1 +#define MODE_RW_64 0x0 +#define MODE_RW_32 0x1 + +#define MODE_EL_SHIFT 0x2 +#define MODE_EL_MASK 0x3 +#define MODE_EL3 0x3 +#define MODE_EL2 0x2 +#define MODE_EL1 0x1 +#define MODE_EL0 0x0 + +#define MODE_SP_SHIFT 0x0 +#define MODE_SP_MASK 0x1 +#define MODE_SP_EL0 0x0 +#define MODE_SP_ELX 0x1 + +#define SPSR_DAIF_SHIFT 6 +#define SPSR_DAIF_MASK 0x0f + +#define DAIF_FIQ_BIT (1<<0) +#define DAIF_IRQ_BIT (1<<0) +#define DAIF_ABT_BIT (1<<0) +#define DAIF_DBG_BIT (1<<0) +#define DISABLE_ALL_EXECPTIONS \ + (DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT) + +#define SPSR_64 (el, sp, daif) \ + (MODE_RW_64 << MODE_RW_SHIFT | \ + ((el) & MODE_EL_MASK) << MODE_EL_SHIFT | \ + ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT | \ + ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT) + +/******************************************************************************* +* Constants to indicate type of exception to the common exception handler. +******************************************************************************/ +#define SYNC_EXCEPTION_SP_EL0 0x0 +#define IRQ_SP_EL0 0x1 +#define FIQ_SP_EL0 0x2 +#define SERROR_SP_EL0 0x3 +#define SYNC_EXCEPTION_SP_ELX 0x4 +#define IRQ_SP_ELX 0x5 +#define FIQ_SP_ELX 0x6 +#define SERROR_SP_ELX 0x7 +#define SYNC_EXCEPTION_AARCH64 0x8 +#define IRQ_AARCH64 0x9 +#define FIQ_AARCH64 0xa +#define SERROR_AARCH64 0xb +#define SYNC_EXCEPTION_AARCH32 0xc +#define IRQ_AARCH32 0xd +#define FIQ_AARCH32 0xe +#define SERROR_AARCH32 0xf + +#define SPSR_USE_L 0 +#define SPSR_USE_H 1 +#define SPSR_L_H_MASK 1 +#define SPSR_M_SHIFT 4 +#define SPSR_ERET_32 (1 << SPSR_M_SHIFT) +#define SPSR_ERET_64 (0 << SPSR_M_SHIFT) +#define SPSR_FIQ (1 << 6) +#define SPSR_IRQ (1 << 7) +#define SPSR_SERROR (1 << 8) +#define SPSR_DEBUG (1 << 9) +#define SPSR_EXCEPTION_MASK (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG) + +#ifndef __ASSEMBLY__ + +/******************************************************************************* +* Structure used for telling the next BL how much of a particular type of +* memory is available for its use and how much is already used. +******************************************************************************/ +struct aapcs64_params_t { +unsigned long arg0; +unsigned long arg1; +unsigned long arg2; +unsigned long arg3; +unsigned long arg4; +unsigned long arg5; +unsigned long arg6; +unsigned long arg7; +}; + +/*************************************************************************** +* This structure provides version information and the size of the +* structure, attributes for the structure it represents +***************************************************************************/ +struct param_header_t { +uint8_t type;/* type of the structure */ +uint8_t version;/* version of this structure */ +uint16_t size;/* size of this structure in bytes */ +uint32_t attr;/* attributes: unused bits SBZ */ +}; + +/***************************************************************************** +* This structure represents the superset of information needed while +* switching exception levels. The only two mechanisms to do so are +* ERET & SMC. Security state is indicated using bit zero of header +* attribute +* NOTE: BL1 expects entrypoint followed by spsr at an offset from the start +* of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while +* processing SMC to jump to BL31. +*****************************************************************************/ +struct entry_point_info_t { +struct param_header_t h; +uintptr_t pc; +uint32_t spsr; +struct aapcs64_params_t args; +}; + +/***************************************************************************** +* Image info binary provides information from the image loader that +* can be used by the firmware to manage available trusted RAM. +* More advanced firmware image formats can provide additional +* information that enables optimization or greater flexibility in the +* common firmware code +*****************************************************************************/ +struct atf_image_info_t { +struct param_header_t h; +uintptr_t image_base;/* physical address of base of image */ +uint32_t image_size;/* bytes read from image file */ +}; + +/***************************************************************************** +* The image descriptor struct definition. +*****************************************************************************/ +struct image_desc_t { +/* Contains unique image id for the image. */ +unsigned int image_id; +/* +* This member contains Image state information. +* Refer IMAGE_STATE_XXX defined above. +*/ +unsigned int state; +uint32_t copied_size;/* image size copied in blocks */ +struct atf_image_info_t atf_image_info; +struct entry_point_info_t ep_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that can be passed to +* BL31 e.g. while passing control to it from BL2. The BL32 parameters will be +* populated only if BL2 detects its presence. A pointer to a structure of this +* type should be passed in X0 to BL31's cold boot entrypoint. +* +* Use of this structure and the X0 parameter is not mandatory: the BL31 +* platform code can use other mechanisms to provide the necessary information +* about BL32 and BL33 to the common and SPD code. +* +* BL31 image information is mandatory if this structure is used. If either of +* the optional BL32 and BL33 image information is not provided, this is +* indicated by the respective image_info pointers being zero. +******************************************************************************/ +struct bl31_params_t { +struct param_header_t h; +struct atf_image_info_t *bl31_image_info; +struct entry_point_info_t *bl32_ep_info; +struct atf_image_info_t *bl32_image_info; +struct entry_point_info_t *bl33_ep_info; +struct atf_image_info_t *bl33_image_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that is passed to +* BL31, e.g. while passing control to it from BL2, bl31_params +* and other platform specific params +******************************************************************************/ +struct bl2_to_bl31_params_mem_t { +struct bl31_params_t bl31_params; +struct atf_image_info_t bl31_image_info; +struct atf_image_info_t bl32_image_info; +struct atf_image_info_t bl33_image_info; +struct entry_point_info_t bl33_ep_info; +struct entry_point_info_t bl32_ep_info; +struct entry_point_info_t bl31_ep_info; +}; + +#endif /*__ASSEMBLY__*/ + +#endif /* __BL_COMMON_H__ */ include/spl.h +0
-0
include/spl.h int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr); · line 255
Quoted 5 lines
include/spl.h +1
-0@@ -251,4 +251,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);int spl_mmc_load_image (struct spl_image_info *spl_image,struct spl_boot_device *bootdev); +void bl31_entry (void );#endif
Thanks,
Michal
RFC PATCH 3/3 spl: add support to booting with ATF
Hi Michal,
Thanks for your comments.
Quoted 6 lines · Michal Simek
Quoted 4 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
OK, will follow your comment in next patch.
Quoted 9 lines
Quoted 5 lines
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
Yes, I use FIT format to contain BL33 and BL32 and SPL load all of them.
Quoted 4 lines
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
This is a good idea, and it look fine for support ATF in SPL in local source code,
but it will be better if we have an official support for ATF, right?
Quoted 5 lines
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF.
ThenSPL_ATF_TEXT_BASEaddress is not needed because it will be read from fit format.
Yes, you are right, SPL_ATF_TEXT_BASE is not a must, we gen get it from fit.
Quoted 74 lines
Quoted 74 lines
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+)create mode 100644
common/spl/spl_atf.c
create mode 100644include/atf_common.hcommon/spl/Kconfig +14
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at: https://github.com/ARM-software/arm-trusted-firmware + +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +4
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +0
-0@@ -0,0 +1,91 @@
Quoted 8 lines
common/spl/spl_atf.c line 7
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0+/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+ this is not based on gpl file that's why license should be different.
Sorry, I do not get what your mean, I'm not good at license policy, ARM ATF use its own license, what should I do for license when I use code from ATF?
Quoted 41 lines
Quoted 41 lines
common/spl/spl_atf.c new file +39
-0+*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept +* aside to pass platform specific and trusted firmware related information +* to BL31. This memory is allocated by allocating memory to +* bl2_to_bl31_params_mem_t structure which is a superset of all the +* structure whose information is passed to BL31 +* NOTE: This function should be called only once and should be done +* before generating params to BL31 +******************************************************************************/ +struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ +memset (&bl31_params_mem,0 ,sizeof (struct bl2_to_bl31_params_mem_t )); + +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ +SET_PARAM_HEAD (bl2_to_bl31_params->bl31_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); + +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE
Quoted 8 lines
common/spl/spl_atf.c line 53
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0+ bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_ep_info,PARAM_EP , +VERSION_1 ,0 ); + bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); +#endif /* BL32_BASE */ Is this used?
Not use for me now, but it may useful later, because we need to fill info about bl32 if there is.
Quoted 11 lines
Quoted 11 lines
common/spl/spl_atf.c new file +10
-0+ +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */ + bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
Quoted 8 lines
common/spl/spl_atf.c line 70
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0+ DISABLE_ALL_EXECPTIONS); + + bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); + +double lines.
Will fix in next patch, confuse why checkpatch did not find this.
Quoted 49 lines
Quoted 49 lines
common/spl/spl_atf.c new file +21
-0+return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; +void (*entry)(struct bl31_params_t *params,void *plat_params) =NULL ; + + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +21
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* +* Redistributions of source code must retain the above copyright notice, this +* list of conditions and the following disclaimer. +* +* Redistributions in binary form must reproduce the above copyright notice, +* this list of conditions and the following disclaimer in the documentation +* and/or other materials provided with the distribution. +* +* Neither the name of ARM nor the names of its contributors may be used +* to endorse or promote products derived from this software without specific +* prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
Quoted 8 lines
include/atf_common.h line 28
Quoted 7 lines
include/atf_common.h new file +7
-0+* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. This should be probably in SPDX format.
Like previous reply, I don't know how to deal with this license, do you mean I can use
SPDX license without any information about the origin License?
Thanks,
- Kever
Quoted 286 lines
Quoted 275 lines
include/atf_common.h new file +267
-0+*/ + +#ifndef __BL_COMMON_H__ +#define __BL_COMMON_H__ + +#define SECURE 0x0 +#define NON_SECURE 0x1 +#define sec_state_is_valid (s) (((s) == SECURE) || ((s) == NON_SECURE)) + +#define UP 1 +#define DOWN 0 + +/******************************************************************************* +* Constants to identify the location of a memory region in a given memory +* layout. +******************************************************************************/ +#define TOP 0x1 +#define BOTTOM !TOP + +/******************************************************************************* +* Constants that allow assembler code to access members of and the +* 'entry_point_info' structure at their correct offsets. +******************************************************************************/ +#define ENTRY_POINT_INFO_PC_OFFSET 0x08 +#define ENTRY_POINT_INFO_ARGS_OFFSET 0x18 + +/* The following are used to set/get image attributes. */ +#define PARAM_EP_SECURITY_MASK (0x1) + +#define GET_SECURITY_STATE (x) (x & PARAM_EP_SECURITY_MASK) +#define SET_SECURITY_STATE (x, security) \ + ((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security)) + + +/* +* The following are used for image state attributes. +* Image can only be in one of the following state. +*/ +#define IMAGE_STATE_RESET 0 +#define IMAGE_STATE_COPIED 1 +#define IMAGE_STATE_COPYING 2 +#define IMAGE_STATE_AUTHENTICATED 3 +#define IMAGE_STATE_EXECUTED 4 +#define IMAGE_STATE_INTERRUPTED 5 + +#define EP_SECURE 0x0 +#define EP_NON_SECURE 0x1 + +#define EP_EE_MASK 0x2 +#define EP_EE_LITTLE 0x0 +#define EP_EE_BIG 0x2 +#define EP_GET_EE (x) (x & EP_EE_MASK) +#define EP_SET_EE (x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee)) + +#define EP_ST_MASK 0x4 +#define EP_ST_DISABLE 0x0 +#define EP_ST_ENABLE 0x4 +#define EP_GET_ST (x) (x & EP_ST_MASK) +#define EP_SET_ST (x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee)) + +#define EP_EXE_MASK 0x8 +#define NON_EXECUTABLE 0x0 +#define EXECUTABLE 0x8 +#define EP_GET_EXE (x) (x & EP_EXE_MASK) +#define EP_SET_EXE (x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee)) + +#define PARAM_EP 0x01 +#define PARAM_IMAGE_BINARY 0x02 +#define PARAM_BL31 0x03 + +#define VERSION_1 0x01 + +#define INVALID_IMAGE_ID (0xFFFFFFFF) + +#define SET_PARAM_HEAD (_p, _type, _ver, _attr) do { \ + (_p)->h.type = (uint8_t)(_type); \ + (_p)->h.version = (uint8_t)(_ver); \ + (_p)->h.size = (uint16_t)sizeof(*_p); \ + (_p)->h.attr = (uint32_t)(_attr) ; \ + } while (0) + +/* Following is used for populating structure members statically. */ +#define SET_STATIC_PARAM_HEAD (_p, _type, _ver, _p_type, _attr) \ + ._p.h.type = (uint8_t)(_type), \ + ._p.h.version = (uint8_t)(_ver), \ + ._p.h.size = (uint16_t)sizeof(_p_type), \ + ._p.h.attr = (uint32_t)(_attr) + +#define MODE_RW_SHIFT 0x4 +#define MODE_RW_MASK 0x1 +#define MODE_RW_64 0x0 +#define MODE_RW_32 0x1 + +#define MODE_EL_SHIFT 0x2 +#define MODE_EL_MASK 0x3 +#define MODE_EL3 0x3 +#define MODE_EL2 0x2 +#define MODE_EL1 0x1 +#define MODE_EL0 0x0 + +#define MODE_SP_SHIFT 0x0 +#define MODE_SP_MASK 0x1 +#define MODE_SP_EL0 0x0 +#define MODE_SP_ELX 0x1 + +#define SPSR_DAIF_SHIFT 6 +#define SPSR_DAIF_MASK 0x0f + +#define DAIF_FIQ_BIT (1<<0) +#define DAIF_IRQ_BIT (1<<0) +#define DAIF_ABT_BIT (1<<0) +#define DAIF_DBG_BIT (1<<0) +#define DISABLE_ALL_EXECPTIONS \ + (DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT) + +#define SPSR_64 (el, sp, daif) \ + (MODE_RW_64 << MODE_RW_SHIFT | \ + ((el) & MODE_EL_MASK) << MODE_EL_SHIFT | \ + ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT | \ + ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT) + +/******************************************************************************* +* Constants to indicate type of exception to the common exception handler. +******************************************************************************/ +#define SYNC_EXCEPTION_SP_EL0 0x0 +#define IRQ_SP_EL0 0x1 +#define FIQ_SP_EL0 0x2 +#define SERROR_SP_EL0 0x3 +#define SYNC_EXCEPTION_SP_ELX 0x4 +#define IRQ_SP_ELX 0x5 +#define FIQ_SP_ELX 0x6 +#define SERROR_SP_ELX 0x7 +#define SYNC_EXCEPTION_AARCH64 0x8 +#define IRQ_AARCH64 0x9 +#define FIQ_AARCH64 0xa +#define SERROR_AARCH64 0xb +#define SYNC_EXCEPTION_AARCH32 0xc +#define IRQ_AARCH32 0xd +#define FIQ_AARCH32 0xe +#define SERROR_AARCH32 0xf + +#define SPSR_USE_L 0 +#define SPSR_USE_H 1 +#define SPSR_L_H_MASK 1 +#define SPSR_M_SHIFT 4 +#define SPSR_ERET_32 (1 << SPSR_M_SHIFT) +#define SPSR_ERET_64 (0 << SPSR_M_SHIFT) +#define SPSR_FIQ (1 << 6) +#define SPSR_IRQ (1 << 7) +#define SPSR_SERROR (1 << 8) +#define SPSR_DEBUG (1 << 9) +#define SPSR_EXCEPTION_MASK (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG) + +#ifndef __ASSEMBLY__ + +/******************************************************************************* +* Structure used for telling the next BL how much of a particular type of +* memory is available for its use and how much is already used. +******************************************************************************/ +struct aapcs64_params_t { +unsigned long arg0; +unsigned long arg1; +unsigned long arg2; +unsigned long arg3; +unsigned long arg4; +unsigned long arg5; +unsigned long arg6; +unsigned long arg7; +}; + +/*************************************************************************** +* This structure provides version information and the size of the +* structure, attributes for the structure it represents +***************************************************************************/ +struct param_header_t { +uint8_t type;/* type of the structure */ +uint8_t version;/* version of this structure */ +uint16_t size;/* size of this structure in bytes */ +uint32_t attr;/* attributes: unused bits SBZ */ +}; + +/***************************************************************************** +* This structure represents the superset of information needed while +* switching exception levels. The only two mechanisms to do so are +* ERET & SMC. Security state is indicated using bit zero of header +* attribute +* NOTE: BL1 expects entrypoint followed by spsr at an offset from the start +* of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while +* processing SMC to jump to BL31. +*****************************************************************************/ +struct entry_point_info_t { +struct param_header_t h; +uintptr_t pc; +uint32_t spsr; +struct aapcs64_params_t args; +}; + +/***************************************************************************** +* Image info binary provides information from the image loader that +* can be used by the firmware to manage available trusted RAM. +* More advanced firmware image formats can provide additional +* information that enables optimization or greater flexibility in the +* common firmware code +*****************************************************************************/ +struct atf_image_info_t { +struct param_header_t h; +uintptr_t image_base;/* physical address of base of image */ +uint32_t image_size;/* bytes read from image file */ +}; + +/***************************************************************************** +* The image descriptor struct definition. +*****************************************************************************/ +struct image_desc_t { +/* Contains unique image id for the image. */ +unsigned int image_id; +/* +* This member contains Image state information. +* Refer IMAGE_STATE_XXX defined above. +*/ +unsigned int state; +uint32_t copied_size;/* image size copied in blocks */ +struct atf_image_info_t atf_image_info; +struct entry_point_info_t ep_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that can be passed to +* BL31 e.g. while passing control to it from BL2. The BL32 parameters will be +* populated only if BL2 detects its presence. A pointer to a structure of this +* type should be passed in X0 to BL31's cold boot entrypoint. +* +* Use of this structure and the X0 parameter is not mandatory: the BL31 +* platform code can use other mechanisms to provide the necessary information +* about BL32 and BL33 to the common and SPD code. +* +* BL31 image information is mandatory if this structure is used. If either of +* the optional BL32 and BL33 image information is not provided, this is +* indicated by the respective image_info pointers being zero. +******************************************************************************/ +struct bl31_params_t { +struct param_header_t h; +struct atf_image_info_t *bl31_image_info; +struct entry_point_info_t *bl32_ep_info; +struct atf_image_info_t *bl32_image_info; +struct entry_point_info_t *bl33_ep_info; +struct atf_image_info_t *bl33_image_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that is passed to +* BL31, e.g. while passing control to it from BL2, bl31_params +* and other platform specific params +******************************************************************************/ +struct bl2_to_bl31_params_mem_t { +struct bl31_params_t bl31_params; +struct atf_image_info_t bl31_image_info; +struct atf_image_info_t bl32_image_info; +struct atf_image_info_t bl33_image_info; +struct entry_point_info_t bl33_ep_info; +struct entry_point_info_t bl32_ep_info; +struct entry_point_info_t bl31_ep_info; +}; + +#endif /*__ASSEMBLY__*/ + +#endif /* __BL_COMMON_H__ */ include/spl.h +0
-0include/spl.h int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr); · line 255
Quoted 5 lines
include/spl.h +1
-0@@ -251,4 +251,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);int spl_mmc_load_image (struct spl_image_info *spl_image,struct spl_boot_device *bootdev); +void bl31_entry (void );#endif Thanks,
Michal
RFC PATCH 3/3 spl: add support to booting with ATF
Hi,
Quoted 18 lines · Kever Yang
Hi Michal,
Thanks for your comments.
Quoted 6 lines · Michal Simek
Quoted 4 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
OK, will follow your comment in next patch.
Quoted 5 lines
Quoted 5 lines
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
Quoted 6 lines
Quoted 4 lines
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
Yes, I use FIT format to contain BL33 and BL32 and SPL load all of them.
Do you have some logs? I didn't check the latest code but IIRC it was possible to handle one image and dt not several images which has to be supported. There is also loadables section in fit which can help with this.
Quoted 8 lines
Quoted 4 lines
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
This is a good idea, and it look fine for support ATF in SPL in local source code,
but it will be better if we have an official support for ATF, right?
Definitely having support just for ATF is much better solution than what I use in ZynqMP.
Quoted 9 lines
Quoted 6 lines
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF.
ThenSPL_ATF_TEXT_BASEaddress is not needed because it will be read from fit format.Yes, you are right,
SPL_ATF_TEXT_BASEis not a must, we gen get it from fit.
ok.
Quoted 85 lines
Quoted 76 lines
Quoted 76 lines
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+)create mode 100644
common/spl/spl_atf.c
create mode 100644include/atf_common.hcommon/spl/Kconfig +6
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at:https://github.com/ARM-software/arm-trusted-firmware
common/spl/Kconfig +7
-0+ +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +3
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +0
-0@@ -0,0 +1,91 @@Quoted 8 lines
common/spl/spl_atf.c line 7
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0+/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+ this is not based on gpl file that's why license should be different.
Quoted 3 lines
Sorry, I do not get what your mean, I'm not good at license policy, ARM ATF use its own license, what should I do for license when I use code from ATF?
I am not that guy too. But if you took parts of code which is not GPL you can't label it as a GPL.
Quoted 60 lines
Quoted 51 lines
Quoted 51 lines
common/spl/spl_atf.c new file +12
-0+*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept
+ * aside to pass platform specific and trusted firmware related information
+ * to BL31. This memory is allocated by allocating memory to
+ *bl2_to_bl31_params_mem_tstructure which is a superset of all the
+ * structure whose information is passed to BL31
+ * NOTE: This function should be called only once and should be done
+ * before generating params to BL31
+ ******************************************************************************/common/spl/spl_atf.c new file +9
-0+struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ + memset(&bl31_params_mem, 0, sizeof(struct
bl2_to_bl31_params_mem_t));common/spl/spl_atf.c new file +7
-0+ +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ + SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info,
PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +6
-0+ VERSION_1, 0); + +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE + bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; + SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,Quoted 8 lines
common/spl/spl_atf.c
Quoted 7 lines
common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); + bl2_to_bl31_params->bl32_image_info =&bl31_params_mem.bl32_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); +#endif /* BL32_BASE */ Is this used?
Quoted 2 lines
Not use for me now, but it may useful later, because we need to fill info about bl32 if there is.
I think that make sense to look at fit format and try to integrate all these stuff together.
Quoted 97 lines
Quoted 13 lines
Quoted 13 lines
common/spl/spl_atf.c new file +12
-0+ +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */ + bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr =SPSR_64 (MODE_EL2 ,MODE_SP_ELX , +DISABLE_ALL_EXECPTIONS ); +Quoted 8 lines
common/spl/spl_atf.c line 54
Quoted 7 lines
+
bl2_to_bl31_params->bl33_image_info=
&bl31_params_mem.bl33_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +3
-0+ VERSION_1, 0); + +double lines.
Will fix in next patch, confuse why checkpatch did not find this.
Quoted 65 lines
Quoted 65 lines
common/spl/spl_atf.c new file +12
-0+return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; + void (*entry)(struct bl31_params_t *params, void *plat_params) =NULL;
common/spl/spl_atf.c new file +9
-0+ + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +2
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * Neither the name of ARM nor the names of its contributors may be used
+ * to endorse or promote products derived from this software without specific
+ * prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESSQuoted 8 lines
Quoted 7 lines
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.This should be probably in SPDX format.
Quoted 3 lines
Like previous reply, I don't know how to deal with this license, do you mean I can use
SPDX license without any information about the origin License?
Tom, Simon: Please correct me if I am wrong.
There are 2 things here.
0. Identify license and especially this part. The rest looks like BSD license
* Neither the name of ARM nor the names of its contributors may be used
* to endorse or promote products derived from this software without specific
* prior written permission.
1. If this ARM license is compatible with u-boot Licensing model
2. We have Licenses folder where License can go but not sure if only SPDX licenses can got there and if ARM published this license there to have official SPDX tag. Please look at Licenses/README
Thanks,
Michal
RFC PATCH 3/3 spl: add support to booting with ATF
+Tom for license comment
Hi,
Quoted 141 lines · Michal Simek
Hi,
Quoted 18 lines · Kever Yang
Hi Michal,
Thanks for your comments.
Quoted 6 lines · Michal Simek
Quoted 4 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
OK, will follow your comment in next patch.
Quoted 5 lines
Quoted 5 lines
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
Quoted 6 lines
Quoted 4 lines
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
Yes, I use FIT format to contain BL33 and BL32 and SPL load all of them.
Do you have some logs? I didn't check the latest code but IIRC it was possible to handle one image and dt not several images which has to be supported. There is also loadables section in fit which can help with this.
Quoted 8 lines
Quoted 4 lines
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
This is a good idea, and it look fine for support ATF in SPL in local source code,
but it will be better if we have an official support for ATF, right?Definitely having support just for ATF is much better solution than what I use in ZynqMP.
Quoted 9 lines
Quoted 6 lines
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF.
ThenSPL_ATF_TEXT_BASEaddress is not needed because it will be read from fit format.Yes, you are right,
SPL_ATF_TEXT_BASEis not a must, we gen get it from fit.ok.
Quoted 85 lines
Quoted 84 lines
Quoted 76 lines
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+)create mode 100644
common/spl/spl_atf.c
create mode 100644include/atf_common.hcommon/spl/Kconfig +6
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at:https://github.com/ARM-software/arm-trusted-firmware
common/spl/Kconfig +7
-0+ +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +3
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +0
-0common/spl/spl_atf.c line 7
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0@@ -0,0 +1,91 @@ +/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+ this is not based on gpl file that's why license should be different.
Quoted 6 lines
Quoted 3 lines
Sorry, I do not get what your mean, I'm not good at license policy, ARM ATF use its own license, what should I do for license when I use code from ATF?
I am not that guy too. But if you took parts of code which is not GPL you can't label it as a GPL.
That's right. Perhaps rewrite and rename bl2_plat_get_bl31_params()? Then you can say that it is inspired by that file, and perhaps avoid the license problem.
The macros are horrible anyway.
Quoted 176 lines
Quoted 62 lines
Quoted 53 lines
Quoted 53 lines
common/spl/spl_atf.c new file +12
-0+*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept
+ * aside to pass platform specific and trusted firmware related information
+ * to BL31. This memory is allocated by allocating memory to
+ *bl2_to_bl31_params_mem_tstructure which is a superset of all the
+ * structure whose information is passed to BL31
+ * NOTE: This function should be called only once and should be done
+ * before generating params to BL31
+ ******************************************************************************/common/spl/spl_atf.c new file +9
-0+struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ + memset(&bl31_params_mem, 0, sizeof(struct
bl2_to_bl31_params_mem_t));common/spl/spl_atf.c new file +7
-0+ +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ + SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info,
PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +6
-0+ VERSION_1, 0); + +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE + bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; + SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,Quoted 8 lines
common/spl/spl_atf.c
Quoted 7 lines
common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); + bl2_to_bl31_params->bl32_image_info =&bl31_params_mem.bl32_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); +#endif /* BL32_BASE */ Is this used?
Quoted 2 lines
Not use for me now, but it may useful later, because we need to fill info about bl32 if there is.
I think that make sense to look at fit format and try to integrate all these stuff together.
Quoted 97 lines
Quoted 13 lines
Quoted 13 lines
common/spl/spl_atf.c new file +12
-0+ +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */ + bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr =SPSR_64 (MODE_EL2 ,MODE_SP_ELX , +DISABLE_ALL_EXECPTIONS ); +Quoted 8 lines
common/spl/spl_atf.c line 54
Quoted 7 lines
+
bl2_to_bl31_params->bl33_image_info=
&bl31_params_mem.bl33_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +3
-0+ VERSION_1, 0); + +double lines.
Will fix in next patch, confuse why checkpatch did not find this.
Quoted 65 lines
Quoted 65 lines
common/spl/spl_atf.c new file +12
-0+return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; + void (*entry)(struct bl31_params_t *params, void *plat_params) =NULL;
common/spl/spl_atf.c new file +9
-0+ + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +2
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * Neither the name of ARM nor the names of its contributors may be used
+ * to endorse or promote products derived from this software without specific
+ * prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESSQuoted 8 lines
Quoted 7 lines
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.This should be probably in SPDX format.
Quoted 3 lines
Like previous reply, I don't know how to deal with this license, do you mean I can use
SPDX license without any information about the origin License?Tom, Simon: Please correct me if I am wrong.
There are 2 things here.0. Identify license and especially this part. The rest looks like BSD license
Quoted 8 lines
* Neither the name of ARM nor the names of its contributors may be used
* to endorse or promote products derived from this software without specific
* prior written permission.1. If this ARM license is compatible with u-boot Licensing model
2. We have Licenses folder where License can go but not sure if only SPDX licenses can got there and if ARM published this license there to have official SPDX tag. Please look at Licenses/README
Perhaps we need to get this licence registered with SPDX?
https://spdx.org/spdx-license-list/request-new-license
But it would be better to get that piece removed somehow. It makes a non-standard license. Failing that, add a new licence:
1. Add to Licenses directory with a suitable identifier (see for example commit 0f4d2f8e)
2. Submit a new license registration request
Regards,
Simon
RFC PATCH 3/3 spl: add support to booting with ATF
Quoted 346 lines · Simon Glass
+Tom for license comment
Hi,
Quoted 141 lines · Michal Simek
Hi,
Quoted 18 lines · Kever Yang
Hi Michal,
Thanks for your comments.
Quoted 6 lines · Michal Simek
Quoted 4 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
OK, will follow your comment in next patch.
Quoted 5 lines
Quoted 5 lines
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
Quoted 6 lines
Quoted 4 lines
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
Yes, I use FIT format to contain BL33 and BL32 and SPL load all of them.
Do you have some logs? I didn't check the latest code but IIRC it was possible to handle one image and dt not several images which has to be supported. There is also loadables section in fit which can help with this.
Quoted 8 lines
Quoted 4 lines
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
This is a good idea, and it look fine for support ATF in SPL in local source code,
but it will be better if we have an official support for ATF, right?Definitely having support just for ATF is much better solution than what I use in ZynqMP.
Quoted 9 lines
Quoted 6 lines
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF.
ThenSPL_ATF_TEXT_BASEaddress is not needed because it will be read from fit format.Yes, you are right,
SPL_ATF_TEXT_BASEis not a must, we gen get it from fit.ok.
Quoted 85 lines
Quoted 84 lines
Quoted 76 lines
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+)create mode 100644
common/spl/spl_atf.c
create mode 100644include/atf_common.hcommon/spl/Kconfig +6
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at:https://github.com/ARM-software/arm-trusted-firmware
common/spl/Kconfig +7
-0+ +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +3
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +0
-0common/spl/spl_atf.c line 7
Quoted 7 lines
common/spl/spl_atf.c new file +7
-0@@ -0,0 +1,91 @@ +/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+ this is not based on gpl file that's why license should be different.
Quoted 6 lines
Quoted 3 lines
Sorry, I do not get what your mean, I'm not good at license policy, ARM ATF use its own license, what should I do for license when I use code from ATF?
I am not that guy too. But if you took parts of code which is not GPL you can't label it as a GPL.
That's right. Perhaps rewrite and rename
bl2_plat_get_bl31_params()? Then you can say that it is inspired by that file, and perhaps avoid the license problem.The macros are horrible anyway.
Quoted 176 lines
Quoted 62 lines
Quoted 53 lines
Quoted 53 lines
common/spl/spl_atf.c new file +12
-0+*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept
+ * aside to pass platform specific and trusted firmware related information
+ * to BL31. This memory is allocated by allocating memory to
+ *bl2_to_bl31_params_mem_tstructure which is a superset of all the
+ * structure whose information is passed to BL31
+ * NOTE: This function should be called only once and should be done
+ * before generating params to BL31
+ ******************************************************************************/common/spl/spl_atf.c new file +9
-0+struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ + memset(&bl31_params_mem, 0, sizeof(struct
bl2_to_bl31_params_mem_t));common/spl/spl_atf.c new file +7
-0+ +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ + SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info,
PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +6
-0+ VERSION_1, 0); + +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE + bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; + SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,Quoted 8 lines
common/spl/spl_atf.c
Quoted 7 lines
common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); + bl2_to_bl31_params->bl32_image_info =&bl31_params_mem.bl32_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +2
-0+ VERSION_1, 0); +#endif /* BL32_BASE */ Is this used?
Quoted 2 lines
Not use for me now, but it may useful later, because we need to fill info about bl32 if there is.
I think that make sense to look at fit format and try to integrate all these stuff together.
Quoted 97 lines
Quoted 13 lines
Quoted 13 lines
common/spl/spl_atf.c new file +12
-0+ +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */ + bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr =SPSR_64 (MODE_EL2 ,MODE_SP_ELX , +DISABLE_ALL_EXECPTIONS ); +Quoted 8 lines
common/spl/spl_atf.c line 54
Quoted 7 lines
+
bl2_to_bl31_params->bl33_image_info=
&bl31_params_mem.bl33_image_info;
+ SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY,common/spl/spl_atf.c new file +3
-0+ VERSION_1, 0); + +double lines.
Will fix in next patch, confuse why checkpatch did not find this.
Quoted 65 lines
Quoted 65 lines
common/spl/spl_atf.c new file +12
-0+return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; + void (*entry)(struct bl31_params_t *params, void *plat_params) =NULL;
common/spl/spl_atf.c new file +9
-0+ + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +2
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * Neither the name of ARM nor the names of its contributors may be used
+ * to endorse or promote products derived from this software without specific
+ * prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESSQuoted 8 lines
Quoted 7 lines
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.This should be probably in SPDX format.
Quoted 3 lines
Like previous reply, I don't know how to deal with this license, do you mean I can use
SPDX license without any information about the origin License?Tom, Simon: Please correct me if I am wrong.
There are 2 things here.0. Identify license and especially this part. The rest looks like BSD license
Quoted 8 lines
* Neither the name of ARM nor the names of its contributors may be used
* to endorse or promote products derived from this software without specific
* prior written permission.1. If this ARM license is compatible with u-boot Licensing model
2. We have Licenses folder where License can go but not sure if only SPDX licenses can got there and if ARM published this license there to have official SPDX tag. Please look at Licenses/READMEPerhaps we need to get this licence registered with SPDX?
Quoted 8 lines
https://spdx.org/spdx-license-list/request-new-license
But it would be better to get that piece removed somehow. It makes a non-standard license. Failing that, add a new licence:
1. Add to Licenses directory with a suitable identifier (see for example commit
0f4d2f8e)
2. Submit a new license registration request
For both license questions, we must be correct. The copy of ATF that I just looked at has a 3 clause BSD license, with the 3rd clause worded perhaps oddly, but, that looks right for 3-clause BSD. This is compatible with U-Boot. If on the other hand ATF has some data structures/etc that non-ATF code really needs to know and it is not BSD-3-Clause, we should perhaps try and contact someone with the authority to re-license it, using include/android_image.h as an example of where U-Boot hosts, officially, this kind of information in a more permissible license. Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170128/b4b9927a/attachment.sig>
RFC PATCH 3/3 spl: add support to booting with ATF
In reply to Kever YangHi
Who is suppose to maintain source in "common/spl"?
I hope to get more comments before my first version patch without 'RFC' and with suggestion from Michal.
Thanks,
- Kever
Quoted 482 lines · Kever Yang
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
Signed-off-by Kever Yang <[email protected]>---
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+)create mode 100644
common/spl/spl_atf.c
create mode 100644include/atf_common.hcommon/spl/Kconfig +14
-0@@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT + bool "Support ARM trust firmware" + depends on SPL + help + ATF(ARM Trust Firmware) is component for ARM arch64 which need to + load by SPL(consider as BL2 in ATF). + More detail at: https://github.com/ARM-software/arm-trusted-firmware + +config SPL_ATF_TEXT_BASE + depends on SPL_ATF_SUPPORT + hex "ATF TEXT BASE addr" + help + This is the base address in memory for ATF text and entry point. + config TPL_ENV_SUPPORT bool "Support an environment" depends on TPLcommon/spl/Makefile +1
-0@@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.ocommon/spl/spl.c +4
-0@@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024);#endif +#ifdef CONFIG_SPL_ATF_SUPPORT +bl31_entry (); +#endif +debug ("loaded - jumping to U-Boot..." );spl_board_prepare_for_boot ();jump_to_image_no_args (&spl_image);common/spl/spl_atf.c new file +91
-0@@ -0,0 +1,91 @@ +/* +* Copyright (C) 2016 Rockchip Electronic Co.,Ltd +* Written by Kever Yang <[email protected]> +* +* origin from arm-trust-firmware +* plat/arm/common/arm_bl2_setup.c +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h> + +static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params; + +/******************************************************************************* +* This function assigns a pointer to the memory that the platform has kept +* aside to pass platform specific and trusted firmware related information +* to BL31. This memory is allocated by allocating memory to +* bl2_to_bl31_params_mem_t structure which is a superset of all the +* structure whose information is passed to BL31 +* NOTE: This function should be called only once and should be done +* before generating params to BL31 +******************************************************************************/ +struct bl31_params_t *bl2_plat_get_bl31_params (void ) +{ +struct entry_point_info_t *bl33_ep_info; + +/* +* Initialise the memory for all the arguments that needs to +* be passed to BL31 +*/ +memset (&bl31_params_mem,0 ,sizeof (struct bl2_to_bl31_params_mem_t )); + +/* Assign memory for TF related information */ + bl2_to_bl31_params = &bl31_params_mem.bl31_params; +SET_PARAM_HEAD (bl2_to_bl31_params,PARAM_BL31 ,VERSION_1 ,0 ); + +/* Fill BL31 related information */ +SET_PARAM_HEAD (bl2_to_bl31_params->bl31_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); + +/* Fill BL32 related information if it exists */ +#ifdef BL32_BASE + bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_ep_info,PARAM_EP , +VERSION_1 ,0 ); + bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl32_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); +#endif /* BL32_BASE */ + +/* Fill BL33 related information */ + bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info; + bl33_ep_info = &bl31_params_mem.bl33_ep_info; +SET_PARAM_HEAD (bl33_ep_info,PARAM_EP ,VERSION_1 ,EP_NON_SECURE ); + +/* BL33 expects to receive the primary CPU MPID (through x0) */ + bl33_ep_info->args.arg0 =0xffff &read_mpidr (); + bl33_ep_info->pc =CONFIG_SYS_TEXT_BASE ; + bl33_ep_info->spsr =SPSR_64 (MODE_EL2 ,MODE_SP_ELX , +DISABLE_ALL_EXECPTIONS ); + + bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info; +SET_PARAM_HEAD (bl2_to_bl31_params->bl33_image_info,PARAM_IMAGE_BINARY , +VERSION_1 ,0 ); + + +return bl2_to_bl31_params; +} + +void raw_write_daif (uint32_t daif) +{ + __asm__ __volatile__("msr DAIF, %0\n\t" : :"r" (daif) :"memory" ); +} + +void bl31_entry (void ) +{ +struct bl31_params_t *bl31_params; +void (*entry)(struct bl31_params_t *params,void *plat_params) =NULL ; + + bl31_params =bl2_plat_get_bl31_params (); + entry = (void *)CONFIG_SPL_ATF_TEXT_BASE ; + +raw_write_daif (SPSR_EXCEPTION_MASK ); +dcache_disable (); + +entry (bl31_params,NULL ); +}include/atf_common.h new file +295
-0@@ -0,0 +1,295 @@ +/* +* Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* +* Redistributions of source code must retain the above copyright notice, this +* list of conditions and the following disclaimer. +* +* Redistributions in binary form must reproduce the above copyright notice, +* this list of conditions and the following disclaimer in the documentation +* and/or other materials provided with the distribution. +* +* Neither the name of ARM nor the names of its contributors may be used +* to endorse or promote products derived from this software without specific +* prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ + +#ifndef __BL_COMMON_H__ +#define __BL_COMMON_H__ + +#define SECURE 0x0 +#define NON_SECURE 0x1 +#define sec_state_is_valid (s) (((s) == SECURE) || ((s) == NON_SECURE)) + +#define UP 1 +#define DOWN 0 + +/******************************************************************************* +* Constants to identify the location of a memory region in a given memory +* layout. +******************************************************************************/ +#define TOP 0x1 +#define BOTTOM !TOP + +/******************************************************************************* +* Constants that allow assembler code to access members of and the +* 'entry_point_info' structure at their correct offsets. +******************************************************************************/ +#define ENTRY_POINT_INFO_PC_OFFSET 0x08 +#define ENTRY_POINT_INFO_ARGS_OFFSET 0x18 + +/* The following are used to set/get image attributes. */ +#define PARAM_EP_SECURITY_MASK (0x1) + +#define GET_SECURITY_STATE (x) (x & PARAM_EP_SECURITY_MASK) +#define SET_SECURITY_STATE (x, security) \ + ((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security)) + + +/* +* The following are used for image state attributes. +* Image can only be in one of the following state. +*/ +#define IMAGE_STATE_RESET 0 +#define IMAGE_STATE_COPIED 1 +#define IMAGE_STATE_COPYING 2 +#define IMAGE_STATE_AUTHENTICATED 3 +#define IMAGE_STATE_EXECUTED 4 +#define IMAGE_STATE_INTERRUPTED 5 + +#define EP_SECURE 0x0 +#define EP_NON_SECURE 0x1 + +#define EP_EE_MASK 0x2 +#define EP_EE_LITTLE 0x0 +#define EP_EE_BIG 0x2 +#define EP_GET_EE (x) (x & EP_EE_MASK) +#define EP_SET_EE (x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee)) + +#define EP_ST_MASK 0x4 +#define EP_ST_DISABLE 0x0 +#define EP_ST_ENABLE 0x4 +#define EP_GET_ST (x) (x & EP_ST_MASK) +#define EP_SET_ST (x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee)) + +#define EP_EXE_MASK 0x8 +#define NON_EXECUTABLE 0x0 +#define EXECUTABLE 0x8 +#define EP_GET_EXE (x) (x & EP_EXE_MASK) +#define EP_SET_EXE (x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee)) + +#define PARAM_EP 0x01 +#define PARAM_IMAGE_BINARY 0x02 +#define PARAM_BL31 0x03 + +#define VERSION_1 0x01 + +#define INVALID_IMAGE_ID (0xFFFFFFFF) + +#define SET_PARAM_HEAD (_p, _type, _ver, _attr) do { \ + (_p)->h.type = (uint8_t)(_type); \ + (_p)->h.version = (uint8_t)(_ver); \ + (_p)->h.size = (uint16_t)sizeof(*_p); \ + (_p)->h.attr = (uint32_t)(_attr) ; \ + } while (0) + +/* Following is used for populating structure members statically. */ +#define SET_STATIC_PARAM_HEAD (_p, _type, _ver, _p_type, _attr) \ + ._p.h.type = (uint8_t)(_type), \ + ._p.h.version = (uint8_t)(_ver), \ + ._p.h.size = (uint16_t)sizeof(_p_type), \ + ._p.h.attr = (uint32_t)(_attr) + +#define MODE_RW_SHIFT 0x4 +#define MODE_RW_MASK 0x1 +#define MODE_RW_64 0x0 +#define MODE_RW_32 0x1 + +#define MODE_EL_SHIFT 0x2 +#define MODE_EL_MASK 0x3 +#define MODE_EL3 0x3 +#define MODE_EL2 0x2 +#define MODE_EL1 0x1 +#define MODE_EL0 0x0 + +#define MODE_SP_SHIFT 0x0 +#define MODE_SP_MASK 0x1 +#define MODE_SP_EL0 0x0 +#define MODE_SP_ELX 0x1 + +#define SPSR_DAIF_SHIFT 6 +#define SPSR_DAIF_MASK 0x0f + +#define DAIF_FIQ_BIT (1<<0) +#define DAIF_IRQ_BIT (1<<0) +#define DAIF_ABT_BIT (1<<0) +#define DAIF_DBG_BIT (1<<0) +#define DISABLE_ALL_EXECPTIONS \ + (DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT) + +#define SPSR_64 (el, sp, daif) \ + (MODE_RW_64 << MODE_RW_SHIFT | \ + ((el) & MODE_EL_MASK) << MODE_EL_SHIFT | \ + ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT | \ + ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT) + +/******************************************************************************* +* Constants to indicate type of exception to the common exception handler. +******************************************************************************/ +#define SYNC_EXCEPTION_SP_EL0 0x0 +#define IRQ_SP_EL0 0x1 +#define FIQ_SP_EL0 0x2 +#define SERROR_SP_EL0 0x3 +#define SYNC_EXCEPTION_SP_ELX 0x4 +#define IRQ_SP_ELX 0x5 +#define FIQ_SP_ELX 0x6 +#define SERROR_SP_ELX 0x7 +#define SYNC_EXCEPTION_AARCH64 0x8 +#define IRQ_AARCH64 0x9 +#define FIQ_AARCH64 0xa +#define SERROR_AARCH64 0xb +#define SYNC_EXCEPTION_AARCH32 0xc +#define IRQ_AARCH32 0xd +#define FIQ_AARCH32 0xe +#define SERROR_AARCH32 0xf + +#define SPSR_USE_L 0 +#define SPSR_USE_H 1 +#define SPSR_L_H_MASK 1 +#define SPSR_M_SHIFT 4 +#define SPSR_ERET_32 (1 << SPSR_M_SHIFT) +#define SPSR_ERET_64 (0 << SPSR_M_SHIFT) +#define SPSR_FIQ (1 << 6) +#define SPSR_IRQ (1 << 7) +#define SPSR_SERROR (1 << 8) +#define SPSR_DEBUG (1 << 9) +#define SPSR_EXCEPTION_MASK (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG) + +#ifndef __ASSEMBLY__ + +/******************************************************************************* +* Structure used for telling the next BL how much of a particular type of +* memory is available for its use and how much is already used. +******************************************************************************/ +struct aapcs64_params_t { +unsigned long arg0; +unsigned long arg1; +unsigned long arg2; +unsigned long arg3; +unsigned long arg4; +unsigned long arg5; +unsigned long arg6; +unsigned long arg7; +}; + +/*************************************************************************** +* This structure provides version information and the size of the +* structure, attributes for the structure it represents +***************************************************************************/ +struct param_header_t { +uint8_t type;/* type of the structure */ +uint8_t version;/* version of this structure */ +uint16_t size;/* size of this structure in bytes */ +uint32_t attr;/* attributes: unused bits SBZ */ +}; + +/***************************************************************************** +* This structure represents the superset of information needed while +* switching exception levels. The only two mechanisms to do so are +* ERET & SMC. Security state is indicated using bit zero of header +* attribute +* NOTE: BL1 expects entrypoint followed by spsr at an offset from the start +* of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while +* processing SMC to jump to BL31. +*****************************************************************************/ +struct entry_point_info_t { +struct param_header_t h; +uintptr_t pc; +uint32_t spsr; +struct aapcs64_params_t args; +}; + +/***************************************************************************** +* Image info binary provides information from the image loader that +* can be used by the firmware to manage available trusted RAM. +* More advanced firmware image formats can provide additional +* information that enables optimization or greater flexibility in the +* common firmware code +*****************************************************************************/ +struct atf_image_info_t { +struct param_header_t h; +uintptr_t image_base;/* physical address of base of image */ +uint32_t image_size;/* bytes read from image file */ +}; + +/***************************************************************************** +* The image descriptor struct definition. +*****************************************************************************/ +struct image_desc_t { +/* Contains unique image id for the image. */ +unsigned int image_id; +/* +* This member contains Image state information. +* Refer IMAGE_STATE_XXX defined above. +*/ +unsigned int state; +uint32_t copied_size;/* image size copied in blocks */ +struct atf_image_info_t atf_image_info; +struct entry_point_info_t ep_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that can be passed to +* BL31 e.g. while passing control to it from BL2. The BL32 parameters will be +* populated only if BL2 detects its presence. A pointer to a structure of this +* type should be passed in X0 to BL31's cold boot entrypoint. +* +* Use of this structure and the X0 parameter is not mandatory: the BL31 +* platform code can use other mechanisms to provide the necessary information +* about BL32 and BL33 to the common and SPD code. +* +* BL31 image information is mandatory if this structure is used. If either of +* the optional BL32 and BL33 image information is not provided, this is +* indicated by the respective image_info pointers being zero. +******************************************************************************/ +struct bl31_params_t { +struct param_header_t h; +struct atf_image_info_t *bl31_image_info; +struct entry_point_info_t *bl32_ep_info; +struct atf_image_info_t *bl32_image_info; +struct entry_point_info_t *bl33_ep_info; +struct atf_image_info_t *bl33_image_info; +}; + +/******************************************************************************* +* This structure represents the superset of information that is passed to +* BL31, e.g. while passing control to it from BL2, bl31_params +* and other platform specific params +******************************************************************************/ +struct bl2_to_bl31_params_mem_t { +struct bl31_params_t bl31_params; +struct atf_image_info_t bl31_image_info; +struct atf_image_info_t bl32_image_info; +struct atf_image_info_t bl33_image_info; +struct entry_point_info_t bl33_ep_info; +struct entry_point_info_t bl32_ep_info; +struct entry_point_info_t bl31_ep_info; +}; + +#endif /*__ASSEMBLY__*/ + +#endif /* __BL_COMMON_H__ */ include/spl.h +1
-0@@ -251,4 +251,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);int spl_mmc_load_image (struct spl_image_info *spl_image,struct spl_boot_device *bootdev); +void bl31_entry (void );#endif
cover arm64: rk3399: enable SPL with ATF support
In reply to Kever YangHi,
separation of rk3399 SPL from ATF SPL would be worth.
Thanks,
Michal
Quoted 51 lines · Kever Yang
RK3399 SPL is not enable bacause of the lack of ATF support in SPL, after port some source code from ATF, I manage to enable the support for ATF in SPL. This patch set depends on some patch for SPL support multi binary in FIT which is from Andre.
The patch of sdram controller is still not clean, because I want to get comments from upstream.I port this driver from coreboot, and I don't want to make much change on it. The rk3399 dram controller has many registers to config, I want to make them directly used by driver instead of parse and copy them one by one from dts.
Another part of comment is welcome for the implementation of
spl_atf.cand the entry inspl.cKever Yang (3):
arm64: rk3399: add SPL support
arm64: rk3399: add ddr controller driver
spl: add support to booting with ATFarch/arm/Kconfig | 1 + arch/arm/dts/rk3399.dtsi | 24 + arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 118 ++ arch/arm/include/asm/arch-rockchip/sdram_rk3399.h | 188 +++ arch/arm/mach-rockchip/Kconfig | 2 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/rk3399-board-spl.c | 157 ++ arch/arm/mach-rockchip/rk3399/Makefile | 1 + arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc | 1565 ++++++++++++++++++++ arch/arm/mach-rockchip/rk3399/sdram_rk3399.c | 1121 ++++++++++++++ common/spl/Kconfig | 14 + common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++ configs/evb-rk3399_defconfig | 17 + drivers/clk/rockchip/clk_rk3399.c | 42 +- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 106 -- include/atf_common.h | 295 ++++ include/configs/rk3399_common.h | 11 + include/dt-bindings/clock/rk3399-cru.h | 16 +- include/spl.h | 1 + 21 files changed, 3663 insertions(+), 113 deletions(-) create mode 100644 arch/arm/include/asm/arch-rockchip/sdram_rk3399.h create mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c create mode 100644 arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc create mode 100644 arch/arm/mach-rockchip/rk3399/sdram_rk3399.c create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.h
cover arm64: rk3399: enable SPL with ATF support
Hi Michal,
Quoted 3 lines · Michal Simek
Hi,
separation of rk3399 SPL from ATF SPL would be worth.
Thanks for your comment, this may help my rk3399 SPL patches get accept faster.
I will do it after people get back from holiday and send me other comments.
Thanks,
- Kever
Quoted 58 lines
Thanks,
MichalQuoted 50 lines · Kever Yang
RK3399 SPL is not enable bacause of the lack of ATF support in SPL, after port some source code from ATF, I manage to enable the support for ATF in SPL. This patch set depends on some patch for SPL support multi binary in FIT which is from Andre.
The patch of sdram controller is still not clean, because I want to get comments from upstream.I port this driver from coreboot, and I don't want to make much change on it. The rk3399 dram controller has many registers to config, I want to make them directly used by driver instead of parse and copy them one by one from dts.
Another part of comment is welcome for the implementation of
spl_atf.cand the entry inspl.cKever Yang (3):
arm64: rk3399: add SPL support
arm64: rk3399: add ddr controller driver
spl: add support to booting with ATFarch/arm/Kconfig | 1 + arch/arm/dts/rk3399.dtsi | 24 + arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 118 ++ arch/arm/include/asm/arch-rockchip/sdram_rk3399.h | 188 +++ arch/arm/mach-rockchip/Kconfig | 2 + arch/arm/mach-rockchip/Makefile | 1 + arch/arm/mach-rockchip/rk3399-board-spl.c | 157 ++ arch/arm/mach-rockchip/rk3399/Makefile | 1 + arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inc | 1565 ++++++++++++++++++++ arch/arm/mach-rockchip/rk3399/sdram_rk3399.c | 1121 ++++++++++++++ common/spl/Kconfig | 14 + common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++ configs/evb-rk3399_defconfig | 17 + drivers/clk/rockchip/clk_rk3399.c | 42 +- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 106 -- include/atf_common.h | 295 ++++ include/configs/rk3399_common.h | 11 + include/dt-bindings/clock/rk3399-cru.h | 16 +- include/spl.h | 1 + 21 files changed, 3663 insertions(+), 113 deletions(-)create mode 100644
arch/arm/include/asm/arch-rockchip/sdram_rk3399.hcreate mode 100644arch/arm/mach-rockchip/rk3399-board-spl.ccreate mode 100644arch/arm/mach-rockchip/rk3399/sdram-lpddr3-4GB.inccreate mode 100644arch/arm/mach-rockchip/rk3399/sdram_rk3399.ccreate mode 100644common/spl/spl_atf.c
create mode 100644include/atf_common.h