Messages in this thread | | | Date | Tue, 24 Feb 2009 22:55:45 -0300 | Subject | Re: [patch/RESEND 2.6.29-rc3-git] NAND: davinci_nand driver | From | Thiago Galesi <> |
| |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |