diff mbox

[U-Boot] mx31: provide readable WEIM CS accessor

Message ID 1317214100-1379-2-git-send-email-helmut.raiger@hale.at
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Helmut Raiger Sept. 28, 2011, 12:48 p.m. UTC
Some macros are added to support the setup for i.MX31 WEIM
chip selects. As a compromise between verbosity and readability
an ASCII-art'ish bit comment is used instead of bitfields.
All i.MX31 boards have been patched to use this approach using a helper
program to verify the changes.

Signed-off-by: Helmut Raiger <helmut.raiger@hale.at>
---
 arch/arm/include/asm/arch-mx31/imx-regs.h   |   38 ++++++++++++-
 board/davedenx/qong/qong.c                  |   79 +++++++++------------------
 board/freescale/mx31ads/mx31ads.c           |   13 +++--
 board/freescale/mx31pdk/mx31pdk.c           |   11 +++-
 board/imx31_phycore/imx31_phycore.c         |   36 +++++++++---
 board/logicpd/imx31_litekit/imx31_litekit.c |   24 ++++++--
 6 files changed, 123 insertions(+), 78 deletions(-)

Comments

Stefano Babic Sept. 28, 2011, 3:14 p.m. UTC | #1
On 09/28/2011 02:48 PM, Helmut Raiger wrote:
> Some macros are added to support the setup for i.MX31 WEIM
> chip selects. As a compromise between verbosity and readability
> an ASCII-art'ish bit comment is used instead of bitfields.
> All i.MX31 boards have been patched to use this approach using a helper
> program to verify the changes.
> 
> Signed-off-by: Helmut Raiger <helmut.raiger@hale.at>

Hi Helmut,

> ---
>  arch/arm/include/asm/arch-mx31/imx-regs.h   |   38 ++++++++++++-
>  board/davedenx/qong/qong.c                  |   79 +++++++++------------------
>  board/freescale/mx31ads/mx31ads.c           |   13 +++--
>  board/freescale/mx31pdk/mx31pdk.c           |   11 +++-
>  board/imx31_phycore/imx31_phycore.c         |   36 +++++++++---
>  board/logicpd/imx31_litekit/imx31_litekit.c |   24 ++++++--
>  6 files changed, 123 insertions(+), 78 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
> index 2064870..d535830 100644
> --- a/arch/arm/include/asm/arch-mx31/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
> @@ -25,6 +25,7 @@
>  #define __ASM_ARCH_MX31_IMX_REGS_H
>  
>  #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__))
> +#include <asm/io.h>
>  #include <asm/types.h>
>  
>  /* Clock control module registers */
> @@ -534,10 +535,41 @@ enum iomux_pins {
>  #define ESDCTL_BL(x)			((x) << 7)
>  #define ESDCTL_PRCT(x)			((x) << 0)
>  
> +/* 13 fields of the upper CS control register */
> +#define CSCR_U(sp, wp, bcd, bcs, psz, pme, sync, dol, \
> +		cnc, wsc, ew, wws, edc) \
> +	((sp) << 31 | (wp) << 30 | (bcd) << 28 | (psz) << 22 | (pme) << 21 |\
> +	 (sync) << 20 | (dol) << 16 | (cnc) << 14 | (wsc) << 8 | (ew) << 7 |\
> +	 (wws) << 4 | (edc) << 0)
> +/* 12 fields of the lower CS control register */
> +#define CSCR_L(oea, oen, ebwa, ebwn, \
> +		csa, ebc, dsz, csn, psr, cre, wrap, csen) \
> +	((oea) << 28 | (oen) << 24 | (ebwa) << 20 | (ebwn) << 16 |\
> +	 (csa) << 12 | (ebc) << 11 | (dsz) << 8 | (csn) << 4 |\
> +	 (psr) << 3 | (cre) << 2 | (wrap) << 1 | (csen) << 0)
> +/* 14 fields of the additional CS control register */
> +#define CSCR_A(ebra, ebrn, rwa, rwn, mum, lah, lbn, lba, dww, dct, \
> +		wwu, age, cnc2, fce) \
> +	((ebra) << 28 | (ebrn) << 24 | (rwa) << 20 | (rwn) << 16 |\
> +	 (mum) << 15 | (lah) << 13 | (lbn) << 10 | (lba) << 8 |\
> +	 (dww) << 6 | (dct) << 4 | (wwu) << 3 |\
> +	 (age) << 2 | (cnc2) << 1 | (fce) << 0)
> +
>  #define WEIM_BASE	0xb8002000
> -#define CSCR_U(x)	(WEIM_BASE + (x) * 0x10)
> -#define CSCR_L(x)	(WEIM_BASE + 4 + (x) * 0x10)
> -#define CSCR_A(x)	(WEIM_BASE + 8 + (x) * 0x10)
> +#define WEIM_CSCR_U(x)	(WEIM_BASE + (x) * 0x10)
> +#define WEIM_CSCR_L(x)	(WEIM_BASE + 4 + (x) * 0x10)
> +#define WEIM_CSCR_A(x)	(WEIM_BASE + 8 + (x) * 0x10)
> +
> +#ifndef __ASSEMBLER__
> +static inline void mx31_setup_weimcs(int cs,

Is there a reason to embed this function in imx-regs.h ? Why not in
./arch/arm/cpu/arm1136/mx31/generic.c, where I think this function
belongs ?

We are trying to get consistency among the several i.MX SOCs. For this
reason, a general function should not have a specific SOC prefix.
You introduce now a new accessor to set up the WEIM registers. We have
not yet such as function, but we can have then for other SOCs, too.
Rename your function as mxc_setup_weimcs(), and when an accessor will be
supplied for MX5 (or MX*), the same name must be used.

> +		unsigned int upper, unsigned int lower, unsigned int add)
> +{
> +	writel(upper, WEIM_CSCR_U(cs));
> +	writel(lower, WEIM_CSCR_L(cs));
> +	writel(add, WEIM_CSCR_A(cs));
> +}

You are using offests to access registers. Why not to set a structure as:

struct weim_regs {
	u32 upper;
	u32 lower;
	u32 adder;
	u32 reserved;
}

and then :

struct weim {
	struct weim_regs cs[6];
};

...or something like that.

Passing the register values to the function makes the accessor too
striclty bound to the mx31. But if you pass a struct weim*, that is void
mxc_setup_weimcs(struct weim *), we can have the same accessor (with a
different implementation, of course) for the other SOCs, too. I can
imagine we can have MX5 (at the moment I see only the mx53ard) using the
same way to set up the WEIM interface.

Best regards,
Stefano Babic
Helmut Raiger Sept. 29, 2011, 6:32 a.m. UTC | #2
On 09/28/2011 05:14 PM, Stefano Babic wrote
>> Is there a reason to embed this function in imx-regs.h ? Why not in
>> ./arch/arm/cpu/arm1136/mx31/generic.c, where I think this function
>> belongs ?
>>
I took it from the kernel where it is done that way and didn't ask why. 
I'll move it.

>> We are trying to get consistency among the several i.MX SOCs. For this
>> reason, a general function should not have a specific SOC prefix.
>> You introduce now a new accessor to set up the WEIM registers. We have
>> not yet such as function, but we can have then for other SOCs, too.
>> Rename your function as mxc_setup_weimcs(), and when an accessor will be
>> supplied for MX5 (or MX*), the same name must be used.
>>
>> +		unsigned int upper, unsigned int lower, unsigned int add)
>> +{
>> +	writel(upper, WEIM_CSCR_U(cs));
>> +	writel(lower, WEIM_CSCR_L(cs));
>> +	writel(add, WEIM_CSCR_A(cs));
>> +}
> You are using offests to access registers. Why not to set a structure as:
>
> struct weim_regs {
> 	u32 upper;
> 	u32 lower;
> 	u32 adder;
> 	u32 reserved;
> }
>
> and then :
>
> struct weim {
> 	struct weim_regs cs[6];
> };
>
> ...or something like that.
>
> Passing the register values to the function makes the accessor too
> striclty bound to the mx31. But if you pass a struct weim*, that is void
> mxc_setup_weimcs(struct weim *), we can have the same accessor (with a
> different implementation, of course) for the other SOCs, too. I can
> imagine we can have MX5 (at the moment I see only the mx53ard) using the
> same way to set up the WEIM interface.

I used the writel register access to assure correct memory barriers, but 
this might not be a problem with the CS registers. However passing the 
complete set of chip selects wouldn't work, as only a few will be setup 
in the function, while others aren't touched (we could pass a bitmap to 
select which ones should be set, but this seems flamboyant).

What about:

void mxc_setup_weimcs(int cs, const struct mxc_weimcs *cs)
{
...
}

void some_board_init_func(void)
{
     /* CS5: CPLD incl. network controller */
     static const struct mxc_weimcs cs5 = {
         /*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
         CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 24, 0,  4,  3),
         /*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
         CSCR_L(2,  2,   2,   5,  2,  0,  5,  2,  0,  0,   0,   1),
         /*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
         CSCR_A(2,   2,  2,  2,  0,  0,  2,  2,  0,  0,  0,  0,   0,  0)
     };

     mxc_setup_weimcs(5, &cs5);
}

This should still work for different SOCs (with different struct 
mxc_weimcs). CSCR_U et al. will be mx31 specific defines.

Helmut


--
Scanned by MailScanner.
Stefano Babic Sept. 29, 2011, 6:59 a.m. UTC | #3
On 09/29/2011 08:32 AM, Helmut Raiger wrote:
> On 09/28/2011 05:14 PM, Stefano Babic wrote
>>> Is there a reason to embed this function in imx-regs.h ? Why not in
>>> ./arch/arm/cpu/arm1136/mx31/generic.c, where I think this function
>>> belongs ?
>>>
> I took it from the kernel where it is done that way and didn't ask why.
> I'll move it.
> 
>>> We are trying to get consistency among the several i.MX SOCs. For this
>>> reason, a general function should not have a specific SOC prefix.
>>> You introduce now a new accessor to set up the WEIM registers. We have
>>> not yet such as function, but we can have then for other SOCs, too.
>>> Rename your function as mxc_setup_weimcs(), and when an accessor will be
>>> supplied for MX5 (or MX*), the same name must be used.
>>>
>>> +        unsigned int upper, unsigned int lower, unsigned int add)
>>> +{
>>> +    writel(upper, WEIM_CSCR_U(cs));
>>> +    writel(lower, WEIM_CSCR_L(cs));
>>> +    writel(add, WEIM_CSCR_A(cs));
>>> +}
>> You are using offests to access registers. Why not to set a structure as:
>>
>> struct weim_regs {
>>     u32 upper;
>>     u32 lower;
>>     u32 adder;
>>     u32 reserved;
>> }
>>
>> and then :
>>
>> struct weim {
>>     struct weim_regs cs[6];
>> };
>>
>> ...or something like that.
>>
>> Passing the register values to the function makes the accessor too
>> striclty bound to the mx31. But if you pass a struct weim*,

Note: I understand now the misunderstanding. I want to pass a struct
weim_regs *, not weim*.

> that is void
>> mxc_setup_weimcs(struct weim *), we can have the same accessor (with a
>> different implementation, of course) for the other SOCs, too. I can
>> imagine we can have MX5 (at the moment I see only the mx53ard) using the
>> same way to set up the WEIM interface.
> 
> I used the writel register access to assure correct memory barriers,

This is ok

> but
> this might not be a problem with the CS registers. However passing the
> complete set of chip selects wouldn't work,

This is not what I meant. I want that the function change only one
chipselect, not all chipselects in one shot.

> as only a few will be setup
> in the function, while others aren't touched (we could pass a bitmap to
> select which ones should be set, but this seems flamboyant).

No bitmap please...

> 
> What about:
> 
> void mxc_setup_weimcs(int cs, const struct mxc_weimcs *cs)
> {
> ...
> }

This is what I meant ! Only to check the names: mxc_weimcs is what I
described as weim_regs, right ? And this structure can be specified for
each SOC.

> 
> void some_board_init_func(void)
> {
>     /* CS5: CPLD incl. network controller */
>     static const struct mxc_weimcs cs5 = {
>         /*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
>         CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 24, 0,  4,  3),
>         /*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
>         CSCR_L(2,  2,   2,   5,  2,  0,  5,  2,  0,  0,   0,   1),
>         /*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
>         CSCR_A(2,   2,  2,  2,  0,  0,  2,  2,  0,  0,  0,  0,   0,  0)
>     };
> 
>     mxc_setup_weimcs(5, &cs5);

Yes, right

> }
> 
> This should still work for different SOCs (with different struct
> mxc_weimcs).

Exactly.

> CSCR_U et al. will be mx31 specific defines.

This is not a problem - other SOCc have or can have a different layout.
It is correct to define these macro into imx-regs.h, as you already did.

Best regards,
Stefano Babic
Helmut Raiger Sept. 29, 2011, 7:30 a.m. UTC | #4
On 09/28/2011 05:14 PM, Stefano Babic wrote:
>> +#ifndef __ASSEMBLER__
>> +static inline void mx31_setup_weimcs(int cs,
> Is there a reason to embed this function in imx-regs.h ? Why not in
> ./arch/arm/cpu/arm1136/mx31/generic.c, where I think this function
> belongs ?
>

I re-checked, it makes a lot of sense to inline this function as it 
results into 3 simple register writes (addresses are compile time 
calculated if 'cs' is a constant)!

Helmut




--
Scanned by MailScanner.
Stefano Babic Sept. 29, 2011, 9:17 a.m. UTC | #5
On 09/29/2011 09:30 AM, Helmut Raiger wrote:
> On 09/28/2011 05:14 PM, Stefano Babic wrote:
>>> +#ifndef __ASSEMBLER__
>>> +static inline void mx31_setup_weimcs(int cs,
>> Is there a reason to embed this function in imx-regs.h ? Why not in
>> ./arch/arm/cpu/arm1136/mx31/generic.c, where I think this function
>> belongs ?
>>
> 
> I re-checked, it makes a lot of sense to inline this function as it
> results into 3 simple register writes (addresses are compile time
> calculated if 'cs' is a constant)!

Well, this is correct, but I wonder if this is the right function to be
optimized. I cannot imagine that this function runs a lot of times. It
is used to initialize the chipselects, and it is normally called not
more as one time for each chip select, or less. For the i.MX31, not more
as 6 times.

What I am trying in any case to avoid is that the code becomes messy.
And I am trying to have the code as consistent as possible among the
several i.MX SOCs. The i.MX31 is the older, and only part of code was
cleaned up. You see for example a lot of accesses using the __REG macro
inside the i.MX31 code, and this is not accepted for new code.So there
are some "unwritten" rules, that can be acquired reading the code for
the i.MX processors.

- each i.MX SOC has a imx-regs.h file, where the registers and the
layout of the SOC is described. Neither functions nor prototypes must be
inserted here.

- for clock related defines, a crm_regs.h file is defined

- common prototypes are put in sys_proto.h (same name as other ARM SOCs,
see OMAP/TI/s5p).

So please do not put inline functions inside imx-regs.h, and feel free
to add a sys_proto.h (check for the same file for MX35/MX5) for your
purposes.

Best regards,
Stefano Babic
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index 2064870..d535830 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -25,6 +25,7 @@ 
 #define __ASM_ARCH_MX31_IMX_REGS_H
 
 #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__))
+#include <asm/io.h>
 #include <asm/types.h>
 
 /* Clock control module registers */
@@ -534,10 +535,41 @@  enum iomux_pins {
 #define ESDCTL_BL(x)			((x) << 7)
 #define ESDCTL_PRCT(x)			((x) << 0)
 
+/* 13 fields of the upper CS control register */
+#define CSCR_U(sp, wp, bcd, bcs, psz, pme, sync, dol, \
+		cnc, wsc, ew, wws, edc) \
+	((sp) << 31 | (wp) << 30 | (bcd) << 28 | (psz) << 22 | (pme) << 21 |\
+	 (sync) << 20 | (dol) << 16 | (cnc) << 14 | (wsc) << 8 | (ew) << 7 |\
+	 (wws) << 4 | (edc) << 0)
+/* 12 fields of the lower CS control register */
+#define CSCR_L(oea, oen, ebwa, ebwn, \
+		csa, ebc, dsz, csn, psr, cre, wrap, csen) \
+	((oea) << 28 | (oen) << 24 | (ebwa) << 20 | (ebwn) << 16 |\
+	 (csa) << 12 | (ebc) << 11 | (dsz) << 8 | (csn) << 4 |\
+	 (psr) << 3 | (cre) << 2 | (wrap) << 1 | (csen) << 0)
+/* 14 fields of the additional CS control register */
+#define CSCR_A(ebra, ebrn, rwa, rwn, mum, lah, lbn, lba, dww, dct, \
+		wwu, age, cnc2, fce) \
+	((ebra) << 28 | (ebrn) << 24 | (rwa) << 20 | (rwn) << 16 |\
+	 (mum) << 15 | (lah) << 13 | (lbn) << 10 | (lba) << 8 |\
+	 (dww) << 6 | (dct) << 4 | (wwu) << 3 |\
+	 (age) << 2 | (cnc2) << 1 | (fce) << 0)
+
 #define WEIM_BASE	0xb8002000
-#define CSCR_U(x)	(WEIM_BASE + (x) * 0x10)
-#define CSCR_L(x)	(WEIM_BASE + 4 + (x) * 0x10)
-#define CSCR_A(x)	(WEIM_BASE + 8 + (x) * 0x10)
+#define WEIM_CSCR_U(x)	(WEIM_BASE + (x) * 0x10)
+#define WEIM_CSCR_L(x)	(WEIM_BASE + 4 + (x) * 0x10)
+#define WEIM_CSCR_A(x)	(WEIM_BASE + 8 + (x) * 0x10)
+
+#ifndef __ASSEMBLER__
+static inline void mx31_setup_weimcs(int cs,
+		unsigned int upper, unsigned int lower, unsigned int add)
+{
+	writel(upper, WEIM_CSCR_U(cs));
+	writel(lower, WEIM_CSCR_L(cs));
+	writel(add, WEIM_CSCR_A(cs));
+}
+#endif
+
 
 #define IOMUXC_BASE	0x43FAC000
 #define IOMUXC_GPR	(IOMUXC_BASE + 0x8)
diff --git a/board/davedenx/qong/qong.c b/board/davedenx/qong/qong.c
index 99432ed..6cd9e10 100644
--- a/board/davedenx/qong/qong.c
+++ b/board/davedenx/qong/qong.c
@@ -25,7 +25,6 @@ 
 #include <netdev.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
-#include <asm/io.h>
 #include <nand.h>
 #include <fsl_pmic.h>
 #include <asm/gpio.h>
@@ -61,11 +60,15 @@  static void qong_fpga_reset(void)
 int board_early_init_f (void)
 {
 #ifdef CONFIG_QONG_FPGA
-	/* CS1: FPGA/Network Controller/GPIO */
-	/* 16-bit, no DTACK */
-	__REG(CSCR_U(1)) = 0x00000A01;
-	__REG(CSCR_L(1)) = 0x20040501;
-	__REG(CSCR_A(1)) = 0x04020C00;
+	/* CS1: FPGA/Network Controller/GPIO, 16-bit, no DTACK */
+	mx31_setup_weimcs(1,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  0, 10, 0,  0,  1),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(2,  0,   0,   4,  0,  0,  5,  0,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   4,  0,  2,  0,  0,  3,  0,  0,  0,  0,  0,   0,  0)
+	);
 
 	/* setup pins for FPGA */
 	mx31_gpio_mux(IOMUX_MODE(0x76, MUX_CTL_GPIO));
@@ -146,50 +149,14 @@  int board_init (void)
 	/* Chip selects */
 	/* CS0: Nor Flash #0 - it must be init'ed when executing from DDR */
 	/* Assumptions: HCLK = 133 MHz, tACC = 130ns */
-	__REG(CSCR_U(0)) = ((0 << 31)	| /* SP */
-						(0 << 30)	| /* WP */
-						(0 << 28)	| /* BCD */
-						(0 << 24)	| /* BCS */
-						(0 << 22)	| /* PSZ */
-						(0 << 21)	| /* PME */
-						(0 << 20)	| /* SYNC */
-						(0 << 16)	| /* DOL */
-						(3 << 14)	| /* CNC */
-						(21 << 8)	| /* WSC */
-						(0 << 7)	| /* EW */
-						(0 << 4)	| /* WWS */
-						(6 << 0)	  /* EDC */
-					   );
-
-	__REG(CSCR_L(0)) = ((2 << 28)	| /* OEA */
-						(1 << 24)	| /* OEN */
-						(3 << 20)	| /* EBWA */
-						(3 << 16)	| /* EBWN */
-						(1 << 12)	| /* CSA */
-						(1 << 11)	| /* EBC */
-						(5 << 8)	| /* DSZ */
-						(1 << 4)	| /* CSN */
-						(0 << 3)	| /* PSR */
-						(0 << 2)	| /* CRE */
-						(0 << 1)	| /* WRAP */
-						(1 << 0)	  /* CSEN */
-					   );
-
-	__REG(CSCR_A(0)) = ((2 << 28)	| /* EBRA */
-						(1 << 24)	| /* EBRN */
-						(2 << 20)	| /* RWA */
-						(2 << 16)	| /* RWN */
-						(0 << 15)	| /* MUM */
-						(0 << 13)	| /* LAH */
-						(2 << 10)	| /* LBN */
-						(0 << 8)	| /* LBA */
-						(0 << 6)	| /* DWW */
-						(0 << 4)	| /* DCT */
-						(0 << 3)	| /* WWU */
-						(0 << 2)	| /* AGE */
-						(0 << 1)	| /* CNC2 */
-						(0 << 0)	  /* FCE */
-					   );
+	mx31_setup_weimcs(0,
+		/*     sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 21, 0,  0,  6),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(0,  1,   3,   3,  1,  1,  5,  1,  0,  0,   0,  1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   1,  2,  2,  0,  0,  2,  0,  0,  0,  0,  0,   0,  0)
+	);
 
 	/* board id for linux */
 	gd->bd->bi_arch_number = MACH_TYPE_QONG;
@@ -249,9 +216,15 @@  static void board_nand_setup(void)
 {
 
 	/* CS3: NAND 8-bit */
-	__REG(CSCR_U(3)) = 0x00004f00;
-	__REG(CSCR_L(3)) = 0x20013b31;
-	__REG(CSCR_A(3)) = 0x00020800;
+	mx31_setup_weimcs(3,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  1, 15, 0,  0,  0),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(2,  0,   0,   1,  3,  1,  3,  3,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   0,  0,  2,  0,  0,  2,  0,  0,  0,  0,  0,  0,   0)
+	);
+
 	__REG(IOMUXC_GPR) |= 1 << 13;
 
 	mx31_gpio_mux(IOMUX_MODE(MUX_CTL_NFC_WP, MUX_CTL_IN_GPIO));
diff --git a/board/freescale/mx31ads/mx31ads.c b/board/freescale/mx31ads/mx31ads.c
index 7637c92..eafb57c 100644
--- a/board/freescale/mx31ads/mx31ads.c
+++ b/board/freescale/mx31ads/mx31ads.c
@@ -22,7 +22,6 @@ 
 
 #include <common.h>
 #include <netdev.h>
-#include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
 
@@ -54,9 +53,15 @@  int board_early_init_f(void)
 	 * the only non-zero field "Wait State Control" is set to half the
 	 * default value.
 	 */
-	__REG(CSCR_U(0)) = 0x00000f00;
-	__REG(CSCR_L(0)) = 0x10000D03;
-	__REG(CSCR_A(0)) = 0x00720900;
+	mx31_setup_weimcs(0,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  0, 15, 0,  0,  0),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(1,  0,   0,   0,  0,  1,  5,  0,  0,  0,   1,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   0,  7,  2,  0,  0,  2,  1,  0,  0,  0,  0,   0,   0)
+	);
+
 
 	/* setup pins for UART1 */
 	mx31_gpio_mux(MUX_RXD1__UART1_RXD_MUX);
diff --git a/board/freescale/mx31pdk/mx31pdk.c b/board/freescale/mx31pdk/mx31pdk.c
index f6e190a..3d5e312 100644
--- a/board/freescale/mx31pdk/mx31pdk.c
+++ b/board/freescale/mx31pdk/mx31pdk.c
@@ -56,9 +56,14 @@  void dram_init_banksize(void)
 int board_early_init_f(void)
 {
 	/* CS5: CPLD incl. network controller */
-	__REG(CSCR_U(5)) = 0x0000d843;
-	__REG(CSCR_L(5)) = 0x22252521;
-	__REG(CSCR_A(5)) = 0x22220a00;
+	mx31_setup_weimcs(5,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 24, 0,  4,  3),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(2,  2,   2,   5,  2,  0,  5,  2,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(2,   2,  2,  2,  0,  0,  2,  2,  0,  0,  0,  0,   0,  0)
+	);
 
 	/* Setup UART1 and SPI2 pins */
 	mx31_uart1_hw_init();
diff --git a/board/imx31_phycore/imx31_phycore.c b/board/imx31_phycore/imx31_phycore.c
index 773900e..f4ad681 100644
--- a/board/imx31_phycore/imx31_phycore.c
+++ b/board/imx31_phycore/imx31_phycore.c
@@ -49,17 +49,35 @@  int board_init(void)
 
 int board_early_init_f(void)
 {
-	__REG(CSCR_U(0)) = 0x0000cf03; /* CS0: Nor Flash */
-	__REG(CSCR_L(0)) = 0x10000d03;
-	__REG(CSCR_A(0)) = 0x00720900;
-
-	__REG(CSCR_U(1)) = 0x0000df06; /* CS1: Network Controller */
-	__REG(CSCR_L(1)) = 0x444a4541;
-	__REG(CSCR_A(1)) = 0x44443302;
-
-	__REG(CSCR_U(4)) = 0x0000d843; /* CS4: SRAM */
-	__REG(CSCR_L(4)) = 0x22252521;
-	__REG(CSCR_A(4)) = 0x22220a00;
+	/* CS0: Nor Flash */
+	mx31_setup_weimcs(0,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 15, 0,  0,  3),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(1,  0,   0,   0,  0,  1,  5,  0,  0,  0,   1,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   0,  7,  2,  0,  0,  2,  1,  0,  0,  0,  0,   0,  0)
+	);
+
+	/* CS1: Network Controller */
+	mx31_setup_weimcs(1,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 31, 0,  0,  6),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(4,  4,   4,  10,  4,  0,  5,  4,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(4,   4,  4,  4,  0,  1,  4,  3,  0,  0,  0,  0,   1,  0)
+	);
+
+	/* CS4: SRAM */
+	mx31_setup_weimcs(4,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 24, 0,  4,  3),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(2,  2,   2,   5,  2,  0,  5,  2,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(2,   2,  2,  2,  0,  0,  2,  2,  0,  0,  0,  0,   0,  0)
+	);
 
 	/* setup pins for UART1 */
 	mx31_gpio_mux(MUX_RXD1__UART1_RXD_MUX);
diff --git a/board/logicpd/imx31_litekit/imx31_litekit.c b/board/logicpd/imx31_litekit/imx31_litekit.c
index 09cc9c5..2805cfe 100644
--- a/board/logicpd/imx31_litekit/imx31_litekit.c
+++ b/board/logicpd/imx31_litekit/imx31_litekit.c
@@ -45,13 +45,25 @@  void dram_init_banksize(void)
 
 int board_early_init_f(void)
 {
-	__REG(CSCR_U(0)) = 0x0000cf03; /* CS0: Nor Flash */
-	__REG(CSCR_L(0)) = 0xa0330d01;
-	__REG(CSCR_A(0)) = 0x00220800;
+	/* CS0: Nor Flash */
+	mx31_setup_weimcs(0,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 15, 0,  0,  3),
+		/*    oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(10,  0,   3,   3,  0,  1,  5,  0,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(0,   0,  2,  2,  0,  0,  2,  0,  0,  0,  0,  0,   0,  0)
+	);
 
-	__REG(CSCR_U(4)) = 0x0000dcf6; /* CS4: Network Controller */
-	__REG(CSCR_L(4)) = 0x444a4541;
-	__REG(CSCR_A(4)) = 0x44443302;
+	/* CS4: Network Controller */
+	mx31_setup_weimcs(4,
+		/*    sp wp bcd bcs psz pme sync dol cnc wsc ew wws edc */
+		CSCR_U(0, 0,  0,  0,  0,  0,   0,  0,  3, 28, 1,  7,  6),
+		/*   oea oen ebwa ebwn csa ebc dsz csn psr cre wrap csen */
+		CSCR_L(4,  4,   4,  10,  4,  0,  5,  4,  0,  0,   0,   1),
+		/*  ebra ebrn rwa rwn mum lah lbn lba dww dct wwu age cnc2 fce*/
+		CSCR_A(4,   4,  4,  4,  0,  1,  4,  3,  0,  0,  0,  0,   1,  0)
+	);
 
 	/* setup pins for UART1 */
 	mx31_gpio_mux(MUX_RXD1__UART1_RXD_MUX);