From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bill Pringlemeir <bpringlemeir@nbsps.com>
Date: Wed, 06 Aug 2014 19:01:19 -0400
Subject: [U-Boot] [PATCH 3/4] mtd: nand: add Freescale NFC driver
In-Reply-To: <4ea124ab817db6e3dee2067aadd6db14643990f5.1407312577.git.stefan@agner.ch>
	(Stefan Agner's message of "Wed, 6 Aug 2014 10:59:37 +0200")
References: <cover.1407312577.git.stefan@agner.ch>
	<4ea124ab817db6e3dee2067aadd6db14643990f5.1407312577.git.stefan@agner.ch>
Message-ID: <87ha1p9q9s.fsf@nbsps.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On  6 Aug 2014, stefan at agner.ch 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/fsl_nfc.c b/drivers/mtd/nand/fsl_nfc.c
> new file mode 100644
> index 0000000..df2c8be
> --- /dev/null
> +++ b/drivers/mtd/nand/fsl_nfc.c
> @@ -0,0 +1,676 @@

[snip]

> +#define NFC_TIMEOUT	(1000)
> +
> +/* ECC status placed at end of buffers. */
> +#define ECC_SRAM_ADDR	((PAGE_2K+256-8) >> 3)
> +#define ECC_STATUS_MASK	0x80
> +#define ECC_ERR_COUNT	0x3F
> +
> +struct fsl_nfc {
> +	struct mtd_info	  *mtd;
> +	struct nand_chip   chip;
> +	struct device	  *dev;
> +	void __iomem	  *regs;
> +	//wait_queue_head_t  irq_waitq;

I think u-boot doesn't like C++ comments?

> +	uint               column;
> +	int                spareonly;
> +	int                page;
> +	/* Status and ID are in alternate locations. */
> +	int                alt_buf;
> +#define ALT_BUF_ID   1
> +#define ALT_BUF_STAT 2
> +	struct clk        *clk;
> +};
> +
> +#define mtd_to_nfc(_mtd) (struct fsl_nfc *)((struct nand_chip
> *)_mtd->priv)->priv;

[snip]

> +static int nfc_correct_data(struct mtd_info *mtd, u_char *dat,
> +				 u_char *read_ecc, u_char *calc_ecc)
> +{
> +	struct fsl_nfc *nfc = mtd_to_nfc(mtd);
> +	u32 ecc_status;
> +	u8 ecc_count;
> +	int flip;
> +
> +	/* 

Some extra ws here  Ie, '/* ' with a space after the star.  Maybe that
is from me?

> +	 * Errata: ECC status is stored at NFC_CFG[ECCADD] +4 for
> +	 * little-endian and +7 for big-endian SOC.  Access as 32 bits
> +	 * and use low byte.
> +	 */

This appears to be documented in the latest Vybrid manual (Rev7).

> +	ecc_status = __raw_readl(nfc->regs + ECC_SRAM_ADDR * 8 + 4);
> +	ecc_count = ecc_status & ECC_ERR_COUNT;
> +	if (!(ecc_status & ECC_STATUS_MASK))
> +		return ecc_count;

[snip]

> +static void nfc_enable_hwecc(struct mtd_info *mtd, int mode)
> +{
> +}
> +
> +struct nfc_config {
> +	int hardware_ecc;
> +	int width;
> +	int flash_bbt;
> +};
> +
> +//static int nfc_probe(struct platform_device *pdev)

Another C++ comment.

All minor nits.  I am certainly ok with this code being included in
u-boot.

Regards,
Bill Pringlemeir.