[U-Boot] [PATCH v2 1/4] Flex-OneNAND driver

[Scott Wood]

On Thu, Dec 11, 2008 at 07:57:18PM +0530, Rohit Hagargundgi wrote:
> This patch adds support for MLC OneNAND and Flex-OneNAND devices.

Patch does not apply to u-boot-nand-flash/next (the master branch is for
2008.12 bugfixes only).

It's looking a lot better (though I'm not familiar enough with this
hardware to comment on the functional bits), though while you're
rebasing, I have some style nits:

> +	unsigned boundary, blk, die = 0;

"unsigned int"

> +inline int flexonenand_region(struct mtd_info *mtd, loff_t addr)

Compiler should be able to figure out inlines by itself.

> +{
> +	int i;
> +
> +	for (i = 0; i < mtd->numeraseregions &&
> +		addr >= mtd->eraseregions[i].offset; i++)
> +		;

Align the continuation line with "i = 0", making it distinct from the
if-body.

Move the addr >= mtd->eraseregions[i].offset test into the if-body, for
readability.

> +	i--;
> +	return i;
> +}

Just return i - 1.

> +			ret = ret ? onenand_recover_lsb(mtd, from, ret) : ret;

if (ret)
	ret = onenand_recover_lsb(mtd, from, ret);

The ?: usually decreases readability rather than increases it; it's best
to avoid unless using it would eliminate duplication or a temporary
variable.

>  	/* Block lock scheme */
> -	for (block = start; block < end; block++) {
> +	for (block = start; block < end + 1; block++) {

block <= end

> +		locked = bdry >> FLEXONENAND_PI_UNLOCK_SHIFT;
> +		locked = (locked == 0x3) ? 0 : 1;

if ((bdry >> FLEXONENAND_PI_UNLOCK_SHIFT) == 3)
	locked = 0;
else
	locked = 1;

> +	eraseshift = this->erase_shift - 1;
> +
> +
> +	mtd->numeraseregions = this->dies << 1;

Only one blank line.

> +	mtd->erasesize = 1 << (this->erase_shift);

Unnecessary parens.

-Scott