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

Wolfgang Denk
2011-07-08T11:29:20Z

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

--
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@netlabs.com>