Dear Minkyu Kang,
In message 4AA0CE3F.60304@samsung.com you wrote:
This patch includes the onenand driver for s5pc1xx
Signed-off-by: Minkyu Kang mk7.kang@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
...
+static int s3c_read_reg(int offset) +{
- return readl(onenand->base + offset);
+}
+static void s3c_write_reg(int value, int offset) +{
- writel(value, onenand->base + offset);
+}
+static int s3c_read_cmd(unsigned int cmd) +{
- return readl(onenand->ahb_addr + cmd);
+}
+static void s3c_write_cmd(int value, unsigned int cmd) +{
- writel(value, onenand->ahb_addr + cmd);
+}
PLease see comments to previous patch about usage of register plus offset versus using proper C structures.
Please change this code to use structs, too.
+static unsigned int s5pc100_mem_addr(int fba, int fpa, int fsa) +{
- return (fba << 13) | (fpa << 7) | (fsa << 5);
+}
This function needs explanation. Please add a comment what it does, and how.
...
+static int s3c_onenand_bbt_wait(struct mtd_info *mtd, int state) +{
- unsigned int flags = INT_ACT | LOAD_CMP;
- unsigned int stat;
- unsigned long timeout = 0x10000;
- while (1 && timeout--) {
Please do not add bogus code. Remove the "1 &&"
...
+void s3c_onenand_init(struct mtd_info *mtd) +{
- struct onenand_chip *this = mtd->priv;
- onenand = malloc(sizeof(struct s3c_onenand));
- if (!onenand)
return;- onenand->page_buf = malloc(SZ_4K * sizeof(char));
- if (!onenand->page_buf)
return;- memset(onenand->page_buf, 0xFF, SZ_4K);
- onenand->oob_buf = malloc(128 * sizeof(char));
- if (!onenand->oob_buf)
return;- memset(onenand->oob_buf, 0xFF, 128);
- onenand->mtd = mtd;
+#ifdef CONFIG_S5PC1XX
- /* S5PC100 specific values */
- onenand->base = (void *) 0xE7100000;
- onenand->ahb_addr = (void *) 0xB0000000;
- onenand->mem_addr = s5pc100_mem_addr;
+#else +#error Please fix it at s3c6410 +#endif
What does this mean?
diff --git a/include/samsung_onenand.h b/include/samsung_onenand.h new file mode 100644 index 0000000..91b40e1 --- /dev/null +++ b/include/samsung_onenand.h
...
+/*
- OneNAND Controller
- */
+#define MEM_CFG_OFFSET 0x0000 +#define BURST_LEN_OFFSET 0x0010 +#define MEM_RESET_OFFSET 0x0020 +#define INT_ERR_STAT_OFFSET 0x0030 +#define INT_ERR_MASK_OFFSET 0x0040 +#define INT_ERR_ACK_OFFSET 0x0050
Please use a C struct instead.
Best regards,
Wolfgang Denk