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;elsethis->IO_ADDR_W = &nand->nfdata;if (ctrl & NAND_NCE)s3c_nand_select_chip(mtd, *(int *)this->priv);elses3c_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