Re: [U-Boot] [PATCH] TI: Davinci: NAND Driver Cleanup

2010-03-16 Thread Nick Thompson 
On 14/03/10 21:14, s-paul...@ti.com wrote:
 From: Cyril Chemparathy cy...@ti.com
 
 Modified to use IO accessor routines consistently.  Eliminated volatile usage
 to keep checkpatch.pl happy.
 Patch was tested on DM355, DM365 and DM6446 EVMs
 
 Signed-off-by: Cyril Chemparathy cy...@ti.com
 Tested-by: Sandeep Paulraj s-paul...@ti.com
 ---
  drivers/mtd/nand/davinci_nand.c  |  126 
 --
  include/asm-arm/arch-davinci/emif_defs.h |   80 +--
  2 files changed, 104 insertions(+), 102 deletions(-)
 
 diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
 index bfc2acf..61cba14 100644
 --- a/drivers/mtd/nand/davinci_nand.c
 +++ b/drivers/mtd/nand/davinci_nand.c
 @@ -57,7 +57,8 @@
  #define ECC_STATE_ERR_CORR_COMP_P0x2
  #define ECC_STATE_ERR_CORR_COMP_N0x3
  
 -static emif_registers *const emif_regs = (void *) 
 DAVINCI_ASYNC_EMIF_CNTRL_BASE;
 +static struct davinci_emif_regs *emif_regs =
 + (struct  davinci_emif_regs *) DAVINCI_ASYNC_EMIF_CNTRL_BASE;

Since this is really just a constant, why setup a variable locally where
ever EMIF registers are accessed? What's wrong with the define you removed
below? ...

 diff --git a/include/asm-arm/arch-davinci/emif_defs.h 
 b/include/asm-arm/arch-davinci/emif_defs.h
 index aa57703..3d77bfc 100644
 --- a/include/asm-arm/arch-davinci/emif_defs.h
 +++ b/include/asm-arm/arch-davinci/emif_defs.h
 @@ -24,50 +24,42 @@
  
  #include asm/arch/hardware.h
  
 -typedef struct davinci_emif_regs {
 - dv_reg  ERCSR;
 - dv_reg  AWCCR;
 - dv_reg  SDBCR;
 - dv_reg  SDRCR;
 - dv_reg  AB1CR;
 - dv_reg  AB2CR;
 - dv_reg  AB3CR;
 - dv_reg  AB4CR;
 - dv_reg  SDTIMR;
 - dv_reg  DDRSR;
 - dv_reg  DDRPHYCR;
 - dv_reg  DDRPHYSR;
 - dv_reg  TOTAR;
 - dv_reg  TOTACTR;
 - dv_reg  DDRPHYID_REV;
 - dv_reg  SDSRETR;
 - dv_reg  EIRR;
 - dv_reg  EIMR;
 - dv_reg  EIMSR;
 - dv_reg  EIMCR;
 - dv_reg  IOCTRLR;
 - dv_reg  IOSTATR;
 - u_int8_tRSVD0[8];
 - dv_reg  NANDFCR;
 - dv_reg  NANDFSR;
 - u_int8_tRSVD1[8];
 - dv_reg  NANDFECC[4];
 - u_int8_tRSVD2[60];
 - dv_reg  NAND4BITECCLOAD;
 - dv_reg  NAND4BITECC1;
 - dv_reg  NAND4BITECC2;
 - dv_reg  NAND4BITECC3;
 - dv_reg  NAND4BITECC4;
 - dv_reg  NANDERRADD1;
 - dv_reg  NANDERRADD2;
 - dv_reg  NANDERRVAL1;
 - dv_reg  NANDERRVAL2;
 -} emif_registers;
 -
 -typedef emif_registers   *emifregs;
 -
 -#define davinci_emif_regs \
 - ((struct davinci_emif_regs *)DAVINCI_ASYNC_EMIF_CNTRL_BASE)

...This one.

Nick.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] TI: Davinci: NAND Driver Cleanup

2010-03-16 Thread Chemparathy, Cyril 
Hi Scott,

 Configuring for davinci_schmoogie board...
 ...
 Should be lowercase?

Thank you.  I will be sending out a v2 shortly, and this time around all 8 
davinci based boards build fine.

 Also, any particular reason to use the raw version of the accessors?

Please correct me if I am wrong here, but my understanding is that the raw 
variants are for native-endian access, while the non-raw ones could potentially 
force little-endian conversions for PCI.

If so, since these on-chip registers must always be accessed native-endian, I 
felt that the raw accessors would be appropriate.

Any thoughts?

Regards
Cyril.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] TI: Davinci: NAND Driver Cleanup

2010-03-16 Thread Scott Wood 
Chemparathy, Cyril wrote:
 Please correct me if I am wrong here, but my understanding is that
 the raw variants are for native-endian access, while the non-raw ones
 could potentially force little-endian conversions for PCI.

The non-raw ones also provide ordering on some architectures, though I 
don't think this matters on ARM.  Unfortunately there doesn't seem to be 
an arch-neutral way to unbundle these.

 If so, since these on-chip registers must always be accessed native-endian, I 
 felt that the raw accessors would be appropriate.

OK.

-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] TI: Davinci: NAND Driver Cleanup

2010-03-15 Thread Scott Wood 
On Sun, Mar 14, 2010 at 05:14:31PM -0400, s-paul...@ti.com wrote:
 - emif_regs-AB1CR = acfg1; /* CS2 */
 + __raw_writel(acfg1, emif_regs-AB1CR); /* CS2 */

Configuring for davinci_schmoogie board...
davinci_nand.c: In function ‘nand_flash_init’:
davinci_nand.c:586: error: ‘struct davinci_emif_regs’ has no member named 
‘AB1CR’
make[1]: *** [davinci_nand.o] Error 1
make: *** [drivers/mtd/nand/libnand.a] Error 2

Should be lowercase?

Also, any particular reason to use the raw version of the accessors?

-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] TI: Davinci: NAND Driver Cleanup

2010-03-14 Thread s-paulraj 
From: Cyril Chemparathy cy...@ti.com

Modified to use IO accessor routines consistently.  Eliminated volatile usage
to keep checkpatch.pl happy.
Patch was tested on DM355, DM365 and DM6446 EVMs

Signed-off-by: Cyril Chemparathy cy...@ti.com
Tested-by: Sandeep Paulraj s-paul...@ti.com
---
 drivers/mtd/nand/davinci_nand.c  |  126 --
 include/asm-arm/arch-davinci/emif_defs.h |   80 +--
 2 files changed, 104 insertions(+), 102 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index bfc2acf..61cba14 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -57,7 +57,8 @@
 #define ECC_STATE_ERR_CORR_COMP_P  0x2
 #define ECC_STATE_ERR_CORR_COMP_N  0x3
 
-static emif_registers *const emif_regs = (void *) 
DAVINCI_ASYNC_EMIF_CNTRL_BASE;
+static struct davinci_emif_regs *emif_regs =
+   (struct  davinci_emif_regs *) DAVINCI_ASYNC_EMIF_CNTRL_BASE;
 
 /*
  * Exploit the little endianness of the ARM to do multi-byte transfers
@@ -93,7 +94,7 @@ static void nand_davinci_read_buf(struct mtd_info *mtd, 
uint8_t *buf, int len)
 
/* copy aligned data */
while (len = 4) {
-   *(u32 *)buf = readl(nand);
+   *(u32 *)buf = __raw_readl(nand);
buf += 4;
len -= 4;
}
@@ -138,7 +139,7 @@ static void nand_davinci_write_buf(struct mtd_info *mtd, 
const uint8_t *buf,
 
/* copy aligned data */
while (len = 4) {
-   writel(*(u32 *)buf, nand);
+   __raw_writel(*(u32 *)buf, nand);
buf += 4;
len -= 4;
}
@@ -156,7 +157,8 @@ static void nand_davinci_write_buf(struct mtd_info *mtd, 
const uint8_t *buf,
}
 }
 
-static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int 
ctrl)
+static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd,
+   unsigned int ctrl)
 {
struct  nand_chip *this = mtd-priv;
u_int32_t   IO_ADDR_W = (u_int32_t)this-IO_ADDR_W;
@@ -164,9 +166,9 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, 
int cmd, unsigned int c
if (ctrl  NAND_CTRL_CHANGE) {
IO_ADDR_W = ~(MASK_ALE|MASK_CLE);
 
-   if ( ctrl  NAND_CLE )
+   if (ctrl  NAND_CLE)
IO_ADDR_W |= MASK_CLE;
-   if ( ctrl  NAND_ALE )
+   if (ctrl  NAND_ALE)
IO_ADDR_W |= MASK_ALE;
this-IO_ADDR_W = (void __iomem *) IO_ADDR_W;
}
@@ -181,24 +183,25 @@ static void nand_davinci_enable_hwecc(struct mtd_info 
*mtd, int mode)
 {
u_int32_t   val;
 
-   (void)readl((emif_regs-NANDFECC[CONFIG_SYS_NAND_CS - 2]));
+   (void)__raw_readl((emif_regs-nandfecc[CONFIG_SYS_NAND_CS - 2]));
 
-   val = readl(emif_regs-NANDFCR);
+   val = __raw_readl(emif_regs-nandfcr);
val |= DAVINCI_NANDFCR_NAND_ENABLE(CONFIG_SYS_NAND_CS);
val |= DAVINCI_NANDFCR_1BIT_ECC_START(CONFIG_SYS_NAND_CS);
-   writel(val, emif_regs-NANDFCR);
+   __raw_writel(val, emif_regs-nandfcr);
 }
 
 static u_int32_t nand_davinci_readecc(struct mtd_info *mtd, u_int32_t region)
 {
u_int32_t   ecc = 0;
 
-   ecc = readl((emif_regs-NANDFECC[region - 1]));
+   ecc = __raw_readl((emif_regs-nandfecc[region - 1]));
 
-   return(ecc);
+   return ecc;
 }
 
-static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat, 
u_char *ecc_code)
+static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
+   u_char *ecc_code)
 {
u_int32_t   tmp;
const int region = 1;
@@ -232,7 +235,8 @@ static int nand_davinci_calculate_ecc(struct mtd_info *mtd, 
const u_char *dat, u
return 0;
 }
 
-static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat, u_char 
*read_ecc, u_char *calc_ecc)
+static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat,
+   u_char *read_ecc, u_char *calc_ecc)
 {
struct nand_chip *this = mtd-priv;
u_int32_t ecc_nand = read_ecc[0] | (read_ecc[1]  8) |
@@ -268,7 +272,7 @@ static int nand_davinci_correct_data(struct mtd_info *mtd, 
u_char *dat, u_char *
return -1;
}
}
-   return(0);
+   return 0;
 }
 #endif /* CONFIG_SYS_NAND_HW_ECC */
 
@@ -315,15 +319,15 @@ static void nand_davinci_4bit_enable_hwecc(struct 
mtd_info *mtd, int mode)
 * Start a new ECC calculation for reading or writing 512 bytes
 * of data.
 */
-   val = readl(emif_regs-NANDFCR);
+   val = __raw_readl(emif_regs-nandfcr);
val = ~DAVINCI_NANDFCR_4BIT_ECC_SEL_MASK;
val |= DAVINCI_NANDFCR_NAND_ENABLE(CONFIG_SYS_NAND_CS);
val |= DAVINCI_NANDFCR_4BIT_ECC_SEL(CONFIG_SYS_NAND_CS);