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

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@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
+} + +void spear_enable_hwecc(struct mtd_info *mtd, int mode) +{ + writel(readl(&fsmc_regs_p->genmemctrl_pc) & ~0x80, + &fsmc_regs_p->genmemctrl_pc); + writel(readl(&fsmc_regs_p->genmemctrl_pc) & ~FSMC_ECCEN, + &fsmc_regs_p->genmemctrl_pc); + writel(readl(&fsmc_regs_p->genmemctrl_pc) | FSMC_ECCEN, + &fsmc_regs_p->genmemctrl_pc); +} + +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. These are just minor things. Change is optional. +}
Tom

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@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

Scott Wood wrote:
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@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.
You are correct! And I was not..
+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.
Yes. That is perfectly valid.
Tom
-Scott
participants (2)
-
Scott Wood
-
Tom