[U-Boot] [PATCH v4 05/12] SPEAr : nand driver support for SPEAr SoCs

[Scott Wood]

On Wed, Jan 13, 2010 at 07:13:17AM -0600, Tom wrote:
> SPEAr SoCs contain an FSMC controller which can be used to interface
> with a range of memories eg. NAND, SRAM, NOR.
> Currently, this driver supports interfacing FSMC with NAND memories
> 
> Signed-off-by: Vipin <vipin.kumar at st.com>
> ---
>   drivers/mtd/nand/Makefile             |    1 +
>   drivers/mtd/nand/spr_nand.c           |  123 +++++++++++++++++++++++++++++++++
>   include/asm-arm/arch-spear/spr_nand.h |   57 +++++++++++++++
>   3 files changed, 181 insertions(+), 0 deletions(-)
>   create mode 100755 drivers/mtd/nand/spr_nand.c
>   create mode 100644 include/asm-arm/arch-spear/spr_nand.h
> 
> <snip>
> +}
> +
> +static int spear_read_hwecc(struct mtd_info *mtd,
> +			    const u_char *data, u_char ecc[3])
> +{
> +	u_int ecc_tmp;
> +
> +	/* read the h/w ECC */
> +	ecc_tmp = readl(&fsmc_regs_p->genmemctrl_ecc);
> +
> +	ecc[0] = (u_char) (ecc_tmp & 0xFF);
> +	ecc[1] = (u_char) ((ecc_tmp & 0xFF00) >> 8);
> +	ecc[2] = (u_char) ((ecc_tmp & 0xFF0000) >> 16);
> +
> +	return 0;
> 
> Always returning 0.
> return can be switched to void

No it can't, because it needs to match the signature of ecc.calculate.

> +int spear_nand_init(struct nand_chip *nand)
> +{
> +	writel(FSMC_DEVWID_8 | FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON,
> +	       &fsmc_regs_p->genmemctrl_pc);
> +	writel(readl(&fsmc_regs_p->genmemctrl_pc) | FSMC_TCLR_1 | FSMC_TAR_1,
> +	       &fsmc_regs_p->genmemctrl_pc);
> +	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
> +	       &fsmc_regs_p->genmemctrl_comm);
> +	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
> +	       &fsmc_regs_p->genmemctrl_attrib);
> +
> +	nand->options = 0;
> +	nand->ecc.mode = NAND_ECC_HW;
> +	nand->ecc.layout = &spear_nand_ecclayout;
> +	nand->ecc.size = 512;
> +	nand->ecc.bytes = 3;
> +	nand->ecc.calculate = spear_read_hwecc;
> +	nand->ecc.hwctl = spear_enable_hwecc;
> +	nand->ecc.correct = nand_correct_data;
> +	nand->cmd_ctrl = spear_nand_hwcontrol;
> +	return 0;
> Same here for returning void.

IMHO it's better to preserve the ability to return an error in the future,
especially in init functions.

-Scott