S3C24XX: Add support to MINI2416 board

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- Coding style cleanup
- Use of clrsetbits_le32()
- Use of register bit macros instead of magic numbers

Changes for v3:
- Removed magic numbers
- Removed a macro to declare a void printf()
- Replaced one printf() with a puts()

---
 drivers/mtd/nand/Makefile       |    1 +
 drivers/mtd/nand/s3c24xx_nand.c |  246  
+++++++++++++++++++++++++++++++++++++++
 2 files changed, 247 insertions(+)
 create mode 100644 drivers/mtd/nand/s3c24xx_nand.c

drivers/mtd/nand/Makefile +1 -0

index 29dc20e..791ec44 100644
@@ -60,6 +60,7 @@ COBJS-$(CONFIG_NAND_MXS) += mxs_nand.o
 COBJS-$(CONFIG_NAND_NDFC) += ndfc.o
 COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
+COBJS-$(CONFIG_NAND_S3C24XX) += s3c24xx_nand.o
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o

drivers/mtd/nand/s3c24xx_nand.c new file +34 -0

new file mode 100644
index 0000000..3c13709
@@ -0,0 +1,246 @@
+/*
+ * (C) Copyright 2012 INOV - INESC Inovacao
+ * Jose Goncalves <[email protected]>
+ *
+ * Based on drivers/mtd/nand/s3c64xx.c and U-Boot 1.3.4 from Samsung.
+ * Supports only SLC NAND Flash chips.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+#include <asm/arch/s3c24xx_cpu.h>
+
+#define TACLS_VAL	7	/* CLE & ALE duration setting (0~7) */
+#define	TWRPH0_VAL	7	/* TWRPH0 duration setting  

(0~7) */
+#define TWRPH1_VAL 7 /* TWRPH1 duration setting (0~7) */

drivers/mtd/nand/s3c24xx_nand.c new file +5 -0

+
+	nfcont = readl(&nand->nfcont);
+
+	switch (chip) {
+	case -1:

drivers/mtd/nand/s3c24xx_nand.c new file +9 -0

+	}
+
+	writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,  

unsigned int ctrl)

drivers/mtd/nand/s3c24xx_nand.c new file +10 -0

+{
+	struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+	struct nand_chip *this = mtd->priv;
+
+	if (ctrl & NAND_CTRL_CHANGE) {
+		if (ctrl & NAND_CLE)
+			this->IO_ADDR_W = &nand->nfcmmd;
+		else if (ctrl & NAND_ALE)
+			this->IO_ADDR_W = &nand->nfaddr;
+		else

drivers/mtd/nand/s3c24xx_nand.c new file +18 -0

+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+	static int chip_n;
+	struct s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+
+	if (chip_n == 0) {
+		/* Extend NAND timings to the maximum */
+		clrsetbits_le32(&nand_reg->nfconf,
+				NFCONF_TACLS_MASK | NFCONF_TWRPH0_MASK |
+				NFCONF_TWRPH1_MASK,
+				NFCONF_TACLS(TACLS_VAL) |
+				NFCONF_TWRPH0(TWRPH0_VAL) |
+				NFCONF_TWRPH1(TWRPH1_VAL));
+
+		/* Disable chip selects and soft lock, enable  

controller */

drivers/mtd/nand/s3c24xx_nand.c new file +2 -0

+		clrsetbits_le32(&nand_reg->nfcont, NFCONT_WP,
+				NFCONT_NCE1 | NFCONT_NCE0 |  

NFCONT_ENABLE);

drivers/mtd/nand/s3c24xx_nand.c new file +19 -0

+	} else if (chip_n >= MAX_CHIPS) {
+		return -ENODEV;
+	}
+
+	nand->IO_ADDR_R = &nand_reg->nfdata;
+	nand->IO_ADDR_W = &nand_reg->nfdata;
+	nand->cmd_ctrl = s3c_nand_hwcontrol;
+	nand->dev_ready = s3c_nand_device_ready;
+	nand->select_chip = s3c_nand_select_chip;
+	nand->options = 0;
+#ifdef CONFIG_SPL_BUILD
+	nand->read_buf = nand_read_buf;
+#endif
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+	nand->ecc.hwctl = s3c_nand_enable_hwecc;
+	nand->ecc.calculate = s3c_nand_calculate_ecc;
+	nand->ecc.correct = s3c_nand_correct_data;
+	nand->ecc.mode = NAND_ECC_HW_OOB_FIRST;

Quoted 67 lines · Jos? Miguel Gon?alves

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- Coding style cleanup
- Use of clrsetbits_le32()
- Use of register bit macros instead of magic numbers

Changes for v3:
- Removed magic numbers
- Removed a macro to declare a void printf()
- Replaced one printf() with a puts()

---
 drivers/mtd/nand/Makefile       |    1 +
 drivers/mtd/nand/s3c24xx_nand.c |  246 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 247 insertions(+)
 create mode 100644 drivers/mtd/nand/s3c24xx_nand.c

drivers/mtd/nand/Makefile +1 -0

index 29dc20e..791ec44 100644
@@ -60,6 +60,7 @@ COBJS-$(CONFIG_NAND_MXS) += mxs_nand.o
 COBJS-$(CONFIG_NAND_NDFC) += ndfc.o
 COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
+COBJS-$(CONFIG_NAND_S3C24XX) += s3c24xx_nand.o
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o

drivers/mtd/nand/s3c24xx_nand.c new file +29 -0

new file mode 100644
index 0000000..3c13709
@@ -0,0 +1,246 @@
+/*
+ * (C) Copyright 2012 INOV - INESC Inovacao
+ * Jose Goncalves <[email protected]>
+ *
+ * Based on drivers/mtd/nand/s3c64xx.c and U-Boot 1.3.4 from Samsung.
+ * Supports only SLC NAND Flash chips.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>

drivers/mtd/nand/s3c24xx_nand.c line 35

Quoted 6 lines

drivers/mtd/nand/s3c24xx_nand.c new file +6 -0

+#include <asm/errno.h>
+#include <asm/arch/s3c24xx_cpu.h>
+
+#define TACLS_VAL    7    /* CLE & ALE duration setting (0~7) */
+#define    TWRPH0_VAL    7    /* TWRPH0 duration setting (0~7) */
+#define TWRPH1_VAL    7    /* TWRPH1 duration setting (0~7) */

Please use space, not tab, as a word separator (after the second #define).

drivers/mtd/nand/s3c24xx_nand.c line 38

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+
+#define MAX_CHIPS    2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };

This needs explanation (and const). Better would be to use a priv struct, as discussed before.

drivers/mtd/nand/s3c24xx_nand.c line 43

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +4 -0

+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    u_long nfcont;

s/u_long/u32/

Quoted 10 lines

drivers/mtd/nand/s3c24xx_nand.c new file +9 -0

+
+    nfcont = readl(&nand->nfcont);
+
+    switch (chip) {
+    case -1:
+        nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+        break;
+    case 0:
+        nfcont &= ~NFCONT_NCE0;

drivers/mtd/nand/s3c24xx_nand.c line 58

Quoted 6 lines

drivers/mtd/nand/s3c24xx_nand.c new file +6 -0

+        break;
+    case 1:
+        nfcont &= ~NFCONT_NCE1;
+        break;
+    default:
+        return;

error message on default?

Quoted 25 lines

drivers/mtd/nand/s3c24xx_nand.c new file +24 -0

+    }
+
+    writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    struct nand_chip *this = mtd->priv;
+
+    if (ctrl & NAND_CTRL_CHANGE) {
+        if (ctrl & NAND_CLE)
+            this->IO_ADDR_W = &nand->nfcmmd;
+        else if (ctrl & NAND_ALE)
+            this->IO_ADDR_W = &nand->nfaddr;
+        else
+            this->IO_ADDR_W = &nand->nfdata;
+        if (ctrl & NAND_NCE)
+            s3c_nand_select_chip(mtd, *(int *)this->priv);
+        else
+            s3c_nand_select_chip(mtd, -1);

drivers/mtd/nand/s3c24xx_nand.c line 87

Quoted 5 lines

drivers/mtd/nand/s3c24xx_nand.c new file +5 -0

+    }
+
+    if (cmd != NAND_CMD_NONE)
+        writeb(cmd, this->IO_ADDR_W);
+}

As discussed earlier, do you really need to mess with IO_ADDR_W or can you do it the way ndfc.c does?

I.e.:

static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)

{
    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
    struct nand_chip *this = mtd->priv;

    if (cmd == NAND_CMD_NONE)
        return;

    if (ctrl & NAND_CLE)
        writeb(cmd, &nand->nfcmmd);
    else
        writeb(cmd, &nand->nfaddr);
}

Quoted 43 lines

drivers/mtd/nand/s3c24xx_nand.c new file +43 -0

+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+    static int chip_n;
+    struct s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+
+    if (chip_n == 0) {
+        /* Extend NAND timings to the maximum */
+        clrsetbits_le32(&nand_reg->nfconf,
+                NFCONF_TACLS_MASK | NFCONF_TWRPH0_MASK |
+                NFCONF_TWRPH1_MASK,
+                NFCONF_TACLS(TACLS_VAL) |
+                NFCONF_TWRPH0(TWRPH0_VAL) |
+                NFCONF_TWRPH1(TWRPH1_VAL));
+
+        /* Disable chip selects and soft lock, enable controller */
+        clrsetbits_le32(&nand_reg->nfcont, NFCONT_WP,
+                NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE);
+    } else if (chip_n >= MAX_CHIPS) {
+        return -ENODEV;
+    }
+
+    nand->IO_ADDR_R = &nand_reg->nfdata;
+    nand->IO_ADDR_W = &nand_reg->nfdata;
+    nand->cmd_ctrl = s3c_nand_hwcontrol;
+    nand->dev_ready = s3c_nand_device_ready;
+    nand->select_chip = s3c_nand_select_chip;
+    nand->options = 0;
+#ifdef CONFIG_SPL_BUILD
+    nand->read_buf = nand_read_buf;
+#endif
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+    nand->ecc.hwctl = s3c_nand_enable_hwecc;
+    nand->ecc.calculate = s3c_nand_calculate_ecc;
+    nand->ecc.correct = s3c_nand_correct_data;
+    nand->ecc.mode = NAND_ECC_HW_OOB_FIRST;
+    nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE;
+    nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES;
+#else
+    nand->ecc.mode = NAND_ECC_SOFT;

drivers/mtd/nand/s3c24xx_nand.c line 246

Quoted 6 lines

drivers/mtd/nand/s3c24xx_nand.c new file +6 -0

+#endif /* ! CONFIG_S3C24XX_NAND_HWECC */
+
+    nand->priv = nand_cs + chip_n++;
+
+    return 0;
+}

Please consider using the new SELF_INIT mechanism.

Quoted 7 lines · Scott Wood

drivers/mtd/nand/s3c24xx_nand.c line 35

Quoted 3 lines · Jos? Miguel Gon?alves

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+#define TACLS_VAL    7    /* CLE & ALE duration setting (0~7) */
+#define    TWRPH0_VAL    7    /* TWRPH0 duration setting (0~7) */
+#define TWRPH1_VAL    7    /* TWRPH1 duration setting (0~7) */

Please use space, not tab, as a word separator (after the second #define).

OK.

Quoted 8 lines

drivers/mtd/nand/s3c24xx_nand.c line 38

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+
+#define MAX_CHIPS    2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };

This needs explanation (and const). Better would be to use a priv struct, as discussed before.

Quoted 6 lines

drivers/mtd/nand/s3c24xx_nand.c line 43

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +4 -0

+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    u_long nfcont;

s/u_long/u32/

Didn't catch this...

Quoted 15 lines

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +4 -0

+    nfcont = readl(&nand->nfcont);
+
+    switch (chip) {
+    case -1:

drivers/mtd/nand/s3c24xx_nand.c line 58

Quoted 10 lines

drivers/mtd/nand/s3c24xx_nand.c new file +10 -0

+        nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+        break;
+    case 0:
+        nfcont &= ~NFCONT_NCE0;
+        break;
+    case 1:
+        nfcont &= ~NFCONT_NCE1;
+        break;
+    default:
+        return;

Quoted 1 line

error message on default?

OK.

Quoted 26 lines

Quoted 26 lines

drivers/mtd/nand/s3c24xx_nand.c new file +9 -0

+    }
+
+    writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,  

unsigned int ctrl)

drivers/mtd/nand/s3c24xx_nand.c new file +15 -0

+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    struct nand_chip *this = mtd->priv;
+
+    if (ctrl & NAND_CTRL_CHANGE) {
+        if (ctrl & NAND_CLE)
+            this->IO_ADDR_W = &nand->nfcmmd;
+        else if (ctrl & NAND_ALE)
+            this->IO_ADDR_W = &nand->nfaddr;
+        else
+            this->IO_ADDR_W = &nand->nfdata;
+        if (ctrl & NAND_NCE)
+            s3c_nand_select_chip(mtd, *(int *)this->priv);
+        else
+            s3c_nand_select_chip(mtd, -1);

Quoted 8 lines

drivers/mtd/nand/s3c24xx_nand.c line 87

Quoted 5 lines

drivers/mtd/nand/s3c24xx_nand.c new file +5 -0

+    }
+
+    if (cmd != NAND_CMD_NONE)
+        writeb(cmd, this->IO_ADDR_W);
+}

As discussed earlier, do you really need to mess with IO_ADDR_W or can you do it the way ndfc.c does?

I will take a look at ndfc.c.

Quoted 10 lines

I.e.:

static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)

{
    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
    struct nand_chip *this = mtd->priv;

Quoted 8 lines

    if (cmd == NAND_CMD_NONE)
        return;

    if (ctrl & NAND_CLE)
        writeb(cmd, &nand->nfcmmd);
    else
        writeb(cmd, &nand->nfaddr);
}

Quoted 44 lines

Quoted 44 lines

drivers/mtd/nand/s3c24xx_nand.c new file +43 -0

+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+    static int chip_n;
+    struct s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+
+    if (chip_n == 0) {
+        /* Extend NAND timings to the maximum */
+        clrsetbits_le32(&nand_reg->nfconf,
+                NFCONF_TACLS_MASK | NFCONF_TWRPH0_MASK |
+                NFCONF_TWRPH1_MASK,
+                NFCONF_TACLS(TACLS_VAL) |
+                NFCONF_TWRPH0(TWRPH0_VAL) |
+                NFCONF_TWRPH1(TWRPH1_VAL));
+
+        /* Disable chip selects and soft lock, enable controller */
+        clrsetbits_le32(&nand_reg->nfcont, NFCONT_WP,
+                NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE);
+    } else if (chip_n >= MAX_CHIPS) {
+        return -ENODEV;
+    }
+
+    nand->IO_ADDR_R = &nand_reg->nfdata;
+    nand->IO_ADDR_W = &nand_reg->nfdata;
+    nand->cmd_ctrl = s3c_nand_hwcontrol;
+    nand->dev_ready = s3c_nand_device_ready;
+    nand->select_chip = s3c_nand_select_chip;
+    nand->options = 0;
+#ifdef CONFIG_SPL_BUILD
+    nand->read_buf = nand_read_buf;
+#endif
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+    nand->ecc.hwctl = s3c_nand_enable_hwecc;
+    nand->ecc.calculate = s3c_nand_calculate_ecc;
+    nand->ecc.correct = s3c_nand_correct_data;
+    nand->ecc.mode = NAND_ECC_HW_OOB_FIRST;
+    nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE;
+    nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES;
+#else
+    nand->ecc.mode = NAND_ECC_SOFT;

Quoted 9 lines

drivers/mtd/nand/s3c24xx_nand.c line 246

Quoted 6 lines

drivers/mtd/nand/s3c24xx_nand.c new file +6 -0

+#endif /* ! CONFIG_S3C24XX_NAND_HWECC */
+
+    nand->priv = nand_cs + chip_n++;
+
+    return 0;
+}

Please consider using the new SELF_INIT mechanism.

Can you explain and/or point_to_resources for what this means (I'm a U-Boot newbie)...

Quoted 25 lines · Jos? Miguel Gon?alves

Quoted 6 lines · Scott Wood

drivers/mtd/nand/s3c24xx_nand.c line 35

Quoted 3 lines · Jos? Miguel Gon?alves

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+#define TACLS_VAL    7    /* CLE & ALE duration setting (0~7) */
+#define    TWRPH0_VAL    7    /* TWRPH0 duration setting (0~7) */
+#define TWRPH1_VAL    7    /* TWRPH1 duration setting (0~7) */

Please use space, not tab, as a word separator (after the second #define).

OK.

Quoted 14 lines

drivers/mtd/nand/s3c24xx_nand.c line 38

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+
+#define MAX_CHIPS    2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };

This needs explanation (and const). Better would be to use a priv struct, as discussed before.

drivers/mtd/nand/s3c24xx_nand.c line 43

Quoted 4 lines

drivers/mtd/nand/s3c24xx_nand.c new file +4 -0

+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    u_long nfcont;

s/u_long/u32/

Quoted 1 line

Didn't catch this...

Replace u_long with u32.

Quoted 55 lines

Quoted 16 lines

Quoted 5 lines

drivers/mtd/nand/s3c24xx_nand.c new file +4 -0

+    nfcont = readl(&nand->nfcont);
+
+    switch (chip) {
+    case -1:

drivers/mtd/nand/s3c24xx_nand.c line 58

Quoted 10 lines

drivers/mtd/nand/s3c24xx_nand.c new file +10 -0

+        nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+        break;
+    case 0:
+        nfcont &= ~NFCONT_NCE0;
+        break;
+    case 1:
+        nfcont &= ~NFCONT_NCE1;
+        break;
+    default:
+        return;

Quoted 1 line

error message on default?

OK.

Quoted 26 lines

Quoted 26 lines

drivers/mtd/nand/s3c24xx_nand.c new file +9 -0

+    }
+
+    writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int 

ctrl)

drivers/mtd/nand/s3c24xx_nand.c new file +15 -0

+{
+    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+    struct nand_chip *this = mtd->priv;
+
+    if (ctrl & NAND_CTRL_CHANGE) {
+        if (ctrl & NAND_CLE)
+            this->IO_ADDR_W = &nand->nfcmmd;
+        else if (ctrl & NAND_ALE)
+            this->IO_ADDR_W = &nand->nfaddr;
+        else
+            this->IO_ADDR_W = &nand->nfdata;
+        if (ctrl & NAND_NCE)
+            s3c_nand_select_chip(mtd, *(int *)this->priv);
+        else
+            s3c_nand_select_chip(mtd, -1);

Quoted 8 lines

drivers/mtd/nand/s3c24xx_nand.c line 87

Quoted 5 lines

drivers/mtd/nand/s3c24xx_nand.c new file +5 -0

+    }
+
+    if (cmd != NAND_CMD_NONE)
+        writeb(cmd, this->IO_ADDR_W);
+}

As discussed earlier, do you really need to mess with IO_ADDR_W or can you do it the way ndfc.c does?

Quoted 1 line

I will take a look at ndfc.c.

Most of this driver was copy-paste from s3c64xx.c driver and an older patched U-Boot sources from Samsung, so I did not make any real code examination after it started to work...

Quoted 9 lines

Quoted 9 lines

I.e.:

static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)

{
    struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
    struct nand_chip *this = mtd->priv;

Quoted 8 lines

Quoted 8 lines

    if (cmd == NAND_CMD_NONE)
        return;

    if (ctrl & NAND_CLE)
        writeb(cmd, &nand->nfcmmd);
    else
        writeb(cmd, &nand->nfaddr);
}

Oh sorry -- the above is missing the NCE part. Before the NAND_CMD_NONE check insert:

    if (ctrl & NAND_CTRL_CHANGE) {
        if (ctrl & NAND_NCE)
            s3c_nand_select_chip(mtd, *(const int *)this->priv);
        else
            s3c_nand_select_chip(mtd, -1);
    }

Quoted 54 lines

Quoted 53 lines

Quoted 40 lines

drivers/mtd/nand/s3c24xx_nand.c new file +39 -0

+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+    static int chip_n;
+    struct s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+
+    if (chip_n == 0) {
+        /* Extend NAND timings to the maximum */
+        clrsetbits_le32(&nand_reg->nfconf,
+                NFCONF_TACLS_MASK | NFCONF_TWRPH0_MASK |
+                NFCONF_TWRPH1_MASK,
+                NFCONF_TACLS(TACLS_VAL) |
+                NFCONF_TWRPH0(TWRPH0_VAL) |
+                NFCONF_TWRPH1(TWRPH1_VAL));
+
+        /* Disable chip selects and soft lock, enable controller */
+        clrsetbits_le32(&nand_reg->nfcont, NFCONT_WP,
+                NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE);
+    } else if (chip_n >= MAX_CHIPS) {
+        return -ENODEV;
+    }
+
+    nand->IO_ADDR_R = &nand_reg->nfdata;
+    nand->IO_ADDR_W = &nand_reg->nfdata;
+    nand->cmd_ctrl = s3c_nand_hwcontrol;
+    nand->dev_ready = s3c_nand_device_ready;
+    nand->select_chip = s3c_nand_select_chip;
+    nand->options = 0;
+#ifdef CONFIG_SPL_BUILD
+    nand->read_buf = nand_read_buf;
+#endif
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+    nand->ecc.hwctl = s3c_nand_enable_hwecc;
+    nand->ecc.calculate = s3c_nand_calculate_ecc;
+    nand->ecc.correct = s3c_nand_correct_data;
+    nand->ecc.mode = NAND_ECC_HW_OOB_FIRST;

drivers/mtd/nand/s3c24xx_nand.c line 246

Quoted 10 lines

drivers/mtd/nand/s3c24xx_nand.c new file +10 -0

+    nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE;
+    nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES;
+#else
+    nand->ecc.mode = NAND_ECC_SOFT;
+#endif /* ! CONFIG_S3C24XX_NAND_HWECC */
+
+    nand->priv = nand_cs + chip_n++;
+
+    return 0;
+}

Please consider using the new SELF_INIT mechanism.

Quoted 2 lines

Can you explain and/or point_to_resources for what this means (I'm a U-Boot newbie)...

See CONFIG_SYS_NAND_SELF_INIT in doc/README.nand

I'd like the old way to be removed at some point.

Quoted 10 lines · Scott Wood

Quoted 6 lines · Jos? Miguel Gon?alves

Quoted 2 lines · Scott Wood

Please consider using the new SELF_INIT mechanism.

Can you explain and/or point_to_resources for what this means (I'm a U-Boot newbie)...

See CONFIG_SYS_NAND_SELF_INIT in doc/README.nand

Quoted 1 line

I'd like the old way to be removed at some point.

From what I've seen there is an incompatibility between the SPL simple nand driver and the SELF_INIT mechanism.
The nand_init() function in drivers/mtd/nand/nand_spl_simple.c calls board_nand_init() with a pointer to a struct nand_chip as an argument (the old way). This needs to be changed to be able to use that SPL driver. Can you give me an hint on the proper way to handle this?