[U-Boot] [U-Boot,05/10] mtd: nand: s3c: Add S3C2440 specifics
Scott Wood
scottwood at freescale.com
Thu Nov 27 03:20:53 CET 2014
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;
> if (!(ctrl & NAND_ALE))
> - 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);
> else
> - 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
More information about the U-Boot
mailing list