Hi Stefano, On Sunday, September 30, 2012 3:04:14 PM, Stefano Babic wrote: > 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.thebaud...@advansee.com> > > Cc: Stefano Babic <sba...@denx.de> > > Cc: John Rigby <jcri...@gmail.com> > > Cc: Matthias Weisser <weiss...@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 ?
Will do. Best regards, Benoît _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
