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

[Scott Wood]

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?

We probably should add them given they're the standard arch and bus
independent accessors in Linux, but you could also use readl()/writel().
Why did you choose the raw version?

> 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().

Yes, in Linux this would have been handled through the DMA API.

-Scott