On Fri, May 18, 2007 at 03:54:14PM +0800, Eric Chen wrote:
> Log:
> This nand chip-select patch for HXD8, support up to 4G nand flash.
Index: linux-2.6.21/include/asm-arm/arch-s3c2440/hxd8.h
===================================================================
--- linux-2.6.21.orig/include/asm-arm/arch-s3c2440/hxd8.h 2007-05-18
13:32:17.000000000 +0800
+++ linux-2.6.21/include/asm-arm/arch-s3c2440/hxd8.h 2007-05-18
13:41:40.000000000 +0800
@@ -12,5 +12,5 @@
#define HXD8_GPIO_PCF50606 S3C2410_GPF6
#define HXD8_IRQ_PCF50606 IRQ_EINT6
-
+#define HXD8_NAND_CHIP_SELECT 1
#endif
Index: linux-2.6.21/arch/arm/mach-s3c2440/mach-hxd8.c
===================================================================
--- linux-2.6.21.orig/arch/arm/mach-s3c2440/mach-hxd8.c 2007-05-18
13:32:17.000000000 +0800
+++ linux-2.6.21/arch/arm/mach-s3c2440/mach-hxd8.c 2007-05-18
13:49:44.000000000 +0800
@@ -74,6 +74,12 @@
#include <asm/plat-s3c24xx/devs.h>
#include <asm/plat-s3c24xx/cpu.h>
#include <asm/plat-s3c24xx/pm.h>
+#include "asm-arm/delay.h"
You should use <asm/delay.h> to include this file, but I belive
the correct thing to do is to #include <linux/delay.h> which
includes <asm/delay.h> and ensures all sleep functions are defined
+#if HXD8_NAND_CHIP_SELECT
+void hxd8_select_chip(struct s3c2410_nand_set *, int chip);
this really should be declared static, you don't want to be exporting
these.
+void hxd8_nand_chip_select_init();
the above point, plus the decleration is not complete without a
void in the arguments.
+#endif
static struct map_desc hxd8_iodesc[] __initdata = {
/* ISA IO Space map (memory space selected by A24) */
@@ -133,14 +139,17 @@
[0] = {
.name = "hxd8-nand",
.nr_chips = 1,
+ .working_index = 0,
},
[1] = {
.name = "hxd8-nand-1",
.nr_chips = 1,
+ .working_index = 1,
},
[2] = {
.name = "hxd8-nand-2",
.nr_chips = 1,
+ .working_index = 2,
},
};
@@ -154,6 +163,9 @@
.twrph1 = 20,
.nr_sets = ARRAY_SIZE(hxd8_nand_sets),
.sets = hxd8_nand_sets,
+#if HXD8_NAND_CHIP_SELECT
+ .select_chip = hxd8_select_chip,
+#endif
};
If you'd declared the code above, you could have done
#if HXD8_NAND_CHIP_SELECT
static void hxd8_select_chip(..args..)
{
code here;
}
static void hxd8_nand_chip_select_init(void)
{
code here;
}
#else
#define hxd8_select_chip NULL
#define hxd8_nand_chip_select_init() do { } while(0)
#endif
and then you'd not need the #if around .select_chip
/* PMU configuration */
@@ -386,8 +398,78 @@
platform_device_register(&hxd8_pmu_dev);
s3c2410_pm_init();
+#if HXD8_NAND_CHIP_SELECT
+ hxd8_nand_chip_select_init();
+#endif
+}
+#if HXD8_NAND_CHIP_SELECT
+void hxd8_nand_chip_select_init()
+{
+ /* Nand Flash 3G glue logic initialization
+ * nCE1 ~ nCE4 as output mode
+ */
+ s3c2410_gpio_cfgpin(S3C2410_GPA14, S3C2410_GPA14_OUT);
+ s3c2410_gpio_cfgpin(S3C2410_GPA15, S3C2410_GPA15_OUT);
+ s3c2410_gpio_cfgpin(S3C2410_GPA16, S3C2410_GPA16_OUT);
+ s3c2410_gpio_setpin(S3C2410_GPA14, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 1);
+ s3c2410_gpio_cfgpin(S3C2410_GPG1, S3C2410_GPG1_OUTP);
+ s3c2410_gpio_setpin(S3C2410_GPG1, 0);
+ /* RnB 2~4 as input mode */
+ s3c2410_gpio_cfgpin(S3C2410_GPC5, S3C2410_GPG5_INP);
+ s3c2410_gpio_cfgpin(S3C2410_GPC6, S3C2410_GPG6_INP);
+ s3c2410_gpio_cfgpin(S3C2410_GPC7, S3C2410_GPG7_INP);
}
If you'd put this above, you'd not have needed the
prototype in the first place. Also see notes about
removing #if statements.
+void hxd8_select_chip(struct s3c2410_nand_set * nset,
+ int chip)
+{
+
+ if (chip == -1){
space between the ) and { is usual.
+ s3c2410_gpio_setpin(S3C2410_GPG1, 0);//note here,
please put informative comments in here, this should either
be fleshed out, or removed entirely.
+ s3c2410_gpio_setpin(S3C2410_GPA14, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 1);
+ udelay(3);
+ return;
+ }
+
+ switch (nset->working_index){
+ case 0:
+ s3c2410_gpio_setpin(S3C2410_GPG1, 0);//note here,
+ s3c2410_gpio_setpin(S3C2410_GPA14, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 1);
+ udelay(3);
+ break;
+ case 1:
+ s3c2410_gpio_setpin(S3C2410_GPG1, 1);//note here,
+ s3c2410_gpio_setpin(S3C2410_GPA14, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 0);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 1);
+ udelay(3);
+ break;
+ case 2:
+ s3c2410_gpio_setpin(S3C2410_GPG1, 1);//note here,
+ s3c2410_gpio_setpin(S3C2410_GPA14, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 0);
+ udelay(3);
+ break;
+ case 3:
+ s3c2410_gpio_setpin(S3C2410_GPG1, 1);//note here,
+ s3c2410_gpio_setpin(S3C2410_GPA14, 0);
+ s3c2410_gpio_setpin(S3C2410_GPA15, 1);
+ s3c2410_gpio_setpin(S3C2410_GPA16, 1);
+ udelay(3);
+ break;
another way of doing this which would have produced less code
is the following:
int sel = (chip == -1) ? -1 : nset->working_index;
BUG_ON(sel > 3 || sel < -1);
s3c2410_gpio_setpin(S3C2410_GPG1, (sel == 0) ? 0 : 1);
s3c2410_gpio_setpin(S3C2410_GPA15, (sel == 1) ? 0 : 1);
s3c2410_gpio_setpin(S3C2410_GPA16, (sel == 2) ? 0 : 1);
s3c2410_gpio_setpin(S3C2410_GPA14, (sel == 3) ? 0 : 1);
+ default:
+ printk("nand select error!");
BUG(); is also useful instead of a printk.
Note, the printk should have been printk(KERN_ERR "nand select error!");
+ }
+
+}
+#endif
MACHINE_START(HXD8, "HXD8")
/* Maintainer: Harald Welte <[EMAIL PROTECTED]> */
.phys_io = S3C2410_PA_UART,
Index: linux-2.6.21/include/asm-arm/arch-s3c2410/nand.h
===================================================================
--- linux-2.6.21.orig/include/asm-arm/arch-s3c2410/nand.h 2007-04-26
11:08:32.000000000 +0800
+++ linux-2.6.21/include/asm-arm/arch-s3c2410/nand.h 2007-05-18
13:55:02.000000000 +0800
@@ -20,13 +20,16 @@
* nr_map = map for low-layer logical to physical chip numbers (option)
* partitions = mtd partition list
*/
-
+#include <asm-arm/arch-s3c2440/hxd8.h>
struct s3c2410_nand_set {
int nr_chips;
int nr_partitions;
char *name;
int *nr_map;
struct mtd_partition *partitions;
+#if HXD8_NAND_CHIP_SELECT
+ int working_index;
+#endif
};
configuration specific bits in platform code is never very good.
I belive working index could have removed by using the nr_chips
field.
struct s3c2410_platform_nand {
Index: linux-2.6.21/drivers/mtd/nand/s3c2410.c
===================================================================
--- linux-2.6.21.orig/drivers/mtd/nand/s3c2410.c 2007-05-18
13:32:17.000000000 +0800
+++ linux-2.6.21/drivers/mtd/nand/s3c2410.c 2007-05-18 14:57:16.000000000
+0800
@@ -63,6 +63,7 @@
#include <asm/arch/regs-nand.h>
#include <asm/arch/nand.h>
+#include <asm-arm/arch-s3c2440/hxd8.h>
#ifdef CONFIG_MTD_NAND_S3C2410_HWECC
static int hardware_ecc = 1;
#else
@@ -254,6 +255,13 @@
if (chip == -1) {
cur |= info->sel_bit;
+#if HXD8_NAND_CHIP_SELECT
+ /* GPIO return to default when unselect chip*/
+ if (info->platform != NULL) {
+ if (info->platform->select_chip != NULL)
+ (info->platform->select_chip) (nmtd->set, chip);
+ }
+#endif
} else {
if (nmtd->set != NULL && chip > nmtd->set->nr_chips) {
dev_err(info->device, "invalid chip %d\n", chip);
@@ -323,9 +331,26 @@
static int s3c2440_nand_devready(struct mtd_info *mtd)
{
struct s3c2410_nand_info *info = s3c2410_nand_mtd_toinfo(mtd);
+ struct s3c2410_nand_mtd *nmtd = s3c2410_nand_mtd_toours(mtd);
+
+#if HXD8_NAND_CHIP_SELECT
+ switch(nmtd->set->working_index){
+ case 0:
+ return readb(info->regs + S3C2440_NFSTAT) &
S3C2440_NFSTAT_READY;
+ case 1:
+ return (s3c2410_gpio_getpin(S3C2410_GPC6)>>6) &
S3C2440_NFSTAT_READY;
+ case 2:
+ return (s3c2410_gpio_getpin(S3C2410_GPC7)>>7) &
S3C2440_NFSTAT_READY;
+ case 3:
+ return (s3c2410_gpio_getpin(S3C2410_GPC5)>>5) &
S3C2440_NFSTAT_READY;
+ default:
+ return readb(info->regs + S3C2440_NFSTAT) &
S3C2440_NFSTAT_READY;
+ }
+#endif
Firstly, you didn't need S3C2440_NFSTAT_READY on this, it is either
zero for busy, or non-zero for ready, thus also making the shifts
redundant.
Secondly, why did the hardware engineers make 3 copies of what is
an open-collector signal, in a system where the nand controller
can only access one chip at a time? RnB should have been common
to all three chips.
I think we need to add some form of override for the
devready function passed through with the platform
data.
return readb(info->regs + S3C2440_NFSTAT) & S3C2440_NFSTAT_READY;
}
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
