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