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

Reply via email to