Dear Ajay Bhargav,

In message 867041231.46865.1310120597114.JavaMail.root@ahm.einfochips.com you wrote:

Thanks for info, I have seen that. But i am not adding support for GPIO commands.

Like the gpio command, your code should use the same generic gpio API.

In reference to

...

...

+#define GPLR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x00) +#define GPDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x0c) +#define GPSR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x18) +#define GPCR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x24) +#define GSDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x54) +#define GCDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x60)

...

Please use a C struct to dsescribe the register layout.

I have followed similar way of using GPIO as used by "drivers/gpio/kw_gpio.c" and defined in "include/asm/arch-kirkwood/gpio.h",

To me this looks different.

so actually i am not adding GPIO support rather i wrote functions that are required by the Ethernet and other drivers.

You are not getting the point. I'm asking for two things:

- Do not use a "base address + offset" notation (with a declartion of register offsets in your header files), but use C structs instead to describe the regioster layout, and then use proper I/O accessor functions on them.

- Do not implement your own GPIO framework, but instead use existing code.

Best regards,

Wolfgang Denk