On 18/11/09 20:22, Scott Wood wrote:
> On Mon, Nov 16, 2009 at 05:49:55PM +0000, Nick Thompson wrote:
>>  static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
>>  {
>> -    int             dummy;
>> +    u_int32_t       val;
>>  
>> -    dummy = emif_regs->NANDF1ECC;
>> +    val = readl(&emif_regs->NANDF1ECC);
> 
> "val =" can be omitted, which would keep it clear that it is a dummy read.

Sounds good - I'll follow Wolfgang's idea and add a (void) as well.

> 
>> -    /* FIXME:  only chipselect 0 is supported for now */
>> -    emif_regs->NANDFCR |= 1 << 8;
>> +    val = readl(&emif_regs->NANDFCR);
>> +    val |= DAVINCI_NANDFCR_1BIT_ECC_START(CONFIG_SYS_NAND_CS);
>> +    writel(val, &emif_regs->NANDFCR);
> 
> Do you need to clear the bit corresponding to the previous chipselect?

No, the bits clear themselves when the ECC result is read. The device
is actually capable of running multiple ECC1's at the same time. It
can't do multiple ECC4's, but the select for that is coded to make
this clear.

> 
> Otherwise, ACK.

I'm going to resubmit as your first comment revealed a latent bug.
That dummy read is what ensures the ECC start bit is clear, but It only
clear the CS2 ECC, rather than the config selected ECC.

Thanks,
Nick.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to