Re: GPIO support for HTC Dream

["Arve Hjønnevåg" <arve@android.com>]

On Fri, Dec 11, 2009 at 2:10 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
> Ok, thanks to Ryan and everyone... You were right, the code was way
> too complex.
>
> Cleaning it up according to Hartley's great howto resulted in:
>
>  arch/arm/Kconfig                     |    1
>  arch/arm/mach-msm/board-dream-gpio.c |  327 +++++++++--------------------------
>  arch/arm/mach-msm/board-dream.c      |   24 --
>  arch/arm/mach-msm/generic_gpio.c     |  270 ----------------------------
>  arch/arm/mach-msm/gpio_chip.h        |   30 ---
>  kernel/printk.c                      |    6
>  6 files changed, 102 insertions(+), 556 deletions(-)
>
>> As Ryan says, using gpiolib will clean this up immensely.
>>
>> It appears the what you really have for gpio's is 7 8-bit ports that start
>> with gpio number 128.  Each port appears to only have one control register.
>> This register is written with a '1' to drive the gpio as a high output and
>> '0' to drive it as a low output or to use it as an input.  Not really
>> sure if this is correct since the code is a bit screwy.
>
> It seems it is, because it seems to work.
>
>> Your current code is written so that all of the gpio's are in one "chip".
>> Because of this you are having to calculate the 'reg' needed to access the
>> gpio whenever you need to read or write to a gpio.
>>
>> If you follow the ep93xx implementation you can break the whole thing down
>> into 'banks' and simplify everything.  Something like:
>
> ...and get 100 lines of results, instead of 600... Yep, thanks.
>
> Now, it will still need some cleanups -- I'm not sure if gpios are
> dream-specific or generic for whole msm.... I kind of assume they
> should be generic for msm. Google people, can you help?

I'm not exactly sure what you are asking. Gpios in general are not
dream specific, the code in board-dream-gpio.c is. Some gpios (e.g.
DREAM_4_BALL_UP_0) are used for a dream specific function, but point
to a gpio that is generic to the msm architecture.

>
>> static struct dream_gpio_chip dream_gpio_banks[] = {
>>       DREAM_GPIO_BANK("MISC2", 0x00, DREAM_GPIO_MISC2_BASE),
>>       DREAM_GPIO_BANK("MISC3", 0x02, DREAM_GPIO_MISC3_BASE),
>>       DREAM_GPIO_BANK("MISC4", 0x04, DREAM_GPIO_MISC4_BASE),
>>       DREAM_GPIO_BANK("MISC5", 0x06, DREAM_GPIO_MISC5_BASE),
>>       DREAM_GPIO_BANK("INT2", 0x08, DREAM_GPIO_INT2_BASE),
>>       DREAM_GPIO_BANK("MISC1", 0x0a, DREAM_GPIO_MISC1_BASE),
>>       DREAM_GPIO_BANK("VIRTUAL", 0x12, DREAM_GPIO_VIRTUAL_BASE),
>> };
>
> Yep, that did the trick.
>
>> With this you can now just access the dream_gpio_chip data in all
>> the member function using the to_dream_gpio_chip() macro.
>>
>> I agree with Ryan and think you would be better off in the long
>> run to re-code this and use gpiolib.  You might also want to break
>> out all the gpio interrupt stuff and submit it as a seperate patch.
>
> Will do. Here's how the patch looks now.
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 1c4119c..8bb8546 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -565,6 +565,8 @@ config ARCH_MSM
>        select CPU_V6
>        select GENERIC_TIME
>        select GENERIC_CLOCKEVENTS
> +       select GENERIC_GPIO
> +       select ARCH_REQUIRE_GPIOLIB

Please do not do this in a board specific change. You have not changed
the msm generic gpio support to use gpiolib.

-- 
Arve Hjønnevåg