Dear Minkyu Kang,
In message <4aa0ce3f.60...@samsung.com> you wrote:
> This patch includes the onenand driver for s5pc1xx
>
> Signed-off-by: Minkyu Kang <mk7.k...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Lack of skill dictates economy of style. - Joey Ramone
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
