[U-Boot] [PATCH v3] mx25: Clean up lowlevel_init

Sun Sep 30 15:04:14 CEST 2012

On 20/08/2012 21:00, Benoît Thébaudeau wrote:
> Clean up mx25 lowlevel_init:
>  - Add comments.
>  - Do not use write32 repeatedly with the same value in order not to increase
>    code size.
>  - Make register values configurable.
>  - Use macro parameters with default 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>
> Cc: John Rigby <jcrigby at gmail.com>
> Cc: Matthias Weisser <weisserm at arcor.de>
> ---

Hi Benoît,

> Changes for v2:
>  - Use macro arguments with default values instead of #define's.
> Changes for v3:
>  - Fix comment for default MAX MPR value.
> 
>  .../arch/arm/include/asm/arch-mx25/macro.h         |   87 +++++++++++++++-----
>  .../board/karo/tx25/lowlevel_init.S                |   34 +-------
>  2 files changed, 68 insertions(+), 53 deletions(-)
> 
> diff --git u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx25/macro.h u-boot-4d3c95f/arch/arm/include/asm/arch-mx25/macro.h
> index 3b694da..56cae36 100644
> --- u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx25/macro.h
> +++ u-boot-4d3c95f/arch/arm/include/asm/arch-mx25/macro.h
> @@ -32,32 +32,75 @@
>  
>  #include <asm/arch/imx-regs.h>
>  #include <generated/asm-offsets.h>
> +#include <asm/macro.h>
>  
> -.macro init_aips
> -	write32	IMX_AIPS1_BASE + AIPS_MPR_0_7, 0x77777777
> -	write32	IMX_AIPS1_BASE + AIPS_MPR_8_15, 0x77777777
> -	write32	IMX_AIPS2_BASE + AIPS_MPR_0_7, 0x77777777
> -	write32	IMX_AIPS2_BASE + AIPS_MPR_8_15, 0x77777777
> +/*
> + * AIPS setup - Only setup MPROTx registers.
> + * The PACR default values are good.
> + *
> + * Default argument values:
> + *  - MPR: Set all MPROTx to be non-bufferable, trusted for R/W, not forced to
> + *    user-mode.
> + */
> +.macro init_aips mpr=0x77777777
> +	ldr	r0, =IMX_AIPS1_BASE
> +	ldr	r1, =\mpr
> +	str	r1, [r0, #AIPS_MPR_0_7]
> +	str	r1, [r0, #AIPS_MPR_8_15]
> +	ldr	r2, =IMX_AIPS2_BASE
> +	str	r1, [r2, #AIPS_MPR_0_7]
> +	str	r1, [r2, #AIPS_MPR_8_15]
>  .endm
>  
> -.macro init_max
> -	write32	IMX_MAX_BASE + MAX_MPR0, 0x43210
> -	write32	IMX_MAX_BASE + MAX_MPR1, 0x43210
> -	write32	IMX_MAX_BASE + MAX_MPR2, 0x43210
> -	write32	IMX_MAX_BASE + MAX_MPR3, 0x43210
> -	write32	IMX_MAX_BASE + MAX_MPR4, 0x43210
> -
> -	write32	IMX_MAX_BASE + MAX_SGPCR0, 0x10
> -	write32	IMX_MAX_BASE + MAX_SGPCR1, 0x10
> -	write32	IMX_MAX_BASE + MAX_SGPCR2, 0x10
> -	write32	IMX_MAX_BASE + MAX_SGPCR3, 0x10
> -	write32	IMX_MAX_BASE + MAX_SGPCR4, 0x10
> +/*
> + * MAX (Multi-Layer AHB Crossbar Switch) setup
> + *
> + * Default argument values:
> + *  - MPR: priority is IAHB > DAHB > USBOTG > RTIC > eSDHC2/SDMA
> + *  - SGPCR: always park on last master
> + *  - MGPCR: restore default values
> + */
> +.macro init_max mpr=0x00043210, sgpcr=0x00000010, mgpcr=0x00000000
> +	ldr	r0, =IMX_MAX_BASE
> +	ldr	r1, =\mpr
> +	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, =\sgpcr
> +	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, =\mgpcr
> +	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 */
> +.endm
>  
> -	write32	IMX_MAX_BASE + MAX_MGPCR0, 0x0
> -	write32	IMX_MAX_BASE + MAX_MGPCR1, 0x0
> -	write32	IMX_MAX_BASE + MAX_MGPCR2, 0x0
> -	write32	IMX_MAX_BASE + MAX_MGPCR3, 0x0
> -	write32	IMX_MAX_BASE + MAX_MGPCR4, 0x0
> +/*
> + * M3IF setup
> + *
> + * Default argument values:
> + *  - CTL:
> + * MRRP[0] = LCDC on priority list (1 << 0)			= 0x00000001
> + * MRRP[1] = MAX1 not on priority list (0 << 1)			= 0x00000000
> + * MRRP[2] = MAX0 not on priority list (0 << 2)			= 0x00000000
> + * MRRP[3] = USBH not on priority list (0 << 3)			= 0x00000000
> + * MRRP[4] = SDMA not on priority list (0 << 4)			= 0x00000000
> + * MRRP[5] = eSDHC1/ATA/FEC not on priority list (0 << 5)	= 0x00000000
> + * MRRP[6] = LCDC/SLCDC/MAX2 not on priority list (0 << 6)	= 0x00000000
> + * MRRP[7] = CSI not on priority list (0 << 7)			= 0x00000000
> + *								------------
> + *								  0x00000001
> + */
> +.macro init_m3if ctl=0x00000001
> +	/* M3IF Control Register (M3IFCTL) */
> +	write32	IMX_M3IF_CTRL_BASE, \ctl
>  .endm
>  
>  #endif /* __ASSEMBLY__ */
> diff --git u-boot-4d3c95f.orig/board/karo/tx25/lowlevel_init.S u-boot-4d3c95f/board/karo/tx25/lowlevel_init.S
> index 823df10..0421237 100644
> --- u-boot-4d3c95f.orig/board/karo/tx25/lowlevel_init.S
> +++ u-boot-4d3c95f/board/karo/tx25/lowlevel_init.S
> @@ -22,37 +22,7 @@
>   */
>  
>  #include <asm/macro.h>
> -
> -.macro init_aips
> -	write32	0x43f00000, 0x77777777
> -	write32	0x43f00004, 0x77777777
> -	write32	0x43f00000, 0x77777777
> -	write32	0x53f00004, 0x77777777
> -.endm
> -
> -.macro init_max
> -	write32	0x43f04000, 0x43210
> -	write32	0x43f04100, 0x43210
> -	write32	0x43f04200, 0x43210
> -	write32	0x43f04300, 0x43210
> -	write32	0x43f04400, 0x43210
> -
> -	write32	0x43f04010, 0x10
> -	write32	0x43f04110, 0x10
> -	write32	0x43f04210, 0x10
> -	write32	0x43f04310, 0x10
> -	write32	0x43f04410, 0x10
> -
> -	write32	0x43f04800, 0x0
> -	write32	0x43f04900, 0x0
> -	write32	0x43f04a00, 0x0
> -	write32	0x43f04b00, 0x0
> -	write32	0x43f04c00, 0x0
> -.endm
> -
> -.macro init_m3if
> -	write32 0xb8003000, 0x1
> -.endm
> +#include <asm/arch/macro.h>
>  
>  .macro init_clocks
>  	/*
> @@ -64,6 +34,8 @@
>  	 * 0x00600000 makes CLKO parent clk the USB clk
>  	 */
>  	write32	0x53f80064, 0x45600000
> +
> +	/* CCTL: ARM = 399 MHz, AHB = 133 MHz */
>  	write32	0x53f80008, 0x20034000
>  
>  	/*
> 

You only moved code and set default values for macros: I do not see
issues. But checkpatch reports several errors, and, even if some of them
can be related to the fact this file is in assembly, most of them can be
fixed.

Could you take a look ?

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================