On Sat, Oct 11, 2014 at 06:42:53PM +0200, Marek Vasut wrote:
+#ifdef CONFIG_S3C2410 #define S3C2410_NFCONF_TACLS(x) ((x)<<8) #define S3C2410_NFCONF_TWRPH0(x) ((x)<<4) #define S3C2410_NFCONF_TWRPH1(x) ((x)<<0) +#else /* S3C2412, S3C2440 */ +#define S3C2410_NFCONF_TACLS(x) ((x)<<12) +#define S3C2410_NFCONF_TWRPH0(x) ((x)<<8) +#define S3C2410_NFCONF_TWRPH1(x) ((x)<<4) +#endif
Is there any chance there will be a third variant? Perhaps the S3C2440 variant should be explicitly requested with an #error if no variant is selected.
#define S3C2410_ADDR_NALE 4 #define S3C2410_ADDR_NCLE 8 @@ -42,25 +51,30 @@ static void s3c24x0_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl) { struct nand_chip *chip = mtd->priv; struct s3c24x0_nand *nand = s3c24x0_get_base_nand();
uint32_t sel_reg, sel_bit;
debug("hwcontrol(): 0x%02x 0x%08x\n", cmd & 0xff, ctrl);
if (ctrl & NAND_CTRL_CHANGE) {
ulong IO_ADDR_W = (ulong)nand;
chip->IO_ADDR_W = &nand->nfconf;if (!(ctrl & NAND_CLE))
IO_ADDR_W |= S3C2410_ADDR_NCLE;
chip->IO_ADDR_W = &nand->nfaddr;
IO_ADDR_W |= S3C2410_ADDR_NALE;
chip->IO_ADDR_W = &nand->nfcmd;
chip->IO_ADDR_W = (void *)IO_ADDR_W;+#ifdef CONFIG_S3C2410
sel_reg = (uint32_t)&nand->nfconf;sel_bit = S3C2410_NFCONF_nFCE;+#else
sel_reg = (uint32_t)&nand->nfcont;sel_bit = S3C2440_NFCONT_nFCE;+#endif
Why are you casting &nand->nfcon[ft] to an integer...
if (ctrl & NAND_NCE)
writel(readl(&nand->nfconf) & ~S3C2410_NFCONF_nFCE,&nand->nfconf);
clrbits_le32(sel_reg, sel_bit);
writel(readl(&nand->nfconf) | S3C2410_NFCONF_nFCE,&nand->nfconf);
setbits_le32(sel_reg, sel_bit);
...only to pass that integer to something that normally accepts a pointer, which doesn't cause a warning only because of ARM's crappy I/O accessors that don't do type checking?
At least the code that was there before used ulong for inappropriate pointer-to-integer casts rather than uint32_t. :-P
- writel(readl(&clk_power->clkcon) | (1 << 4), &clk_power->clkcon);
- setbits_le32(&clk_power->clkcon, 1 << 4);
This change seems unrelated to adding s3c2440 support. It'd be clearer if the {clr,set,clrset}bits_le32() conversion were done separately, before the s3c2440 stuff. I'm not insisting that you redo this one, but next time try to keep cleanup separate.
-Scott