On 4/16/2011 1:50 PM, Artem Bityutskiy wrote:

On Fri, 2011-04-15 at 20:20 +0530, Murali Nalajala wrote:

quoted

On 4/15/2011 3:21 PM, Artem Bityutskiy wrote:

quoted

On Tue, 2011-03-01 at 06:17 +0530, Murali Nalajala wrote:

quoted

+#define pr_fmt(fmt) "%s:" fmt, __func__
+
+#include<linux/kernel.h>
+#include<linux/mtd/mtd.h>
+#include<linux/mtd/nand.h>
+#include<linux/mtd/partitions.h>
+#include<linux/platform_device.h>
+#include<linux/sched.h>
+#include<linux/slab.h>
+#include<linux/dma-mapping.h>
+#include<asm/mach/flash.h>
+#include<mach/dma.h>
+
+#include "msm_nand.h"
+
+unsigned long msm_nand_phys;

No global variables like this please. Here is how you use them:

+extern unsigned long msm_nand_phys;

extern declaration in the 'C' file causing warning. We are in plan to
upload the OneNAND changes soon which makes use of the same .h file.

So you say that OneNAND (an independent driver) is going to use this
variable? Are you also going to export it?

I haven't decided yet when oneNAND driver come in still it is under 
discussion. There is a chance that if OneNAND comes as an individual 
driver then i have to export this variable.

Can i export this variable now? What is your suggestion?

quoted

quoted

+#define MSM_NAND_REG(off) (msm_nand_phys + (off))
+
+#define MSM_NAND_FLASH_CMD            MSM_NAND_REG(0x0000)
+#define MSM_NAND_ADDR0                MSM_NAND_REG(0x0004)

Could you please make the macros to take the "struct msm_nand_chip
*chip" argument instead, and store the pase address there. Do not hide
the fact that those macros are actually functions, not constant - this
is error prone.

Besides, I'm do not know your HW, but if you have several controllers
with various base addresses - your driver won't work.

you are correct, we have multiple controllers, which breaks this logic
in future.

So then make your macros to accept the base address as an argument
instead please.

Thanks,
Murali N

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.