[U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support

[Masahiro Yamada]

Hello Scott, Chin,


> > +/* this is a helper macro that allows us to
> > + * format the bank into the proper bits for the controller */
> > +#define BANK(x) ((x) << 24)
> > +
> > +/* Interrupts are cleared by writing a 1 to the appropriate status bit */
> > +static inline void clear_interrupt(uint32_t irq_mask)
> > +{
> > +	uint32_t intr_status_reg = 0;
> > +	intr_status_reg = INTR_STATUS(denali.flash_bank);
> > +	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
> > +}
> 
> Why are you using raw I/O accessors?  The Linux driver doesn't do this.

Add ioread32/iowrite32 to arch/arm/include/asm/io.h
and use them?



> > +
> > +static uint32_t wait_for_irq(uint32_t irq_mask)
> > +{
> > +	unsigned long comp_res = 1000;
> > +	uint32_t intr_status = 0;
> > +
> > +	do {
> > +		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
> > +		if (intr_status & irq_mask) {
> > +			denali.irq_status &= ~irq_mask;
> > +			/* our interrupt was detected */
> > +			break;
> > +		}
> > +		udelay(1);
> > +		comp_res--;
> > +	} while (comp_res != 0);
> 
> This looks like a much shorter timeout than Linux uses (1000us versus
> 1000ms).  Though FWIW the Linux timeout code looks buggy.
> 
> Also, comp_res is a very odd name for a timeout variable.

Right. I think wait time is too short.
When I tested this code on my board, timeout error always
occurred on page write command.

I had to fix it to run write command.
    s/udelay(1)/udelay(1000)/




There is another problem.
I think there is a cache coherency problem in this driver code.
DMA is used in this driver but D-cache is never flushed.

When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined),
ARM processor writes to/reads from the buffer through D-cache.
On the other hand, Denali NAND controller always wites to/reads from
the buffer on physical memory.
So, this driver writes/reads wrong data.

I had to add flush_dcache_range() function call
in denali_setup_dma_sequence().


  @@ -689,6 +689,8 @@ static void denali_setup_dma_sequence(int op)
          uint32_t mode;
          uint32_t addr = (uint32_t)denali.buf.dma_buf;
   
  +       flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
  +
          mode = MODE_10 | BANK(denali.flash_bank);
   
          /* DMA is a four step process */




Best Regards
Masahiro Yamada