From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Jos=E9_Miguel_Gon=E7alves?= Date: Tue, 18 Sep 2012 19:22:58 +0100 Subject: [U-Boot] [PATCH v3 09/11] S3C24XX: Add NAND Flash driver In-Reply-To: <1347991325.15284.8@snotra> References: <1347991325.15284.8@snotra> Message-ID: <5058BC02.7070705@inov.pt> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 18-09-2012 19:02, Scott Wood wrote: > On 09/18/2012 12:40:36 PM, Jos? Miguel Gon?alves wrote: >> NAND Flash driver with HW ECC for the S3C24XX SoCs. >> Currently it only supports SLC NAND chips. >> >> Signed-off-by: Jos? Miguel Gon?alves >> --- >> Changes for v2: >> - Coding style cleanup >> - Use of clrsetbits_le32() >> - Use of register bit macros instead of magic numbers >> >> Changes for v3: >> - Removed magic numbers >> - Removed a macro to declare a void printf() >> - Replaced one printf() with a puts() >> --- >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/s3c24xx_nand.c | 246 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 247 insertions(+) >> create mode 100644 drivers/mtd/nand/s3c24xx_nand.c >> >> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile >> index 29dc20e..791ec44 100644 >> --- a/drivers/mtd/nand/Makefile >> +++ b/drivers/mtd/nand/Makefile >> @@ -60,6 +60,7 @@ COBJS-$(CONFIG_NAND_MXS) += mxs_nand.o >> COBJS-$(CONFIG_NAND_NDFC) += ndfc.o >> COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o >> COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o >> +COBJS-$(CONFIG_NAND_S3C24XX) += s3c24xx_nand.o >> COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o >> COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o >> COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o >> diff --git a/drivers/mtd/nand/s3c24xx_nand.c b/drivers/mtd/nand/s3c24xx_nand.c >> new file mode 100644 >> index 0000000..3c13709 >> --- /dev/null >> +++ b/drivers/mtd/nand/s3c24xx_nand.c >> @@ -0,0 +1,246 @@ >> +/* >> + * (C) Copyright 2012 INOV - INESC Inovacao >> + * Jose Goncalves >> + * >> + * Based on drivers/mtd/nand/s3c64xx.c and U-Boot 1.3.4 from Samsung. >> + * Supports only SLC NAND Flash chips. >> + * >> + * See file CREDITS for list of people who contributed to this >> + * project. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >> + * MA 02111-1307 USA >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define TACLS_VAL 7 /* CLE & ALE duration setting (0~7) */ >> +#define TWRPH0_VAL 7 /* TWRPH0 duration setting (0~7) */ >> +#define TWRPH1_VAL 7 /* TWRPH1 duration setting (0~7) */ > > Please use space, not tab, as a word separator (after the second #define). OK. > >> + >> +#define MAX_CHIPS 2 >> +static int nand_cs[MAX_CHIPS] = { 0, 1 }; > > This needs explanation (and const). Better would be to use a priv struct, as > discussed before. > >> +static void s3c_nand_select_chip(struct mtd_info *mtd, int chip) >> +{ >> + struct s3c24xx_nand *const nand = s3c24xx_get_base_nand(); >> + u_long nfcont; > > s/u_long/u32/ Didn't catch this... > >> + >> + nfcont = readl(&nand->nfcont); >> + >> + switch (chip) { >> + case -1: >> + nfcont |= NFCONT_NCE1 | NFCONT_NCE0; >> + break; >> + case 0: >> + nfcont &= ~NFCONT_NCE0; >> + break; >> + case 1: >> + nfcont &= ~NFCONT_NCE1; >> + break; >> + default: >> + return; > > error message on default? OK. > >> + } >> + >> + writel(nfcont, &nand->nfcont); >> +} >> + >> +/* >> + * Hardware specific access to control-lines function >> + */ >> +static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl) >> +{ >> + struct s3c24xx_nand *const nand = s3c24xx_get_base_nand(); >> + struct nand_chip *this = mtd->priv; >> + >> + if (ctrl & NAND_CTRL_CHANGE) { >> + if (ctrl & NAND_CLE) >> + this->IO_ADDR_W = &nand->nfcmmd; >> + else if (ctrl & NAND_ALE) >> + this->IO_ADDR_W = &nand->nfaddr; >> + else >> + this->IO_ADDR_W = &nand->nfdata; >> + if (ctrl & NAND_NCE) >> + s3c_nand_select_chip(mtd, *(int *)this->priv); >> + else >> + s3c_nand_select_chip(mtd, -1); >> + } >> + >> + if (cmd != NAND_CMD_NONE) >> + writeb(cmd, this->IO_ADDR_W); >> +} > > As discussed earlier, do you really need to mess with IO_ADDR_W or can you do it > the way ndfc.c does? I will take a look at ndfc.c. Most of this driver was copy-paste from s3c64xx.c driver and an older patched U-Boot sources from Samsung, so I did not make any real code examination after it started to work... > > I.e.: > > static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl) > { > struct s3c24xx_nand *const nand = s3c24xx_get_base_nand(); > struct nand_chip *this = mtd->priv; > > if (cmd == NAND_CMD_NONE) > return; > > if (ctrl & NAND_CLE) > writeb(cmd, &nand->nfcmmd); > else > writeb(cmd, &nand->nfaddr); > } > >> +/* >> + * Board-specific NAND initialization. >> + */ >> +int board_nand_init(struct nand_chip *nand) >> +{ >> + static int chip_n; >> + struct s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand(); >> + >> + if (chip_n == 0) { >> + /* Extend NAND timings to the maximum */ >> + clrsetbits_le32(&nand_reg->nfconf, >> + NFCONF_TACLS_MASK | NFCONF_TWRPH0_MASK | >> + NFCONF_TWRPH1_MASK, >> + NFCONF_TACLS(TACLS_VAL) | >> + NFCONF_TWRPH0(TWRPH0_VAL) | >> + NFCONF_TWRPH1(TWRPH1_VAL)); >> + >> + /* Disable chip selects and soft lock, enable controller */ >> + clrsetbits_le32(&nand_reg->nfcont, NFCONT_WP, >> + NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE); >> + } else if (chip_n >= MAX_CHIPS) { >> + return -ENODEV; >> + } >> + >> + nand->IO_ADDR_R = &nand_reg->nfdata; >> + nand->IO_ADDR_W = &nand_reg->nfdata; >> + nand->cmd_ctrl = s3c_nand_hwcontrol; >> + nand->dev_ready = s3c_nand_device_ready; >> + nand->select_chip = s3c_nand_select_chip; >> + nand->options = 0; >> +#ifdef CONFIG_SPL_BUILD >> + nand->read_buf = nand_read_buf; >> +#endif >> + >> +#ifdef CONFIG_S3C24XX_NAND_HWECC >> + nand->ecc.hwctl = s3c_nand_enable_hwecc; >> + nand->ecc.calculate = s3c_nand_calculate_ecc; >> + nand->ecc.correct = s3c_nand_correct_data; >> + nand->ecc.mode = NAND_ECC_HW_OOB_FIRST; >> + nand->ecc.size = CONFIG_SYS_NAND_ECCSIZE; >> + nand->ecc.bytes = CONFIG_SYS_NAND_ECCBYTES; >> +#else >> + nand->ecc.mode = NAND_ECC_SOFT; >> +#endif /* ! CONFIG_S3C24XX_NAND_HWECC */ >> + >> + nand->priv = nand_cs + chip_n++; >> + >> + return 0; >> +} > > Please consider using the new SELF_INIT mechanism. > Can you explain and/or point_to_resources for what this means (I'm a U-Boot newbie)... Best regards, Jos? Gon?alves