From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Mon, 27 Oct 2008 12:35:46 -0500
Subject: [U-Boot] [PATCH 08/13 v4] ARM: OMAP3: Add NAND support
In-Reply-To: <48f8e774.1f145e0a.149f.554e@mx.google.com>
References: <48f8e774.1f145e0a.149f.554e@mx.google.com>
Message-ID: <20081027173546.GA20556@loki.buserror.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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