uboot-hxd8-nand-chip-select.patch

Hi Eric!

Some remarks:

1) the GPIO initialization should not be neccessary.  It's
much better
   to fix the GPIO initialization in hxd8.c

2) selecting the various NAND chips can be done via the
existing
   board_nand_select_device() callback mechanism

3) The actual GPIO chip select function can be implemented a
bit
   shorter when not using a case statement

4) The changes to drivers/nand/nand.c should not be
neccessary if '3' is
   done correctly

Here's my [untested] counter-proposal.  Please note that
this is a patch
against the patch.   I would like to ask you to test this
patch, and fix
in case you run into problems.


Index: uboot-hxd8.patch
============================================================
=======
--- uboot-hxd8.patch	(revision 2074)
+++ uboot-hxd8.patch	(working copy)
 -88,7
+88,7 
 ===========================================================
========
 --- /dev/null
 +++ u-boot/board/hxd8/hxd8.c
- -0,0 +1,172 
+ -0,0 +1,204 
 +/*
 + * (C) Copyright 2007 by OpenMoko, Inc.
 + * Author: Harald Welte <laforgeopenmoko.org>
 -120,6
+120,8 
 + */
 +
 +#include <common.h>
++#include <nand.h>
++#include <pcf50606.h>
 +#include <s3c2440.h>
 +
 +DECLARE_GLOBAL_DATA_PTR;
 -190,7
+192,7 
 +	gpio->GPADAT = 0x0001C000;
 +	gpio->GPBCON = 0x00045542;
 +	gpio->GPBUP = 0x000007FF;
-+	gpio->GPCCON = 0xAAAA55A9;
++	gpio->GPCCON = 0xAAAA02A9;
 +	gpio->GPCUP = 0x0000FFFF;
 +	gpio->GPDCON = 0xAAAAAAAA;
 +	gpio->GPDUP = 0x0000FFFF;
 -260,7
+262,37 
 +char *dynpart_names[] = {
 +    "u-boot", "u-boot_env",
"kernel", "splash", "rootfs",
NULL };
 +
++/* 4G Nand flash chip select function */
++void board_nand_select_device(struct nand_chip *this, int
chip)
++{
++	S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
 +
++	if (chip == 0)
++		gpio->GPGDAT &=  ~(1 << 1);
++	else
++		gpio->GPGDAT |=  (1 << 1);
++
++	if (chip == 1)
++		gpio->GPADAT &=  ~(1 << 15);
++	else
++		gpio->GPADAT |= (1 << 15);
++
++	if (chip == 2)
++		gpio->GPADAT &=  ~(1 << 16);
++	else
++		gpio->GPADAT |=  (1 << 16);
++
++	if (chip == 3)
++		gpio->GPADAT &=  ~(1 << 14);
++	else
++		gpio->GPADAT |= (1 << 14);
++
++	/* UGLY: ew don't have mtd_info pointer, but know that
++	 * s3c24xx hwcontrol function does not use it for CLRNCE
*/
++	if (chip == -1)
++		this->hwcontrol(NULL, NAND_CTL_CLRNCE);
++}
++
 Index: u-boot/board/hxd8/lowlevel_init.S
 ===========================================================
========
 --- /dev/null
 -441,7
+473,7 
 ===========================================================
========
 --- /dev/null
 +++ u-boot/include/configs/hxd8.h
- -0,0 +1,275 
+ -0,0 +1,278 
 +/*
 + * (C) Copyright 2007 OpenMoko, Inc.
 + * Author: Harald Welte <laforgeopenmoko.org>
 -484,6
+516,7 
 +#define CONFIG_ARM920T		1	/* This is an ARM920T Core	*/
 +#define	CONFIG_S3C2440		1	/* in a SAMSUNG S3C2440 SoC    
*/
 +#define CONFIG_SMDK2440		1	/* on a SAMSUNG SMDK2440 Board 
*/
++#define CONFIG_HXD8		1	/* on a FIC HXD8 Board  */
 +
 +/* input clock of PLL */
 +#define CONFIG_SYS_CLK_FREQ	16934400/* the HXD8 has this
input clock */
 -649,9
+682,11 
 +#define CFG_ENV_OFFSET_OOB   	1               /* Location
of ENV stored in block 0 OOB */
 +#define	CFG_PREBOOT_OVERRIDE	1	/* allow preboot from
memory */
 +
-+#define NAND_MAX_CHIPS		1
++#define NAND_MAX_CHIPS		3
 +#define CFG_NAND_BASE		0x4e000000
-+#define CFG_MAX_NAND_DEVICE	1
++#define CFG_MAX_NAND_DEVICE	NAND_MAX_CHIPS
++#define CFG_NAND_BASE_LIST	{ CFG_NAND_BASE, CFG_NAND_BASE,
CFG_NAND_BASE }
++#define CFG_NAND_SELECT_DEVICE
 +
 +#define CONFIG_MMC		1
 +#define CFG_MMC_BASE		0xff000000
 -1072,3
+1107,70 
 +	.bss : { *(.bss) }
 +	_end = .;
 +}
+Index: u-boot/cpu/arm920t/s3c24x0/nand.c
+===========================================================
========
+--- u-boot.orig/cpu/arm920t/s3c24x0/nand.c
++++ u-boot/cpu/arm920t/s3c24x0/nand.c
+ -83,6 +83,26 
+ #define	NFDATA		__REGb(NF_BASE + oNFDATA)
+ #define	NFSTAT		__REGb(NF_BASE + oNFSTAT)
+ 
++#if defined(CONFIG_HXD8)
++static int hxd8_dev_ready(struct mtd_info *mtd)
++{
++	S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
++	u_int32_t val = gpio->GPCDAT;
++
++	switch (nand_curr_device) {
++		case 0:
++			return (NFSTAT & 0x01);
++		case 1:	/* RnB 3 */
++			return ((val>>6) & 0x01);
++		case 2:	/* RnB 4 */
++			return ((val>>7) & 0x01);
++		case 3:	/* RnB 2 */
++			return  ((val>>5) & 0x01);
++		default:
++			return 0;
++	}
++}
++#endif
+ 
+ static void s3c2410_hwcontrol(struct mtd_info *mtd, int
cmd)
+ {
+ -251,6 +271,12 
+ 	nand->eccmode = NAND_ECC_SOFT;
+ #endif
+ 
++#if defined(CONFIG_HXD8)
++	//printk("Initial new csn");
++	//nand->select_chip = hxd8_nand_select_chip;
++	nand->dev_ready = hxd8_dev_ready;
++#endif
++
+ #ifdef CONFIG_S3C2410_NAND_BBT
+ 	nand->options = NAND_USE_FLASH_BBT |
NAND_DONT_CREATE_BBT;
+ #else
+Index: u-boot/drivers/nand/nand.c
+===========================================================
========
+--- u-boot.orig/drivers/nand/nand.c
++++ u-boot/drivers/nand/nand.c
+ -64,14 +64,17 
+ {
+ 	int i;
+ 	unsigned int size = 0;
++
+ 	for (i = 0; i < CFG_MAX_NAND_DEVICE; i++) {
++		//nand_curr_device = i;
+ 		nand_init_chip(&nand_info[i], &nand_chip[i],
base_address[i]);
+ 		size += nand_info[i].size;
+ 		if (nand_curr_device == -1)
+ 			nand_curr_device = i;
+ 	}
+ 	printf("%lu MiBn", size / (1024 * 1024));
+-
++	// Always select the 1st nand device
++	nand_curr_device = 0;
+ #ifdef CFG_NAND_SELECT_DEVICE
+ 	/*
+ 	 * Select the chip in the board/cpu specific driver

-- 
- Harald Welte <laforgeopenmoko.org>          	
       http://openmoko.org/
============================================================
================
Software for the world's first truly open Free Software
mobile phone

Hi, Harald,

I added minimum changes to make it work.
(1) select_chip is mandatory.
(2) nand_curr_device should be selected before nand_init.

Regards,
Eric

----- Original Message ----- 
From: "Harald Welte" <laforgeopenmoko.org>
To: "Eric Chen" <eric_chenfiwin.com.tw>
Cc: <openmoko-ubootlists.openmoko.org>
Sent: Friday, May 25, 2007 6:51 AM
Subject: Re: [PATCH] uboot-hxd8-nand-chip-select.patch


> Hi Eric!
>
> Some remarks:
>
> 1) the GPIO initialization should not be neccessary. 
It's much better
>   to fix the GPIO initialization in hxd8.c
>
> 2) selecting the various NAND chips can be done via the
existing
>   board_nand_select_device() callback mechanism
>
> 3) The actual GPIO chip select function can be
implemented a bit
>   shorter when not using a case statement
>
> 4) The changes to drivers/nand/nand.c should not be
neccessary if '3' is
>   done correctly
>
> Here's my [untested] counter-proposal.  Please note
that this is a patch
> against the patch.   I would like to ask you to test
this patch, and fix
> in case you run into problems.
>
>
> Index: uboot-hxd8.patch
>
============================================================
=======
> --- uboot-hxd8.patch (revision 2074)
> +++ uboot-hxd8.patch (working copy)
>  -88,7 +88,7 
>
============================================================
=======
> --- /dev/null
> +++ u-boot/board/hxd8/hxd8.c
> - -0,0 +1,172 
> + -0,0 +1,204 
> +/*
> + * (C) Copyright 2007 by OpenMoko, Inc.
> + * Author: Harald Welte <laforgeopenmoko.org>
>  -120,6 +120,8 
> + */
> +
> +#include <common.h>
> ++#include <nand.h>
> ++#include <pcf50606.h>
> +#include <s3c2440.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>  -190,7 +192,7 
> + gpio->GPADAT = 0x0001C000;
> + gpio->GPBCON = 0x00045542;
> + gpio->GPBUP = 0x000007FF;
> -+ gpio->GPCCON = 0xAAAA55A9;
> ++ gpio->GPCCON = 0xAAAA02A9;
> + gpio->GPCUP = 0x0000FFFF;
> + gpio->GPDCON = 0xAAAAAAAA;
> + gpio->GPDUP = 0x0000FFFF;
>  -260,7 +262,37 
> +char *dynpart_names[] = {
> +    "u-boot", "u-boot_env",
"kernel", "splash", "rootfs",
NULL };
> +
> ++/* 4G Nand flash chip select function */
> ++void board_nand_select_device(struct nand_chip *this,
int chip)
> ++{
> ++ S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
> +
> ++ if (chip == 0)
> ++ gpio->GPGDAT &=  ~(1 << 1);
> ++ else
> ++ gpio->GPGDAT |=  (1 << 1);
> ++
> ++ if (chip == 1)
> ++ gpio->GPADAT &=  ~(1 << 15);
> ++ else
> ++ gpio->GPADAT |= (1 << 15);
> ++
> ++ if (chip == 2)
> ++ gpio->GPADAT &=  ~(1 << 16);
> ++ else
> ++ gpio->GPADAT |=  (1 << 16);
> ++
> ++ if (chip == 3)
> ++ gpio->GPADAT &=  ~(1 << 14);
> ++ else
> ++ gpio->GPADAT |= (1 << 14);
> ++
> ++ /* UGLY: ew don't have mtd_info pointer, but know
that
> ++ * s3c24xx hwcontrol function does not use it for
CLRNCE */
> ++ if (chip == -1)
> ++ this->hwcontrol(NULL, NAND_CTL_CLRNCE);
> ++}
> ++
> Index: u-boot/board/hxd8/lowlevel_init.S
>
============================================================
=======
> --- /dev/null
>  -441,7 +473,7 
>
============================================================
=======
> --- /dev/null
> +++ u-boot/include/configs/hxd8.h
> - -0,0 +1,275 
> + -0,0 +1,278 
> +/*
> + * (C) Copyright 2007 OpenMoko, Inc.
> + * Author: Harald Welte <laforgeopenmoko.org>
>  -484,6 +516,7 
> +#define CONFIG_ARM920T 1 /* This is an ARM920T Core
*/
> +#define CONFIG_S3C2440 1 /* in a SAMSUNG S3C2440 SoC  
  */
> +#define CONFIG_SMDK2440 1 /* on a SAMSUNG SMDK2440
Board  */
> ++#define CONFIG_HXD8 1 /* on a FIC HXD8 Board  */
> +
> +/* input clock of PLL */
> +#define CONFIG_SYS_CLK_FREQ 16934400/* the HXD8 has
this input clock */
>  -649,9 +682,11 
> +#define CFG_ENV_OFFSET_OOB   1               /*
Location of ENV stored in 
> block 0 OOB */
> +#define CFG_PREBOOT_OVERRIDE 1 /* allow preboot from
memory */
> +
> -+#define NAND_MAX_CHIPS 1
> ++#define NAND_MAX_CHIPS 3
> +#define CFG_NAND_BASE 0x4e000000
> -+#define CFG_MAX_NAND_DEVICE 1
> ++#define CFG_MAX_NAND_DEVICE NAND_MAX_CHIPS
> ++#define CFG_NAND_BASE_LIST { CFG_NAND_BASE,
CFG_NAND_BASE, 
> CFG_NAND_BASE }
> ++#define CFG_NAND_SELECT_DEVICE
> +
> +#define CONFIG_MMC 1
> +#define CFG_MMC_BASE 0xff000000
>  -1072,3 +1107,70 
> + .bss : { *(.bss) }
> + _end = .;
> +}
> +Index: u-boot/cpu/arm920t/s3c24x0/nand.c
>
+===========================================================
========
> +--- u-boot.orig/cpu/arm920t/s3c24x0/nand.c
> ++++ u-boot/cpu/arm920t/s3c24x0/nand.c
> + -83,6 +83,26 
> + #define NFDATA __REGb(NF_BASE + oNFDATA)
> + #define NFSTAT __REGb(NF_BASE + oNFSTAT)
> +
> ++#if defined(CONFIG_HXD8)
> ++static int hxd8_dev_ready(struct mtd_info *mtd)
> ++{
> ++ S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
> ++ u_int32_t val = gpio->GPCDAT;
> ++
> ++ switch (nand_curr_device) {
> ++ case 0:
> ++ return (NFSTAT & 0x01);
> ++ case 1: /* RnB 3 */
> ++ return ((val>>6) & 0x01);
> ++ case 2: /* RnB 4 */
> ++ return ((val>>7) & 0x01);
> ++ case 3: /* RnB 2 */
> ++ return  ((val>>5) & 0x01);
> ++ default:
> ++ return 0;
> ++ }
> ++}
> ++#endif
> +
> + static void s3c2410_hwcontrol(struct mtd_info *mtd,
int cmd)
> + {
> + -251,6 +271,12 
> + nand->eccmode = NAND_ECC_SOFT;
> + #endif
> +
> ++#if defined(CONFIG_HXD8)
> ++ //printk("Initial new csn");
> ++ //nand->select_chip = hxd8_nand_select_chip;
> ++ nand->dev_ready = hxd8_dev_ready;
> ++#endif
> ++
> + #ifdef CONFIG_S3C2410_NAND_BBT
> + nand->options = NAND_USE_FLASH_BBT |
NAND_DONT_CREATE_BBT;
> + #else
> +Index: u-boot/drivers/nand/nand.c
>
+===========================================================
========
> +--- u-boot.orig/drivers/nand/nand.c
> ++++ u-boot/drivers/nand/nand.c
> + -64,14 +64,17 
> + {
> + int i;
> + unsigned int size = 0;
> ++
> + for (i = 0; i < CFG_MAX_NAND_DEVICE; i++) {
> ++ //nand_curr_device = i;
> + nand_init_chip(&nand_info[i], &nand_chip[i],
base_address[i]);
> + size += nand_info[i].size;
> + if (nand_curr_device == -1)
> + nand_curr_device = i;
> + }
> + printf("%lu MiBn", size / (1024 * 1024));
> +-
> ++ // Always select the 1st nand device
> ++ nand_curr_device = 0;
> + #ifdef CFG_NAND_SELECT_DEVICE
> + /*
> + * Select the chip in the board/cpu specific driver
>
> -- 
> - Harald Welte <laforgeopenmoko.org> 
> http://openmoko.org/
>
============================================================
================
> Software for the world's first truly open Free Software
mobile phone 

On Tue, May 29, 2007 at 10:49:35AM +0800, Eric Chen wrote:
> Hi, Harald,
> 
> I added minimum changes to make it work.
> (1) select_chip is mandatory.

I disagree.  This basically circumvents the standard method
of selecting
different MTD devices (CFG_NAND_SELECT_DEVICE and the
board_nand_select_device() callback).  If this is not
working, then we
need to debug why it is not working, rather than just
abandoning the
standard method and doing it our own way.

> (2) nand_curr_device should be selected before
nand_init.

This once again should not be required if we manage to get
the
select_device method working.

Please investigate,
-- 
- Harald Welte <laforgeopenmoko.org>          	
       http://openmoko.org/
============================================================
================
Software for the world's first truly open Free Software
mobile phone

Hi, Harald,

You're right, I updated the patch again.
Regards,
Eric
----- Original Message ----- 
From: "Harald Welte" <laforgeopenmoko.org>
To: "Eric Chen" <eric_chenfiwin.com.tw>
Cc: <openmoko-ubootlists.openmoko.org>
Sent: Thursday, May 31, 2007 5:49 PM
Subject: Re: [PATCH] uboot-hxd8-nand-chip-select.patch


> On Tue, May 29, 2007 at 10:49:35AM +0800, Eric Chen
wrote:
>> Hi, Harald,
>>
>> I added minimum changes to make it work.
>> (1) select_chip is mandatory.
>
> I disagree.  This basically circumvents the standard
method of selecting
> different MTD devices (CFG_NAND_SELECT_DEVICE and the
> board_nand_select_device() callback).  If this is not
working, then we
> need to debug why it is not working, rather than just
abandoning the
> standard method and doing it our own way.
>
>> (2) nand_curr_device should be selected before
nand_init.
>
> This once again should not be required if we manage to
get the
> select_device method working.
>
> Please investigate,
> -- 
> - Harald Welte <laforgeopenmoko.org> 
> http://openmoko.org/
>
============================================================
================
> Software for the world's first truly open Free Software
mobile phone