On 09/18/2012 01:22:58 PM, José Miguel Gonçalves wrote:
On 18-09-2012 19:02, Scott Wood wrote:
On 09/18/2012 12:40:36 PM, José Miguel Gonçalves wrote:
+#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.


+
+#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.

+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...

Replace u_long with u32.

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

error message on default?

OK.


+    }
+
+    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);
+    }
+
+    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.



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...


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);
}

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);
        }




+/*
+ * 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;
+#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)...

See CONFIG_SYS_NAND_SELF_INIT in doc/README.nand

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

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

Reply via email to