On 14-09-2012 19:21, Marek Vasut wrote:
Dear 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 <jose.goncal...@inov.pt>
[...]
+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS 2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)
This doesn't seem quite right ...
1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro
1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it
would blown up the internal SoC RAM space available.
So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?
+#endif
+
+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+ struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ u_long nfcont;
+
+ 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;
+ }
+
+ 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;
Why don't you use local variable here?
Because you need to retain the NAND controller register to use between calls to
s3c_nand_hwcontrol().
If you call the function without the NAND_CTRL_CHANGE bit set in the parameter
'ctrl' you must use the register used on the last call on the next access.
+ if (ctrl & NAND_NCE)
+ s3c_nand_select_chip(mtd, *(int *)this->priv);
Uh, how's this *(int *) supposed to work?
*(int *)this->priv contains an integer that is the chip id (0 or 1).
Best regards,
José Gonçalves
Best regards,
José Gonçalves
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
