[U-Boot] [PATCH 3/3] mx35: Clean up lowlevel_init

Benoît Thébaudeau benoit.thebaudeau at advansee.com
Mon Aug 20 14:09:20 CEST 2012


Hi Stefano,

> Clean up mx35 lowlevel_init:
>  - Indent with tabs.
>  - Fix comments.
>  - Use defined values instead of literal constants.
>  - Use defined macros instead of duplicating code.
> 
> Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> Cc: Stefano Babic <sbabic at denx.de>
> ---
>  .../arm/include/asm/arch-mx35/lowlevel_macro.S     |  153
>  ++++++++------------
>  .../board/CarMediaLab/flea3/lowlevel_init.S        |   19 +--
>  .../board/freescale/mx35pdk/lowlevel_init.S        |  119
>  +--------------
>  .../board/freescale/mx35pdk/mx35pdk.h              |   14 +-
>  4 files changed, 78 insertions(+), 227 deletions(-)
> 
> diff --git
> u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx35/lowlevel_macro.S
> u-boot-4d3c95f/arch/arm/include/asm/arch-mx35/lowlevel_macro.S
> index 05aa951..9d7c133 100644
> ---
> u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx35/lowlevel_macro.S
> +++ u-boot-4d3c95f/arch/arm/include/asm/arch-mx35/lowlevel_macro.S
> @@ -19,122 +19,93 @@
>   * MA 02111-1307 USA
>   */
>  
> +#include <asm/arch/imx-regs.h>
> +#include <generated/asm-offsets.h>
> +#include <asm/macro.h>
> +
>  /*
>   * AIPS setup - Only setup MPROTx registers.
>   * The PACR default values are good.
>   */
>  .macro init_aips
> -	/*
> -	 * Set all MPROTx to be non-bufferable, trusted for R/W,
> -	 * not forced to user-mode.
> -	 */
> -	ldr r0, =AIPS1_BASE_ADDR
> -	ldr r1, =AIPS_MPR_CONFIG
> -	str r1, [r0, #0x00]
> -	str r1, [r0, #0x04]
> -	ldr r0, =AIPS2_BASE_ADDR
> -	str r1, [r0, #0x00]
> -	str r1, [r0, #0x04]
> +	ldr	r0, =AIPS1_BASE_ADDR
> +	ldr	r1, =AIPS_MPR_CONFIG
> +	str	r1, [r0, #AIPS_MPR_0_7]
> +	str	r1, [r0, #AIPS_MPR_8_15]
> +	ldr	r2, =AIPS2_BASE_ADDR
> +	str	r1, [r2, #AIPS_MPR_0_7]
> +	str	r1, [r2, #AIPS_MPR_8_15]
>  
> -	/*
> -	 * Clear the on and off peripheral modules Supervisor Protect bit
> -	 * for SDMA to access them. Did not change the AIPS control
> registers
> -	 * (offset 0x20) access type
> -	 */
> -	ldr r0, =AIPS1_BASE_ADDR
> -	ldr r1, =AIPS_OPACR_CONFIG
> -	str r1, [r0, #0x40]
> -	str r1, [r0, #0x44]
> -	str r1, [r0, #0x48]
> -	str r1, [r0, #0x4C]
> -	str r1, [r0, #0x50]
> -	ldr r0, =AIPS2_BASE_ADDR
> -	str r1, [r0, #0x40]
> -	str r1, [r0, #0x44]
> -	str r1, [r0, #0x48]
> -	str r1, [r0, #0x4C]
> -	str r1, [r0, #0x50]
> +	/* Did not change the AIPS control registers access type. */
> +	ldr	r1, =AIPS_OPACR_CONFIG
> +	str	r1, [r0, #AIPS_OPACR_0_7]
> +	str	r1, [r0, #AIPS_OPACR_8_15]
> +	str	r1, [r0, #AIPS_OPACR_16_23]
> +	str	r1, [r0, #AIPS_OPACR_24_31]
> +	str	r1, [r0, #AIPS_OPACR_32_39]
> +	str	r1, [r2, #AIPS_OPACR_0_7]
> +	str	r1, [r2, #AIPS_OPACR_8_15]
> +	str	r1, [r2, #AIPS_OPACR_16_23]
> +	str	r1, [r2, #AIPS_OPACR_24_31]
> +	str	r1, [r2, #AIPS_OPACR_32_39]
>  .endm
>  
>  /* MAX (Multi-Layer AHB Crossbar Switch) setup */
>  .macro init_max
> -	ldr r0, =MAX_BASE_ADDR
> -	/* MPR - priority is M4 > M2 > M3 > M5 > M0 > M1 */
> -	ldr r1, =MAX_MPR_CONFIG
> -	str r1, [r0, #0x000]        /* for S0 */
> -	str r1, [r0, #0x100]        /* for S1 */
> -	str r1, [r0, #0x200]        /* for S2 */
> -	str r1, [r0, #0x300]        /* for S3 */
> -	str r1, [r0, #0x400]        /* for S4 */
> -	/* SGPCR - always park on last master */
> -	ldr r1, =MAX_SGPCR_CONFIG
> -	str r1, [r0, #0x010]        /* for S0 */
> -	str r1, [r0, #0x110]        /* for S1 */
> -	str r1, [r0, #0x210]        /* for S2 */
> -	str r1, [r0, #0x310]        /* for S3 */
> -	str r1, [r0, #0x410]        /* for S4 */
> -	/* MGPCR - restore default values */
> -	ldr r1, =MAX_MGPCR_CONFIG
> -	str r1, [r0, #0x800]        /* for M0 */
> -	str r1, [r0, #0x900]        /* for M1 */
> -	str r1, [r0, #0xA00]        /* for M2 */
> -	str r1, [r0, #0xB00]        /* for M3 */
> -	str r1, [r0, #0xC00]        /* for M4 */
> -	str r1, [r0, #0xD00]        /* for M5 */
> +	ldr	r0, =MAX_BASE_ADDR
> +	ldr	r1, =MAX_MPR_CONFIG
> +	str	r1, [r0, #MAX_MPR0]	/* for S0 */
> +	str	r1, [r0, #MAX_MPR1]	/* for S1 */
> +	str	r1, [r0, #MAX_MPR2]	/* for S2 */
> +	str	r1, [r0, #MAX_MPR3]	/* for S3 */
> +	str	r1, [r0, #MAX_MPR4]	/* for S4 */
> +	ldr	r1, =MAX_SGPCR_CONFIG
> +	str	r1, [r0, #MAX_SGPCR0]	/* for S0 */
> +	str	r1, [r0, #MAX_SGPCR1]	/* for S1 */
> +	str	r1, [r0, #MAX_SGPCR2]	/* for S2 */
> +	str	r1, [r0, #MAX_SGPCR3]	/* for S3 */
> +	str	r1, [r0, #MAX_SGPCR4]	/* for S4 */
> +	ldr	r1, =MAX_MGPCR_CONFIG
> +	str	r1, [r0, #MAX_MGPCR0]	/* for M0 */
> +	str	r1, [r0, #MAX_MGPCR1]	/* for M1 */
> +	str	r1, [r0, #MAX_MGPCR2]	/* for M2 */
> +	str	r1, [r0, #MAX_MGPCR3]	/* for M3 */
> +	str	r1, [r0, #MAX_MGPCR4]	/* for M4 */
> +	str	r1, [r0, #MAX_MGPCR5]	/* for M5 */
>  .endm
>  
>  /* M3IF setup */
>  .macro init_m3if
> -	/* Configure M3IF registers */
> -	ldr r1, =M3IF_BASE_ADDR
> -	/*
> -	* M3IF Control Register (M3IFCTL)
> -	* MRRP[0] = L2CC0 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[1] = L2CC1 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[2] = MBX not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[3] = MAX1 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[4] = SDMA not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[5] = MPEG4 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[6] = IPU1 on priority list (1 << 6)	= 0x00000040
> -	* MRRP[7] = IPU2 not on priority list (0 << 0)	= 0x00000000
> -	*						------------
> -	*						  0x00000040
> -	*/
> -	ldr r0, =M3IF_CONFIG
> -	str r0, [r1]  /* M3IF control reg */
> +	/* M3IF Control Register (M3IFCTL) */
> +	write32	M3IF_BASE_ADDR, M3IFCTL_CONFIG
>  .endm
>  
>  .macro core_init
> -	mrc 15, 0, r1, c1, c0, 0
> +	mrc	p15, 0, r1, c1, c0, 0
>  
> -	mrc 15, 0, r0, c1, c0, 1
> -	orr r0, r0, #7
> -	mcr 15, 0, r0, c1, c0, 1
> -	orr r1, r1, #(1<<11)
> +	/* Set branch prediction enable */
> +	mrc	p15, 0, r0, c1, c0, 1
> +	orr	r0, r0, #7
> +	mcr	p15, 0, r0, c1, c0, 1
> +	orr	r1, r1, #1 << 11
>  
>  	/* Set unaligned access enable */
> -	orr r1, r1, #(1<<22)
> +	orr	r1, r1, #1 << 22
>  
>  	/* Set low int latency enable */
> -	orr r1, r1, #(1<<21)
> +	orr	r1, r1, #1 << 21
>  
> -	mcr 15, 0, r1, c1, c0, 0
> +	mcr	p15, 0, r1, c1, c0, 0
>  
> -	mov r0, #0
> +	mov	r0, #0
>  
> -	/* Set branch prediction enable */
> -	mcr 15, 0, r0, c15, c2, 4
> +	mcr	p15, 0, r0, c15, c2, 4
>  
> -	mcr 15, 0, r0, c7, c7, 0        /* invalidate I cache and D cache
> */
> -	mcr 15, 0, r0, c8, c7, 0        /* invalidate TLBs */
> -	mcr 15, 0, r0, c7, c10, 4       /* Drain the write buffer */
> +	mcr	p15, 0, r0, c7, c7, 0	/* Invalidate I cache and D cache */
> +	mcr	p15, 0, r0, c8, c7, 0	/* Invalidate TLBs */
> +	mcr	p15, 0, r0, c7, c10, 4	/* Drain the write buffer */
>  
> -	/*
> -	 * initializes very early AIPS
> -	 * Then it also initializes Multi-Layer AHB Crossbar Switch,
> -	 * M3IF
> -	 * Also setup the Peripheral Port Remap register inside the core
> -	 */
> -	ldr r0, =0x40000015        /* start from AIPS 2GB region */
> -	mcr p15, 0, r0, c15, c2, 4
> +	/* Setup the Peripheral Port Memory Remap Register */
> +	ldr	r0, =0x40000015		/* Start from AIPS 2-GB region */
> +	mcr	p15, 0, r0, c15, c2, 4
>  .endm
> diff --git
> u-boot-4d3c95f.orig/board/CarMediaLab/flea3/lowlevel_init.S
> u-boot-4d3c95f/board/CarMediaLab/flea3/lowlevel_init.S
> index 2f42fc9..e7e5cd7 100644
> --- u-boot-4d3c95f.orig/board/CarMediaLab/flea3/lowlevel_init.S
> +++ u-boot-4d3c95f/board/CarMediaLab/flea3/lowlevel_init.S
> @@ -22,9 +22,6 @@
>   */
>  
>  #include <config.h>
> -#include <asm-offsets.h>
> -#include <asm/arch/imx-regs.h>
> -#include <generated/asm-offsets.h>
>  
>  /*
>   * Configuration for the flea3 board.
> @@ -46,19 +43,17 @@
>  /*
>   * M3IF Control Register (M3IFCTL)
>   * MRRP[0] = L2CC0 not on priority list (0 << 0) = 0x00000000
> - * MRRP[1] = L2CC1 not on priority list (0 << 0) = 0x00000000
> - * MRRP[2] = MBX not on priority list (0 << 0)   = 0x00000000
> - * MRRP[3] = MAX1 not on priority list (0 << 0)  = 0x00000000
> - * MRRP[4] = SDMA not on priority list (0 << 0)  = 0x00000000
> - * MRRP[5] = MPEG4 not on priority list (0 << 0) = 0x00000000
> + * MRRP[1] = L2CC1 not on priority list (0 << 1) = 0x00000000
> + * MRRP[2] = MBX not on priority list (0 << 2)   = 0x00000000
> + * MRRP[3] = MAX1 not on priority list (0 << 3)  = 0x00000000
> + * MRRP[4] = SDMA not on priority list (0 << 4)  = 0x00000000
> + * MRRP[5] = MPEG4 not on priority list (0 << 5) = 0x00000000
>   * MRRP[6] = IPU1 on priority list (1 << 6)      = 0x00000040
> - * MRRP[7] = IPU2 not on priority list (0 << 0)  = 0x00000000
> + * MRRP[7] = IPU2 not on priority list (0 << 7)  = 0x00000000
>   *                                               ------------
>   *                                                 0x00000040
>   */
> -#define M3IF_CONFIG		0x00000040
> -
> -#define CCM_PDR0_CONFIG		0x00801000
> +#define M3IFCTL_CONFIG		0x00000040
>  
>  /*
>   * includes MX35 utility macros
> diff --git
> u-boot-4d3c95f.orig/board/freescale/mx35pdk/lowlevel_init.S
> u-boot-4d3c95f/board/freescale/mx35pdk/lowlevel_init.S
> index 698c4cf..75bb958 100644
> --- u-boot-4d3c95f.orig/board/freescale/mx35pdk/lowlevel_init.S
> +++ u-boot-4d3c95f/board/freescale/mx35pdk/lowlevel_init.S
> @@ -23,6 +23,7 @@
>  #include <asm/arch/imx-regs.h>
>  #include <generated/asm-offsets.h>
>  #include "mx35pdk.h"
> +#include <asm/arch/lowlevel_macro.S>
>  
>  /*
>   * return soc version
> @@ -40,91 +41,6 @@
>  	addne \ret, \ret, #0x10
>  .endm
>  
> -/*
> - * AIPS setup - Only setup MPROTx registers.
> - * The PACR default values are good.
> - */
> -.macro init_aips
> -	/*
> -	 * Set all MPROTx to be non-bufferable, trusted for R/W,
> -	 * not forced to user-mode.
> -	 */
> -	ldr r0, =AIPS1_BASE_ADDR
> -	ldr r1, =AIPS_MPR_CONFIG
> -	str r1, [r0, #0x00]
> -	str r1, [r0, #0x04]
> -	ldr r0, =AIPS2_BASE_ADDR
> -	str r1, [r0, #0x00]
> -	str r1, [r0, #0x04]
> -
> -	/*
> -	 * Clear the on and off peripheral modules Supervisor Protect bit
> -	 * for SDMA to access them. Did not change the AIPS control
> registers
> -	 * (offset 0x20) access type
> -	 */
> -	ldr r0, =AIPS1_BASE_ADDR
> -	ldr r1, =AIPS_OPACR_CONFIG
> -	str r1, [r0, #0x40]
> -	str r1, [r0, #0x44]
> -	str r1, [r0, #0x48]
> -	str r1, [r0, #0x4C]
> -	str r1, [r0, #0x50]
> -	ldr r0, =AIPS2_BASE_ADDR
> -	str r1, [r0, #0x40]
> -	str r1, [r0, #0x44]
> -	str r1, [r0, #0x48]
> -	str r1, [r0, #0x4C]
> -	str r1, [r0, #0x50]
> -.endm
> -
> -/* MAX (Multi-Layer AHB Crossbar Switch) setup */
> -.macro init_max
> -	ldr r0, =MAX_BASE_ADDR
> -	/* MPR - priority is M4 > M2 > M3 > M5 > M0 > M1 */
> -	ldr r1, =MAX_MPR_CONFIG
> -	str r1, [r0, #0x000]        /* for S0 */
> -	str r1, [r0, #0x100]        /* for S1 */
> -	str r1, [r0, #0x200]        /* for S2 */
> -	str r1, [r0, #0x300]        /* for S3 */
> -	str r1, [r0, #0x400]        /* for S4 */
> -	/* SGPCR - always park on last master */
> -	ldr r1, =MAX_SGPCR_CONFIG
> -	str r1, [r0, #0x010]        /* for S0 */
> -	str r1, [r0, #0x110]        /* for S1 */
> -	str r1, [r0, #0x210]        /* for S2 */
> -	str r1, [r0, #0x310]        /* for S3 */
> -	str r1, [r0, #0x410]        /* for S4 */
> -	/* MGPCR - restore default values */
> -	ldr r1, =MAX_MGPCR_CONFIG
> -	str r1, [r0, #0x800]        /* for M0 */
> -	str r1, [r0, #0x900]        /* for M1 */
> -	str r1, [r0, #0xA00]        /* for M2 */
> -	str r1, [r0, #0xB00]        /* for M3 */
> -	str r1, [r0, #0xC00]        /* for M4 */
> -	str r1, [r0, #0xD00]        /* for M5 */
> -.endm
> -
> -/* M3IF setup */
> -.macro init_m3if
> -	/* Configure M3IF registers */
> -	ldr r1, =M3IF_BASE_ADDR
> -	/*
> -	* M3IF Control Register (M3IFCTL)
> -	* MRRP[0] = L2CC0 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[1] = L2CC1 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[2] = MBX not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[3] = MAX1 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[4] = SDMA not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[5] = MPEG4 not on priority list (0 << 0)	= 0x00000000
> -	* MRRP[6] = IPU1 on priority list (1 << 6)	= 0x00000040
> -	* MRRP[7] = IPU2 not on priority list (0 << 0)	= 0x00000000
> -	*						------------
> -	*						  0x00000040
> -	*/
> -	ldr r0, =M3IF_CONFIG
> -	str r0, [r1]  /* M3IF control reg */
> -.endm
> -
>  /* CPLD on CS5 setup */
>  .macro init_debug_board
>  	ldr r0, =DBG_BASE_ADDR
> @@ -210,38 +126,7 @@
>  lowlevel_init:
>  	mov r10, lr
>  
> -	mrc 15, 0, r1, c1, c0, 0
> -
> -	mrc 15, 0, r0, c1, c0, 1
> -	orr r0, r0, #7
> -	mcr 15, 0, r0, c1, c0, 1
> -	orr r1, r1, #(1<<11)
> -
> -	/* Set unaligned access enable */
> -	orr r1, r1, #(1<<22)
> -
> -	/* Set low int latency enable */
> -	orr r1, r1, #(1<<21)
> -
> -	mcr 15, 0, r1, c1, c0, 0
> -
> -	mov r0, #0
> -
> -	/* Set branch prediction enable */
> -	mcr 15, 0, r0, c15, c2, 4
> -
> -	mcr 15, 0, r0, c7, c7, 0        /* invalidate I cache and D cache
> */
> -	mcr 15, 0, r0, c8, c7, 0        /* invalidate TLBs */
> -	mcr 15, 0, r0, c7, c10, 4       /* Drain the write buffer */
> -
> -	/*
> -	 * initializes very early AIPS
> -	 * Then it also initializes Multi-Layer AHB Crossbar Switch,
> -	 * M3IF
> -	 * Also setup the Peripheral Port Remap register inside the core
> -	 */
> -	ldr r0, =0x40000015        /* start from AIPS 2GB region */
> -	mcr p15, 0, r0, c15, c2, 4
> +	core_init
>  
>  	init_aips
>  
> diff --git u-boot-4d3c95f.orig/board/freescale/mx35pdk/mx35pdk.h
> u-boot-4d3c95f/board/freescale/mx35pdk/mx35pdk.h
> index 6aeb218..9e44f1f 100644
> --- u-boot-4d3c95f.orig/board/freescale/mx35pdk/mx35pdk.h
> +++ u-boot-4d3c95f/board/freescale/mx35pdk/mx35pdk.h
> @@ -39,17 +39,17 @@
>  /*
>   * M3IF Control Register (M3IFCTL)
>   * MRRP[0] = L2CC0 not on priority list (0 << 0) = 0x00000000
> - * MRRP[1] = L2CC1 not on priority list (0 << 0) = 0x00000000
> - * MRRP[2] = MBX not on priority list (0 << 0)   = 0x00000000
> - * MRRP[3] = MAX1 not on priority list (0 << 0)  = 0x00000000
> - * MRRP[4] = SDMA not on priority list (0 << 0)  = 0x00000000
> - * MRRP[5] = MPEG4 not on priority list (0 << 0) = 0x00000000
> + * MRRP[1] = L2CC1 not on priority list (0 << 1) = 0x00000000
> + * MRRP[2] = MBX not on priority list (0 << 2)   = 0x00000000
> + * MRRP[3] = MAX1 not on priority list (0 << 3)  = 0x00000000
> + * MRRP[4] = SDMA not on priority list (0 << 4)  = 0x00000000
> + * MRRP[5] = MPEG4 not on priority list (0 << 5) = 0x00000000
>   * MRRP[6] = IPU1 on priority list (1 << 6)      = 0x00000040
> - * MRRP[7] = IPU2 not on priority list (0 << 0)  = 0x00000000
> + * MRRP[7] = IPU2 not on priority list (0 << 7)  = 0x00000000
>   *                                               ------------
>   *                                                 0x00000040
>   */
> -#define M3IF_CONFIG	0x00000040
> +#define M3IFCTL_CONFIG		0x00000040
>  
>  #define DBG_BASE_ADDR		WEIM_CTRL_CS5
>  #define DBG_CSCR_U_CONFIG	0x0000D843
> 

Note that I will send a v2 for this patch replacing the configs with macro
parameters having default values.

Do you think that it's worth keeping the configs in mx35pdk.h after that (just
in case, so that they can be easily changed without modifying
board/freescale/mx35pdk/lowlevel_init.S)? IMHO, it's better to remove them if
they have the default values of macro parameters.

Best regards,
Benoît


More information about the U-Boot mailing list