Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
Hi Marek, On Fri, 4 Jul 2014 16:34:11 +0200 Marek Vasut ma...@denx.de wrote: +static void read_data_from_flash_mem(uint8_t *buf, int len) +{ + int i; + uint32_t *buf32; + + /* transfer the data from the flash */ + buf32 = (uint32_t *)buf; + for (i = 0; i len / 4; i++) + *buf32++ = readl(denali_flash_mem + INDEX_DATA_REG); Won't this trigger unaligned access if $buf is not aligned to 4-byte boundary ? Thanks for catching this. Actually, it would seldom happen because the aligned load address is generally given. But I will add a sanity check here in v2, just in case. + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. Actually, this part is the same as that of the normal Denali driver by Chin Liang See http://patchwork.ozlabs.org/patch/357717/ Moreover, it is the same as in Linux Kernel. I understand what you mean, but I guess we can excuse here. + cmd = MODE_01 | addr; + writel(cmd, denali_flash_mem + INDEX_CTRL_REG); Somehow, eventually, this should be migrated to struct based register access instead of such offset computation. If you feel like doing it, that'd be nice ;-) Dito. I am following the coding style of the normal Denali driver and the Linux one. We might perhaps do that migration as the separated task in future, but not now. Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
On Thursday, July 10, 2014 at 12:06:20 PM, Masahiro Yamada wrote: Hi Marek, Hi! On Fri, 4 Jul 2014 16:34:11 +0200 Marek Vasut ma...@denx.de wrote: +static void read_data_from_flash_mem(uint8_t *buf, int len) +{ + int i; + uint32_t *buf32; + + /* transfer the data from the flash */ + buf32 = (uint32_t *)buf; + for (i = 0; i len / 4; i++) + *buf32++ = readl(denali_flash_mem + INDEX_DATA_REG); Won't this trigger unaligned access if $buf is not aligned to 4-byte boundary ? Thanks for catching this. Actually, it would seldom happen because the aligned load address is generally given. But I will add a sanity check here in v2, just in case. Or just use put_unaligned() ? + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. Actually, this part is the same as that of the normal Denali driver by Chin Liang See http://patchwork.ozlabs.org/patch/357717/ Moreover, it is the same as in Linux Kernel. I understand what you mean, but I guess we can excuse here. OK, I see. Is there any chance to fix this in Linux maybe eventually? [...] Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
Hi Marek, On Thu, 10 Jul 2014 12:10:25 +0200 Marek Vasut ma...@denx.de wrote: On Fri, 4 Jul 2014 16:34:11 +0200 Marek Vasut ma...@denx.de wrote: +static void read_data_from_flash_mem(uint8_t *buf, int len) +{ + int i; + uint32_t *buf32; + + /* transfer the data from the flash */ + buf32 = (uint32_t *)buf; + for (i = 0; i len / 4; i++) + *buf32++ = readl(denali_flash_mem + INDEX_DATA_REG); Won't this trigger unaligned access if $buf is not aligned to 4-byte boundary ? Thanks for catching this. Actually, it would seldom happen because the aligned load address is generally given. But I will add a sanity check here in v2, just in case. Or just use put_unaligned() ? Oh, year! put_unaligned() would be better. Thanks! + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. Actually, this part is the same as that of the normal Denali driver by Chin Liang See http://patchwork.ozlabs.org/patch/357717/ Moreover, it is the same as in Linux Kernel. I understand what you mean, but I guess we can excuse here. OK, I see. Is there any chance to fix this in Linux maybe eventually? OK. I will send a patch to linux-mtd ML to use #define PIPELINE_ACCESS2000 Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
On Thursday, July 10, 2014 at 12:52:19 PM, Masahiro Yamada wrote: Hi Marek, Hi! [...] Or just use put_unaligned() ? Oh, year! put_unaligned() would be better. Thanks! + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. Actually, this part is the same as that of the normal Denali driver by Chin Liang See http://patchwork.ozlabs.org/patch/357717/ Moreover, it is the same as in Linux Kernel. I understand what you mean, but I guess we can excuse here. OK, I see. Is there any chance to fix this in Linux maybe eventually? OK. I will send a patch to linux-mtd ML to use #define PIPELINE_ACCESS2000 Ack on both, thank you ! Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
Hi Marek, On Thu, 10 Jul 2014 13:30:38 +0200 Marek Vasut ma...@denx.de wrote: + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. Actually, this part is the same as that of the normal Denali driver by Chin Liang See http://patchwork.ozlabs.org/patch/357717/ Moreover, it is the same as in Linux Kernel. I understand what you mean, but I guess we can excuse here. OK, I see. Is there any chance to fix this in Linux maybe eventually? OK. I will send a patch to linux-mtd ML to use #define PIPELINE_ACCESS2000 Ack on both, thank you ! Done. http://patchwork.ozlabs.org/patch/368933/ Best Regards Masahiro Yamada ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL
On Friday, July 04, 2014 at 12:19:13 PM, Masahiro Yamada wrote: The SPL-mode driver for Denali(Cadence) NAND Flash Memory Controller IP. This driver requires two CONFIG macros: - CONFIG_NAND_DENALI Define to enable this driver. - CONFIG_SYS_NAND_BAD_BLOCK_POS Specify bad block mark position in the oob space. Typically 0. Signed-off-by: Masahiro Yamada yamad...@jp.panasonic.com Cc: Chin Liang See cl...@altera.com Cc: Scott Wood scottw...@freescale.com [...] +static void read_data_from_flash_mem(uint8_t *buf, int len) +{ + int i; + uint32_t *buf32; + + /* transfer the data from the flash */ + buf32 = (uint32_t *)buf; + for (i = 0; i len / 4; i++) + *buf32++ = readl(denali_flash_mem + INDEX_DATA_REG); Won't this trigger unaligned access if $buf is not aligned to 4-byte boundary ? +} + +int denali_send_pipeline_cmd(int page, int ecc_en, int access_type) +{ + uint32_t addr, cmd; + static uint32_t page_count = 1; + + writel(ecc_en, denali_flash_reg + ECC_ENABLE); + + /* clear all bits of intr_status. */ + writel(0x, denali_flash_reg + INTR_STATUS(flash_bank)); + + addr = BANK(flash_bank) | page; + + /* setup the acccess type */ + cmd = MODE_10 | addr; + index_addr(cmd, access_type); + + /* setup the pipeline command */ + index_addr(cmd, 0x2000 | page_count); Magic value 0x2000 should be fixed here. + cmd = MODE_01 | addr; + writel(cmd, denali_flash_mem + INDEX_CTRL_REG); Somehow, eventually, this should be migrated to struct based register access instead of such offset computation. If you feel like doing it, that'd be nice ;-) + return wait_for_irq(INTR_STATUS__LOAD_COMP); +} [...] Thanks! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot