From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Wed, 16 May 2012 11:54:50 -0500
Subject: [U-Boot] [PATCH V3 RESEND 2/4] mtd/NAND: Add FSMC driver support
In-Reply-To: <4dc294de330f527ba943deb9230fcccd46229064.1337169928.git.amit.virdi@st.com>
References: <cover.1337169928.git.amit.virdi@st.com>
	<4dc294de330f527ba943deb9230fcccd46229064.1337169928.git.amit.virdi@st.com>
Message-ID: <4FB3DBDA.2070801@freescale.com>
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 05/16/2012 07:06 AM, Amit Virdi wrote:
> +		if ((bits_ecc + bits_data) <= 8) {
> +			if (bits_data)
> +				memset(dat, 0xff, 512);
> +			return bits_data;

return bits_data + bits_ecc;

> +	i = 0;
> +	while (num_err--) {
> +		change_bit(0, &err_idx[i]);
> +		change_bit(1, &err_idx[i]);

Where is change_bit defined?  I see __change_bit in
arch/arm/include/asm/bitops.h, but change_bit is defined as an extern
prototype.  In Linux change_bit (without the __) is defined as an atomic
operation, which probably isn't appropriate here.

These two in particular could just be err_idx[i] ^= 3, right?

> +		if (err_idx[i] < 512 * 8) {
> +			change_bit(err_idx[i], dat);
> +			i++;
> +		}

Increment i unconditionally.

-Scott