[U-Boot] [PATCH 08/13 v4] ARM: OMAP3: Add NAND support

[Scott Wood]

On Fri, Oct 17, 2008 at 09:28:50PM +0200, dirk.behme at googlemail.com wrote:
> +	if (nand_curr_device < 0 ||
> +		nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
> +		!nand_info[nand_curr_device].name) {
> +		printf("Error: Can't switch ecc, no devices available\n");
> +		return;
> +	}

It's much more readable if you align the continuation lines with the
beginning of the condition, rather than with the if-body.

> +	mtd = &nand_info[nand_curr_device];
> +	nand = mtd->priv;
> +
> +	/* clean up allocated buffers */
> +	nand_release(mtd);
> +	/* mark me unscanned */
> +	nand->options &= ~NAND_BBT_SCANNED;

Better to set it back to the original value explicitly, in case other
internal NAND flags crop up.

> +	/* Setup the ecc configurations again */
> +	if (!hardware) {
> +		nand->ecc.mode = NAND_ECC_SOFT;
> +		/* Use mtd default settings */
> +		nand->ecc.layout = NULL;
> +	} else {
> +		nand->ecc.mode = NAND_ECC_HW;
> +		nand->ecc.layout = &hw_nand_oob;
> +		nand->ecc.size = 512;
> +		nand->ecc.bytes = 3;
> +		nand->ecc.steps = hw_nand_oob.eccbytes / nand->ecc.bytes;

No need to set ecc.steps; nand_scan_tail() will do it.

> +/* Small Page x8 NAND device Layout */
> +#ifdef GPMC_NAND_ECC_SP_x8_LAYOUT
> +#define GPMC_NAND_HW_ECC_LAYOUT {\
> +	.eccbytes = 3,\
> +	.eccpos = {1, 2, 3},\
> +	.oobfree = {\
> +		{.offset = 4,\
> +		 .length = 13 } } \
> +}

.length = 12

> +#endif
> +
> +/* Small Page x16 NAND device Layout */
> +#ifdef GPMC_NAND_ECC_SP_x16_LAYOUT
> +#define GPMC_NAND_HW_ECC_LAYOUT {\
> +	.eccbytes = 3,\
> +	.eccpos = {2, 3, 4},\
> +	.oobfree = {\
> +		{.offset = 4,\
> +		 .length = 12 } } \
> +}

.offset = 5
.length = 11

Otherwise ACK, if the ARM maintainers are OK with passing integers as
addresses to writel().

-Scott