linux-kernel - Re: [patch/RESEND 2.6.29-rc3-git] NAND: davinci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82ecf08e0902241755y5a7be725gb84116433b7d3be2@mail.gmail.com>
Date:	Tue, 24 Feb 2009 22:55:45 -0300
From:	Thiago Galesi <thiagogalesi@...il.com>
To:	dbrownell@...rs.sourceforge.net
Cc:	Linux MTD <linux-mtd@...ts.infradead.org>,
	lkml <linux-kernel@...r.kernel.org>,
	DaVinci <davinci-linux-open-source@...ux.davincidsp.com>
Subject: Re: [patch/RESEND 2.6.29-rc3-git] NAND: davinci_nand driver

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/