[U-Boot] [PATCH v2 1/3] sunxi: nand: Add basic sunxi NAND driver for SPL with DMA support

Boris Brezillon boris.brezillon at free-electrons.com
Mon Jul 20 18:13:35 CEST 2015


Hi Piotr,

Here is a quick review.

On Mon, 20 Jul 2015 14:37:25 +0200
Piotr Zierhoffer <pzierhoffer at antmicro.com> wrote:

> From: Piotr Zierhoffer <piotr.zierhoffer at cs.put.poznan.pl>
> 
> This driver adds NAND support to SPL.
> It was tested on AllWinner A20.
> 
> Signed-off-by: Peter Gielda <pgielda at antmicro.com>
> Signed-off-by: Tomasz Gorochowik <tgorochowik at antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko at antmicro.com>
> Signed-off-by: Piotr Zierhoffer <pzierhoffer at antmicro.com>
> Signed-off-by: Karol Gugala <kgugala at 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;
> +		else
> +			current_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

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


More information about the U-Boot mailing list