[U-Boot] [PATCH 3/4] mtd: nand: add Freescale NFC driver

[Stefan Agner <stefan@agner.ch>]

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 <stefan@agner.ch>
>> ---
>>  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