linux-mips
[Top] [All Lists]

Re: [PATCH] au1000: convert to using gpiolib

To: Manuel Lauss <mano@roarinelk.homelinux.net>
Subject: Re: [PATCH] au1000: convert to using gpiolib
From: Florian Fainelli <florian@openwrt.org>
Date: Fri, 16 Jan 2009 11:10:25 +0100
Cc: ralf@linux-mips.org, Linux-MIPS <linux-mips@linux-mips.org>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:in-reply-to :references:date:x-google-sender-auth:message-id:subject:from:to:cc :content-type; bh=dsgDLXtW9cKPTv1SIMBiK5iWWrY7WGkFWapSiH95juM=; b=EdUMvDQbNlR8cRxAIeCjUGRyuSjHte/5YdrxQ4/Z6j7N9hsigCkCbG2BiRT5M+lBLv I0fCvBPkm/gdFYg1UWTAIYmE/JfAowFrR0f4DFT4ZwVo/GS/sgp6k1Jb7Pa6szty5ESi hrov/7Di0oX6OVFLEWi6XjG5FcyQ+Elo5pfa8=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; b=ob0WkB6fdd2Hxmk0/p2G2K3pKwVIy0fQiOkRRlHGbC0YQ5SH5aDcou4eAz8tJ1FSTy LJGOgHDvonL+S9IuoKa9b38kKddfJ0Bwdv3eCAKLCuRUnZtdp+KVYYoV8vZZX+cDi2Go yyb2vVkfq4blVyTz8W+GD/awYzzXgzVp7ZbzM=
In-reply-to: <20090115205851.GD8656@roarinelk.homelinux.net>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <200901151646.49591.florian@openwrt.org> <20090115205851.GD8656@roarinelk.homelinux.net>
Sender: linux-mips-bounce@linux-mips.org
Hi Manuel,

2009/1/15 Manuel Lauss <mano@roarinelk.homelinux.net>
Hi Florian,

On Thu, Jan 15, 2009 at 04:46:48PM +0100, Florian Fainelli wrote:
> This patch converts the GPIO board code to use gpiolib

Because of the 'enable | value' scheme I believe you don't require any
locking here.

Right, thanks.
 



> +struct au1000_gpio_chip au1000_gpio_chip[] = {
> +     [0] = {
> +             .regbase                        = (void __iomem *)SYS_BASE,
> +             .chip = {
> +                     .label                  = "au1000-gpio1",
> +                     .direction_input        = au1000_gpio1_direction_input,
> +                     .direction_output       = au1000_gpio1_direction_output,
> +                     .get                    = au1000_gpio1_get,
> +                     .set                    = au1000_gpio1_set,
> +                     .base                   = 0,
> +                     .ngpio                  = 32,
> +             },
> +     },
> +#if !defined(CONFIG_SOC_AU1000)
> +     [1] = {
> +             .regbase                        = (void __iomem *)GPIO2_BASE,
> +             .chip = {
> +                     .label                  = "au1000-gpio2",
> +                     .direction_input        = au1000_gpio2_direction_input,
> +                     .direction_output       = au1000_gpio2_direction_output,
> +                     .get                    = au1000_gpio2_get,
> +                     .set                    = au1000_gpio2_set,
> +                     .base                   = AU1XXX_GPIO_BASE,
> +                     .ngpio                  = 32,
> +             },
> +     },
>  #endif
> -     else
> -             return au1xxx_gpio1_read(gpio);
> -}
> -EXPORT_SYMBOL(au1xxx_gpio_get_value);
> +};
[...]
> +static int __init au1000_gpio_init(void)
>  {
> -     if (gpio >= AU1XXX_GPIO_BASE)
> -#if defined(CONFIG_SOC_AU1000)
> -             ;
> -#else
> -             au1xxx_gpio2_write(gpio, value);
> -#endif
> -     else
> -             au1xxx_gpio1_write(gpio, value);
> -}
> -EXPORT_SYMBOL(au1xxx_gpio_set_value);
> +     gpiochip_add(&au1000_gpio_chip[0].chip);
> +#if !defined(CONFIG_SOC_AU1000)
> +     gpiochip_add(&au1000_gpio_chip[1].chip);
>
[...]
> +arch_initcall(au1000_gpio_init);

Can you please make the gpiolib registration dependent on a
CONFIG symbol?  I.e. make the au1000_gpio{,2}_direction() and
friends calls globally visible but let the individual boards
decide whether they want to use the gpio numbering imposed by
this patch.

Would something like #ifdef CONFIG_AU1000_NON_STD_GPIOS be ok with you ?
Or maybe we could get the base information from board-specific code ?
 


Long explanation:  I maintain a number of modules with a common connector
interface, based on different architectures (sh, mips and arm so far).
I also maintain a few baseboards which can carry theese modules.  Modules
provide 16 gpios numbered 0-15, which are used by the baseboards.  Since
I need to keep the baseboard code free of arch-specific hacks, every module
provides its own gpio-chip which distributes the gpio-lib calls to various
on- and off-chip peripherals.  On my alchemy board in particular, those 16
gpios are provided by a mixture of gpio1, gpio2 and FPGA based pins (yes
I repeatedly moanoed to the hw guys about this but pin multiplexing and
required features make a sane implementation difficult; but at least I was
allowed to write the VHDL for the fpga-based gpios).

Your explanation makes perfect sense to me.
 


If this explanation doesn't make sense I'll gladly whip up an addon patch.


> diff --git a/arch/mips/include/asm/mach-au1x00/gpio.h b/arch/mips/include/asm/mach-au1x00/gpio.h
> index 2dc61e0..34d9b72 100644
> --- a/arch/mips/include/asm/mach-au1x00/gpio.h
> +++ b/arch/mips/include/asm/mach-au1x00/gpio.h
> @@ -5,65 +5,29 @@
>
>  #define AU1XXX_GPIO_BASE     200

please change this to AU1XXX_GPIO2_BASE (it's the base number
of the GPIO2 block pins after all)

Ok.
 
Thanks for your comments, I will respin with your comments.

<Prev in Thread] Current Thread [Next in Thread>