From mboxrd@z Thu Jan  1 00:00:00 1970
From: Amit Virdi <amit.virdi@st.com>
Date: Thu, 17 May 2012 17:49:16 +0530
Subject: [U-Boot] [PATCH V3 RESEND 2/4] mtd/NAND: Add FSMC driver support
In-Reply-To: <4FB3DBDA.2070801@freescale.com>
References: <cover.1337169928.git.amit.virdi@st.com>
	<4dc294de330f527ba943deb9230fcccd46229064.1337169928.git.amit.virdi@st.com>
	<4FB3DBDA.2070801@freescale.com>
Message-ID: <4FB4ECC4.1050005@st.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 5/16/2012 10:24 PM, Scott Wood wrote:
> 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;
>

Ok.

>> +	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

change_bit is defined in this patchset.

> prototype.  In Linux change_bit (without the __) is defined as an atomic
> operation, which probably isn't appropriate here.
>

Yes, as the interrupts are not enabled in the u-boot so this has 
virtually no effect. So effectively, I need to use __change_bit here.

Since __change_bit was not exported so we defined a wrapper "change_bit" 
to use it. Any other suggestion?

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

Correct. I'll take it.

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

Yeah, you are correct. Thanks for your inputs.


Regards
Amit Virdi