Albert/Wolfgang,

I believe this patchset should be GTG. Are there any objections outstanding that I've failed to answer?

Thanks,

Tom

On Tue, May 3, 2011 at 12:29 PM, Mike Frysinger vapier@gentoo.org wrote:

On Tue, May 3, 2011 at 12:49, Michael Walle wrote:

...

Am Di, 3.05.2011, 00:45, schrieb Tom Warren:

...

Changes in V2:       - use 'gpio_pin' enum in gpio.h (Simon Glass review request)       - change 'GPIO_PORT8' to 'GPIO_FULLPORT' (Simon Glass request)       - change 'offset' to 'pin' globally

arch/arm/include/asm/arch-tegra2/gpio.h |  244

++++++++++++++++++++++++-

...

drivers/gpio/Makefile                   |    1 +  drivers/gpio/tegra2_gpio.c              |  299

Does this (SoC) driver belong to drivers/gpio/ or arch/arm/cpu/armv7/tegra2/? To me it seems that both are used for architectures/SoCs specific drivers (eg. blackfin, omap in arch/ and at91 in drivers/gpio/). What is the preferred directory?

following the Linux style makes sense to me.  only gpio expanders and such live in drivers/gpio/.  but until we get a more unified api effort, i'm not sure sweating the location of the driver is important. -mike

On Thu, Jun 2, 2011 at 1:56 PM, Wolfgang Denk wd@denx.de wrote:

Dear Tom Warren,

In message BANLkTik3Em_qLP7AHfpx2SvVMvo0EnsqoA@mail.gmail.com you wrote:

...

I believe this patchset should be GTG.  Are there any objections outstanding that I've failed to answer?

GTG ???

Good To Go.

I did not see any cleanup resulting from Mike's comment here:

05/02   Mike Frysinger     Re: [PATCH V2 1/2] GPIO: Tegra2: add GPIO driver for Tegra2        http://article.gmane.org/gmane.comp.boot-loaders.u-boot/99112 05/02   Tom Warren         Re: [PATCH V2 1/2] GPIO: Tegra2: add GPIO driver for Tegra2        http://article.gmane.org/gmane.comp.boot-loaders.u-boot/99114

Let me double-check. IIRC, one (cmd_gpio) didn't apply since it was from another SoC and didn't have the commands we use on Tegra, and the other (where the driver should go) ended with Mike saying "i'm not sure sweating the location of the driver is important", so I've left it with the other GPIO drivers.

Mike - you good with that status?

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@denx.de Little known fact about Middle Earth:   The Hobbits had a very sophi- sticated computer network!   It was a Tolkien Ring...

On Thu, Jun 2, 2011 at 2:34 PM, Wolfgang Denk wd@denx.de wrote:

Dear Tom Warren,

In message BANLkTi=w1u7q+W-VKRYCq-wB50e7edFAdA@mail.gmail.com you wrote:

...

Let me double-check. IIRC, one (cmd_gpio) didn't apply since it was from another SoC and didn't have the commands we use on Tegra, and the other (where the driver should go) ended with Mike saying "i'm not sure sweating the location of the driver is important", so I've left it with the other GPIO drivers.

What I'm referring to is the "lots of duplicated code" part.

Actually, if you do a kompare between cmd_gpio.c and tegra2_gpio.c, there's virtually no 'duplicated' code, just 2 different implementations of do_gpio, with different args and parsing of params, etc. I use info, port, input and output, and cmd_gpio uses input, set, clear and toggle. Since cmd_gpio.c isn't built, it's not duplicating any code in the U-Boot binary, space-wise. So there's no advantage to using it instead of my own driver's cmd parser.

Note that the PCA953x GPIO driver also does it's own cmd parsing in do_pca953x().

Tom

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@denx.de Computers are not intelligent. They only think they are.

On Thursday, June 02, 2011 17:55:55 Tom Warren wrote:

Actually, if you do a kompare between cmd_gpio.c and tegra2_gpio.c, there's virtually no 'duplicated' code, just 2 different implementations of do_gpio, with different args and parsing of params, etc. I use info, port, input and output, and cmd_gpio uses input, set, clear and toggle.

you're duplicating functionality. if the common cmd_gpio.c lacks features that yours adds, then send a patch to extend cmd_gpio.c. the fact that yours takes different arguments to do the same thing is irrelevant. change your scripts and/or muscle memory to the common cmd_gpio.c.

Note that the PCA953x GPIO driver also does it's own cmd parsing in do_pca953x().

then someone should clean it up -mike

On Thu, Jun 2, 2011 at 9:53 PM, Mike Frysinger vapier@gentoo.org wrote:

On Thursday, June 02, 2011 17:30:09 Tom Warren wrote:

...

Let me double-check. IIRC, one (cmd_gpio) didn't apply since it was from another SoC

no, it isnt.  it's using the generic GPIO API as defined by Linux and implemented by many ports in u-boot.  any new GPIO provider in u-boot should probably be implementing that API too. -mike

Alright. I'll look at using cmd_gpio instead of my code, adding any missing GPIO API functions to my driver.

Tom