Hi Masahiro,
On Tue, 2014-03-04 at 19:31 +0900, Masahiro Yamada wrote:
> 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?
>
Changed all to use standard writel and readl.
>
>
> > > +
> > > +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)/
>
>
Hmmm that is interesting.
Probably I speed up the NAND controller to max.
Anyway, the delay now is 1s.
>
>
> 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 */
>
>
I miss out this as I didn't enable the cache.
Thanks and I will put this into new patches
Chin Liang
>
>
> Best Regards
> Masahiro Yamada
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
