[U-Boot] [PATCH] denali: review feedback

For Chin Liang See
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com --- drivers/mtd/nand/denali_nand.c | 42 +++++++++++++++++++----------------------- drivers/mtd/nand/denali_nand.h | 3 +++ 2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/mtd/nand/denali_nand.c b/drivers/mtd/nand/denali_nand.c index ffa69ad..a713cca 100644 --- a/drivers/mtd/nand/denali_nand.c +++ b/drivers/mtd/nand/denali_nand.c @@ -55,8 +55,6 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS; #define DENALI_READ 0 #define DENALI_WRITE 0x100
-#define DENALI_DATA_OFFSET_ADDRESS 0x10 - /* types of device accesses. We can issue commands and get status */ #define COMMAND_CYCLE 0 #define ADDR_CYCLE 1 @@ -129,15 +127,15 @@ static uint32_t wait_for_irq(uint32_t irq_mask) */ static void index_addr(uint32_t address, uint32_t data) { - writel(address, denali.flash_mem); - writel(data, denali.flash_mem + DENALI_DATA_OFFSET_ADDRESS); + writel(address, denali.flash_mem + INDEX_CTRL_REG); + writel(data, denali.flash_mem + INDEX_DATA_REG); }
/* Perform an indexed read of the device */ static void index_addr_read_data(uint32_t address, uint32_t *pdata) { - writel(address, denali.flash_mem); - *pdata = readl(denali.flash_mem + DENALI_DATA_OFFSET_ADDRESS); + writel(address, denali.flash_mem + INDEX_CTRL_REG); + *pdata = readl(denali.flash_mem + INDEX_DATA_REG); }
/* We need to buffer some data for some of the NAND core routines. @@ -576,7 +574,7 @@ static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
if (access_type != SPARE_ACCESS) { cmd = MODE_01 | addr; - writel(cmd, denali.flash_mem); + writel(cmd, denali.flash_mem + INDEX_CTRL_REG); } return 0; } @@ -593,7 +591,7 @@ static int write_data_to_flash_mem(const void *buf, int len) /* write the data to the flash memory */ buf32 = (uint32_t *)buf; for (i = 0; i < len / 4; i++) - writel(*buf32++, denali.flash_mem + DENALI_DATA_OFFSET_ADDRESS); + writel(*buf32++, denali.flash_mem + INDEX_DATA_REG); return i * 4; /* intent is to return the number of bytes read */ }
@@ -626,7 +624,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
/* We need to write to buffer first through MAP00 command */ cmd = MODE_00 | BANK(denali.flash_bank); - writel(cmd, denali.flash_mem); + writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
/* send the data into flash buffer */ write_data_to_flash_mem(buf, mtd->oobsize); @@ -664,7 +662,7 @@ static void denali_enable_dma(bool en) }
/* setups the HW to perform the data DMA */ -static void denali_setup_dma_sequence(int op) +static void denali_setup_dma(int op) { const int page_count = 1; uint32_t mode; @@ -672,26 +670,24 @@ static void denali_setup_dma_sequence(int op)
flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
- mode = MODE_10 | BANK(denali.flash_bank); - - /* DMA is a four step process */ + mode = MODE_10 | BANK(denali.flash_bank) | denali.page;
- /* 1. setup transfer type and # of pages */ - index_addr(mode | denali.page, 0x2000 | op | page_count); + /* DMA is a three step process */
- /* 2. set memory high address bits 23:8 */ - index_addr(mode | ((uint32_t)(addr >> 16) << 8), 0x2200); + /* 1. setup transfer type and # of pages + interrupt when complete, burst len = 64 bytes */ + index_addr(mode, 0x01002000 | (64 << 16) | op | page_count);
- /* 3. set memory low address bits 23:8 */ - index_addr(mode | ((uint32_t)addr << 8), 0x2300); + /* 2. set memory low address bits 31:0 */ + index_addr(mode, addr);
- /* 4. interrupt when complete, burst len = 64 bytes*/ - index_addr(mode | 0x14000, 0x2400); + /* 3. set memory high address bits 64:32 */ + index_addr(mode, 0); }
/* Common DMA function */ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer, - uint32_t irq_mask, int oob_required) + uint32_t irq_mask, int oob_required) { uint32_t irq_status = 0; /* setup_ecc_for_xfer(bool ecc_en, bool transfer_spare) */ @@ -704,7 +700,7 @@ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer, denali_enable_dma(true);
/* setup the DMA */ - denali_setup_dma_sequence(ops); + denali_setup_dma(ops);
/* wait for operation to complete */ irq_status = wait_for_irq(irq_mask); diff --git a/drivers/mtd/nand/denali_nand.h b/drivers/mtd/nand/denali_nand.h index 4fd9ffc..c668d8c 100644 --- a/drivers/mtd/nand/denali_nand.h +++ b/drivers/mtd/nand/denali_nand.h @@ -413,6 +413,9 @@ typedef int irqreturn_t; #ifndef _LLD_NAND_ #define _LLD_NAND_
+#define INDEX_CTRL_REG 0x0 +#define INDEX_DATA_REG 0x10 + #define MODE_00 0x00000000 #define MODE_01 0x04000000 #define MODE_10 0x08000000

Hi Masahiro,
On Fri, 7 Mar 2014 21:52:54 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
For Chin Liang See
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
The patch subject / commit summary definitely needs to be more descriptive of the why and what of the patch content.
Amicalement,

Hello Albert,
On Fri, 7 Mar 2014 21:52:54 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
For Chin Liang See
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
The patch subject / commit summary definitely needs to be more descriptive of the why and what of the patch content.
Amicalement,
Albert.
This is not meant to be applied. I just sent a code diff to Chin because I wanted to suggest how his patch file should be fixed in the next version.
I wish there had been a good way to prevent Patchwok from picking up emails which include code diffs.
Best Regards Masahiro Yamada
participants (2)
-
Albert ARIBAUD
-
Masahiro Yamada