On Thu, 2015-05-21 at 15:59 +0200, Roy Spliet wrote:
+#ifdef CONFIG_NAND_SUNXI +#include <nand.h> +#endif
Why do you need the ifdef?
+#include <common.h>
+#include <config.h> +#include <asm/io.h> +#include <nand.h>
+/* DMAC */ +#define DMAC_BASE 0x01c02000 +#define DMAC_REG(a) (DMAC_BASE + a)
+#define DMAC_INT DMAC_REG(0x000) +#define DMAC_DDMA_CFG DMAC_REG(0x300) +#define DMAC_DDMA_SRC DMAC_REG(0x304) +#define DMAC_DDMA_DST DMAC_REG(0x308) +#define DMAC_DDMA_BYTE_COUNT DMAC_REG(0x30C) +#define DMAC_DDMA_PARAM DMAC_REG(0x318)
+/* NAND controller */ +#define NANDFLASHC_BASE 0x01c03000 +#define NREG(a) (0x01c03000 + a)
+#define NANDFLASHC_CTL NREG(0x00) +#define NANDFLASHC_CTL_EN 0x00000001 +#define NANDFLASHC_CTL_RST 0x00000002 +#define NANDFLASHC_CTL_RAM_METHOD 0x00004000
+#define NANDFLASHC_ST NREG(0x004) +#define NANDFLASHC_INT NREG(0x008) +#define NANDFLASHC_TIMING_CTL NREG(0x00C) +#define NANDFLASHC_TIMING_CFG NREG(0x010) +#define NANDFLASHC_ADDR_LOW NREG(0x014) +#define NANDFLASHC_ADDR_HIGH NREG(0x018) +#define NANDFLASHC_SECTOR_NUM NREG(0x01C) +#define NANDFLASHC_CNT NREG(0x020)
+#define NANDFLASHC_CMD NREG(0x024) +#define NANDFLASHC_SEND_CMD1 (1 << 22) +#define NANDFLASHC_WAIT_FLAG (1 << 23)
+#define NANDFLASHC_RCMD_SET NREG(0x028) +#define NANDFLASHC_WCMD_SET NREG(0x02C) +#define NANDFLASHC_IO_DATA NREG(0x030) +#define NANDFLASHC_ECC_CTL NREG(0x034) +#define NANDFLASHC_ECC_ST NREG(0x038) +#define NANDFLASHC_DEBUG NREG(0x03c) +#define NANDFLASHC_ECC_CNT0 NREG(0x040) +#define NANDFLASHC_ECC_CNT1 NREG(0x044) +#define NANDFLASHC_ECC_CNT2 NREG(0x048) +#define NANDFLASHC_ECC_CNT3 NREG(0x04c) +#define NANDFLASHC_USER_DATA_BASE NREG(0x050) +#define NANDFLASHC_EFNAND_STATUS NREG(0x090) +#define NANDFLASHC_SPARE_AREA NREG(0x0A0) +#define NANDFLASHC_PATTERN_ID NREG(0x0A4) +#define NANDFLASHC_RAM0_BASE NREG(0x400) +#define NANDFLASHC_RAM1_BASE NREG(0x800)
Shouldn't these be in a header file so they can be shared with a non- SPL driver?
+void +nand_init(void) +{
Don't put a newline after the return type.
uint32_t val;board_nand_init();val = readl(NANDFLASHC_CTL);val |= NANDFLASHC_CTL_RST;writel(val, NANDFLASHC_CTL);/* Wait until reset pin is deasserted */do {val = readl(NANDFLASHC_CTL);if (!(val & NANDFLASHC_CTL_RST))break;} while (1);
Add a timeout to delay loops.
/** \todo Chip select, currently kind of static */val = readl(NANDFLASHC_CTL);val &= 0xf0fff0f2;
Don't put magic numbers in the code -- use symbolic constants taht describe what the fields mean.
+/* random seed */
+static 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,+};
Why is randomness needed?
+uint32_t ecc_errors = 0;
Why is this global?
+int +nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) +{
dma_addr_t dst_block;dma_addr_t dst_end;phys_addr_t addr = offs;dst_end = ((dma_addr_t) dest) + size;memset((void *)dest, 0x0, size);
Unnecessary cast.
-Scott