Dear Matthias, > Dear Benoît > > Am 14.08.2012 23:25, schrieb Benoît Thébaudeau: > > Clean up mx25 lowlevel_init: > > - Add comments. > > - Do not use write32 repeatedly with the same value in order no > > to increase > > code size. > > - Make register values configurable. > > - Use defined 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> > > --- > > .../arch/arm/include/asm/arch-mx25/macro.h | 57 > > +++++++++++++------- > > .../board/karo/tx25/lowlevel_init.S | 37 > > +++---------- > > .../board/syteco/zmx25/lowlevel_init.S | 6 +++ > > 3 files changed, 51 insertions(+), 49 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..9550ade 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,49 @@ > > > > #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 > > - 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 > > + ldr r0, =IMX_AIPS1_BASE > > + ldr r1, =AIPS_MPR_CONFIG > > + 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 > > > > +/* MAX (Multi-Layer AHB Crossbar Switch) setup */ > > .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 > > + ldr r0, =IMX_MAX_BASE > > + 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 */ > > +.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 */ > > +.macro init_m3if > > + /* M3IF Control Register (M3IFCTL) */ > > + write32 IMX_M3IF_CTRL_BASE, M3IFCTL_CONFIG > > .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..cdd9bda 100644 > > --- u-boot-4d3c95f.orig/board/karo/tx25/lowlevel_init.S > > +++ u-boot-4d3c95f/board/karo/tx25/lowlevel_init.S > > @@ -23,36 +23,13 @@ > > > > #include <asm/macro.h> > > > > -.macro init_aips > > - write32 0x43f00000, 0x77777777 > > - write32 0x43f00004, 0x77777777 > > - write32 0x43f00000, 0x77777777 > > - write32 0x53f00004, 0x77777777 > > -.endm > > +#define AIPS_MPR_CONFIG 0x77777777 > > +#define MAX_MPR_CONFIG 0x43210 > > +#define MAX_SGPCR_CONFIG 0x10 > > +#define MAX_MGPCR_CONFIG 0x0 > > +#define M3IFCTL_CONFIG 0x1 > > I think it would be nicer if the init_max macro can get these values > as > parameters and not as define which has to be there before including > the > header file. You could even use default arguments so that the macro > can > be used without any argument in the default case. What do you think?
The solution that I applied here comes from the i.MX35 lowlevel_init code. I like your suggestion. I think I will do that in my i.MX35 ll series too. > > -.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 +41,8 @@ > > * 0x00600000 makes CLKO parent clk the USB clk > > */ > > write32 0x53f80064, 0x45600000 > > + > > + /* CCTL: ARM = 399 MHz, AHB = 133 MHz */ > > write32 0x53f80008, 0x20034000 > > > > /* > > diff --git u-boot-4d3c95f.orig/board/syteco/zmx25/lowlevel_init.S > > u-boot-4d3c95f/board/syteco/zmx25/lowlevel_init.S > > index 1002674..575755c 100644 > > --- u-boot-4d3c95f.orig/board/syteco/zmx25/lowlevel_init.S > > +++ u-boot-4d3c95f/board/syteco/zmx25/lowlevel_init.S > > @@ -25,6 +25,12 @@ > > */ > > > > #include <asm/macro.h> > > + > > +#define AIPS_MPR_CONFIG 0x77777777 > > +#define MAX_MPR_CONFIG 0x43210 > > +#define MAX_SGPCR_CONFIG 0x10 > > +#define MAX_MGPCR_CONFIG 0x0 > > + > > #include <asm/arch/macro.h> > > #include <asm/arch/imx-regs.h> > > #include <generated/asm-offsets.h> > > I didn't build and run this but from a functional point it looks > good. Great. > BTW: > It would be nice if you put the maintainer of a changed file on CC. I > didn't check the ML all the time and maybe missed this change. Sure, sorry. I Cc'ed the custodian, but forgot the maintainers. I add the other board maintainer to Cc. Best regards, Benoît _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
