[U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
NAND Flash driver with HW ECC for the S3C24XX SoCs.
Currently it only supports SLC NAND chips.
Signed-off-by: José Miguel Gonçalves jose.goncal...@inov.pt
---
drivers/mtd/nand/Makefile |1 +
drivers/mtd/nand/s3c24xx_nand.c | 269 +++
2 files changed, 270 insertions(+)
create mode 100644 drivers/mtd/nand/s3c24xx_nand.c
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 29dc20e..791ec44 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -60,6 +60,7 @@ COBJS-$(CONFIG_NAND_MXS) += mxs_nand.o
COBJS-$(CONFIG_NAND_NDFC) += ndfc.o
COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
+COBJS-$(CONFIG_NAND_S3C24XX) += s3c24xx_nand.o
COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o
COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
diff --git a/drivers/mtd/nand/s3c24xx_nand.c b/drivers/mtd/nand/s3c24xx_nand.c
new file mode 100644
index 000..eed72d5
--- /dev/null
+++ b/drivers/mtd/nand/s3c24xx_nand.c
@@ -0,0 +1,269 @@
+/*
+ * (C) Copyright 2012 INOV - INESC Inovacao
+ * Jose Goncalves jose.goncal...@inov.pt
+ *
+ * Based on drivers/mtd/nand/s3c64xx.c and U-Boot 1.3.4 from Samsung.
+ * Supports only SLC NAND Flash chips.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include common.h
+#include nand.h
+#include asm/io.h
+#include asm/arch/s3c24xx_cpu.h
+#include asm/errno.h
+
+#define NFCONT_ECC_ENC (118)
+#define NFCONT_WP (116)
+#define NFCONT_MECCLOCK(17)
+#define NFCONT_SECCLOCK(16)
+#define NFCONT_INITMECC(15)
+#define NFCONT_INITSECC(14)
+#define NFCONT_NCE1(12)
+#define NFCONT_NCE0(11)
+#define NFCONT_ENABLE (10)
+
+#define NFSTAT_RNB (10)
+
+#define MAX_CHIPS 2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)
+#endif
+
+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ u_long nfcont;
+
+ nfcont = readl(nand-nfcont);
+
+ switch (chip) {
+ case -1:
+ nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+ break;
+ case 0:
+ nfcont = ~NFCONT_NCE0;
+ break;
+ case 1:
+ nfcont = ~NFCONT_NCE1;
+ break;
+ default:
+ return;
+ }
+
+ writel(nfcont, nand-nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int
ctrl)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ struct nand_chip *this = mtd-priv;
+
+ if (ctrl NAND_CTRL_CHANGE) {
+ if (ctrl NAND_CLE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+ else if (ctrl NAND_ALE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+ else
+ this-IO_ADDR_W = (void __iomem *)nand-nfdata;
+ if (ctrl NAND_NCE)
+ s3c_nand_select_chip(mtd, *(int *)this-priv);
+ else
+ s3c_nand_select_chip(mtd, -1);
+ }
+
+ if (cmd != NAND_CMD_NONE)
+ writeb(cmd, this-IO_ADDR_W);
+}
+
+/*
+ * Function for checking device ready pin
+ */
+static int s3c_nand_device_ready(struct mtd_info *mtdinfo)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+
+ return readl(nand-nfstat) NFSTAT_RNB;
+}
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+/*
+ * This function is called before encoding ECC codes to ready ECC engine.
+ */
+static void s3c_nand_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ u_long nfcont, nfconf;
+
+ /* Set 1-bit ECC */
+ nfconf = readl(nand-nfconf);
+#if defined(CONFIG_S3C2412) || defined(CONFIG_S3C2413)
+ nfconf = ~(0x1 24);
+#else
+ nfconf = ~(0x3 23);
+#endif
+ writel(nfconf, nand-nfconf);
+
+ /* Initialize unlock ECC */
+ nfcont = readl(nand-nfcont);
+ nfcont |= NFCONT_INITMECC;
+
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Dear José Miguel Gonçalves,
NAND Flash driver with HW ECC for the S3C24XX SoCs.
Currently it only supports SLC NAND chips.
Signed-off-by: José Miguel Gonçalves jose.goncal...@inov.pt
---
drivers/mtd/nand/Makefile |1 +
drivers/mtd/nand/s3c24xx_nand.c | 269
+++ 2 files changed, 270 insertions(+)
create mode 100644 drivers/mtd/nand/s3c24xx_nand.c
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 29dc20e..791ec44 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -60,6 +60,7 @@ COBJS-$(CONFIG_NAND_MXS) += mxs_nand.o
COBJS-$(CONFIG_NAND_NDFC) += ndfc.o
COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
+COBJS-$(CONFIG_NAND_S3C24XX) += s3c24xx_nand.o
COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o
COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
diff --git a/drivers/mtd/nand/s3c24xx_nand.c
b/drivers/mtd/nand/s3c24xx_nand.c new file mode 100644
index 000..eed72d5
--- /dev/null
+++ b/drivers/mtd/nand/s3c24xx_nand.c
@@ -0,0 +1,269 @@
+/*
+ * (C) Copyright 2012 INOV - INESC Inovacao
+ * Jose Goncalves jose.goncal...@inov.pt
+ *
+ * Based on drivers/mtd/nand/s3c64xx.c and U-Boot 1.3.4 from Samsung.
+ * Supports only SLC NAND Flash chips.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include common.h
+#include nand.h
+#include asm/io.h
+#include asm/arch/s3c24xx_cpu.h
+#include asm/errno.h
+
+#define NFCONT_ECC_ENC (118)
+#define NFCONT_WP(116)
+#define NFCONT_MECCLOCK (17)
+#define NFCONT_SECCLOCK (16)
+#define NFCONT_INITMECC (15)
+#define NFCONT_INITSECC (14)
+#define NFCONT_NCE1 (12)
+#define NFCONT_NCE0 (11)
+#define NFCONT_ENABLE(10)
+
+#define NFSTAT_RNB (10)
+
+#define MAX_CHIPS2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)
+#endif
+
+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ u_long nfcont;
+
+ nfcont = readl(nand-nfcont);
+
+ switch (chip) {
+ case -1:
+ nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+ break;
+ case 0:
+ nfcont = ~NFCONT_NCE0;
+ break;
+ case 1:
+ nfcont = ~NFCONT_NCE1;
+ break;
+ default:
+ return;
+ }
+
+ writel(nfcont, nand-nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int
ctrl) +{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ struct nand_chip *this = mtd-priv;
+
+ if (ctrl NAND_CTRL_CHANGE) {
+ if (ctrl NAND_CLE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+ else if (ctrl NAND_ALE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+ else
+ this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
+ if (ctrl NAND_NCE)
+ s3c_nand_select_chip(mtd, *(int *)this-priv);
+ else
+ s3c_nand_select_chip(mtd, -1);
+ }
+
+ if (cmd != NAND_CMD_NONE)
+ writeb(cmd, this-IO_ADDR_W);
+}
+
+/*
+ * Function for checking device ready pin
+ */
+static int s3c_nand_device_ready(struct mtd_info *mtdinfo)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+
+ return readl(nand-nfstat) NFSTAT_RNB;
+}
+
+#ifdef CONFIG_S3C24XX_NAND_HWECC
+/*
+ * This function is called before encoding ECC codes to ready ECC engine.
+ */
+static void s3c_nand_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ u_long nfcont, nfconf;
+
+ /* Set 1-bit ECC */
+ nfconf = readl(nand-nfconf);
+#if defined(CONFIG_S3C2412) || defined(CONFIG_S3C2413)
+ nfconf = ~(0x1 24);
+#else
+ nfconf = ~(0x3 23);
+#endif
Magic
+ writel(nfconf,
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int
ctrl) +{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ struct nand_chip *this = mtd-priv;
+
+ if (ctrl NAND_CTRL_CHANGE) {
+ if (ctrl NAND_CLE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+ else if (ctrl NAND_ALE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+ else
+ this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+ static int chip_n = 0;
+ s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+ u_long nfconf, nfcont;
+
+ if (chip_n == 0) {
+ /* Extend NAND timings to the maximum */
+ nfconf = readl(nand_reg-nfconf);
+ nfconf |= 0x7770;
Magic
+ writel(nfconf, nand_reg-nfconf);
+
+ /* Disable chip selects and soft lock, enable controller */
+ nfcont = readl(nand_reg-nfcont);
+ nfcont = ~NFCONT_WP;
+ nfcont |= NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE;
+ writel(nfcont, nand_reg-nfcont);
use clrsetbits_le32()
I will do that and also define some macros for the magic values.
Regards,
José Gonçalves
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
On 09/12/2012 06:16 PM, José Miguel Gonçalves wrote:
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int
ctrl) +{
+s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+struct nand_chip *this = mtd-priv;
+
+if (ctrl NAND_CTRL_CHANGE) {
+if (ctrl NAND_CLE)
+this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+else if (ctrl NAND_ALE)
+this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+else
+this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Why do you have volatile in your s3c24xx_nand struct?
-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Dear José Miguel Gonçalves,
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned
int ctrl) +{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ struct nand_chip *this = mtd-priv;
+
+ if (ctrl NAND_CTRL_CHANGE) {
+ if (ctrl NAND_CLE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+ else if (ctrl NAND_ALE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+ else
+ this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Not that you need to do the assignment into the structure use local
variable, no ?
+/*
+ * Board-specific NAND initialization.
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+ static int chip_n = 0;
+ s3c24xx_nand *const nand_reg = s3c24xx_get_base_nand();
+ u_long nfconf, nfcont;
+
+ if (chip_n == 0) {
+ /* Extend NAND timings to the maximum */
+ nfconf = readl(nand_reg-nfconf);
+ nfconf |= 0x7770;
Magic
+ writel(nfconf, nand_reg-nfconf);
+
+ /* Disable chip selects and soft lock, enable controller */
+ nfcont = readl(nand_reg-nfcont);
+ nfcont = ~NFCONT_WP;
+ nfcont |= NFCONT_NCE1 | NFCONT_NCE0 | NFCONT_ENABLE;
+ writel(nfcont, nand_reg-nfcont);
use clrsetbits_le32()
I will do that and also define some macros for the magic values.
Regards,
José Gonçalves
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
On 09/13/2012 12:45 AM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned
int ctrl) +{
+ s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+ struct nand_chip *this = mtd-priv;
+
+ if (ctrl NAND_CTRL_CHANGE) {
+ if (ctrl NAND_CLE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+ else if (ctrl NAND_ALE)
+ this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+ else
+ this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Not that you need to do the assignment into the structure use local
variable, no ?
Understood. I agree, it makes more sense to use a local variable to
address the proper NAND controller register.
I will update that.
Best regards,
José Gonçalves
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Hi Scott,
On 09/13/2012 12:20 AM, Scott Wood wrote:
On 09/12/2012 06:16 PM, José Miguel Gonçalves wrote:
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int
ctrl) +{
+s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+struct nand_chip *this = mtd-priv;
+
+if (ctrl NAND_CTRL_CHANGE) {
+if (ctrl NAND_CLE)
+this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+else if (ctrl NAND_ALE)
+this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+else
+this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Why do you have volatile in your s3c24xx_nand struct?
I use that as a rule to memory mapping of hardware registers.
Without it GCC optimization sometimes do bad things, like completely
removing sequences of code.
For instance, if you need to pause in a loop until some bit of a
register is changed (as it's done in the serial driver) and the struct
were this register is mapped don't have the volatile attribute, the GCC
optimizer removes the loop.
Regards,
José Gonçalves
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Dear José Miguel Gonçalves,
Hi Scott,
On 09/13/2012 12:20 AM, Scott Wood wrote:
On 09/12/2012 06:16 PM, José Miguel Gonçalves wrote:
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int
ctrl) +{
+s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+struct nand_chip *this = mtd-priv;
+
+if (ctrl NAND_CTRL_CHANGE) {
+if (ctrl NAND_CLE)
+this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+else if (ctrl NAND_ALE)
+this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+else
+this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Why do you have volatile in your s3c24xx_nand struct?
I use that as a rule to memory mapping of hardware registers.
Without it GCC optimization sometimes do bad things, like completely
removing sequences of code.
Not true unless your gcc is broken. Use proper accessors (readl()/writel()),
they have proper barriers already.
For instance, if you need to pause in a loop until some bit of a
register is changed (as it's done in the serial driver) and the struct
were this register is mapped don't have the volatile attribute, the GCC
optimizer removes the loop.
Yes, see above.
Regards,
José Gonçalves
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
On 09/13/2012 01:24 AM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Scott,
On 09/13/2012 12:20 AM, Scott Wood wrote:
On 09/12/2012 06:16 PM, José Miguel Gonçalves wrote:
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int
ctrl) +{
+s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+struct nand_chip *this = mtd-priv;
+
+if (ctrl NAND_CTRL_CHANGE) {
+if (ctrl NAND_CLE)
+this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+else if (ctrl NAND_ALE)
+this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+else
+this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile' qualifier
from pointer target type [enabled by default]
Why do you have volatile in your s3c24xx_nand struct?
I use that as a rule to memory mapping of hardware registers.
Without it GCC optimization sometimes do bad things, like completely
removing sequences of code.
Not true unless your gcc is broken. Use proper accessors (readl()/writel()),
they have proper barriers already.
For instance, if you need to pause in a loop until some bit of a
register is changed (as it's done in the serial driver) and the struct
were this register is mapped don't have the volatile attribute, the GCC
optimizer removes the loop.
Yes, see above.
When I was debugging U-Boot on the MIN2416 I saw this over-optimization
situation in the serial driver so I added the volatile attribute to all
structs that map the SoC registers. But, after you pointed to me that
the I/O macros have already incorporated the proper barriers, I looked
again to the serial driver source and noticed that I forgot to use that
macros on register accesses! I will change this and test it tomorrow
before resubmitting the patch.
Best regards,
José Gonçalves
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] S3C24XX: Add NAND Flash driver
Dear José Miguel Gonçalves,
On 09/13/2012 01:24 AM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Scott,
On 09/13/2012 12:20 AM, Scott Wood wrote:
On 09/12/2012 06:16 PM, José Miguel Gonçalves wrote:
Hi Marek,
On 09/12/2012 10:11 PM, Marek Vasut wrote:
Dear José Miguel Gonçalves,
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int
ctrl) +{
+s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+struct nand_chip *this = mtd-priv;
+
+if (ctrl NAND_CTRL_CHANGE) {
+if (ctrl NAND_CLE)
+this-IO_ADDR_W = (void __iomem *)nand-nfcmmd;
+else if (ctrl NAND_ALE)
+this-IO_ADDR_W = (void __iomem *)nand-nfaddr;
+else
+this-IO_ADDR_W = (void __iomem *)nand-nfdata;
Do you need this cast ?
Without it gcc gives me a warning:
s3c24xx_nand.c:90:20: warning: assignment discards `volatile'
qualifier from pointer target type [enabled by default]
Why do you have volatile in your s3c24xx_nand struct?
I use that as a rule to memory mapping of hardware registers.
Without it GCC optimization sometimes do bad things, like completely
removing sequences of code.
Not true unless your gcc is broken. Use proper accessors
(readl()/writel()), they have proper barriers already.
For instance, if you need to pause in a loop until some bit of a
register is changed (as it's done in the serial driver) and the struct
were this register is mapped don't have the volatile attribute, the GCC
optimizer removes the loop.
Yes, see above.
When I was debugging U-Boot on the MIN2416 I saw this over-optimization
situation in the serial driver
I just noticed that all those uart-something in your serial driver are
actually
register accesses. So that's flat wrong, use writel()/readl() etc accessors. Of
course doing it like you do without memory barriers will make it go south.
so I added the volatile attribute to all
structs that map the SoC registers. But, after you pointed to me that
the I/O macros have already incorporated the proper barriers, I looked
again to the serial driver source and noticed that I forgot to use that
macros on register accesses! I will change this and test it tomorrow
before resubmitting the patch.
WFM, thanks.
Best regards,
José Gonçalves
Ccing Gabriel, can you look at those patches ?
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
