[U-Boot] [PATCH 0/4] sunxi: nand: Basic NAND driver with SPL support
This is a basic driver for the sunxi NAND controller for Allwinner A20. It supports both SPL and U-Boot.
The driver uses DMA for data transfers. It does not support writing.
To enable user reading from NADN, the a20_nandread command was added.
Piotr Zierhoffer (4): sunxi: nand: Add basic sunxi NAND driver with DMA support sunxi: nand: Add board configuration options sunxi: nand: Add a20_nandread command to load image from NAND in SPL sunxi: nand: Add information to sunxi that it was run from NAND in SPL
arch/arm/cpu/armv7/sunxi/board.c | 4 + board/sunxi/Kconfig | 10 ++ board/sunxi/Makefile | 1 + board/sunxi/nand.c | 315 +++++++++++++++++++++++++++++++++++++++ board/sunxi/nand.h | 146 ++++++++++++++++++ common/Makefile | 1 + common/cmd_a20_nandread.c | 25 ++++ include/configs/sunxi-common.h | 13 ++ 8 files changed, 515 insertions(+) create mode 100644 board/sunxi/nand.c create mode 100644 board/sunxi/nand.h create mode 100644 common/cmd_a20_nandread.c
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
This driver adds NAND support to both SPL and full U-Boot. It was tested on AllWinner A20.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com ---
board/sunxi/Makefile | 1 + board/sunxi/nand.c | 315 +++++++++++++++++++++++++++++++++++++++++++++++++++ board/sunxi/nand.h | 146 ++++++++++++++++++++++++ 3 files changed, 462 insertions(+) create mode 100644 board/sunxi/nand.c create mode 100644 board/sunxi/nand.h
diff --git a/board/sunxi/Makefile b/board/sunxi/Makefile index 43766e0..6f1d571 100644 --- a/board/sunxi/Makefile +++ b/board/sunxi/Makefile @@ -9,6 +9,7 @@ # SPDX-License-Identifier: GPL-2.0+ # obj-y += board.o +obj-$(CONFIG_SUNXI_NAND) += nand.o obj-$(CONFIG_SUNXI_GMAC) += gmac.o obj-$(CONFIG_SUNXI_AHCI) += ahci.o obj-$(CONFIG_MACH_SUN4I) += dram_sun4i_auto.o diff --git a/board/sunxi/nand.c b/board/sunxi/nand.c new file mode 100644 index 0000000..16e8e4d --- /dev/null +++ b/board/sunxi/nand.c @@ -0,0 +1,315 @@ +/* + * Copyright (c) 2014-2015, Antmicro Ltd <www.antmicro.com> + * Copyright (c) 2015, AW-SOM Technologies <www.aw-som.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <config.h> +#include <asm/io.h> +#include <nand.h> +#include "nand.h" /* sunxi specific header */ + +/* minimal "boot0" style NAND support for Allwinner A20 */ + +/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif + +#define MAX_RETRIES 10 + +static int check_value_inner(int offset, int expected_bits, + int max_number_of_retries, int negation) +{ + int retries = 0; + do { + int val = readl(offset) & expected_bits; + if (negation ? !val : val) + return 1; + mdelay(1); + retries++; + } while (retries < max_number_of_retries); + + return 0; +} + +static inline int check_value(int offset, int expected_bits, + int max_number_of_retries) +{ + return check_value_inner(offset, expected_bits, + max_number_of_retries, 0); +} + +static inline int check_value_negated(int offset, int unexpected_bits, + int max_number_of_retries) +{ + return check_value_inner(offset, unexpected_bits, + max_number_of_retries, 1); +} + +void nand_set_clocks(void) +{ + uint32_t val; + + writel(PORTC_PC_CFG0_NRB1 | + PORTC_PC_CFG0_NRB0 | + PORTC_PC_CFG0_NRE | + PORTC_PC_CFG0_NCE0 | + PORTC_PC_CFG0_NCE1 | + PORTC_PC_CFG0_NCLE | + PORTC_PC_CFG0_NALE | + PORTC_PC_CFG0_NWE, PORTC_BASE + PORTC_PC_CFG0); + + writel(PORTC_PC_CFG1_NDQ7 | + PORTC_PC_CFG1_NDQ6 | + PORTC_PC_CFG1_NDQ5 | + PORTC_PC_CFG1_NDQ4 | + PORTC_PC_CFG1_NDQ3 | + PORTC_PC_CFG1_NDQ2 | + PORTC_PC_CFG1_NDQ1 | + PORTC_PC_CFG1_NDQ0, PORTC_BASE + PORTC_PC_CFG1); + + writel(PORTC_PC_CFG2_NCE7 | + PORTC_PC_CFG2_NCE6 | + PORTC_PC_CFG2_NCE5 | + PORTC_PC_CFG2_NCE4 | + PORTC_PC_CFG2_NCE3 | + PORTC_PC_CFG2_NCE2 | + PORTC_PC_CFG2_NWP, PORTC_BASE + PORTC_PC_CFG2); + + writel(PORTC_PC_CFG3_NDQS, PORTC_BASE + PORTC_PC_CFG3); + + val = readl(CCU_BASE + CCU_AHB_GATING_REG0); + writel(CCU_AHB_GATING_REG0_NAND | val, CCU_BASE + CCU_AHB_GATING_REG0); + + val = readl(CCU_BASE + CCU_NAND_SCLK_CFG_REG); + writel(val | CCU_NAND_SCLK_CFG_REG_SCLK_GATING + | CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO, + CCU_BASE + CCU_NAND_SCLK_CFG_REG); +} + +int nand_initialized; + +void nand_init(void) +{ + board_nand_init(NULL); +} + +uint32_t ecc_errors; + +/* read 0x400 bytes from real_addr to temp_buf */ +void nand_read_block(unsigned int real_addr, int syndrome) +{ + uint32_t val; + int ECC_OFF = 0; + uint16_t ecc_mode = 0; + uint16_t rand_seed; + uint32_t page; + uint16_t column; + uint32_t oob_offset; + + if (!nand_initialized) + board_nand_init(NULL); + + switch (CONFIG_SUNXI_ECC_STRENGTH) { + case 16: + ecc_mode = 0; + ECC_OFF = 0x20; + break; + case 24: + ecc_mode = 1; + ECC_OFF = 0x2e; + break; + case 28: + ecc_mode = 2; + ECC_OFF = 0x32; + break; + case 32: + ecc_mode = 3; + ECC_OFF = 0x3c; + break; + case 40: + ecc_mode = 4; + ECC_OFF = 0x4a; + break; + case 48: + ecc_mode = 4; + ECC_OFF = 0x52; + break; + case 56: + ecc_mode = 4; + ECC_OFF = 0x60; + break; + case 60: + ecc_mode = 4; + ECC_OFF = 0x0; + break; + case 64: + ecc_mode = 4; + ECC_OFF = 0x0; + break; + default: + ecc_mode = 0; + ECC_OFF = 0; + } + + if (ECC_OFF == 0) { + printf("Unsupported ECC strength (%d)!\n", + CONFIG_SUNXI_ECC_STRENGTH); + return; + } + + /* clear temp_buf */ + memset((void *)temp_buf, 0, SPL_WRITE_SIZE); + + /* set CMD */ + writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | 0xff, + NANDFLASHC_BASE + NANDFLASHC_CMD); + + if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 1), + MAX_RETRIES)) { + printf("Error while initilizing command interrupt\n"); + return; + } + + page = (real_addr / BLOCK_SIZE); + column = real_addr % BLOCK_SIZE; + + if (syndrome) { + /* shift every 1kB in syndrome */ + column += (column / SPL_WRITE_SIZE) * ECC_OFF; + } + + /* clear ecc status */ + writel(0, NANDFLASHC_BASE + NANDFLASHC_ECC_ST); + + /* Choose correct seed */ + if (syndrome) + rand_seed = random_seed_syndrome; + else + rand_seed = random_seed[page % 128]; + + writel((rand_seed << 16) | NFC_ECC_RANDOM_EN | NFC_ECC_EN + | NFC_ECC_PIPELINE | (ecc_mode << 12), + NANDFLASHC_BASE + NANDFLASHC_ECC_CTL); + + val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL); + writel(val | NFC_CTL_RAM_METHOD, NANDFLASHC_BASE + NANDFLASHC_CTL); + + if (syndrome) { + writel(SPL_WRITE_SIZE, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA); + } else { + oob_offset = BLOCK_SIZE + (column / SPL_WRITE_SIZE) * ECC_OFF; + writel(oob_offset, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA); + } + + /* DMAC */ + writel(0x0, DMAC_BASE + DMAC_CFG_REG0); /* clr dma cmd */ + /* read from REG_IO_DATA */ + writel(NANDFLASHC_BASE + NANDFLASHC_IO_DATA, + DMAC_BASE + DMAC_SRC_START_ADDR_REG0); + writel((uint32_t)temp_buf, + DMAC_BASE + DMAC_DEST_START_ADDRR_REG0); /* read to RAM */ + writel(DMAC_DDMA_PARA_REG_SRC_WAIT_CYC + | DMAC_DDMA_PARA_REG_SRC_BLK_SIZE, + DMAC_BASE + DMAC_DDMA_PARA_REG0); + writel(SPL_WRITE_SIZE, DMAC_BASE + DMAC_DDMA_BC_REG0); /* 1kB */ + writel(DMAC_DDMA_CFG_REG_LOADING + | DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 + | DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 + | DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO + | DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC, + DMAC_BASE + DMAC_CFG_REG0); + + writel(0x00E00530, NANDFLASHC_BASE + NANDFLASHC_RCMD_SET); /* READ */ + writel(1, NANDFLASHC_BASE + NANDFLASHC_SECTOR_NUM); + writel(((page & 0xFFFF) << 16) | column, + NANDFLASHC_BASE + NANDFLASHC_ADDR_LOW); + writel((page >> 16) & 0xFF, NANDFLASHC_BASE + NANDFLASHC_ADDR_HIGH); + writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS | + NFC_PAGE_CMD | NFC_WAIT_FLAG | /* ADDR_CYCLE */ (4 << 16) | + NFC_SEND_ADR | NFC_DATA_SWAP_METHOD | (syndrome ? NFC_SEQ : 0), + NANDFLASHC_BASE + NANDFLASHC_CMD); + + if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 2), + MAX_RETRIES)) { + printf("Error while initializing dma interrupt\n"); + return; + } + + if (!check_value_negated(DMAC_BASE + DMAC_CFG_REG0, + DMAC_DDMA_CFG_REG_LOADING, MAX_RETRIES)) { + printf("Error while waiting for dma transfer to finish\n"); + return; + } + + if (readl(NANDFLASHC_BASE + NANDFLASHC_ECC_ST)) + ecc_errors++; +} + +int helper_load(uint32_t offs, unsigned int size, void *dest) +{ + uint32_t dst; + uint32_t count; + uint32_t count_c; + uint32_t adr = offs; + + memset((void *)dest, 0x0, size); /* clean destination memory */ + ecc_errors = 0; + for (dst = (uint32_t)dest; + dst < ((uint32_t)dest + size); + dst += SPL_WRITE_SIZE) { + /* if < 0x400000 then syndrome read */ + nand_read_block(adr, adr < 0x400000); + count = dst - (uint32_t)dest; + + if (size - count > SPL_WRITE_SIZE) + count_c = SPL_WRITE_SIZE; + else + count_c = size - count; + + memcpy((void *)dst, + (void *)temp_buf, + count_c); + adr += SPL_WRITE_SIZE; + } + return ecc_errors; +} + +int board_nand_init(struct nand_chip *nand) +{ + uint32_t val; + + if (nand_initialized) { + printf("NAND already initialized, should not be here...\n"); + return 0; + } + nand_initialized = 1; + nand_set_clocks(); + val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL); + /* enable and reset CTL */ + writel(val | NFC_CTL_EN | NFC_CTL_RESET, + NANDFLASHC_BASE + NANDFLASHC_CTL); + + if (!check_value_negated(NANDFLASHC_BASE + NANDFLASHC_CTL, + NFC_CTL_RESET, MAX_RETRIES)) { + printf("Couldn't initialize nand\n"); + return 1; + } + + return 0; +} + +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{ + helper_load(offs, size, dest); + return 0; +} + +void nand_deselect(void) {} diff --git a/board/sunxi/nand.h b/board/sunxi/nand.h new file mode 100644 index 0000000..b3c1e93 --- /dev/null +++ b/board/sunxi/nand.h @@ -0,0 +1,146 @@ +#ifndef NAND_H + +#define NAND_H + +#define PORTC_BASE 0x01c20800 +#define CCU_BASE 0x01c20000 +#define NANDFLASHC_BASE 0x01c03000 +#define DMAC_BASE 0x01c02000 + +#define CCU_AHB_GATING_REG0 0x60 +#define CCU_NAND_SCLK_CFG_REG 0x80 +#define CCU_AHB_GATING_REG0_NAND (1 << 13) + +#define CCU_NAND_SCLK_CFG_REG_SCLK_GATING (1 << 31) +#define CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO (1 << 0) + +#define PORTC_PC_CFG0 0x48 +#define PORTC_PC_CFG1 0x4C +#define PORTC_PC_CFG2 0x50 +#define PORTC_PC_CFG3 0x54 + +#define PORTC_PC_CFG0_NRB1 (2 << 28) +#define PORTC_PC_CFG0_NRB0 (2 << 24) +#define PORTC_PC_CFG0_NRE (2 << 20) +#define PORTC_PC_CFG0_NCE0 (2 << 16) +#define PORTC_PC_CFG0_NCE1 (2 << 12) +#define PORTC_PC_CFG0_NCLE (2 << 8) +#define PORTC_PC_CFG0_NALE (2 << 4) +#define PORTC_PC_CFG0_NWE (2 << 0) + +#define PORTC_PC_CFG1_NDQ7 (2 << 28) +#define PORTC_PC_CFG1_NDQ6 (2 << 24) +#define PORTC_PC_CFG1_NDQ5 (2 << 20) +#define PORTC_PC_CFG1_NDQ4 (2 << 16) +#define PORTC_PC_CFG1_NDQ3 (2 << 12) +#define PORTC_PC_CFG1_NDQ2 (2 << 8) +#define PORTC_PC_CFG1_NDQ1 (2 << 4) +#define PORTC_PC_CFG1_NDQ0 (2 << 0) + +#define PORTC_PC_CFG2_NCE7 (2 << 24) +#define PORTC_PC_CFG2_NCE6 (2 << 20) +#define PORTC_PC_CFG2_NCE5 (2 << 16) +#define PORTC_PC_CFG2_NCE4 (2 << 12) +#define PORTC_PC_CFG2_NCE3 (2 << 8) +#define PORTC_PC_CFG2_NCE2 (2 << 4) +#define PORTC_PC_CFG2_NWP (2 << 0) + +#define PORTC_PC_CFG3_NDQS (2 << 0) + +#define BLOCK_SIZE 0x2000 + +#define DMAC_CFG_REG0 0x300 +#define DMAC_SRC_START_ADDR_REG0 0x304 +#define DMAC_DEST_START_ADDRR_REG0 0x308 +#define DMAC_DDMA_BC_REG0 0x30C +#define DMAC_DDMA_PARA_REG0 0x318 + +#define DMAC_DDMA_CFG_REG_LOADING (1 << 31) +#define DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 (2 << 25) +#define DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 (2 << 9) +#define DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO (1 << 5) +#define DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC (3 << 0) + +#define DMAC_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0) +#define DMAC_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8) + +#define NANDFLASHC_CTL 0x00000000 + +#define NFC_CTL_EN (1 << 0) +#define NFC_CTL_RESET (1 << 1) +#define NFC_CTL_RAM_METHOD (1 << 14) + +#define NANDFLASHC_ST 0x00000004 +#define NANDFLASHC_INT 0x00000008 +#define NANDFLASHC_TIMING_CTL 0x0000000C +#define NANDFLASHC_TIMING_CFG 0x00000010 +#define NANDFLASHC_ADDR_LOW 0x00000014 +#define NANDFLASHC_ADDR_HIGH 0x00000018 +#define NANDFLASHC_SECTOR_NUM 0x0000001C +#define NANDFLASHC_CNT 0x00000020 +#define NANDFLASHC_CMD 0x00000024 +#define NANDFLASHC_RCMD_SET 0x00000028 +#define NANDFLASHC_WCMD_SET 0x0000002C +#define NANDFLASHC_IO_DATA 0x00000030 +#define NANDFLASHC_ECC_CTL 0x00000034 + +#define NFC_ECC_EN (1 << 0) +#define NFC_ECC_PIPELINE (1 << 3) +#define NFC_ECC_EXCEPTION (1 << 4) +#define NFC_ECC_BLOCK_SIZE (1 << 5) +#define NFC_ECC_RANDOM_EN (1 << 9) +#define NFC_ECC_RANDOM_DIRECTION (1 << 10) + +#define NANDFLASHC_ECC_ST 0x00000038 +#define NANDFLASHC_DEBUG 0x0000003C +#define NANDFLASHC_ECC_CNT0 0x00000040 +#define NANDFLASHC_ECC_CNT1 0x00000044 +#define NANDFLASHC_ECC_CNT2 0x00000048 +#define NANDFLASHC_ECC_CNT3 0x0000004C +#define NANDFLASHC_USER_DATA_BASE 0x00000050 +#define NANDFLASHC_EFNAND_STATUS 0x00000090 +#define NANDFLASHC_SPARE_AREA 0x000000A0 +#define NANDFLASHC_PATTERN_ID 0x000000A4 +#define NANDFLASHC_RAM0_BASE 0x00000400 +#define NANDFLASHC_RAM1_BASE 0x00000800 + +#define NFC_SEND_ADR (1 << 19) +#define NFC_ACCESS_DIR (1 << 20) +#define NFC_DATA_TRANS (1 << 21) +#define NFC_SEND_CMD1 (1 << 22) +#define NFC_WAIT_FLAG (1 << 23) +#define NFC_SEND_CMD2 (1 << 24) +#define NFC_SEQ (1 << 25) +#define NFC_DATA_SWAP_METHOD (1 << 26) +#define NFC_ROW_AUTO_INC (1 << 27) +#define NFC_SEND_CMD3 (1 << 28) +#define NFC_SEND_CMD4 (1 << 29) + +#define NFC_PAGE_CMD (2 << 30) + +#define SPL_WRITE_SIZE CONFIG_CMD_SPL_WRITE_SIZE + +/* random seed used by linux */ +const uint16_t random_seed[128] = { + 0x2b75, 0x0bd0, 0x5ca3, 0x62d1, 0x1c93, 0x07e9, 0x2162, 0x3a72, + 0x0d67, 0x67f9, 0x1be7, 0x077d, 0x032f, 0x0dac, 0x2716, 0x2436, + 0x7922, 0x1510, 0x3860, 0x5287, 0x480f, 0x4252, 0x1789, 0x5a2d, + 0x2a49, 0x5e10, 0x437f, 0x4b4e, 0x2f45, 0x216e, 0x5cb7, 0x7130, + 0x2a3f, 0x60e4, 0x4dc9, 0x0ef0, 0x0f52, 0x1bb9, 0x6211, 0x7a56, + 0x226d, 0x4ea7, 0x6f36, 0x3692, 0x38bf, 0x0c62, 0x05eb, 0x4c55, + 0x60f4, 0x728c, 0x3b6f, 0x2037, 0x7f69, 0x0936, 0x651a, 0x4ceb, + 0x6218, 0x79f3, 0x383f, 0x18d9, 0x4f05, 0x5c82, 0x2912, 0x6f17, + 0x6856, 0x5938, 0x1007, 0x61ab, 0x3e7f, 0x57c2, 0x542f, 0x4f62, + 0x7454, 0x2eac, 0x7739, 0x42d4, 0x2f90, 0x435a, 0x2e52, 0x2064, + 0x637c, 0x66ad, 0x2c90, 0x0bad, 0x759c, 0x0029, 0x0986, 0x7126, + 0x1ca7, 0x1605, 0x386a, 0x27f5, 0x1380, 0x6d75, 0x24c3, 0x0f8e, + 0x2b7a, 0x1418, 0x1fd1, 0x7dc1, 0x2d8e, 0x43af, 0x2267, 0x7da3, + 0x4e3d, 0x1338, 0x50db, 0x454d, 0x764d, 0x40a3, 0x42e6, 0x262b, + 0x2d2e, 0x1aea, 0x2e17, 0x173d, 0x3a6e, 0x71bf, 0x25f9, 0x0a5d, + 0x7c57, 0x0fbe, 0x46ce, 0x4939, 0x6b17, 0x37bb, 0x3e91, 0x76db, +}; + +/* random seed used for syndrome calls */ +const uint16_t random_seed_syndrome = 0x4a80; + +#endif /* end of include guard: NAND_H */
On Thu, 2015-07-16 at 13:25 +0200, Piotr Zierhoffer wrote:
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
This driver adds NAND support to both SPL and full U-Boot. It was tested on AllWinner A20.
It looks a lot like an SPL-only driver to me...
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com
board/sunxi/Makefile | 1 + board/sunxi/nand.c | 315 +++++++++++++++++++++++++++++++++++++++++++++++++++ board/sunxi/nand.h | 146 ++++++++++++++++++++++++ 3 files changed, 462 insertions(+) create mode 100644 board/sunxi/nand.c create mode 100644 board/sunxi/nand.h
Please keep NAND drivers in drivers/mtd/nand/.
diff --git a/board/sunxi/Makefile b/board/sunxi/Makefile index 43766e0..6f1d571 100644 --- a/board/sunxi/Makefile +++ b/board/sunxi/Makefile @@ -9,6 +9,7 @@ # SPDX-License-Identifier: GPL-2.0+ # obj-y += board.o +obj-$(CONFIG_SUNXI_NAND) += nand.o obj-$(CONFIG_SUNXI_GMAC) += gmac.o obj-$(CONFIG_SUNXI_AHCI) += ahci.o obj-$(CONFIG_MACH_SUN4I) += dram_sun4i_auto.o diff --git a/board/sunxi/nand.c b/board/sunxi/nand.c new file mode 100644 index 0000000..16e8e4d --- /dev/null +++ b/board/sunxi/nand.c @@ -0,0 +1,315 @@ +/*
- Copyright (c) 2014-2015, Antmicro Ltd <www.antmicro.com>
- Copyright (c) 2015, AW-SOM Technologies <www.aw-som.com>
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <config.h> +#include <asm/io.h> +#include <nand.h> +#include "nand.h" /* sunxi specific header */
+/* minimal "boot0" style NAND support for Allwinner A20 */
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
+#define MAX_RETRIES 10
+static int check_value_inner(int offset, int expected_bits,
int max_number_of_retries, int negation)+{
int retries = 0;do {int val = readl(offset) & expected_bits;if (negation ? !val : val)return 1;mdelay(1);retries++;} while (retries < max_number_of_retries);return 0;+}
+static inline int check_value(int offset, int expected_bits,
int max_number_of_retries)+{
return check_value_inner(offset, expected_bits,max_number_of_retries, 0);+}
+static inline int check_value_negated(int offset, int unexpected_bits,
int max_number_of_retries)+{
return check_value_inner(offset, unexpected_bits,max_number_of_retries, 1);+}
+void nand_set_clocks(void) +{
Here and elsewhere, please either use static or less generic name.
+int nand_initialized;
static bool nand_initialized;
Or better yet, get rid of this and stop using init-on-first-use.
+void nand_init(void) +{
board_nand_init(NULL);+}
nand_init() is defined in drivers/mtd/nand/nand.c, which is only optional for SPL -- and I don't see an SPL ifdef here.
+uint32_t ecc_errors;
static
+/* read 0x400 bytes from real_addr to temp_buf */ +void nand_read_block(unsigned int real_addr, int syndrome)
I don't know of any NAND chips whose erase blocks are as small as 0x400 bytes. The read/program unit is called a page, not a block.
Is there a reason to hardcode the page size here? Especially since the comment doesn't appear to match the code, which uses SPL_WRITE_SIZE.
+{
uint32_t val;int ECC_OFF = 0;
Why is this capitalized?
uint16_t ecc_mode = 0;uint16_t rand_seed;uint32_t page;uint16_t column;uint32_t oob_offset;if (!nand_initialized)board_nand_init(NULL);
board_nand_init() is called from nand_init(). Why are you calling it here?
switch (CONFIG_SUNXI_ECC_STRENGTH) {
No documentation for this symbol.
If it's an attribute of the hardware rather than something the user selects, it should be CONFIG_SYS_SUNXI_ECC_STRENGTH, or just SUNXI_ECC_STRENGTH if it doesn't need to integrate with kconfig or makefiles.
/* clear temp_buf */memset((void *)temp_buf, 0, SPL_WRITE_SIZE);
Unnecessary cast.
/* set CMD */writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | 0xff,NANDFLASHC_BASE + NANDFLASHC_CMD);if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 1),MAX_RETRIES)) {printf("Error while initilizing command interrupt\n");return;}page = (real_addr / BLOCK_SIZE);
Unnecessary parens (and inconsistent with the following line).
column = real_addr % BLOCK_SIZE;if (syndrome) {/* shift every 1kB in syndrome */column += (column / SPL_WRITE_SIZE) * ECC_OFF;}/* clear ecc status */writel(0, NANDFLASHC_BASE + NANDFLASHC_ECC_ST);/* Choose correct seed */if (syndrome)rand_seed = random_seed_syndrome;elserand_seed = random_seed[page % 128];writel((rand_seed << 16) | NFC_ECC_RANDOM_EN | NFC_ECC_EN| NFC_ECC_PIPELINE | (ecc_mode << 12),NANDFLASHC_BASE + NANDFLASHC_ECC_CTL);val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL);writel(val | NFC_CTL_RAM_METHOD, NANDFLASHC_BASE + NANDFLASHC_CTL);if (syndrome) {writel(SPL_WRITE_SIZE, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA);} else {oob_offset = BLOCK_SIZE + (column / SPL_WRITE_SIZE) * ECC_OFF;writel(oob_offset, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA);}/* DMAC */writel(0x0, DMAC_BASE + DMAC_CFG_REG0); /* clr dma cmd *//* read from REG_IO_DATA */writel(NANDFLASHC_BASE + NANDFLASHC_IO_DATA,DMAC_BASE + DMAC_SRC_START_ADDR_REG0);writel((uint32_t)temp_buf,DMAC_BASE + DMAC_DEST_START_ADDRR_REG0); /* read to RAM */writel(DMAC_DDMA_PARA_REG_SRC_WAIT_CYC| DMAC_DDMA_PARA_REG_SRC_BLK_SIZE,DMAC_BASE + DMAC_DDMA_PARA_REG0);writel(SPL_WRITE_SIZE, DMAC_BASE + DMAC_DDMA_BC_REG0); /* 1kB */writel(DMAC_DDMA_CFG_REG_LOADING| DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32| DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32| DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO| DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC,DMAC_BASE + DMAC_CFG_REG0);writel(0x00E00530, NANDFLASHC_BASE + NANDFLASHC_RCMD_SET); /* READ */
What is 0x00e00530? Define it symbolically.
writel(1, NANDFLASHC_BASE + NANDFLASHC_SECTOR_NUM);writel(((page & 0xFFFF) << 16) | column,NANDFLASHC_BASE + NANDFLASHC_ADDR_LOW);writel((page >> 16) & 0xFF, NANDFLASHC_BASE + NANDFLASHC_ADDR_HIGH);writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS |NFC_PAGE_CMD | NFC_WAIT_FLAG | /* ADDR_CYCLE */ (4 << 16) |
Why is ADDR_CYCLE commented out, and a magic number used in its place?
+int helper_load(uint32_t offs, unsigned int size, void *dest)
This name is not clear (in addition to being way too vague for a global symbol).
+{
uint32_t dst;
Don't have both "dest" and "dst". It's confusing.
uint32_t count;uint32_t count_c;
What does the "_c" mean?
uint32_t adr = offs;
So address is actually a NAND offset, not a memory address? Confusing.
Don't put addresses in uint32_t (even if it doesn't make a difference on this platform, it's a bad habit and a bad example to others). Use a pointer for virtual addresses, and phys_addr_t for physical addresses.
memset((void *)dest, 0x0, size); /* clean destination memory */
Unnecessary cast.
ecc_errors = 0;for (dst = (uint32_t)dest;dst < ((uint32_t)dest + size);dst += SPL_WRITE_SIZE) {/* if < 0x400000 then syndrome read */nand_read_block(adr, adr < 0x400000);
What does 0x400000 represent?
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{
helper_load(offs, size, dest);return 0;+}
Why didn't you just call helper_load() nand_spl_load_image()?
+void nand_deselect(void) {} diff --git a/board/sunxi/nand.h b/board/sunxi/nand.h new file mode 100644 index 0000000..b3c1e93 --- /dev/null +++ b/board/sunxi/nand.h @@ -0,0 +1,146 @@ +#ifndef NAND_H
+#define NAND_H
Don't use such a generic ifdef protector in a non-generic file. It's just chance that include/nand.h is using _NAND_H_ instead of NAND_H...
-Scott
On Thursday, July 16, 2015 at 11:15:58 PM, Scott Wood wrote:
[...]
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
The DMA I believe accesses it via PA anyway, so mapping it elsewhere would only confuse everyone who's hacking on the driver. Just my 5 cents ;-)
[...]
Best regards, Marek Vasut
On Thu, 2015-07-16 at 23:26 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:15:58 PM, Scott Wood wrote:
[...]
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
The DMA I believe accesses it via PA anyway, so mapping it elsewhere would only confuse everyone who's hacking on the driver. Just my 5 cents ;-)
Hey, if it wakes people up to the fact that they ought to be using virt_to_phys() (or better yet, something that distinguishes DMA addresses from physical), great! :-)
And yes, people go on to do the same "everything is u32" crap in non-platform- specific files. Just look at drivers/block/ahci.c.
-Scott
On Thursday, July 16, 2015 at 11:36:08 PM, Scott Wood wrote:
On Thu, 2015-07-16 at 23:26 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:15:58 PM, Scott Wood wrote:
[...]
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
The DMA I believe accesses it via PA anyway, so mapping it elsewhere would only confuse everyone who's hacking on the driver. Just my 5 cents ;-)
Hey, if it wakes people up to the fact that they ought to be using virt_to_phys() (or better yet, something that distinguishes DMA addresses from physical), great! :-)
Not really useful here, but whatever.
And yes, people go on to do the same "everything is u32" crap in non-platform- specific files. Just look at drivers/block/ahci.c.
Errr, this file is platform specific.
Best regards, Marek Vasut
On Fri, 2015-07-17 at 00:06 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:36:08 PM, Scott Wood wrote:
On Thu, 2015-07-16 at 23:26 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:15:58 PM, Scott Wood wrote:
[...]
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
The DMA I believe accesses it via PA anyway, so mapping it elsewhere would only confuse everyone who's hacking on the driver. Just my 5 cents ;-)
Hey, if it wakes people up to the fact that they ought to be using virt_to_phys() (or better yet, something that distinguishes DMA addresses from physical), great! :-)
Not really useful here, but whatever.
And yes, people go on to do the same "everything is u32" crap in non-platform- specific files. Just look at drivers/block/ahci.c.
Errr, this file is platform specific.
(I'm assuming that by "this file" you mean the sunxi file, not the ahci file.)
The point is that establishing good habits everywhere reduces the likelihood of the bad habits migrating to places where things break.
And no, I don't seriously expect my suggestion of remapping the virtual address to be implemented, but GCC has been known to do all sorts of weird stuff when it thinks it knows a NULL pointer is being dereferenced.
-Scott
On Friday, July 17, 2015 at 12:12:15 AM, Scott Wood wrote:
On Fri, 2015-07-17 at 00:06 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:36:08 PM, Scott Wood wrote:
On Thu, 2015-07-16 at 23:26 +0200, Marek Vasut wrote:
On Thursday, July 16, 2015 at 11:15:58 PM, Scott Wood wrote:
[...]
+/* temporary buffer in internal ram */ +#ifdef CONFIG_SPL_BUILD +/* in SPL temporary buffer cannot be @ 0x0 */ +unsigned char temp_buf[SPL_WRITE_SIZE] __aligned(0x10) __section(".text#"); +#else +/* put temporary buffer @ 0x0 */ +unsigned char *temp_buf = (unsigned char *)0x0; +#endif
If 0x0 is the address of an SRAM, its address should be symbolically defined. Also consider mapping it to a different virtual address, to avoid potential compiler mischief.
The DMA I believe accesses it via PA anyway, so mapping it elsewhere would only confuse everyone who's hacking on the driver. Just my 5 cents ;-)
Hey, if it wakes people up to the fact that they ought to be using virt_to_phys() (or better yet, something that distinguishes DMA addresses from physical), great! :-)
Not really useful here, but whatever.
And yes, people go on to do the same "everything is u32" crap in non-platform- specific files. Just look at drivers/block/ahci.c.
Errr, this file is platform specific.
(I'm assuming that by "this file" you mean the sunxi file, not the ahci file.)
The point is that establishing good habits everywhere reduces the likelihood of the bad habits migrating to places where things break.
And no, I don't seriously expect my suggestion of remapping the virtual address to be implemented, but GCC has been known to do all sorts of weird stuff when it thinks it knows a NULL pointer is being dereferenced.
OK, you have a point there.
Best regards, Marek Vasut
2015-07-16 23:15 GMT+02:00 Scott Wood scottwood@freescale.com:
On Thu, 2015-07-16 at 13:25 +0200, Piotr Zierhoffer wrote:
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
This driver adds NAND support to both SPL and full U-Boot. It was tested on AllWinner A20.
It looks a lot like an SPL-only driver to me...
I am preparing a v2 patch set, that will have SPL-only code.
writel(0x00E00530, NANDFLASHC_BASE + NANDFLASHC_RCMD_SET); /* READ */What is 0x00e00530? Define it symbolically.
It's difficult because we do not have much explanation for this constant. This is how BROM operates. We have tuned it according to http://linux-sunxi.org/NFC_Register_Guide
We have fixed the code according to your comments and will resubmit it shortly.
Best regards
Piotr Zierhoffer Antmicro Ltd | www.antmicro.com
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
When SUNXI_NAND option is selected in config, set some configuration options for sunxi NAND.
This commit also introduces the configurable options in Kconfig.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com ---
board/sunxi/Kconfig | 10 ++++++++++ include/configs/sunxi-common.h | 11 +++++++++++ 2 files changed, 21 insertions(+)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 2a1cd3c..3b76d64 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -227,6 +227,16 @@ config OLD_SUNXI_KERNEL_COMPAT Set this to enable various workarounds for old kernels, this results in sub-optimal settings for newer kernels, only enable if needed.
+config SUNXI_NAND + bool "Support for NAND on Allwinner A20" + depends on MACH_SUN7I + ---help--- + Enable support for internal NAND. This option allows U-Boot to read from + sunxi NAND using DMA transfers. It also adds the a20_nandread command + that allows user to transfer a specified amount of data from NAND to + memory. Both SPL and full U-Boot driver are enabled. Writing is not + supported. + config MMC0_CD_PIN string "Card detect pin for mmc0" default "" diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 9576bc1..83922ab 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -139,6 +139,17 @@ #define CONFIG_INITRD_TAG #define CONFIG_SERIAL_TAG
+#if defined(CONFIG_SUNXI_NAND) +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_SUPPORT +#define CONFIG_CMD_SPL_WRITE_SIZE 0x000400 /* 1024 byte */ + +#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000 +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x008000 +#define CONFIG_SYS_NAND_PAGE_SIZE 0x002000 /* 8kb*/ +#define CONFIG_SUNXI_ECC_STRENGTH 40 +#endif + /* mmc config */ #if !defined(CONFIG_UART0_PORT_F) #define CONFIG_MMC
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
The usage of the command is:
a20_nandread <address> <offset> <bytes>
It allows user to copy data from NAND to memory. It employs nand_spl_load_image from the sunxi NAND driver.
It is added only when the NAND support is enabled.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com ---
common/Makefile | 1 + common/cmd_a20_nandread.c | 25 +++++++++++++++++++++++++ include/configs/sunxi-common.h | 2 ++ 3 files changed, 28 insertions(+) create mode 100644 common/cmd_a20_nandread.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..ef31646 100644 --- a/common/Makefile +++ b/common/Makefile @@ -126,6 +126,7 @@ obj-$(CONFIG_ID_EEPROM) += cmd_mac.o obj-$(CONFIG_CMD_MD5SUM) += cmd_md5sum.o obj-$(CONFIG_CMD_MEMORY) += cmd_mem.o obj-$(CONFIG_CMD_IO) += cmd_io.o +obj-$(CONFIG_CMD_A20_NANDREAD) += cmd_a20_nandread.o obj-$(CONFIG_CMD_MFSL) += cmd_mfsl.o obj-$(CONFIG_MII) += miiphyutil.o obj-$(CONFIG_CMD_MII) += miiphyutil.o diff --git a/common/cmd_a20_nandread.c b/common/cmd_a20_nandread.c new file mode 100644 index 0000000..6469535 --- /dev/null +++ b/common/cmd_a20_nandread.c @@ -0,0 +1,25 @@ +#include <common.h> +#include <command.h> +#include <nand.h> + +static int do_a20_nandread(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + if (argc != 4) { + printf("usage: a20_nandread <destination> <source> <size>\n"); + return 1; + } + + uint32_t dst = simple_strtoul(argv[1], NULL, 16); + uint32_t src = simple_strtoul(argv[2], NULL, 16); + uint32_t cnt = simple_strtoul(argv[3], NULL, 16); + printf("Reading 0x%08X bytes from NAND @ 0x%08X to MEM @ 0x%08X...\n", + cnt, src, dst); + nand_spl_load_image(src, cnt, (void *)dst); + printf("\n"); + return 0; +} + +U_BOOT_CMD(a20_nandread, CONFIG_SYS_MAXARGS, 3, + do_a20_nandread, "a20_nandread", "[destination source size]\n" " " +); diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 83922ab..e9cfa9a 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -148,6 +148,8 @@ #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x008000 #define CONFIG_SYS_NAND_PAGE_SIZE 0x002000 /* 8kb*/ #define CONFIG_SUNXI_ECC_STRENGTH 40 + +#define CONFIG_CMD_A20_NANDREAD #endif
/* mmc config */
On Thu, 2015-07-16 at 13:25 +0200, Piotr Zierhoffer wrote:
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
The usage of the command is:
a20_nandread <address> <offset> <bytes>
It allows user to copy data from NAND to memory. It employs nand_spl_load_image from the sunxi NAND driver.
It is added only when the NAND support is enabled.
Please respond to the comments received when it was posted at http://patchwork.ozlabs.org/patch/466149/
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com
Why does the same code now have a completely different author and sign-off chain?
-Scott
Hello
2015-07-16 23:20 GMT+02:00 Scott Wood scottwood@freescale.com:
On Thu, 2015-07-16 at 13:25 +0200, Piotr Zierhoffer wrote:
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com
Why does the same code now have a completely different author and sign-off chain?
-Scott
This code had a wrong copyright/Signed-off-by chain, and was reverted in da9971d1b3bdb554d4a4ac948119f8b2616bbcce.
I am going to resubmit a v2 patch set without this command.
Best regards
Piotr Zierhoffer Antmicro Ltd | www.antmicro.com
As SPL does not know which source to choose when booting U-Boot, choose NAND if it is capable of doing so.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com
---
arch/arm/cpu/armv7/sunxi/board.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index 5f39aa0..e4b7d63 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -128,6 +128,9 @@ void s_init(void) */ u32 spl_boot_device(void) { +#ifdef CONFIG_SPL_NAND_SUPPORT + return BOOT_DEVICE_NAND; +#else /* * When booting from the SD card, the "eGON.BT0" signature is expected * to be found in memory at the address 0x0004 (see the "mksunxiboot" @@ -148,6 +151,7 @@ u32 spl_boot_device(void) return BOOT_DEVICE_MMC1; else return BOOT_DEVICE_BOARD; +#endif }
/* No confirmation data available in SPL yet. Hardcode bootmode */
This is a basic driver for the sunxi NAND controller for Allwinner A20. It supports only SPL.
The driver uses DMA for data transfers. It does not support writing.
Changes in v2: - removed traces of non-SPL-specific code - moved the driver from boards/sunxi to drivers/mtd/nand - moved magic values to defines (whenever possible) - removed unnecesary late initialisation code - code style changes as suggested for the first patch set: - changed visibility of some symbols - renamed unclear variables - renamed header protector - changed types of pointer variables - other minor changes - renamed defines to be more relevant - moved Kconfig entry for the driver to drivers/mtd/nand - reworded Kconfig entry help
Piotr Zierhoffer (3): sunxi: nand: Add basic sunxi NAND driver for SPL with DMA support sunxi: nand: Add board configuration options sunxi: nand: Add information to sunxi that it was run from NAND in SPL
arch/arm/cpu/armv7/sunxi/board.c | 4 + drivers/mtd/nand/Kconfig | 7 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 289 +++++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/sunxi_nand.h | 151 ++++++++++++++++++++ include/configs/sunxi-common.h | 11 ++ 6 files changed, 463 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.c create mode 100644 drivers/mtd/nand/sunxi_nand.h
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
This driver adds NAND support to SPL. It was tested on AllWinner A20.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com Signed-off-by: Karol Gugala kgugala@antmicro.com ---
Changes in v2: - removed traces of non-SPL-specific code - moved the driver from boards/sunxi to drivers/mtd/nand - moved magic values to defines (whenever possible) - removed unnecesary late initialisation code - code style changes as suggested for the first patch set: - changed visibility of some symbols - renamed unclear variables - renamed header protector - changed types of pointer variables - other minor changes
drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 289 ++++++++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/sunxi_nand.h | 151 ++++++++++++++++++++++ 3 files changed, 441 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.c create mode 100644 drivers/mtd/nand/sunxi_nand.h
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 347ea62..4cf9cee 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -12,6 +12,7 @@ NORMAL_DRIVERS=y endif
obj-$(CONFIG_SPL_NAND_AM33XX_BCH) += am335x_spl_bch.o +obj-$(CONFIG_SPL_NAND_SUNXI) += sunxi_nand.o obj-$(CONFIG_SPL_NAND_DENALI) += denali_spl.o obj-$(CONFIG_SPL_NAND_DOCG4) += docg4_spl.o obj-$(CONFIG_SPL_NAND_SIMPLE) += nand_spl_simple.o diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c new file mode 100644 index 0000000..8d66d2b --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.c @@ -0,0 +1,289 @@ +/* + * Copyright (c) 2014-2015, Antmicro Ltd <www.antmicro.com> + * Copyright (c) 2015, AW-SOM Technologies <www.aw-som.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <config.h> +#include <asm/io.h> +#include <nand.h> +#include "sunxi_nand.h" + +/* minimal "boot0" style NAND support for Allwinner A20 */ + +/* temporary buffer in internal ram */ +unsigned char temp_buf[CONFIG_SYS_NAND_PAGE_SIZE] + __aligned(0x10) __section(".text#"); + +#define MAX_RETRIES 10 + +static int check_value_inner(int offset, int expected_bits, + int max_number_of_retries, int negation) +{ + int retries = 0; + do { + int val = readl(offset) & expected_bits; + if (negation ? !val : val) + return 1; + mdelay(1); + retries++; + } while (retries < max_number_of_retries); + + return 0; +} + +static inline int check_value(int offset, int expected_bits, + int max_number_of_retries) +{ + return check_value_inner(offset, expected_bits, + max_number_of_retries, 0); +} + +static inline int check_value_negated(int offset, int unexpected_bits, + int max_number_of_retries) +{ + return check_value_inner(offset, unexpected_bits, + max_number_of_retries, 1); +} + +static void nand_set_clocks(void) +{ + uint32_t val; + + writel(PORTC_PC_CFG0_NRB1 | + PORTC_PC_CFG0_NRB0 | + PORTC_PC_CFG0_NRE | + PORTC_PC_CFG0_NCE0 | + PORTC_PC_CFG0_NCE1 | + PORTC_PC_CFG0_NCLE | + PORTC_PC_CFG0_NALE | + PORTC_PC_CFG0_NWE, PORTC_BASE + PORTC_PC_CFG0); + + writel(PORTC_PC_CFG1_NDQ7 | + PORTC_PC_CFG1_NDQ6 | + PORTC_PC_CFG1_NDQ5 | + PORTC_PC_CFG1_NDQ4 | + PORTC_PC_CFG1_NDQ3 | + PORTC_PC_CFG1_NDQ2 | + PORTC_PC_CFG1_NDQ1 | + PORTC_PC_CFG1_NDQ0, PORTC_BASE + PORTC_PC_CFG1); + + writel(PORTC_PC_CFG2_NCE7 | + PORTC_PC_CFG2_NCE6 | + PORTC_PC_CFG2_NCE5 | + PORTC_PC_CFG2_NCE4 | + PORTC_PC_CFG2_NCE3 | + PORTC_PC_CFG2_NCE2 | + PORTC_PC_CFG2_NWP, PORTC_BASE + PORTC_PC_CFG2); + + writel(PORTC_PC_CFG3_NDQS, PORTC_BASE + PORTC_PC_CFG3); + + val = readl(CCU_BASE + CCU_AHB_GATING_REG0); + writel(CCU_AHB_GATING_REG0_NAND | val, CCU_BASE + CCU_AHB_GATING_REG0); + + val = readl(CCU_BASE + CCU_NAND_SCLK_CFG_REG); + writel(val | CCU_NAND_SCLK_CFG_REG_SCLK_GATING + | CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO, + CCU_BASE + CCU_NAND_SCLK_CFG_REG); +} + +void nand_init(void) +{ + uint32_t val; + + nand_set_clocks(); + val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL); + /* enable and reset CTL */ + writel(val | NFC_CTL_EN | NFC_CTL_RESET, + NANDFLASHC_BASE + NANDFLASHC_CTL); + + if (!check_value_negated(NANDFLASHC_BASE + NANDFLASHC_CTL, + NFC_CTL_RESET, MAX_RETRIES)) { + printf("Couldn't initialize nand\n"); + } +} + +static uint32_t ecc_errors; + +void nand_read_page(unsigned int real_addr, int syndrome) +{ + uint32_t val; + int ecc_off = 0; + uint16_t ecc_mode = 0; + uint16_t rand_seed; + uint32_t page; + uint16_t column; + uint32_t oob_offset; + + switch (SUNXI_ECC_STRENGTH) { + case 16: + ecc_mode = 0; + ecc_off = 0x20; + break; + case 24: + ecc_mode = 1; + ecc_off = 0x2e; + break; + case 28: + ecc_mode = 2; + ecc_off = 0x32; + break; + case 32: + ecc_mode = 3; + ecc_off = 0x3c; + break; + case 40: + ecc_mode = 4; + ecc_off = 0x4a; + break; + case 48: + ecc_mode = 4; + ecc_off = 0x52; + break; + case 56: + ecc_mode = 4; + ecc_off = 0x60; + break; + case 60: + ecc_mode = 4; + ecc_off = 0x0; + break; + case 64: + ecc_mode = 4; + ecc_off = 0x0; + break; + default: + ecc_mode = 0; + ecc_off = 0; + } + + if (ecc_off == 0) { + printf("Unsupported ECC strength (%d)!\n", + SUNXI_ECC_STRENGTH); + return; + } + + /* clear temp_buf */ + memset(temp_buf, 0, CONFIG_SYS_NAND_PAGE_SIZE); + + /* set CMD */ + writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | 0xff, + NANDFLASHC_BASE + NANDFLASHC_CMD); + + if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 1), + MAX_RETRIES)) { + printf("Error while initilizing command interrupt\n"); + return; + } + + page = real_addr / CONFIG_SYS_NAND_BLOCK_SIZE; + column = real_addr % CONFIG_SYS_NAND_BLOCK_SIZE; + + if (syndrome) { + /* shift every 1kB in syndrome */ + column += (column / CONFIG_SYS_NAND_PAGE_SIZE) * ecc_off; + } + + /* clear ecc status */ + writel(0, NANDFLASHC_BASE + NANDFLASHC_ECC_ST); + + /* Choose correct seed */ + if (syndrome) + rand_seed = random_seed_syndrome; + else + rand_seed = random_seed[page % 128]; + + writel((rand_seed << 16) | NFC_ECC_RANDOM_EN | NFC_ECC_EN + | NFC_ECC_PIPELINE | (ecc_mode << 12), + NANDFLASHC_BASE + NANDFLASHC_ECC_CTL); + + val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL); + writel(val | NFC_CTL_RAM_METHOD, NANDFLASHC_BASE + NANDFLASHC_CTL); + + if (syndrome) { + writel(CONFIG_SYS_NAND_PAGE_SIZE, + NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA); + } else { + oob_offset = CONFIG_SYS_NAND_BLOCK_SIZE + + (column / CONFIG_SYS_NAND_PAGE_SIZE) * ecc_off; + writel(oob_offset, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA); + } + + /* DMAC */ + writel(0x0, DMAC_BASE + DMAC_CFG_REG0); /* clr dma cmd */ + /* read from REG_IO_DATA */ + writel(NANDFLASHC_BASE + NANDFLASHC_IO_DATA, + DMAC_BASE + DMAC_SRC_START_ADDR_REG0); + writel((uint32_t)temp_buf, + DMAC_BASE + DMAC_DEST_START_ADDRR_REG0); /* read to RAM */ + writel(DMAC_DDMA_PARA_REG_SRC_WAIT_CYC + | DMAC_DDMA_PARA_REG_SRC_BLK_SIZE, + DMAC_BASE + DMAC_DDMA_PARA_REG0); + writel(CONFIG_SYS_NAND_PAGE_SIZE, + DMAC_BASE + DMAC_DDMA_BC_REG0); /* 1kB */ + writel(DMAC_DDMA_CFG_REG_LOADING + | DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 + | DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 + | DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO + | DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC, + DMAC_BASE + DMAC_CFG_REG0); + + writel((0xE0 << NFC_RANDOM_READ_CMD1_OFFSET) + | (0x05 << NFC_RANDOM_READ_CMD0_OFFSET) + | (0x30 | NFC_READ_CMD_OFFSET), NANDFLASHC_BASE + + NANDFLASHC_RCMD_SET); + writel(1, NANDFLASHC_BASE + NANDFLASHC_SECTOR_NUM); + writel(((page & 0xFFFF) << 16) | column, + NANDFLASHC_BASE + NANDFLASHC_ADDR_LOW); + writel((page >> 16) & 0xFF, NANDFLASHC_BASE + NANDFLASHC_ADDR_HIGH); + writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS | + NFC_PAGE_CMD | NFC_WAIT_FLAG | (4 << NFC_ADDR_NUM_OFFSET) | + NFC_SEND_ADR | NFC_DATA_SWAP_METHOD | (syndrome ? NFC_SEQ : 0), + NANDFLASHC_BASE + NANDFLASHC_CMD); + + if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 2), + MAX_RETRIES)) { + printf("Error while initializing dma interrupt\n"); + return; + } + + if (!check_value_negated(DMAC_BASE + DMAC_CFG_REG0, + DMAC_DDMA_CFG_REG_LOADING, MAX_RETRIES)) { + printf("Error while waiting for dma transfer to finish\n"); + return; + } + + if (readl(NANDFLASHC_BASE + NANDFLASHC_ECC_ST)) + ecc_errors++; +} + +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{ + void *current_dest; + uint32_t count; + uint32_t current_count; + + memset(dest, 0x0, size); /* clean destination memory */ + ecc_errors = 0; + for (current_dest = dest; + current_dest < (dest + size); + current_dest += CONFIG_SYS_NAND_PAGE_SIZE) { + nand_read_page(offs, offs < SYNDROME_PARTITIONS_END); + count = current_dest - dest; + + if (size - count > CONFIG_SYS_NAND_PAGE_SIZE) + current_count = CONFIG_SYS_NAND_PAGE_SIZE; + else + current_count = size - count; + + memcpy(current_dest, + temp_buf, + current_count); + offs += CONFIG_SYS_NAND_PAGE_SIZE; + } + return ecc_errors; +} + +void nand_deselect(void) {} diff --git a/drivers/mtd/nand/sunxi_nand.h b/drivers/mtd/nand/sunxi_nand.h new file mode 100644 index 0000000..d24f0ef --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.h @@ -0,0 +1,151 @@ +#ifndef SUNXI_NAND_H + +#define SUNXI_NAND_H + +#define PORTC_BASE 0x01c20800 +#define CCU_BASE 0x01c20000 +#define NANDFLASHC_BASE 0x01c03000 +#define DMAC_BASE 0x01c02000 + +#define SYNDROME_PARTITIONS_END 0x00400000 +#define SUNXI_ECC_STRENGTH 40 + +#define CCU_AHB_GATING_REG0 0x60 +#define CCU_NAND_SCLK_CFG_REG 0x80 +#define CCU_AHB_GATING_REG0_NAND (1 << 13) + +#define CCU_NAND_SCLK_CFG_REG_SCLK_GATING (1 << 31) +#define CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO (1 << 0) + +#define PORTC_PC_CFG0 0x48 +#define PORTC_PC_CFG1 0x4C +#define PORTC_PC_CFG2 0x50 +#define PORTC_PC_CFG3 0x54 + +#define PORTC_PC_CFG0_NRB1 (2 << 28) +#define PORTC_PC_CFG0_NRB0 (2 << 24) +#define PORTC_PC_CFG0_NRE (2 << 20) +#define PORTC_PC_CFG0_NCE0 (2 << 16) +#define PORTC_PC_CFG0_NCE1 (2 << 12) +#define PORTC_PC_CFG0_NCLE (2 << 8) +#define PORTC_PC_CFG0_NALE (2 << 4) +#define PORTC_PC_CFG0_NWE (2 << 0) + +#define PORTC_PC_CFG1_NDQ7 (2 << 28) +#define PORTC_PC_CFG1_NDQ6 (2 << 24) +#define PORTC_PC_CFG1_NDQ5 (2 << 20) +#define PORTC_PC_CFG1_NDQ4 (2 << 16) +#define PORTC_PC_CFG1_NDQ3 (2 << 12) +#define PORTC_PC_CFG1_NDQ2 (2 << 8) +#define PORTC_PC_CFG1_NDQ1 (2 << 4) +#define PORTC_PC_CFG1_NDQ0 (2 << 0) + +#define PORTC_PC_CFG2_NCE7 (2 << 24) +#define PORTC_PC_CFG2_NCE6 (2 << 20) +#define PORTC_PC_CFG2_NCE5 (2 << 16) +#define PORTC_PC_CFG2_NCE4 (2 << 12) +#define PORTC_PC_CFG2_NCE3 (2 << 8) +#define PORTC_PC_CFG2_NCE2 (2 << 4) +#define PORTC_PC_CFG2_NWP (2 << 0) + +#define PORTC_PC_CFG3_NDQS (2 << 0) + +#define DMAC_CFG_REG0 0x300 +#define DMAC_SRC_START_ADDR_REG0 0x304 +#define DMAC_DEST_START_ADDRR_REG0 0x308 +#define DMAC_DDMA_BC_REG0 0x30C +#define DMAC_DDMA_PARA_REG0 0x318 + +#define DMAC_DDMA_CFG_REG_LOADING (1 << 31) +#define DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 (2 << 25) +#define DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 (2 << 9) +#define DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO (1 << 5) +#define DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC (3 << 0) + +#define DMAC_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0) +#define DMAC_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8) + +#define NANDFLASHC_CTL 0x00000000 + +#define NFC_CTL_EN (1 << 0) +#define NFC_CTL_RESET (1 << 1) +#define NFC_CTL_RAM_METHOD (1 << 14) + +#define NANDFLASHC_ST 0x00000004 +#define NANDFLASHC_INT 0x00000008 +#define NANDFLASHC_TIMING_CTL 0x0000000C +#define NANDFLASHC_TIMING_CFG 0x00000010 +#define NANDFLASHC_ADDR_LOW 0x00000014 +#define NANDFLASHC_ADDR_HIGH 0x00000018 +#define NANDFLASHC_SECTOR_NUM 0x0000001C +#define NANDFLASHC_CNT 0x00000020 +#define NANDFLASHC_CMD 0x00000024 +#define NANDFLASHC_RCMD_SET 0x00000028 +#define NANDFLASHC_WCMD_SET 0x0000002C +#define NANDFLASHC_IO_DATA 0x00000030 +#define NANDFLASHC_ECC_CTL 0x00000034 + +#define NFC_ECC_EN (1 << 0) +#define NFC_ECC_PIPELINE (1 << 3) +#define NFC_ECC_EXCEPTION (1 << 4) +#define NFC_ECC_BLOCK_SIZE (1 << 5) +#define NFC_ECC_RANDOM_EN (1 << 9) +#define NFC_ECC_RANDOM_DIRECTION (1 << 10) + +#define NANDFLASHC_ECC_ST 0x00000038 +#define NANDFLASHC_DEBUG 0x0000003C +#define NANDFLASHC_ECC_CNT0 0x00000040 +#define NANDFLASHC_ECC_CNT1 0x00000044 +#define NANDFLASHC_ECC_CNT2 0x00000048 +#define NANDFLASHC_ECC_CNT3 0x0000004C +#define NANDFLASHC_USER_DATA_BASE 0x00000050 +#define NANDFLASHC_EFNAND_STATUS 0x00000090 +#define NANDFLASHC_SPARE_AREA 0x000000A0 +#define NANDFLASHC_PATTERN_ID 0x000000A4 +#define NANDFLASHC_RAM0_BASE 0x00000400 +#define NANDFLASHC_RAM1_BASE 0x00000800 + +#define NFC_ADDR_NUM_OFFSET 16 +#define NFC_SEND_ADR (1 << 19) +#define NFC_ACCESS_DIR (1 << 20) +#define NFC_DATA_TRANS (1 << 21) +#define NFC_SEND_CMD1 (1 << 22) +#define NFC_WAIT_FLAG (1 << 23) +#define NFC_SEND_CMD2 (1 << 24) +#define NFC_SEQ (1 << 25) +#define NFC_DATA_SWAP_METHOD (1 << 26) +#define NFC_ROW_AUTO_INC (1 << 27) +#define NFC_SEND_CMD3 (1 << 28) +#define NFC_SEND_CMD4 (1 << 29) + +#define NFC_READ_CMD_OFFSET 0 +#define NFC_RANDOM_READ_CMD0_OFFSET 8 +#define NFC_RANDOM_READ_CMD1_OFFSET 16 + + +#define NFC_PAGE_CMD (2 << 30) + +/* random seed used by linux */ +const uint16_t random_seed[128] = { + 0x2b75, 0x0bd0, 0x5ca3, 0x62d1, 0x1c93, 0x07e9, 0x2162, 0x3a72, + 0x0d67, 0x67f9, 0x1be7, 0x077d, 0x032f, 0x0dac, 0x2716, 0x2436, + 0x7922, 0x1510, 0x3860, 0x5287, 0x480f, 0x4252, 0x1789, 0x5a2d, + 0x2a49, 0x5e10, 0x437f, 0x4b4e, 0x2f45, 0x216e, 0x5cb7, 0x7130, + 0x2a3f, 0x60e4, 0x4dc9, 0x0ef0, 0x0f52, 0x1bb9, 0x6211, 0x7a56, + 0x226d, 0x4ea7, 0x6f36, 0x3692, 0x38bf, 0x0c62, 0x05eb, 0x4c55, + 0x60f4, 0x728c, 0x3b6f, 0x2037, 0x7f69, 0x0936, 0x651a, 0x4ceb, + 0x6218, 0x79f3, 0x383f, 0x18d9, 0x4f05, 0x5c82, 0x2912, 0x6f17, + 0x6856, 0x5938, 0x1007, 0x61ab, 0x3e7f, 0x57c2, 0x542f, 0x4f62, + 0x7454, 0x2eac, 0x7739, 0x42d4, 0x2f90, 0x435a, 0x2e52, 0x2064, + 0x637c, 0x66ad, 0x2c90, 0x0bad, 0x759c, 0x0029, 0x0986, 0x7126, + 0x1ca7, 0x1605, 0x386a, 0x27f5, 0x1380, 0x6d75, 0x24c3, 0x0f8e, + 0x2b7a, 0x1418, 0x1fd1, 0x7dc1, 0x2d8e, 0x43af, 0x2267, 0x7da3, + 0x4e3d, 0x1338, 0x50db, 0x454d, 0x764d, 0x40a3, 0x42e6, 0x262b, + 0x2d2e, 0x1aea, 0x2e17, 0x173d, 0x3a6e, 0x71bf, 0x25f9, 0x0a5d, + 0x7c57, 0x0fbe, 0x46ce, 0x4939, 0x6b17, 0x37bb, 0x3e91, 0x76db, +}; + +/* random seed used for syndrome calls */ +const uint16_t random_seed_syndrome = 0x4a80; + +#endif /* end of include guard: SUNXI_NAND_H */
Hi Piotr,
Here is a quick review.
On Mon, 20 Jul 2015 14:37:25 +0200 Piotr Zierhoffer pzierhoffer@antmicro.com wrote:
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
This driver adds NAND support to SPL. It was tested on AllWinner A20.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com Signed-off-by: Karol Gugala kgugala@antmicro.com
Changes in v2:
- removed traces of non-SPL-specific code
- moved the driver from boards/sunxi to drivers/mtd/nand
- moved magic values to defines (whenever possible)
- removed unnecesary late initialisation code
- code style changes as suggested for the first patch set:
- changed visibility of some symbols
- renamed unclear variables
- renamed header protector
- changed types of pointer variables
- other minor changes
drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 289 ++++++++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/sunxi_nand.h | 151 ++++++++++++++++++++++ 3 files changed, 441 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.c create mode 100644 drivers/mtd/nand/sunxi_nand.h
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 347ea62..4cf9cee 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -12,6 +12,7 @@ NORMAL_DRIVERS=y endif
obj-$(CONFIG_SPL_NAND_AM33XX_BCH) += am335x_spl_bch.o +obj-$(CONFIG_SPL_NAND_SUNXI) += sunxi_nand.o obj-$(CONFIG_SPL_NAND_DENALI) += denali_spl.o obj-$(CONFIG_SPL_NAND_DOCG4) += docg4_spl.o obj-$(CONFIG_SPL_NAND_SIMPLE) += nand_spl_simple.o diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c new file mode 100644 index 0000000..8d66d2b --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.c @@ -0,0 +1,289 @@ +/*
- Copyright (c) 2014-2015, Antmicro Ltd <www.antmicro.com>
- Copyright (c) 2015, AW-SOM Technologies <www.aw-som.com>
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <config.h> +#include <asm/io.h> +#include <nand.h> +#include "sunxi_nand.h"
+/* minimal "boot0" style NAND support for Allwinner A20 */
+/* temporary buffer in internal ram */ +unsigned char temp_buf[CONFIG_SYS_NAND_PAGE_SIZE]
- __aligned(0x10) __section(".text#");
+#define MAX_RETRIES 10
+static int check_value_inner(int offset, int expected_bits,
int max_number_of_retries, int negation)+{
- int retries = 0;
- do {
int val = readl(offset) & expected_bits;if (negation ? !val : val)return 1;mdelay(1);retries++;- } while (retries < max_number_of_retries);
- return 0;
+}
+static inline int check_value(int offset, int expected_bits,
int max_number_of_retries)+{
- return check_value_inner(offset, expected_bits,
max_number_of_retries, 0);+}
+static inline int check_value_negated(int offset, int unexpected_bits,
int max_number_of_retries)+{
- return check_value_inner(offset, unexpected_bits,
max_number_of_retries, 1);+}
+static void nand_set_clocks(void)
You're setting more than the clks in there: you're also doing the pinmux. Could you either rename the function or create a new one for the pinmux setting.
BTW, I'm not sure, but I think the pinmux should be done at the board level.
+{
- uint32_t val;
- writel(PORTC_PC_CFG0_NRB1 |
PORTC_PC_CFG0_NRB0 |PORTC_PC_CFG0_NRE |PORTC_PC_CFG0_NCE0 |PORTC_PC_CFG0_NCE1 |PORTC_PC_CFG0_NCLE |PORTC_PC_CFG0_NALE |PORTC_PC_CFG0_NWE, PORTC_BASE + PORTC_PC_CFG0);
At least the NCEX and NRBX should be board specific ?
- writel(PORTC_PC_CFG1_NDQ7 |
PORTC_PC_CFG1_NDQ6 |PORTC_PC_CFG1_NDQ5 |PORTC_PC_CFG1_NDQ4 |PORTC_PC_CFG1_NDQ3 |PORTC_PC_CFG1_NDQ2 |PORTC_PC_CFG1_NDQ1 |PORTC_PC_CFG1_NDQ0, PORTC_BASE + PORTC_PC_CFG1);- writel(PORTC_PC_CFG2_NCE7 |
PORTC_PC_CFG2_NCE6 |PORTC_PC_CFG2_NCE5 |PORTC_PC_CFG2_NCE4 |PORTC_PC_CFG2_NCE3 |PORTC_PC_CFG2_NCE2 |PORTC_PC_CFG2_NWP, PORTC_BASE + PORTC_PC_CFG2);
Ditto
- writel(PORTC_PC_CFG3_NDQS, PORTC_BASE + PORTC_PC_CFG3);
- val = readl(CCU_BASE + CCU_AHB_GATING_REG0);
- writel(CCU_AHB_GATING_REG0_NAND | val, CCU_BASE + CCU_AHB_GATING_REG0);
- val = readl(CCU_BASE + CCU_NAND_SCLK_CFG_REG);
- writel(val | CCU_NAND_SCLK_CFG_REG_SCLK_GATING
| CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO,CCU_BASE + CCU_NAND_SCLK_CFG_REG);+}
+void nand_init(void) +{
- uint32_t val;
- nand_set_clocks();
- val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL);
- /* enable and reset CTL */
- writel(val | NFC_CTL_EN | NFC_CTL_RESET,
NANDFLASHC_BASE + NANDFLASHC_CTL);- if (!check_value_negated(NANDFLASHC_BASE + NANDFLASHC_CTL,
NFC_CTL_RESET, MAX_RETRIES)) {printf("Couldn't initialize nand\n");- }
+}
+static uint32_t ecc_errors;
You could get rid of this global variable if you pass an ecc_errors argument to the nand_read_page function.
+void nand_read_page(unsigned int real_addr, int syndrome) +{
- uint32_t val;
- int ecc_off = 0;
- uint16_t ecc_mode = 0;
- uint16_t rand_seed;
- uint32_t page;
- uint16_t column;
- uint32_t oob_offset;
- switch (SUNXI_ECC_STRENGTH) {
- case 16:
ecc_mode = 0;ecc_off = 0x20;break;- case 24:
ecc_mode = 1;ecc_off = 0x2e;break;- case 28:
ecc_mode = 2;ecc_off = 0x32;break;- case 32:
ecc_mode = 3;ecc_off = 0x3c;break;- case 40:
ecc_mode = 4;ecc_off = 0x4a;break;- case 48:
ecc_mode = 4;ecc_off = 0x52;break;- case 56:
ecc_mode = 4;ecc_off = 0x60;break;- case 60:
ecc_mode = 4;ecc_off = 0x0;break;- case 64:
ecc_mode = 4;ecc_off = 0x0;break;- default:
ecc_mode = 0;ecc_off = 0;- }
- if (ecc_off == 0) {
printf("Unsupported ECC strength (%d)!\n",SUNXI_ECC_STRENGTH);return;- }
- /* clear temp_buf */
- memset(temp_buf, 0, CONFIG_SYS_NAND_PAGE_SIZE);
- /* set CMD */
- writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | 0xff,
NANDFLASHC_BASE + NANDFLASHC_CMD);
Could you replace the 0xff value by NAND_CMD_RESET, because what you're really trying to do is reset the NAND chip.
- if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 1),
Please replace this magic value by the appropriate MACRO [1].
MAX_RETRIES)) {printf("Error while initilizing command interrupt\n");return;- }
- page = real_addr / CONFIG_SYS_NAND_BLOCK_SIZE;
- column = real_addr % CONFIG_SYS_NAND_BLOCK_SIZE;
- if (syndrome) {
/* shift every 1kB in syndrome */
Well, this scheme is not really related to the ECC syndrome scheme, it comes from the BROM implementation which can only really 1K of data per page. Actually, this is not exactly true, depending on the BROM version it will try different things, see this description [2]. So once more, you're making assumptions that could be wrong on some boards.
column += (column / CONFIG_SYS_NAND_PAGE_SIZE) * ecc_off;- }
- /* clear ecc status */
- writel(0, NANDFLASHC_BASE + NANDFLASHC_ECC_ST);
- /* Choose correct seed */
- if (syndrome)
rand_seed = random_seed_syndrome;- else
rand_seed = random_seed[page % 128];- writel((rand_seed << 16) | NFC_ECC_RANDOM_EN | NFC_ECC_EN
| NFC_ECC_PIPELINE | (ecc_mode << 12),NANDFLASHC_BASE + NANDFLASHC_ECC_CTL);- val = readl(NANDFLASHC_BASE + NANDFLASHC_CTL);
- writel(val | NFC_CTL_RAM_METHOD, NANDFLASHC_BASE + NANDFLASHC_CTL);
- if (syndrome) {
writel(CONFIG_SYS_NAND_PAGE_SIZE,NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA);- } else {
oob_offset = CONFIG_SYS_NAND_BLOCK_SIZE+ (column / CONFIG_SYS_NAND_PAGE_SIZE) * ecc_off;writel(oob_offset, NANDFLASHC_BASE + NANDFLASHC_SPARE_AREA);- }
- /* DMAC */
- writel(0x0, DMAC_BASE + DMAC_CFG_REG0); /* clr dma cmd */
- /* read from REG_IO_DATA */
- writel(NANDFLASHC_BASE + NANDFLASHC_IO_DATA,
DMAC_BASE + DMAC_SRC_START_ADDR_REG0);- writel((uint32_t)temp_buf,
DMAC_BASE + DMAC_DEST_START_ADDRR_REG0); /* read to RAM */- writel(DMAC_DDMA_PARA_REG_SRC_WAIT_CYC
| DMAC_DDMA_PARA_REG_SRC_BLK_SIZE,DMAC_BASE + DMAC_DDMA_PARA_REG0);- writel(CONFIG_SYS_NAND_PAGE_SIZE,
DMAC_BASE + DMAC_DDMA_BC_REG0); /* 1kB */- writel(DMAC_DDMA_CFG_REG_LOADING
| DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32| DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32| DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO| DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC,DMAC_BASE + DMAC_CFG_REG0);- writel((0xE0 << NFC_RANDOM_READ_CMD1_OFFSET)
| (0x05 << NFC_RANDOM_READ_CMD0_OFFSET)| (0x30 | NFC_READ_CMD_OFFSET), NANDFLASHC_BASE+ NANDFLASHC_RCMD_SET);
Same comment as above, please use the appropriate macros:
0xE0 => NAND_CMD_RNDOUTSTART 0x05 => NAND_CMD_RNDOUT 0x30 => NAND_CMD_READSTART
- writel(1, NANDFLASHC_BASE + NANDFLASHC_SECTOR_NUM);
- writel(((page & 0xFFFF) << 16) | column,
NANDFLASHC_BASE + NANDFLASHC_ADDR_LOW);- writel((page >> 16) & 0xFF, NANDFLASHC_BASE + NANDFLASHC_ADDR_HIGH);
- writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS |
NFC_PAGE_CMD | NFC_WAIT_FLAG | (4 << NFC_ADDR_NUM_OFFSET) |NFC_SEND_ADR | NFC_DATA_SWAP_METHOD | (syndrome ? NFC_SEQ : 0),NANDFLASHC_BASE + NANDFLASHC_CMD);- if (!check_value(NANDFLASHC_BASE + NANDFLASHC_ST, (1 << 2),
MAX_RETRIES)) {printf("Error while initializing dma interrupt\n");return;- }
- if (!check_value_negated(DMAC_BASE + DMAC_CFG_REG0,
DMAC_DDMA_CFG_REG_LOADING, MAX_RETRIES)) {printf("Error while waiting for dma transfer to finish\n");return;- }
- if (readl(NANDFLASHC_BASE + NANDFLASHC_ECC_ST))
ecc_errors++;+}
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{
- void *current_dest;
- uint32_t count;
- uint32_t current_count;
- memset(dest, 0x0, size); /* clean destination memory */
- ecc_errors = 0;
- for (current_dest = dest;
current_dest < (dest + size);current_dest += CONFIG_SYS_NAND_PAGE_SIZE) {nand_read_page(offs, offs < SYNDROME_PARTITIONS_END);count = current_dest - dest;if (size - count > CONFIG_SYS_NAND_PAGE_SIZE)current_count = CONFIG_SYS_NAND_PAGE_SIZE;elsecurrent_count = size - count;memcpy(current_dest,temp_buf,current_count);offs += CONFIG_SYS_NAND_PAGE_SIZE;- }
- return ecc_errors;
I'm not sure what's the exact convention for nand_spl_load_image return code, but I'd say that returning a negative error code if ecc_errors != 0 is better than returning the number of ECC errors.
+}
+void nand_deselect(void) {} diff --git a/drivers/mtd/nand/sunxi_nand.h b/drivers/mtd/nand/sunxi_nand.h new file mode 100644 index 0000000..d24f0ef --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.h @@ -0,0 +1,151 @@ +#ifndef SUNXI_NAND_H
+#define SUNXI_NAND_H
+#define PORTC_BASE 0x01c20800 +#define CCU_BASE 0x01c20000 +#define NANDFLASHC_BASE 0x01c03000 +#define DMAC_BASE 0x01c02000
I'm not sure how drivers are supposed to interact with a DMA controller, a clk controller or a gpio/pinmux controller in u-boot, but I would expect those definitions to be common to the sunxi platform and not directly defined in the NAND controller driver.
The same goes for all the following CCU_, PORTC_ and DMAC_ definitions.
+#define SYNDROME_PARTITIONS_END 0x00400000 +#define SUNXI_ECC_STRENGTH 40
These two variables should definitely not be defined in this headers: they are representing NAND chip requirements and these ones are definitely board specific (they depends on the NAND chip you have on your board).
+#define CCU_AHB_GATING_REG0 0x60 +#define CCU_NAND_SCLK_CFG_REG 0x80 +#define CCU_AHB_GATING_REG0_NAND (1 << 13)
+#define CCU_NAND_SCLK_CFG_REG_SCLK_GATING (1 << 31) +#define CCU_NAND_SCLK_CFG_REG_CLK_DIV_RATIO (1 << 0)
+#define PORTC_PC_CFG0 0x48 +#define PORTC_PC_CFG1 0x4C +#define PORTC_PC_CFG2 0x50 +#define PORTC_PC_CFG3 0x54
+#define PORTC_PC_CFG0_NRB1 (2 << 28) +#define PORTC_PC_CFG0_NRB0 (2 << 24) +#define PORTC_PC_CFG0_NRE (2 << 20) +#define PORTC_PC_CFG0_NCE0 (2 << 16) +#define PORTC_PC_CFG0_NCE1 (2 << 12) +#define PORTC_PC_CFG0_NCLE (2 << 8) +#define PORTC_PC_CFG0_NALE (2 << 4) +#define PORTC_PC_CFG0_NWE (2 << 0)
+#define PORTC_PC_CFG1_NDQ7 (2 << 28) +#define PORTC_PC_CFG1_NDQ6 (2 << 24) +#define PORTC_PC_CFG1_NDQ5 (2 << 20) +#define PORTC_PC_CFG1_NDQ4 (2 << 16) +#define PORTC_PC_CFG1_NDQ3 (2 << 12) +#define PORTC_PC_CFG1_NDQ2 (2 << 8) +#define PORTC_PC_CFG1_NDQ1 (2 << 4) +#define PORTC_PC_CFG1_NDQ0 (2 << 0)
+#define PORTC_PC_CFG2_NCE7 (2 << 24) +#define PORTC_PC_CFG2_NCE6 (2 << 20) +#define PORTC_PC_CFG2_NCE5 (2 << 16) +#define PORTC_PC_CFG2_NCE4 (2 << 12) +#define PORTC_PC_CFG2_NCE3 (2 << 8) +#define PORTC_PC_CFG2_NCE2 (2 << 4) +#define PORTC_PC_CFG2_NWP (2 << 0)
+#define PORTC_PC_CFG3_NDQS (2 << 0)
+#define DMAC_CFG_REG0 0x300 +#define DMAC_SRC_START_ADDR_REG0 0x304 +#define DMAC_DEST_START_ADDRR_REG0 0x308 +#define DMAC_DDMA_BC_REG0 0x30C +#define DMAC_DDMA_PARA_REG0 0x318
+#define DMAC_DDMA_CFG_REG_LOADING (1 << 31) +#define DMAC_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 (2 << 25) +#define DMAC_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 (2 << 9) +#define DMAC_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO (1 << 5) +#define DMAC_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC (3 << 0)
+#define DMAC_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0) +#define DMAC_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8)
+#define NANDFLASHC_CTL 0x00000000
+#define NFC_CTL_EN (1 << 0) +#define NFC_CTL_RESET (1 << 1) +#define NFC_CTL_RAM_METHOD (1 << 14)
+#define NANDFLASHC_ST 0x00000004 +#define NANDFLASHC_INT 0x00000008 +#define NANDFLASHC_TIMING_CTL 0x0000000C +#define NANDFLASHC_TIMING_CFG 0x00000010 +#define NANDFLASHC_ADDR_LOW 0x00000014 +#define NANDFLASHC_ADDR_HIGH 0x00000018 +#define NANDFLASHC_SECTOR_NUM 0x0000001C +#define NANDFLASHC_CNT 0x00000020 +#define NANDFLASHC_CMD 0x00000024 +#define NANDFLASHC_RCMD_SET 0x00000028 +#define NANDFLASHC_WCMD_SET 0x0000002C +#define NANDFLASHC_IO_DATA 0x00000030 +#define NANDFLASHC_ECC_CTL 0x00000034
+#define NFC_ECC_EN (1 << 0) +#define NFC_ECC_PIPELINE (1 << 3) +#define NFC_ECC_EXCEPTION (1 << 4) +#define NFC_ECC_BLOCK_SIZE (1 << 5) +#define NFC_ECC_RANDOM_EN (1 << 9) +#define NFC_ECC_RANDOM_DIRECTION (1 << 10)
+#define NANDFLASHC_ECC_ST 0x00000038 +#define NANDFLASHC_DEBUG 0x0000003C +#define NANDFLASHC_ECC_CNT0 0x00000040 +#define NANDFLASHC_ECC_CNT1 0x00000044 +#define NANDFLASHC_ECC_CNT2 0x00000048 +#define NANDFLASHC_ECC_CNT3 0x0000004C +#define NANDFLASHC_USER_DATA_BASE 0x00000050 +#define NANDFLASHC_EFNAND_STATUS 0x00000090 +#define NANDFLASHC_SPARE_AREA 0x000000A0 +#define NANDFLASHC_PATTERN_ID 0x000000A4 +#define NANDFLASHC_RAM0_BASE 0x00000400 +#define NANDFLASHC_RAM1_BASE 0x00000800
+#define NFC_ADDR_NUM_OFFSET 16 +#define NFC_SEND_ADR (1 << 19) +#define NFC_ACCESS_DIR (1 << 20) +#define NFC_DATA_TRANS (1 << 21) +#define NFC_SEND_CMD1 (1 << 22) +#define NFC_WAIT_FLAG (1 << 23) +#define NFC_SEND_CMD2 (1 << 24) +#define NFC_SEQ (1 << 25) +#define NFC_DATA_SWAP_METHOD (1 << 26) +#define NFC_ROW_AUTO_INC (1 << 27) +#define NFC_SEND_CMD3 (1 << 28) +#define NFC_SEND_CMD4 (1 << 29)
+#define NFC_READ_CMD_OFFSET 0 +#define NFC_RANDOM_READ_CMD0_OFFSET 8 +#define NFC_RANDOM_READ_CMD1_OFFSET 16
+#define NFC_PAGE_CMD (2 << 30)
Are all these NFC_ macros used outside of the driver itself. If that's not the case then I would recommend moving them direcly in the .c file.
Moreover, you're sometime mixing the NFC_ and NANDFLASHC_ prefix, is there a reason for doing that ?
Also, I haven't checked if the MACRO names are matching the one defined in the linux driver, but if that's not the case then I would recommend using the same definition since the final goal is to port the Linux driver to u-boot (I know you're just implementing the SPL part, but since you moved your code into drivers/mtd/nand/sunxi_nand* I'll have to merge the Linux implementation into this file).
+/* random seed used by linux */ +const uint16_t random_seed[128] = {
- 0x2b75, 0x0bd0, 0x5ca3, 0x62d1, 0x1c93, 0x07e9, 0x2162, 0x3a72,
- 0x0d67, 0x67f9, 0x1be7, 0x077d, 0x032f, 0x0dac, 0x2716, 0x2436,
- 0x7922, 0x1510, 0x3860, 0x5287, 0x480f, 0x4252, 0x1789, 0x5a2d,
- 0x2a49, 0x5e10, 0x437f, 0x4b4e, 0x2f45, 0x216e, 0x5cb7, 0x7130,
- 0x2a3f, 0x60e4, 0x4dc9, 0x0ef0, 0x0f52, 0x1bb9, 0x6211, 0x7a56,
- 0x226d, 0x4ea7, 0x6f36, 0x3692, 0x38bf, 0x0c62, 0x05eb, 0x4c55,
- 0x60f4, 0x728c, 0x3b6f, 0x2037, 0x7f69, 0x0936, 0x651a, 0x4ceb,
- 0x6218, 0x79f3, 0x383f, 0x18d9, 0x4f05, 0x5c82, 0x2912, 0x6f17,
- 0x6856, 0x5938, 0x1007, 0x61ab, 0x3e7f, 0x57c2, 0x542f, 0x4f62,
- 0x7454, 0x2eac, 0x7739, 0x42d4, 0x2f90, 0x435a, 0x2e52, 0x2064,
- 0x637c, 0x66ad, 0x2c90, 0x0bad, 0x759c, 0x0029, 0x0986, 0x7126,
- 0x1ca7, 0x1605, 0x386a, 0x27f5, 0x1380, 0x6d75, 0x24c3, 0x0f8e,
- 0x2b7a, 0x1418, 0x1fd1, 0x7dc1, 0x2d8e, 0x43af, 0x2267, 0x7da3,
- 0x4e3d, 0x1338, 0x50db, 0x454d, 0x764d, 0x40a3, 0x42e6, 0x262b,
- 0x2d2e, 0x1aea, 0x2e17, 0x173d, 0x3a6e, 0x71bf, 0x25f9, 0x0a5d,
- 0x7c57, 0x0fbe, 0x46ce, 0x4939, 0x6b17, 0x37bb, 0x3e91, 0x76db,
+};
+/* random seed used for syndrome calls */ +const uint16_t random_seed_syndrome = 0x4a80;
The random_seed and random_seed_syndrome variables should not be defined in the header, because this implies duplicating the same global variable if the header is included multiple times (which should trigger a linker error).
+#endif /* end of include guard: SUNXI_NAND_H */
/* SUNXI_NAND_H */ should be enough.
Best Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/mtd/nand/sunxi_nand.c#L84 [2]http://linux-sunxi.org/NAND#More_information_on_BROM_NAND
Hi Boris,
thanks for your review. I have applied most of your comments, but I have few remarks and questions.
2015-07-20 18:13 GMT+02:00 Boris Brezillon boris.brezillon@free-electrons.com:
page = real_addr / CONFIG_SYS_NAND_BLOCK_SIZE;column = real_addr % CONFIG_SYS_NAND_BLOCK_SIZE;if (syndrome) {/* shift every 1kB in syndrome */Well, this scheme is not really related to the ECC syndrome scheme, it comes from the BROM implementation which can only really 1K of data per page. Actually, this is not exactly true, depending on the BROM version it will try different things, see this description [2]. So once more, you're making assumptions that could be wrong on some boards.
Actually, I have only one board and I wouldn't like to submit code that is supposed to be general and was never really tested.
I'd suggest removing the comment and making the following options configurable, with the default values as provided, in Kconfig: CONFIG_NAND_PAGE_SIZE 0x2000 CONFIG_NAND_ECC_PAGE_SIZE 0x400 CONFIG_NAND_SUNXI_ECC_STRENGTH 40 CONFIG_NAND_SYNDROME_PARTITIONS_END 0x400000
Do you think that would be sensible?
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{
void *current_dest;uint32_t count;uint32_t current_count;memset(dest, 0x0, size); /* clean destination memory */ecc_errors = 0;for (current_dest = dest;current_dest < (dest + size);current_dest += CONFIG_SYS_NAND_PAGE_SIZE) {nand_read_page(offs, offs < SYNDROME_PARTITIONS_END);count = current_dest - dest;if (size - count > CONFIG_SYS_NAND_PAGE_SIZE)current_count = CONFIG_SYS_NAND_PAGE_SIZE;elsecurrent_count = size - count;memcpy(current_dest,temp_buf,current_count);offs += CONFIG_SYS_NAND_PAGE_SIZE;}return ecc_errors;I'm not sure what's the exact convention for nand_spl_load_image return code, but I'd say that returning a negative error code if ecc_errors != 0 is better than returning the number of ECC errors.
From my observations it seems that there is no established convention, but
returning -1 as an error indicator can be found here and there, so I may do that.
It's not really interpreted anywhere, though.
Are all these NFC_ macros used outside of the driver itself. If that's not the case then I would recommend moving them direcly in the .c file.
In general you may find that constants regarding NANDs are kept in .h files around U-Boot, so I'd like stick with that convention.
Also, I haven't checked if the MACRO names are matching the one defined in the linux driver, but if that's not the case then I would recommend using the same definition since the final goal is to port the Linux driver to u-boot (I know you're just implementing the SPL part, but since you moved your code into drivers/mtd/nand/sunxi_nand* I'll have to merge the Linux implementation into this file).
I have made the macros consistent, but unfortunately I won't be able to verify them all with the Linux drivers, due to time constraints. It can always be done later as a part of a separate patchset with a Linux driver.
I will submit another version of this patchset later today.
I will follow your suggestion to create a new thread for that.
Boris
Best regards
*Piotr Zierhoffer* Antmicro Ltd | www.antmicro.com
Hi Piotr,
On Wed, 22 Jul 2015 13:27:37 +0200 Piotr Zierhoffer pzierhoffer@antmicro.com wrote:
Hi Boris,
thanks for your review. I have applied most of your comments, but I have few remarks and questions.
2015-07-20 18:13 GMT+02:00 Boris Brezillon boris.brezillon@free-electrons.com:
page = real_addr / CONFIG_SYS_NAND_BLOCK_SIZE;column = real_addr % CONFIG_SYS_NAND_BLOCK_SIZE;if (syndrome) {/* shift every 1kB in syndrome */Well, this scheme is not really related to the ECC syndrome scheme, it comes from the BROM implementation which can only really 1K of data per page. Actually, this is not exactly true, depending on the BROM version it will try different things, see this description [2]. So once more, you're making assumptions that could be wrong on some boards.
Actually, I have only one board and I wouldn't like to submit code that is supposed to be general and was never really tested.
I understand, but I was not talking about testing your code on all existing boards, just making your code generic enough so that other users can test on their platforms without having to modify the SPL code. Of course if they detect that something is buggy or missing, they will provide follow-up patches to fix those problems.
I'd suggest removing the comment and making the following options configurable, with the default values as provided, in Kconfig: CONFIG_NAND_PAGE_SIZE 0x2000 CONFIG_NAND_ECC_PAGE_SIZE 0x400 CONFIG_NAND_SUNXI_ECC_STRENGTH 40 CONFIG_NAND_SYNDROME_PARTITIONS_END 0x400000
Do you think that would be sensible?
Yep, that pretty much what I was expecting. Just put those definition into a board config header, or add a Kconfig entry.
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{
void *current_dest;uint32_t count;uint32_t current_count;memset(dest, 0x0, size); /* clean destination memory */ecc_errors = 0;for (current_dest = dest;current_dest < (dest + size);current_dest += CONFIG_SYS_NAND_PAGE_SIZE) {nand_read_page(offs, offs < SYNDROME_PARTITIONS_END);count = current_dest - dest;if (size - count > CONFIG_SYS_NAND_PAGE_SIZE)current_count = CONFIG_SYS_NAND_PAGE_SIZE;elsecurrent_count = size - count;memcpy(current_dest,temp_buf,current_count);offs += CONFIG_SYS_NAND_PAGE_SIZE;}return ecc_errors;I'm not sure what's the exact convention for nand_spl_load_image return code, but I'd say that returning a negative error code if ecc_errors != 0 is better than returning the number of ECC errors.
From my observations it seems that there is no established convention, but returning -1 as an error indicator can be found here and there, so I may do that.
It's not really interpreted anywhere, though.
Okay, then I'll let u-boot maintainers decide what should be done.
Are all these NFC_ macros used outside of the driver itself. If that's not the case then I would recommend moving them direcly in the .c file.
In general you may find that constants regarding NANDs are kept in .h files around U-Boot, so I'd like stick with that convention.
This is true for all generic definitions, not for driver specific ones.
Also, I haven't checked if the MACRO names are matching the one defined in the linux driver, but if that's not the case then I would recommend using the same definition since the final goal is to port the Linux driver to u-boot (I know you're just implementing the SPL part, but since you moved your code into drivers/mtd/nand/sunxi_nand* I'll have to merge the Linux implementation into this file).
I have made the macros consistent, but unfortunately I won't be able to verify them all with the Linux drivers, due to time constraints.
How about copying the definition from the Linux driver and fixing the mismatch in your implementation ?
It can always be done later as a part of a separate patchset with a Linux driver.
Hm, the merge will be even more complicated if you do that. Could you at least move your code to sunxi_nand_spl.c so that we can directly apply the linux commit adding support for the sunxi NAND controller ?
Thanks.
Best Regards,
Boris
Hello
2015-07-22 13:53 GMT+02:00 Boris Brezillon boris.brezillon@free-electrons.com:
I'd suggest removing the comment and making the following options configurable, with the default values as provided, in Kconfig: CONFIG_NAND_PAGE_SIZE 0x2000 CONFIG_NAND_ECC_PAGE_SIZE 0x400 CONFIG_NAND_SUNXI_ECC_STRENGTH 40 CONFIG_NAND_SYNDROME_PARTITIONS_END 0x400000
Do you think that would be sensible?
Yep, that pretty much what I was expecting. Just put those definition into a board config header, or add a Kconfig entry.
Ok, will be done.
In general you may find that constants regarding NANDs are kept in .h files around U-Boot, so I'd like stick with that convention.
This is true for all generic definitions, not for driver specific ones.
Well, that's not true for all the drivers, as we have tegra_nand.h and atmel_nand_ecc.h, never used elsewhere apart from their corresponding *.c files.
Especially if we'd have two drivers, one header will be needed.
But if that would ease your work, I'll merge sunxi_nand.h to sunxi_nand.c and rename it as you suggest.
Best regards
Piotr Zierhoffer Antmicro Ltd | www.antmicro.com
From: Piotr Zierhoffer piotr.zierhoffer@cs.put.poznan.pl
When SPL_NAND_SUNXI option is selected in config, set some configuration options for sunxi NAND.
This commit also introduces the configurable options in Kconfig.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com Signed-off-by: Karol Gugala kgugala@antmicro.com ---
Changes in v2: - removed traces of non-SPL specific code - renamed defines to be more relevant - moved Kconfig entry for the driver to drivers/mtd/nand - reworded Kconfig entry help
drivers/mtd/nand/Kconfig | 7 +++++++ include/configs/sunxi-common.h | 11 +++++++++++ 2 files changed, 18 insertions(+)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 3024357..2f8dbaf 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -85,6 +85,13 @@ config SPL_NAND_DENALI This is a small implementation of the Denali NAND controller for use on SPL.
+config SPL_NAND_SUNXI + bool "Support for NAND on Allwinner A20 in SPL" + depends on MACH_SUN7I + ---help--- + Enable support for internal NAND. This option allows SPL to read from + sunxi NAND using DMA transfers. Writing is not supported. + endif
endmenu diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 9576bc1..c230cff 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -139,6 +139,17 @@ #define CONFIG_INITRD_TAG #define CONFIG_SERIAL_TAG
+#if defined(CONFIG_SPL_NAND_SUNXI) +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_SUPPORT + +#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000 +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x008000 + +#define CONFIG_SYS_NAND_PAGE_SIZE 0x000400 /* 1kb */ +#define CONFIG_SYS_NAND_BLOCK_SIZE 0x002000 /* 8kb*/ +#endif + /* mmc config */ #if !defined(CONFIG_UART0_PORT_F) #define CONFIG_MMC
As SPL does not know which source to choose when booting U-Boot, choose NAND if it is capable of doing so.
Signed-off-by: Peter Gielda pgielda@antmicro.com Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com Signed-off-by: Mateusz Holenko mholenko@antmicro.com Signed-off-by: Piotr Zierhoffer pzierhoffer@antmicro.com
---
Changes in v2: - none
arch/arm/cpu/armv7/sunxi/board.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index 5f39aa0..e4b7d63 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -128,6 +128,9 @@ void s_init(void) */ u32 spl_boot_device(void) { +#ifdef CONFIG_SPL_NAND_SUPPORT + return BOOT_DEVICE_NAND; +#else /* * When booting from the SD card, the "eGON.BT0" signature is expected * to be found in memory at the address 0x0004 (see the "mksunxiboot" @@ -148,6 +151,7 @@ u32 spl_boot_device(void) return BOOT_DEVICE_MMC1; else return BOOT_DEVICE_BOARD; +#endif }
/* No confirmation data available in SPL yet. Hardcode bootmode */
Hi Piotr,
I don't know what's the policy in u-boot, but in most open source projects we usually start a new thread when sending a new version of a specific patch series.
On Mon, 20 Jul 2015 14:37:24 +0200 Piotr Zierhoffer pzierhoffer@antmicro.com wrote:
This is a basic driver for the sunxi NAND controller for Allwinner A20. It supports only SPL.
The driver uses DMA for data transfers. It does not support writing.
Changes in v2:
- removed traces of non-SPL-specific code
- moved the driver from boards/sunxi to drivers/mtd/nand
- moved magic values to defines (whenever possible)
- removed unnecesary late initialisation code
- code style changes as suggested for the first patch set:
- changed visibility of some symbols
- renamed unclear variables
- renamed header protector
- changed types of pointer variables
- other minor changes
- renamed defines to be more relevant
- moved Kconfig entry for the driver to drivers/mtd/nand
- reworded Kconfig entry help
You also completely dropped the a20_nandread command (which is a good thing IMHO), right ?
Best Regards,
Boris
Hello Boris
2015-07-20 17:05 GMT+02:00 Boris Brezillon boris.brezillon@free-electrons.com:
I don't know what's the policy in u-boot, but in most open source projects we usually start a new thread when sending a new version of a specific patch series.
I have tried to follow the guidelines from http://www.denx.de/wiki/U-Boot/Patches and understood that I should set -r option in patman to the message-id of my previous cover letter - like in the documentation of git-send-email (http://git-scm.com/docs/git-send-email)
Sorry if I got it wrong, should I resubmit it?
You also completely dropped the a20_nandread command (which is a good thing IMHO), right ?
This is true. The command was the only non-SPL part, and since it was not a full driver we have decided to remove it from this patch set.
Best regards
Piotr Zierhoffer Antmicro Ltd | www.antmicro.com
participants (4)
-
Boris Brezillon -
Marek Vasut -
Piotr Zierhoffer -
Scott Wood