[U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD

[Wolfgang Denk]

Dear Ajay Bhargav,

In message <867041231.46865.1310120597114.JavaMail.root at 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Unix is like a toll road on which you have to stop every 50  feet  to
pay another nickel. But hey! You only feel 5 cents poorer each time.
                 - Larry Wall in <1992Aug13.192357.15731 at netlabs.com>