
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