Re: [patch/RESEND 2.6.29-rc3-git] NAND: davinci_nand driver

[Thiago Galesi <thiagogalesi@gmail.com>]

OK a couple of things
 +
> +
> +#ifdef CONFIG_MTD_PARTITIONS
> +static inline int mtd_has_partitions(void) { return 1; }
> +#else
> +static inline int mtd_has_partitions(void) { return 0; }
> +#endif
> +
> +#ifdef CONFIG_MTD_CMDLINE_PARTS
> +static inline int mtd_has_cmdlinepart(void) { return 1; }
> +#else
> +static inline int mtd_has_cmdlinepart(void) { return 0; }
> +#endif

I'm not sure stylewise this is the best way. Even though #ifdefs stays
outside of functions, this is a case where maybe it's better to put it
inside or rething the need / usage for these functions.

> +static int nand_davinci_calculate_1bit(struct mtd_info *mtd,
> +                                     const u_char *dat, u_char *ecc_code)

(and others)

My beef here is the use of u_char. This is really not a standart C
type or a standart Linux type (u8/u32 etc), so we should aim for those
(yes, I hate typing unsigned blah blah blah) but you can use u8 for
that.

> +static void nand_davinci_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +       struct nand_chip *chip = mtd->priv;
> +
> +       if ((0x03 & ((unsigned)buf)) == 0 && (0x03 & len) == 0)
> +               ioread32_rep(chip->IO_ADDR_R, buf, len >> 2);
> +       else if ((0x01 & ((unsigned)buf)) == 0 && (0x01 & len) == 0)

What are those 0x03 and 0x01 (and other places as well), you'll have
to spell out those, preferably using defines.

-- 
-
Thiago Galesi