[U-Boot] [PATCH-OMAP3] OMAP3: Convert readl/writel and replace hardcoded values
Convert readl/writel to base + offset style. Replace hardcoded values with macros.
No functional change.
Signed-off-by: Dirk Behme dirk.behme@googlemail.com
--- cpu/arm_cortexa8/omap3/board.c | 56 ++++++++------- cpu/arm_cortexa8/omap3/clock.c | 47 ++++++++----- cpu/arm_cortexa8/omap3/interrupts.c | 14 +-- cpu/arm_cortexa8/omap3/mem.c | 53 ++++++++------ cpu/arm_cortexa8/omap3/sys_info.c | 14 ++- include/asm-arm/arch-omap3/cpu.h | 129 +++++++++++++++++++----------------- include/asm-arm/arch-omap3/mem.h | 8 +- include/asm-arm/arch-omap3/omap3.h | 2 8 files changed, 180 insertions(+), 143 deletions(-)
Index: u-boot-arm/cpu/arm_cortexa8/omap3/mem.c =================================================================== --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/mem.c +++ u-boot-arm/cpu/arm_cortexa8/omap3/mem.c @@ -106,6 +106,8 @@ u32 *onenand_cs_base;
#endif
+static u32 *sdrc_base = (u32 *)OMAP34XX_SDRC_BASE; + /************************************************************************** * make_cs1_contiguous() - for es2 and above remap cs1 behind cs0 to allow * command line mem=xyz use all memory with out discontinuous support @@ -120,7 +122,7 @@ void make_cs1_contiguous(void) size /= SZ_32M; /* find size to offset CS1 */ a_add_high = (size & 3) << 8; /* set up low field */ a_add_low = (size & 0x3C) >> 2; /* set up high field */ - writel((a_add_high | a_add_low), SDRC_CS_CFG); + writel((a_add_high | a_add_low), sdrc_base + OFFS(SDRC_CS_CFG));
}
@@ -175,36 +177,43 @@ void do_sdrc_init(u32 offset, u32 early)
if (early) { /* reset sdrc controller */ - writel(SOFTRESET, SDRC_SYSCONFIG); - wait_on_value(RESETDONE, RESETDONE, SDRC_STATUS, 12000000); - writel(0, SDRC_SYSCONFIG); + writel(SOFTRESET, sdrc_base + OFFS(SDRC_SYSCONFIG)); + wait_on_value(RESETDONE, RESETDONE, SDRC_STATUS_REG, 12000000); + writel(0, sdrc_base + OFFS(SDRC_SYSCONFIG));
/* setup sdrc to ball mux */ - writel(SDP_SDRC_SHARING, SDRC_SHARING); + writel(SDP_SDRC_SHARING, sdrc_base + OFFS(SDRC_SHARING));
- /* Disble Power Down of CKE cuz of 1 CKE on combo part */ - writel(0x00000081, SDRC_POWER); + /* Disable Power Down of CKE cuz of 1 CKE on combo part */ + writel(SRFRONRESET | PAGEPOLICY_HIGH, + sdrc_base + OFFS(SDRC_POWER));
- writel(0x0000A, SDRC_DLLA_CTRL); + writel(ENADLL | DLLPHASE_90, sdrc_base + OFFS(SDRC_DLLA_CTRL)); sdelay(0x20000); }
- writel(0x02584099, SDRC_MCFG_0 + offset); - writel(0x4e201, SDRC_RFR_CTRL + offset); - writel(0xaa9db4c6, SDRC_ACTIM_CTRLA_0 + actim_offs); - writel(0x11517, SDRC_ACTIM_CTRLB_0 + actim_offs); - - writel(CMD_NOP, SDRC_MANUAL_0 + offset); - writel(CMD_PRECHARGE, SDRC_MANUAL_0 + offset); - writel(CMD_AUTOREFRESH, SDRC_MANUAL_0 + offset); - writel(CMD_AUTOREFRESH, SDRC_MANUAL_0 + offset); - - /* CAS latency 3, Write Burst = Read Burst, Serial Mode, - Burst length = 4 */ - writel(0x00000032, SDRC_MR_0 + offset); + writel(RASWIDTH_13BITS | CASWIDTH_10BITS | ADDRMUXLEGACY | + RAMSIZE_128 | BANKALLOCATION | B32NOT16 | B32NOT16 | + DEEPPD | DDR_SDRAM, sdrc_base + OFFS(SDRC_MCFG_0 + offset)); + writel(ARCV | ARE_ARCV_1, sdrc_base + OFFS(SDRC_RFR_CTRL + offset)); + writel(V_ACTIMA_165, sdrc_base + + OFFS(SDRC_ACTIM_CTRLA_0 + actim_offs)); + writel(V_ACTIMB_165, sdrc_base + + OFFS(SDRC_ACTIM_CTRLB_0 + actim_offs)); + + writel(CMD_NOP, sdrc_base + OFFS(SDRC_MANUAL_0 + offset)); + writel(CMD_PRECHARGE, sdrc_base + OFFS(SDRC_MANUAL_0 + offset)); + writel(CMD_AUTOREFRESH, sdrc_base + OFFS(SDRC_MANUAL_0 + offset)); + writel(CMD_AUTOREFRESH, sdrc_base + OFFS(SDRC_MANUAL_0 + offset)); + + /* + * CAS latency 3, Write Burst = Read Burst, Serial Mode, + * Burst length = 4 + */ + writel(CASL3 | BURSTLENGTH4, sdrc_base + OFFS(SDRC_MR_0 + offset));
if (!mem_ok(offset)) - writel(0, SDRC_MCFG_0 + offset); + writel(0, sdrc_base + OFFS(SDRC_MCFG_0 + offset)); }
void enable_gpmc_config(u32 *gpmc_config, u32 *gpmc_cs_base, u32 base, Index: u-boot-arm/cpu/arm_cortexa8/omap3/sys_info.c =================================================================== --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/sys_info.c +++ u-boot-arm/cpu/arm_cortexa8/omap3/sys_info.c @@ -33,6 +33,8 @@
extern omap3_sysinfo sysinfo; static u32 *gpmc_base = (u32 *)GPMC_BASE; +static u32 *sdrc_base = (u32 *)OMAP34XX_SDRC_BASE; +static u32 *ctrl_base = (u32 *)OMAP34XX_CTRL_BASE;
/****************************************** * get_cpu_rev(void) - extract version info @@ -59,7 +61,8 @@ u32 get_cpu_rev(void) ****************************************************/ u32 is_mem_sdr(void) { - if (readl(SDRC_MR_0 + SDRC_CS0_OSET) == SDP_SDRC_MR_0_SDR) + if (readl(sdrc_base + OFFS(SDRC_MR_0 + SDRC_CS0_OSET)) == + SDP_SDRC_MR_0_SDR) return 1; return 0; } @@ -72,7 +75,7 @@ u32 get_sdr_cs_size(u32 offset) u32 size;
/* get ram size field */ - size = readl(SDRC_MCFG_0 + offset) >> 8; + size = readl(sdrc_base + OFFS(SDRC_MCFG_0 + offset)) >> 8; size &= 0x3FF; /* remove unwanted bits */ size *= SZ_2M; /* find size in MB */ return size; @@ -88,7 +91,7 @@ u32 get_sdr_cs_offset(u32 cs) if (!cs) return 0;
- offset = readl(SDRC_CS_CFG); + offset = readl(sdrc_base + OFFS(SDRC_CS_CFG)); offset = (offset & 15) << 27 | (offset & 0x30) >> 17;
return offset; @@ -240,7 +243,7 @@ u32 is_running_in_sdram(void) ***************************************************************/ u32 get_boot_type(void) { - return (readl(CONTROL_STATUS) & SYSBOOT_MASK); + return (readl(ctrl_base + OFFS(CONTROL_STATUS)) & SYSBOOT_MASK); }
/************************************************************* @@ -248,5 +251,6 @@ u32 get_boot_type(void) *************************************************************/ u32 get_device_type(void) { - return ((readl(CONTROL_STATUS) & (DEVICE_MASK)) >> 8); + return ((readl(ctrl_base + OFFS(CONTROL_STATUS)) & + (DEVICE_MASK)) >> 8); } Index: u-boot-arm/include/asm-arm/arch-omap3/cpu.h =================================================================== --- u-boot-arm.orig/include/asm-arm/arch-omap3/cpu.h +++ u-boot-arm/include/asm-arm/arch-omap3/cpu.h @@ -29,14 +29,7 @@
/* Register offsets of common modules */ /* Control */ -#define CONTROL_STATUS (OMAP34XX_CTRL_BASE + 0x2F0) -#define OMAP34XX_MCR (OMAP34XX_CTRL_BASE + 0x8C) -#define CONTROL_SCALABLE_OMAP_STATUS (OMAP34XX_CTRL_BASE + 0x44C) -#define CONTROL_SCALABLE_OMAP_OCP (OMAP34XX_CTRL_BASE + 0x534) - -/* Tap Information */ -#define TAP_IDCODE_REG (OMAP34XX_TAP_BASE + 0x204) -#define PRODUCTION_ID (OMAP34XX_TAP_BASE + 0x208) +#define CONTROL_STATUS 0x2F0
/* device type */ #define DEVICE_MASK (0x7 << 8) @@ -104,36 +97,52 @@ /* (actual size small port) */
/* SMS */ -#define SMS_SYSCONFIG (OMAP34XX_SMS_BASE + 0x10) -#define SMS_RG_ATT0 (OMAP34XX_SMS_BASE + 0x48) -#define SMS_CLASS_ARB0 (OMAP34XX_SMS_BASE + 0xD0) +#define SMS_SYSCONFIG 0x10 +#define SMS_RG_ATT0 0x48 +#define SMS_CLASS_ARB0 0xD0 #define BURSTCOMPLETE_GROUP7 (0x1 << 31)
/* SDRC */ -#define SDRC_SYSCONFIG (OMAP34XX_SDRC_BASE + 0x10) -#define SDRC_STATUS (OMAP34XX_SDRC_BASE + 0x14) -#define SDRC_CS_CFG (OMAP34XX_SDRC_BASE + 0x40) -#define SDRC_SHARING (OMAP34XX_SDRC_BASE + 0x44) -#define SDRC_DLLA_CTRL (OMAP34XX_SDRC_BASE + 0x60) -#define SDRC_DLLA_STATUS (OMAP34XX_SDRC_BASE + 0x64) -#define SDRC_DLLB_CTRL (OMAP34XX_SDRC_BASE + 0x68) -#define SDRC_DLLB_STATUS (OMAP34XX_SDRC_BASE + 0x6C) -#define DLLPHASE (0x1 << 1) +#define SDRC_SYSCONFIG 0x10 +#define SDRC_STATUS 0x14 +#define SDRC_STATUS_REG (OMAP34XX_SDRC_BASE + SDRC_STATUS) +#define SDRC_CS_CFG 0x40 +#define SDRC_SHARING 0x44 +#define SDRC_DLLA_CTRL 0x60 +#define SDRC_DLLA_STATUS 0x64 +#define SDRC_DLLB_CTRL 0x68 +#define SDRC_DLLB_STATUS 0x6C +#define DLLPHASE_90 (0x1 << 1) #define LOADDLL (0x1 << 2) +#define ENADLL (0x1 << 3) #define DLL_DELAY_MASK 0xFF00 #define DLL_NO_FILTER_MASK ((0x1 << 9) | (0x1 << 8))
-#define SDRC_POWER (OMAP34XX_SDRC_BASE + 0x70) +#define SDRC_POWER 0x70 +#define PAGEPOLICY_HIGH (0x1 << 0) +#define SRFRONRESET (0x1 << 7) #define WAKEUPPROC (0x1 << 26)
-#define SDRC_MCFG_0 (OMAP34XX_SDRC_BASE + 0x80) -#define SDRC_MR_0 (OMAP34XX_SDRC_BASE + 0x84) -#define SDRC_ACTIM_CTRLA_0 (OMAP34XX_SDRC_BASE + 0x9C) -#define SDRC_ACTIM_CTRLB_0 (OMAP34XX_SDRC_BASE + 0xA0) -#define SDRC_ACTIM_CTRLA_1 (OMAP34XX_SDRC_BASE + 0xC4) -#define SDRC_ACTIM_CTRLB_1 (OMAP34XX_SDRC_BASE + 0xC8) -#define SDRC_RFR_CTRL (OMAP34XX_SDRC_BASE + 0xA4) -#define SDRC_MANUAL_0 (OMAP34XX_SDRC_BASE + 0xA8) +#define SDRC_MCFG_0 0x80 +#define DDR_SDRAM (0x1 << 0) +#define DEEPPD (0x1 << 3) +#define B32NOT16 (0x1 << 4) +#define BANKALLOCATION (0x2 << 6) +#define RAMSIZE_128 (0x40 << 8) /* RAM size in 2MB chunks */ +#define ADDRMUXLEGACY (0x1 << 19) +#define CASWIDTH_10BITS (0x5 << 20) +#define RASWIDTH_13BITS (0x2 << 24) +#define SDRC_MR_0 0x84 +#define BURSTLENGTH4 (0x2 << 0) +#define CASL3 (0x3 << 4) +#define SDRC_ACTIM_CTRLA_0 0x9C +#define SDRC_ACTIM_CTRLB_0 0xA0 +#define SDRC_ACTIM_CTRLA_1 0xC4 +#define SDRC_ACTIM_CTRLB_1 0xC8 +#define SDRC_RFR_CTRL 0xA4 +#define ARE_ARCV_1 (0x1 << 0) +#define ARCV (0x4e2 << 8) /* Autorefresh count */ +#define SDRC_MANUAL_0 0xA8 #define OMAP34XX_SDRC_CS0 0x80000000 #define OMAP34XX_SDRC_CS1 0xA0000000 #define CMD_NOP 0x0 @@ -174,6 +183,7 @@ #define WD_UNLOCK2 0x5555
/* PRCM */ +#define PRCM_BASE 0x48004000 #define CM_FCLKEN_IVA2 0x48004000 #define CM_CLKEN_PLL_IVA2 0x48004004 #define CM_IDLEST_PLL_IVA2 0x48004024 @@ -187,16 +197,22 @@ #define CM_ICLKEN1_CORE 0x48004a10 #define CM_ICLKEN2_CORE 0x48004a14 #define CM_CLKSEL_CORE 0x48004a40 +#define CLKSEL_CORE 0xa40 #define CM_FCLKEN_GFX 0x48004b00 #define CM_ICLKEN_GFX 0x48004b10 #define CM_CLKSEL_GFX 0x48004b40 #define CM_FCLKEN_WKUP 0x48004c00 +#define FCLKEN_WKUP 0xc00 #define CM_ICLKEN_WKUP 0x48004c10 +#define ICLKEN_WKUP 0xc10 #define CM_CLKSEL_WKUP 0x48004c40 +#define CLKSEL_WKUP 0xc40 #define CM_IDLEST_WKUP 0x48004c20 #define CM_CLKEN_PLL 0x48004d00 +#define CLKEN_PLL 0xd00 #define CM_IDLEST_CKGEN 0x48004d20 #define CM_CLKSEL1_PLL 0x48004d40 +#define CLKSEL1_PLL 0xd40 #define CM_CLKSEL2_PLL 0x48004d44 #define CM_CLKSEL3_PLL 0x48004d48 #define CM_FCLKEN_DSS 0x48004e00 @@ -210,9 +226,11 @@ #define CM_CLKSEL_PER 0x48005040 #define CM_CLKSEL1_EMU 0x48005140
+#define PRM_BASE 0x48306000 #define PRM_CLKSEL 0x48306d40 #define PRM_RSTCTRL 0x48307250 #define PRM_CLKSRC_CTRL 0x48307270 +#define CLKSRC_CTRL 0x1270 #define SYSCLKDIV_1 (0x1 << 6) #define SYSCLKDIV_2 (0x1 << 7)
@@ -244,35 +262,30 @@ #define PM_OCM_ROM_BASE_ADDR_ARM (SMX_APE_BASE + 0x12C00) #define PM_IVA2_BASE_ADDR_ARM (SMX_APE_BASE + 0x14000)
-#define RT_REQ_INFO_PERMISSION_1 (PM_RT_APE_BASE_ADDR_ARM + 0x68) -#define RT_READ_PERMISSION_0 (PM_RT_APE_BASE_ADDR_ARM + 0x50) -#define RT_WRITE_PERMISSION_0 (PM_RT_APE_BASE_ADDR_ARM + 0x58) -#define RT_ADDR_MATCH_1 (PM_RT_APE_BASE_ADDR_ARM + 0x60) - -#define GPMC_REQ_INFO_PERMISSION_0 (PM_GPMC_BASE_ADDR_ARM + 0x48) -#define GPMC_READ_PERMISSION_0 (PM_GPMC_BASE_ADDR_ARM + 0x50) -#define GPMC_WRITE_PERMISSION_0 (PM_GPMC_BASE_ADDR_ARM + 0x58) - -#define OCM_REQ_INFO_PERMISSION_0 (PM_OCM_RAM_BASE_ADDR_ARM + 0x48) -#define OCM_READ_PERMISSION_0 (PM_OCM_RAM_BASE_ADDR_ARM + 0x50) -#define OCM_WRITE_PERMISSION_0 (PM_OCM_RAM_BASE_ADDR_ARM + 0x58) -#define OCM_ADDR_MATCH_2 (PM_OCM_RAM_BASE_ADDR_ARM + 0x80) - -#define IVA2_REQ_INFO_PERMISSION_0 (PM_IVA2_BASE_ADDR_ARM + 0x48) -#define IVA2_READ_PERMISSION_0 (PM_IVA2_BASE_ADDR_ARM + 0x50) -#define IVA2_WRITE_PERMISSION_0 (PM_IVA2_BASE_ADDR_ARM + 0x58) - -#define IVA2_REQ_INFO_PERMISSION_1 (PM_IVA2_BASE_ADDR_ARM + 0x68) -#define IVA2_READ_PERMISSION_1 (PM_IVA2_BASE_ADDR_ARM + 0x70) -#define IVA2_WRITE_PERMISSION_1 (PM_IVA2_BASE_ADDR_ARM + 0x78) - -#define IVA2_REQ_INFO_PERMISSION_2 (PM_IVA2_BASE_ADDR_ARM + 0x88) -#define IVA2_READ_PERMISSION_2 (PM_IVA2_BASE_ADDR_ARM + 0x90) -#define IVA2_WRITE_PERMISSION_2 (PM_IVA2_BASE_ADDR_ARM + 0x98) - -#define IVA2_REQ_INFO_PERMISSION_3 (PM_IVA2_BASE_ADDR_ARM + 0xA8) -#define IVA2_READ_PERMISSION_3 (PM_IVA2_BASE_ADDR_ARM + 0xB0) -#define IVA2_WRITE_PERMISSION_3 (PM_IVA2_BASE_ADDR_ARM + 0xB8) +#define RT_REQ_INFO_PERMISSION_1 0x68 +#define RT_READ_PERMISSION_0 0x50 +#define RT_WRITE_PERMISSION_0 0x58 +#define RT_ADDR_MATCH_1 0x60 + +#define GPMC_REQ_INFO_PERMISSION_0 0x48 +#define GPMC_READ_PERMISSION_0 0x50 +#define GPMC_WRITE_PERMISSION_0 0x58 + +#define OCM_REQ_INFO_PERMISSION_0 0x48 +#define OCM_READ_PERMISSION_0 0x50 +#define OCM_WRITE_PERMISSION_0 0x58 +#define OCM_ADDR_MATCH_2 0x80 + +#define IVA2_REQ_INFO_PERMISSION_0 0x48 +#define IVA2_READ_PERMISSION_0 0x50 +#define IVA2_WRITE_PERMISSION_0 0x58 + +/* Permission values for registers -Full fledged permissions to all */ +#define UNLOCK_1 0xFFFFFFFF +#define UNLOCK_2 0x00000000 +#define UNLOCK_3 0x0000FFFF + +#define NOT_EARLY 0
/* I2C base */ #define I2C_BASE1 (OMAP34XX_CORE_L4_IO_BASE + 0x70000) Index: u-boot-arm/include/asm-arm/arch-omap3/mem.h =================================================================== --- u-boot-arm.orig/include/asm-arm/arch-omap3/mem.h +++ u-boot-arm/include/asm-arm/arch-omap3/mem.h @@ -80,16 +80,16 @@ typedef enum { #define TRP_165 3 #define TRAS_165 7 #define TRC_165 10 -#define TRFC_165 12 +#define TRFC_165 21 #define V_ACTIMA_165 ((TRFC_165 << 27) | (TRC_165 << 22) | \ (TRAS_165 << 18) | (TRP_165 << 15) | \ (TRCD_165 << 12) | (TRRD_165 << 9) | \ (TDPL_165 << 6) | (TDAL_165))
#define TWTR_165 1 -#define TCKE_165 2 -#define TXP_165 2 -#define XSR_165 20 +#define TCKE_165 1 +#define TXP_165 5 +#define XSR_165 23 #define V_ACTIMB_165 (((TCKE_165 << 12) | (XSR_165 << 0)) | \ (TXP_165 << 8) | (TWTR_165 << 16))
Index: u-boot-arm/cpu/arm_cortexa8/omap3/board.c =================================================================== --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/board.c +++ u-boot-arm/cpu/arm_cortexa8/omap3/board.c @@ -37,13 +37,6 @@ #include <asm/arch/sys_proto.h> #include <asm/arch/mem.h>
-#define NOT_EARLY 0 - -/* Permission values for registers -Full fledged permissions to all */ -#define UNLOCK_1 0xFFFFFFFF -#define UNLOCK_2 0x00000000 -#define UNLOCK_3 0x0000FFFF - extern omap3_sysinfo sysinfo;
/****************************************************************************** @@ -63,27 +56,34 @@ static inline void delay(unsigned long l *****************************************************************************/ void secure_unlock_mem(void) { + u32 *pm_rt_ape_base = (u32 *)PM_RT_APE_BASE_ADDR_ARM; + u32 *pm_gpmc_base = (u32 *)PM_GPMC_BASE_ADDR_ARM; + u32 *pm_ocm_ram_base = (u32 *)PM_OCM_RAM_BASE_ADDR_ARM; + u32 *pm_iva2_base = (u32 *)PM_IVA2_BASE_ADDR_ARM; + u32 *sms_base = (u32 *)OMAP34XX_SMS_BASE; + /* Protection Module Register Target APE (PM_RT) */ - writel(UNLOCK_1, RT_REQ_INFO_PERMISSION_1); - writel(UNLOCK_1, RT_READ_PERMISSION_0); - writel(UNLOCK_1, RT_WRITE_PERMISSION_0); - writel(UNLOCK_2, RT_ADDR_MATCH_1); - - writel(UNLOCK_3, GPMC_REQ_INFO_PERMISSION_0); - writel(UNLOCK_3, GPMC_READ_PERMISSION_0); - writel(UNLOCK_3, GPMC_WRITE_PERMISSION_0); - - writel(UNLOCK_3, OCM_REQ_INFO_PERMISSION_0); - writel(UNLOCK_3, OCM_READ_PERMISSION_0); - writel(UNLOCK_3, OCM_WRITE_PERMISSION_0); - writel(UNLOCK_2, OCM_ADDR_MATCH_2); + writel(UNLOCK_1, pm_rt_ape_base + OFFS(RT_REQ_INFO_PERMISSION_1)); + writel(UNLOCK_1, pm_rt_ape_base + OFFS(RT_READ_PERMISSION_0)); + writel(UNLOCK_1, pm_rt_ape_base + OFFS(RT_WRITE_PERMISSION_0)); + writel(UNLOCK_2, pm_rt_ape_base + OFFS(RT_ADDR_MATCH_1)); + + writel(UNLOCK_3, pm_gpmc_base + OFFS(GPMC_REQ_INFO_PERMISSION_0)); + writel(UNLOCK_3, pm_gpmc_base + OFFS(GPMC_READ_PERMISSION_0)); + writel(UNLOCK_3, pm_gpmc_base + OFFS(GPMC_WRITE_PERMISSION_0)); + + writel(UNLOCK_3, pm_ocm_ram_base + OFFS(OCM_REQ_INFO_PERMISSION_0)); + writel(UNLOCK_3, pm_ocm_ram_base + OFFS(OCM_READ_PERMISSION_0)); + writel(UNLOCK_3, pm_ocm_ram_base + OFFS(OCM_WRITE_PERMISSION_0)); + writel(UNLOCK_2, pm_ocm_ram_base + OFFS(OCM_ADDR_MATCH_2));
/* IVA Changes */ - writel(UNLOCK_3, IVA2_REQ_INFO_PERMISSION_0); - writel(UNLOCK_3, IVA2_READ_PERMISSION_0); - writel(UNLOCK_3, IVA2_WRITE_PERMISSION_0); + writel(UNLOCK_3, pm_iva2_base + OFFS(IVA2_REQ_INFO_PERMISSION_0)); + writel(UNLOCK_3, pm_iva2_base + OFFS(IVA2_READ_PERMISSION_0)); + writel(UNLOCK_3, pm_iva2_base + OFFS(IVA2_WRITE_PERMISSION_0));
- writel(UNLOCK_1, SMS_RG_ATT0); /* SDRC region 0 public */ + /* SDRC region 0 public */ + writel(UNLOCK_1, sms_base + OFFS(SMS_RG_ATT0)); }
/****************************************************************************** @@ -210,7 +210,7 @@ void s_init(void) #endif /* * Writing to AuxCR in U-boot using SMI for GP DEV - * Currently SMI in Kernel on ES2 devices seems to have an isse + * Currently SMI in Kernel on ES2 devices seems to have an issue * Once that is resolved, we can postpone this config to kernel */ if (get_device_type() == GP_DEVICE) @@ -245,6 +245,8 @@ void wait_for_command_complete(unsigned *****************************************************************************/ void watchdog_init(void) { + u32 *wd2_base = (u32 *)WD2_BASE; + /* * There are 3 watch dogs WD1=Secure, WD2=MPU, WD3=IVA. WD1 is * either taken care of by ROM (HS/EMU) or not accessible (GP). @@ -256,9 +258,9 @@ void watchdog_init(void) sr32(CM_ICLKEN_WKUP, 5, 1, 1); wait_on_value(ST_WDT2, 0x20, CM_IDLEST_WKUP, 5); /* some issue here */
- writel(WD_UNLOCK1, WD2_BASE + WSPR); + writel(WD_UNLOCK1, wd2_base + OFFS(WSPR)); wait_for_command_complete(WD2_BASE); - writel(WD_UNLOCK2, WD2_BASE + WSPR); + writel(WD_UNLOCK2, wd2_base + OFFS(WSPR)); }
/****************************************************************************** Index: u-boot-arm/cpu/arm_cortexa8/omap3/interrupts.c =================================================================== --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/interrupts.c +++ u-boot-arm/cpu/arm_cortexa8/omap3/interrupts.c @@ -38,9 +38,6 @@
#define TIMER_LOAD_VAL 0
-/* macro to read the 32 bit timer */ -#define READ_TIMER (*(volatile ulong *)(CONFIG_SYS_TIMERBASE+TCRR)) - #ifdef CONFIG_USE_IRQ /* enable IRQ interrupts */ void enable_interrupts(void) @@ -170,15 +167,16 @@ void do_irq(struct pt_regs *pt_regs)
static ulong timestamp; static ulong lastinc; +static u32 *timer_base = (u32 *)CONFIG_SYS_TIMERBASE;
/* nothing really to do with interrupts, just starts up a counter. */ int interrupt_init(void) { /* start the counter ticking up, reload value on overflow */ - writel(TIMER_LOAD_VAL, CONFIG_SYS_TIMERBASE + TLDR); + writel(TIMER_LOAD_VAL, timer_base + OFFS(TLDR)); /* enable timer */ writel((CONFIG_SYS_PVT << 2) | TCLR_PRE | TCLR_AR | TCLR_ST, - CONFIG_SYS_TIMERBASE + TCLR); + timer_base + OFFS(TCLR));
reset_timer_masked(); /* init the timestamp and lastinc value */
@@ -233,14 +231,14 @@ void udelay(unsigned long usec)
void reset_timer_masked(void) { - /* reset time */ - lastinc = READ_TIMER; /* capture current incrementer value time */ + /* reset time, capture current incrementer value time */ + lastinc = readl(timer_base + OFFS(TCRR)); timestamp = 0; /* start "advancing" time stamp from 0 */ }
ulong get_timer_masked(void) { - ulong now = READ_TIMER; /* current tick value */ + ulong now = readl(timer_base + OFFS(TCRR)); /* current tick value */
if (now >= lastinc) /* normal mode (non roll) */ /* move stamp fordward with absoulte diff ticks */ Index: u-boot-arm/cpu/arm_cortexa8/omap3/clock.c =================================================================== --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/clock.c +++ u-boot-arm/cpu/arm_cortexa8/omap3/clock.c @@ -41,36 +41,46 @@ u32 get_osc_clk_speed(void) { u32 start, cstart, cend, cdiff, val; + u32 *prcm_base = (u32 *)PRCM_BASE; + u32 *prm_base = (u32 *)PRM_BASE; + u32 *gpt1_base = (u32 *)OMAP34XX_GPT1; + u32 *s32k_base = (u32 *)SYNC_32KTIMER_BASE;
- val = readl(PRM_CLKSRC_CTRL); + val = readl(prm_base + OFFS(CLKSRC_CTRL));
/* If SYS_CLK is being divided by 2, remove for now */ val = (val & (~SYSCLKDIV_2)) | SYSCLKDIV_1; - writel(val, PRM_CLKSRC_CTRL); + writel(val, prm_base + OFFS(CLKSRC_CTRL));
/* enable timer2 */ - val = readl(CM_CLKSEL_WKUP) | CLKSEL_GPT1; - writel(val, CM_CLKSEL_WKUP); /* select sys_clk for GPT1 */ + val = readl(prcm_base + OFFS(CLKSEL_WKUP)) | CLKSEL_GPT1; + + /* select sys_clk for GPT1 */ + writel(val, prcm_base + OFFS(CLKSEL_WKUP));
/* Enable I and F Clocks for GPT1 */ - val = readl(CM_ICLKEN_WKUP) | EN_GPT1 | EN_32KSYNC; - writel(val, CM_ICLKEN_WKUP); - val = readl(CM_FCLKEN_WKUP) | EN_GPT1; - writel(val, CM_FCLKEN_WKUP); + val = readl(prcm_base + OFFS(ICLKEN_WKUP)) | EN_GPT1 | EN_32KSYNC; + writel(val, prcm_base + OFFS(ICLKEN_WKUP)); + val = readl(prcm_base + OFFS(FCLKEN_WKUP)) | EN_GPT1; + writel(val, prcm_base + OFFS(FCLKEN_WKUP));
- writel(0, OMAP34XX_GPT1 + TLDR); /* start counting at 0 */ - writel(GPT_EN, OMAP34XX_GPT1 + TCLR); /* enable clock */ + writel(0, gpt1_base + OFFS(TLDR)); /* start counting at 0 */ + writel(GPT_EN, gpt1_base + OFFS(TCLR)); /* enable clock */
/* enable 32kHz source, determine sys_clk via gauging */ - start = 20 + readl(S32K_CR); /* start time in 20 cycles */ - while (readl(S32K_CR) < start) ; /* dead loop till start time */ + + /* start time in 20 cycles */ + start = 20 + readl(s32k_base + OFFS(S32K_CR)); + + /* dead loop till start time */ + while (readl(s32k_base + OFFS(S32K_CR)) < start);
/* get start sys_clk count */ - cstart = readl(OMAP34XX_GPT1 + TCRR); + cstart = readl(gpt1_base + OFFS(TCRR));
/* wait for 40 cycles */ - while (readl(S32K_CR) < (start + 20)) ; - cend = readl(OMAP34XX_GPT1 + TCRR); /* get end sys_clk count */ + while (readl(s32k_base + OFFS(S32K_CR)) < (start + 20)) ; + cend = readl(gpt1_base + OFFS(TCRR)); /* get end sys_clk count */ cdiff = cend - cstart; /* get elapsed ticks */
/* based on number of ticks assign speed */ @@ -123,6 +133,7 @@ void prcm_init(void) int xip_safe, p0, p1, p2, p3; u32 osc_clk = 0, sys_clkin_sel; u32 clk_index, sil_index; + u32 *prcm_base = (u32 *)PRCM_BASE; dpll_param *dpll_param_p;
f_lock_pll = (void *) ((u32) &_end_vect - (u32) &_start + @@ -199,17 +210,17 @@ void prcm_init(void) * if running from flash, jump to small relocated code * area in SRAM. */ - p0 = readl(CM_CLKEN_PLL); + p0 = readl(prcm_base + OFFS(CLKEN_PLL)); sr32((u32) &p0, 0, 3, PLL_FAST_RELOCK_BYPASS); sr32((u32) &p0, 4, 4, dpll_param_p->fsel); /* FREQSEL */
- p1 = readl(CM_CLKSEL1_PLL); + p1 = readl(prcm_base + OFFS(CLKSEL1_PLL)); sr32((u32) &p1, 27, 2, dpll_param_p->m2); /* Set M2 */ sr32((u32) &p1, 16, 11, dpll_param_p->m); /* Set M */ sr32((u32) &p1, 8, 7, dpll_param_p->n); /* Set N */ sr32((u32) &p1, 6, 1, 0); /* set source for 96M */
- p2 = readl(CM_CLKSEL_CORE); + p2 = readl(prcm_base + OFFS(CLKSEL_CORE)); sr32((u32) &p2, 8, 4, CORE_SSI_DIV); /* ssi */ sr32((u32) &p2, 4, 2, CORE_FUSB_DIV); /* fsusb */ sr32((u32) &p2, 2, 2, CORE_L4_DIV); /* l4 */ Index: u-boot-arm/include/asm-arm/arch-omap3/omap3.h =================================================================== --- u-boot-arm.orig/include/asm-arm/arch-omap3/omap3.h +++ u-boot-arm/include/asm-arm/arch-omap3/omap3.h @@ -75,7 +75,7 @@
/* 32KTIMER */ #define SYNC_32KTIMER_BASE 0x48320000 -#define S32K_CR (SYNC_32KTIMER_BASE + 0x10) +#define S32K_CR 0x10
/* OMAP3 GPIO registers */ #define OMAP34XX_GPIO1_BASE 0x48310000
Dear Dirk,
In message 1227445293-23887-1-git-send-email-dirk.behme@googlemail.com you wrote:
Convert readl/writel to base + offset style. Replace hardcoded values with macros.
I think repeating the sequence "base + offset(foo)" again and again is not exactly nice or easy to read.
- writel((a_add_high | a_add_low), SDRC_CS_CFG);
- writel((a_add_high | a_add_low), sdrc_base + OFFS(SDRC_CS_CFG));
I find this new code is even less readable.
At this point I was tempted to compain that you should not re-invent the offsetof() macro when I realized that the actual definition of OFFS is
#define OFFS(x) ((x) >> 2)
Argh... that's awfully error prone. So you rely on havnig to deal with 32 bit register accesses only, without any way of compiler supported type checking?
That's awful.
I am aware that many areas of ARM code use such a horrible program- ming syle of defining only register offsets instead of proper data structures like for example PowerPC has been doing for ages.
Do you see a chance to convert the OMAP3 code to use data structures instead of offset definitions to describe the processor registers and peripherals?
Best regards,
Wolfgang Denk
Dear Wolfgang,
Wolfgang Denk wrote:
Dear Dirk,
In message 1227445293-23887-1-git-send-email-dirk.behme@googlemail.com you wrote:
Convert readl/writel to base + offset style. Replace hardcoded values with macros.
I think repeating the sequence "base + offset(foo)" again and again is not exactly nice or easy to read.
But it's what the patch does ;) Anyway, if I remember correctly this is why you asked me to re-send OMAP3 patch series to mailing list again once all the review comments are fixed and not directly git pull from OMAP3 branch. To get rid of all these "fix here, clean up there" comments.
- writel((a_add_high | a_add_low), SDRC_CS_CFG);
- writel((a_add_high | a_add_low), sdrc_base + OFFS(SDRC_CS_CFG));
I find this new code is even less readable.
At this point I was tempted to compain that you should not re-invent the offsetof() macro when I realized that the actual definition of OFFS is
#define OFFS(x) ((x) >> 2)
Argh... that's awfully error prone. So you rely on havnig to deal with 32 bit register accesses only, without any way of compiler supported type checking?
That's awful.
This style is what Jean-Christophe and Scott asked me to use [1]. I was asked to convert all OMAP3 read/write access to this style to get the patches accepted.
Disussing this at IRC, we agreed on
-- cut -- #define GPMC_BASE 0x6E000000
OFFS(x) ((x) >> 2)
#define GPMC_ECC_CONFIG 0x1F4
static uint32_t *gpmc_base = (uint32_t *)GPMC_BASE; ... writel(0x000, gpmc_base + OFFS(GPMC_ECC_CONFIG)); -- cut --
style for 32-bit accesses. OFFS is used to be able to use offset macros in assembly, too, and to be able to directly use offset addresses from TRM not needing them to divide by 4 "manually" (error prone).
[1] http://lists.denx.de/pipermail/u-boot/2008-October/042483.html
I am aware that many areas of ARM code use such a horrible program- ming syle of defining only register offsets instead of proper data structures like for example PowerPC has been doing for ages.
Do you see a chance to convert the OMAP3 code to use data structures instead of offset definitions to describe the processor registers and peripherals?
Looking at the time I already spent into converting to the style used now, no, unfortunately, not. This was the last read/write converting patch, btw.
Best regards
Dirk
Btw.: Did you notice how e.g. Nomadik patch [2] does it?
-- cut -- #define NOMADIK_GPIO1_BASE 0x101E5000 ... writel(0xC37800F0, NOMADIK_GPIO1_BASE + 0x20); -- cut --
Having this patch applied in this format I slightly wonder
a) why I have to convert all hardcoded values to macros
b) why I have to convert all writex/readx
Style used by Nomadik patch is the same OMAP3 initially used and that wasn't accepted...
[2] http://lists.denx.de/pipermail/u-boot/2008-November/043651.html
Dear Dirk,
In message 4929A58A.5060709@googlemail.com you wrote:
I think repeating the sequence "base + offset(foo)" again and again is not exactly nice or easy to read.
...
- writel((a_add_high | a_add_low), SDRC_CS_CFG);
- writel((a_add_high | a_add_low), sdrc_base + OFFS(SDRC_CS_CFG));
...
Argh... that's awfully error prone. So you rely on havnig to deal with 32 bit register accesses only, without any way of compiler supported type checking?
...
This style is what Jean-Christophe and Scott asked me to use [1]. I was asked to convert all OMAP3 read/write access to this style to get the patches accepted.
I don't see a request for this specific style in the message you referenced.
Anyway - I put both Jean-Christophe and Scott on Cc:, and hereby ask to discuss this again.
Disussing this at IRC, we agreed on
-- cut -- #define GPMC_BASE 0x6E000000
OFFS(x) ((x) >> 2)
#define GPMC_ECC_CONFIG 0x1F4
static uint32_t *gpmc_base = (uint32_t *)GPMC_BASE; ... writel(0x000, gpmc_base + OFFS(GPMC_ECC_CONFIG)); -- cut --
But this is ugly and error prone, and I really do not want to continue adding such code to U-Boot.
style for 32-bit accesses. OFFS is used to be able to use offset macros in assembly, too, and to be able to directly use offset addresses from TRM not needing them to divide by 4 "manually" (error prone).
It may be true that the ARM referecne manuals just list register offsets - PowerPC manuals do the same. But this is in no way a reason to access the registers through a "base address + offset" style thing without type checking.
[1] http://lists.denx.de/pipermail/u-boot/2008-October/042483.html
Do you see a chance to convert the OMAP3 code to use data structures instead of offset definitions to describe the processor registers and peripherals?
Looking at the time I already spent into converting to the style used now, no, unfortunately, not. This was the last read/write converting patch, btw.
Well, if we do not clean this up now, more boards will get added which all use this style, and we will never be able to find resources to clean it up.
Instead of declaring lists of offsets like this:
#define GPMC_ECC_CONFIG 0x1F4 #define GPMC_ECC_CONTROL 0x1F8 #define GPMC_ECC_SIZE_CONFIG 0x1FC #define GPMC_ECC1_RESULT 0x200 #define GPMC_ECC2_RESULT 0x204 #define GPMC_ECC3_RESULT 0x208 #define GPMC_ECC4_RESULT 0x20C #define GPMC_ECC5_RESULT 0x210 #define GPMC_ECC6_RESULT 0x214 #define GPMC_ECC7_RESULT 0x218 #define GPMC_ECC8_RESULT 0x21C #define GPMC_ECC9_RESULT 0x220
you should really use structures like this:
struct ecc_control { u32 ecc_config; u32 ecc_control; u32 ecc_size_config; u32 ecc1_result; u32 ecc2_result; u32 ecc3_result; u32 ecc4_result; u32 ecc5_result; u32 ecc6_result; u32 ecc7_result; u32 ecc8_result; u32 ecc9_result; };
so that instead of
writel(0x000, gpmc_base + OFFS(GPMC_ECC_CONFIG));
you can write
writel(0x000, ptr->ecc_config);
or similar - and the compiler will actually perform type checking.
[Actually thius should be done for ALL such offset lists for all systems that use such stuff, i. e. for probably all ARM systems.]
I am aware that this is a major style change, but it really seems necessary to me.
Best regards,
Wolfgang Denk
On Mon, Nov 24, 2008 at 6:47 AM, Wolfgang Denk wd@denx.de wrote:
Dear Dirk,
you should really use structures like this:
struct ecc_control { u32 ecc_config; u32 ecc_control; u32 ecc_size_config; u32 ecc1_result; u32 ecc2_result; u32 ecc3_result; u32 ecc4_result; u32 ecc5_result; u32 ecc6_result; u32 ecc7_result; u32 ecc8_result; u32 ecc9_result; };so that instead of
writel(0x000, gpmc_base + OFFS(GPMC_ECC_CONFIG));you can write
writel(0x000, ptr->ecc_config);or similar - and the compiler will actually perform type checking.
Wolfgang,
Do you mean to instance the ecc_control struct at a specific memory location and use writel(0x000, &ptr->ecc_config);? If so, why not simply use ptr->ecc_config = 0x0000?
Example:
The AMD sc520 has 4k of Memory Mapped Control Registers (MMCR) that range in size from 8 to 32 bits located at a fixed memory address (0xfffef000)
I am thinking of replacing the read/write_mmcr_byte/word/long () functions into a struct e.g.
struct sc520_mmcr { /* CPU */ u16 revid; u8 cpuctl; u8 pad_0x003[0xd];
/* SDRAM Controller */ u8 drcctl; <etc>
} __attribute__((packed));
struct struct sc520_mmcr* mmcrptr = (volatile struct sc520_mmcr*)0xfffef000;
and replacing: write_mmcr_byte(SC520_DRCCTL, \ (read_mmcr_byte(SC520_DRCCTL) & 0xcf) | (val<<4));
with: mmcrptr->drcctl = (mmcrptr->drcctl & 0xcf) | (val<<4);
Is this appropriate?
Regards,
Graeme
Dear Graeme,
in message d66caabb0811231324q5bcf3076p698840e2f73c0564@mail.gmail.com you wrote:
Do you mean to instance the ecc_control struct at a specific memory location and use writel(0x000, &ptr->ecc_config);? If so, why not
Yes - this, and all other blocks of registers.
simply use ptr->ecc_config = 0x0000?
Because in general it will not work. You may make it work in many situations by making sure that the poointer has the "volatile" attribute, but depending on CPu propoerties and/or compiler opti- mization this may or may not be sufficient. Please see for example Documentation/volatile-considered-harmful.txt in the Linux kernel tree.
The only clean and portable and thus reliable way is to use the (CPU specific) accessor macros or functions.
and replacing: write_mmcr_byte(SC520_DRCCTL, \ (read_mmcr_byte(SC520_DRCCTL) & 0xcf) | (val<<4)); with: mmcrptr->drcctl = (mmcrptr->drcctl & 0xcf) | (val<<4);
Is this appropriate?
Probably not. Check what the Linux kenrel is doing (but then, x86 may be a poor example; these processors are simply not advanced enough).
Best regards,
Wolfgang Denk
Wolfgang,
On Mon, Nov 24, 2008 at 10:04 AM, Wolfgang Denk wd@denx.de wrote:
Dear Graeme,
in message d66caabb0811231324q5bcf3076p698840e2f73c0564@mail.gmail.com you wrote:
Do you mean to instance the ecc_control struct at a specific memory location and use writel(0x000, &ptr->ecc_config);? If so, why not
Yes - this, and all other blocks of registers.
simply use ptr->ecc_config = 0x0000?
mization this may or may not be sufficient. Please see for example Documentation/volatile-considered-harmful.txt in the Linux kernel tree.
Have done
The only clean and portable and thus reliable way is to use the (CPU specific) accessor macros or functions.
Will do :)
It seems to me that more and more of these 'style' issues are cropping up. Should there be a document that covers them so we can start to systematically improve the consistency of the code?
Thanks
Graeme
Dear Graeme,
In message d66caabb0811231536k455da2e6wc0ce1a774d37e4@mail.gmail.com you wrote:
The only clean and portable and thus reliable way is to use the (CPU specific) accessor macros or functions.
Will do :)
thanks.
It seems to me that more and more of these 'style' issues are cropping up. Should there be a document that covers them so we can start to systematically improve the consistency of the code?
Well, there is: http://www.denx.de/wiki/U-Boot/CodingStyle and Linux/Documentation/
The thing is, that only now people start to realize the importance of some of these issues. The PowerPC tree may be well ahead compared to other architectures, because that's where both big fat 64bit server machines on one side and low-end (50 MHz) embedded processors on the other side have been supported for a long time. I think no other architecture does that in a similar way. [x86 may be different, because it always is.]
Best regards,
Wolfgang Denk
On Mon, Nov 24, 2008 at 10:47 AM, Wolfgang Denk wd@denx.de wrote:
Linux/Documentation/
Hmmm, thats a big mouthful to chew for such a small bootloader
Maybe we could start to pick out particular specific pieces that apply directly to the U-Boot source?
architecture does that in a similar way. [x86 may be different, because it always is.]
One could argue that x86 is always the same and everyone else is (thankfully) different ;)
Cheers,
Graeme
On Mon, 24 Nov 2008 10:53:46 +1100 "Graeme Russ" graeme.russ@gmail.com wrote:
On Mon, Nov 24, 2008 at 10:47 AM, Wolfgang Denk wd@denx.de wrote:
Linux/Documentation/
Hmmm, thats a big mouthful to chew for such a small bootloader
I believe he meant linux/Documentation/CodingStyle
Kim
Dear Jean-Christophe,
In message 1227445293-23887-1-git-send-email-dirk.behme@googlemail.com you wrote:
Convert readl/writel to base + offset style. Replace hardcoded values with macros.
It would be quite nice if you could apply this patch to u-boot-arm/omap3 branch independent of read/write discussion. As this is the last clean up patch, after applying this patch we would have the omap3 branch at least in a consistent style for the moment.
Many thanks
Dirk
participants (4)
-
Dirk Behme -
Graeme Russ -
Kim Phillips -
Wolfgang Denk