From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Agner Date: Tue, 12 Aug 2014 23:13:17 +0200 Subject: [U-Boot] [PATCH 3/4] mtd: nand: add Freescale NFC driver In-Reply-To: <1407796426.7427.100.camel@snotra.buserror.net> References: <4ea124ab817db6e3dee2067aadd6db14643990f5.1407312577.git.stefan@agner.ch> <1407796426.7427.100.camel@snotra.buserror.net> Message-ID: <1380dbe3110cbf9359220a78d69e1896@agner.ch> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am 2014-08-12 00:33, schrieb Scott Wood: > On Wed, 2014-08-06 at 10:59 +0200, Stefan Agner wrote: >> This adds initial support for Freescale NFC (NAND Flash Controller). >> The IP is used in ARM based Vybrid SoCs as well as on some PowerPC >> devices. This driver is only tested on Vybrid. >> >> Signed-off-by: Stefan Agner >> --- >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/fsl_nfc.c | 676 +++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 677 insertions(+) >> create mode 100644 drivers/mtd/nand/fsl_nfc.c >> >> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile >> index bf1312a..85cb0dd 100644 >> --- a/drivers/mtd/nand/Makefile >> +++ b/drivers/mtd/nand/Makefile >> @@ -51,6 +51,7 @@ obj-$(CONFIG_NAND_KB9202) += kb9202_nand.o >> obj-$(CONFIG_NAND_KIRKWOOD) += kirkwood_nand.o >> obj-$(CONFIG_NAND_KMETER1) += kmeter1_nand.o >> obj-$(CONFIG_NAND_MPC5121_NFC) += mpc5121_nfc.o >> +obj-$(CONFIG_NAND_FSL_NFC) += fsl_nfc.o >> obj-$(CONFIG_NAND_MXC) += mxc_nand.o > > Could you explain how this differs from mpc5121_nfc, mxc_nand, etc? If > it's truly different enough to deserve its own driver, it should at > least get a more specific name. > I'm not really an expert in NAND devices, but from looking into the reference manuals and drivers, I summarize those differences: mxc_nand: Supports 3 NAND Flash controller found in i.MX ARM SoCs: V1: MX31/MX27, 16 Bit Registers V2.1: MX25/MX35, 16 Bit Registers, V3.2: MX51/MX53, 32 Bit Registers, 12 address registers... All three have no DMA controller integrated. mpc5121_nfc: Supports the MPC5121 Power Architecture Processor NAND flash controller. Big Endian, but other than this, this IP looks very similar to the V2.1 NFC above (looking at the registers and the block diagram). fsl_nfc: Supports VF610 (2011), however should be easily portable to MPC5125 Power Architecture Processor (2009) and MCF5441X ColdFire V4 Microprocessor (2009) All three share the almost identical Register Layout, similar block diagram and have integrated DMA controller. IMHO, this IP really deserves a own driver. Also, from my humble perspective, it might have made more sense to integrate mpc5121_nfc into mxc_nand. In return, splitting out mxc_nand NFC V3.2 (looking at the ifdefs and quite different register layout). Anyway, not part of the topic here.. Regarding naming: A more specific name makes sense since Freescale calls all its Flash Controllers "NFC". I suggest vf610_nfc because that SoC is really is making use of the driver today... Looking at release dates, mpc5125_nfc would make sense as well. >> +static u32 nfc_read(struct mtd_info *mtd, uint reg) >> +{ >> + struct fsl_nfc *nfc = mtd_to_nfc(mtd); >> + >> + if (reg == NFC_FLASH_STATUS1 || >> + reg == NFC_FLASH_STATUS2 || >> + reg == NFC_IRQ_STATUS) >> + return __raw_readl(nfc->regs + reg); >> + /* Gang read/writes together for most registers. */ >> + else >> + return *(u32 *)(nfc->regs + reg); >> +} >> + >> +static void nfc_write(struct mtd_info *mtd, uint reg, u32 val) >> +{ >> + struct fsl_nfc *nfc = mtd_to_nfc(mtd); >> + >> + if (reg == NFC_FLASH_STATUS1 || >> + reg == NFC_FLASH_STATUS2 || >> + reg == NFC_IRQ_STATUS) >> + __raw_writel(val, nfc->regs + reg); >> + /* Gang read/writes together for most registers. */ >> + else >> + *(u32 *)(nfc->regs + reg) = val; >> +} > > You should always be using raw I/O accessors. If the intent is to > bypass I/O ordering for certain registers, the raw accessors should > already be skipping that. > Ok, will do. >> +int board_nand_init(struct nand_chip *chip) >> +{ > [snip] >> + /* second phase scan */ >> + if (nand_scan_tail(mtd)) { >> + err = -ENXIO; >> + goto error; >> + } >> + > > board_nand_init() should only call nand_scan_tail if > CONFIG_SYS_NAND_SELF_INIT is defined -- and if it is defined, then > board_nand_init() takes no arguments. > Ok, we need the page size during initialization, hence nand_scan_ident needs to be in the init code. I remove the argument from board_nand_init then. -- Stefan