* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 15:36 [PATCH] add gpiolib support for mpc5200 Sascha Hauer
@ 2008-04-24 18:45 ` Grant Likely
2008-04-25 10:53 ` Sascha Hauer
2008-04-25 15:30 ` Matt Sealey
2008-04-24 19:11 ` Grant Likely
` (3 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Grant Likely @ 2008-04-24 18:45 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi all,
>
> Feel free to comment on this.
>
> Sascha
>
>
> This patch adds gpiolib support for mpc5200 SOCs. I'm not sure
> whether it's a good idea to make this optional via kconfig.
> The gpt devices only support a single gpio. In the current of_gpio
> implementation each chip consumes 32 GPIOs which leads to huge
> gaps.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Looks pretty good. You've saved me the need to go write a driver
myself. Comments below, but I'll pull it into my tree and give it a
spin.
I don't see any mechanism for setting the open drain state of the pin.
That will either need to be done by platform code or encoded into the
device tree. Does the OF gpio infrastructure provide any callback to
the driver when something requests the pin? That would seem to be the
ideal place to set the open drain state.
You'll also need to document the format of the gpio pin specifier for
these devices (ie. first cell is GPIO number, second cell is ????).
As for the wide spans caused by gpt gpios, it is probably okay for
now, but we can rework it to do something clever (like have a single
registration for all gpt gpios) at a later date.
Cheers,
g.
>
> ---
> arch/powerpc/platforms/52xx/Kconfig | 6
> arch/powerpc/platforms/52xx/Makefile | 2
> arch/powerpc/platforms/52xx/mpc52xx_gpio.c | 408 +++++++++++++++++++++++++++++
> 3 files changed, 416 insertions(+)
>
> Index: arch/powerpc/platforms/52xx/mpc52xx_gpio.c
> ===================================================================
> --- /dev/null
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
> @@ -0,0 +1,408 @@
> +/*
> + * MPC52xx gpio driver
> + *
> + * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/of_gpio.h>
> +#include <linux/io.h>
> +#include <asm/of_platform.h>
> +#include <asm/prom.h>
> +#include <asm/gpio.h>
> +#include <asm/mpc52xx.h>
> +#include <sysdev/fsl_soc.h>
> +
> +static DEFINE_SPINLOCK(gpio_lock);
> +
> +/*
> + * GPIO LIB API implementation for wakeup GPIOs.
> + *
> + * There's a maximum of 8 wakeup GPIOs. Which of these are available
> + * for use depends on your board setup.
> + *
> + * 0 -> GPIO_WKUP_7
> + * 1 -> GPIO_WKUP_6
> + * 2 -> PSC6_1
> + * 3 -> PSC6_0
> + * 4 -> ETH_17
> + * 5 -> PSC3_9
> + * 6 -> PSC2_4
> + * 7 -> PSC1_4
> + *
> + */
> +static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
> +
> + pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
dev_dbg maybe?
> +
> + return ret;
> +}
> +
> +static void mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_8(®s->wkup_dvo);
> + if (val)
> + tmp |= 1 << (7 - gpio);
> + else
> + tmp &= ~(1 << (7 - gpio));
> + out_8(®s->wkup_dvo, tmp);
Rather than read/modify/write of the device register; the function
would probably be faster (one fewer barrier) if you used a shadow
register of the pin state and the critical region would be
shorter/simpler. Also, while this device doesn't have the side
effects associated with shared input/output register, it might still
be good form to use a shadow register just for the sake of clarity.
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_8(®s->wkup_ddr);
> + tmp &= ~(1 << (7 - gpio));
> + out_8(®s->wkup_ddr, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + /* First set initial value */
> + mpc52xx_wkup_gpio_set(gc, gpio, val);
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + /* Then set direction */
> + tmp = in_8(®s->wkup_ddr);
> + tmp |= 1 << (7 - gpio);
> + out_8(®s->wkup_ddr, tmp);
> +
> + /* Finally enable the pin */
> + tmp = in_8(®s->wkup_gpioe);
> + tmp |= 1 << (7 - gpio);
> + out_8(®s->wkup_gpioe, tmp);
Do you want/need the cost of enabling the pin every time dir_out is
called? Can it be done when the pin is requested instead? Or by the
board firmware/platform code? Some drivers (for example the i2c
bitbang driver for the clock signal; see i2c-gpio.c) change the state
by changing the direction of the pin.
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_wkup_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 8;
> + chip->gc.direction_input = mpc52xx_wkup_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_wkup_gpio_dir_out;
> + chip->gc.get = mpc52xx_wkup_gpio_get;
> + chip->gc.set = mpc52xx_wkup_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static int mpc52xx_gpiochip_remove(struct of_device *ofdev)
> +{
> + return -EBUSY;
> +}
> +
> +static struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpio-wkup",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_wkup_gpiochip_driver = {
> + .name = "gpio_wkup",
> + .match_table = mpc52xx_wkup_gpiochip_match,
> + .probe = mpc52xx_wkup_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +/*
> + * GPIO LIB API implementation for simple GPIOs
> + *
> + * There's a maximum of 32 simple GPIOs. Which of these are available
> + * for use depends on your board setup.
> + * The numbering reflects the bit numbering in the port registers:
> + *
> + * 0..1 > reserved
> + * 2..3 > IRDA
> + * 4..7 > ETHR
> + * 8..11 > reserved
> + * 12..15 > USB
> + * 16..17 > reserved
> + * 18..23 > PSC3
> + * 24..27 > PSC2
> + * 28..31 > PSC1
> + */
> +static int mpc52xx_simple_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + ret = (in_be32(®s->simple_ival) >> (31 - gpio)) & 1;
> +
> + return ret;
> +}
> +
> +static void mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_be32(®s->simple_dvo);
> + if (val)
> + tmp |= 1 << (31 - gpio);
> + else
> + tmp &= ~(1 << (31 - gpio));
> + out_be32(®s->simple_dvo, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_simple_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_be32(®s->simple_ddr);
> + tmp &= ~(1 << (31 - gpio));
> + out_be32(®s->simple_ddr, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_simple_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + /* First set initial value */
> + mpc52xx_simple_gpio_set(gc, gpio, val);
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + /* Then set direction */
> + tmp = in_be32(®s->simple_ddr);
> + tmp |= 1 << (31 - gpio);
> + out_be32(®s->simple_ddr, tmp);
> +
> + /* Finally enable the pin */
> + tmp = in_be32(®s->simple_gpioe);
> + tmp |= 1 << (31 - gpio);
> + out_be32(®s->simple_gpioe, tmp);
ditto here
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_simple_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 32;
> + chip->gc.direction_input = mpc52xx_simple_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_simple_gpio_dir_out;
> + chip->gc.get = mpc52xx_simple_gpio_get;
> + chip->gc.set = mpc52xx_simple_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static struct of_device_id mpc52xx_simple_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpio",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_simple_gpiochip_driver = {
> + .name = "gpio",
> + .match_table = mpc52xx_simple_gpiochip_match,
> + .probe = mpc52xx_simple_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +/*
> + * GPIO LIB API implementation for gpt GPIOs.
> + *
> + * Each gpt only has a single GPIO.
> + */
> +static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + return (in_be32(®s->status) & (1 << (31 - 23))) ? 1 : 0;
> +
> + return ret;
> +}
> +
> +static void mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
> +
> + if (val)
> + out_be32(®s->mode, 0x34);
> + else
> + out_be32(®s->mode, 0x24);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt *regs = mm_gc->regs;
> +
> + out_be32(®s->mode, 0x04);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_gpt_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + mpc52xx_gpt_gpio_set(gc, gpio, val);
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_gpt_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 1;
> + chip->gc.direction_input = mpc52xx_gpt_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
> + chip->gc.get = mpc52xx_gpt_gpio_get;
> + chip->gc.set = mpc52xx_gpt_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpt-gpio",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_gpt_gpiochip_driver = {
> + .name = "gpio_gpt",
> + .match_table = mpc52xx_gpt_gpiochip_match,
> + .probe = mpc52xx_gpt_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +static int __init mpc52xx_gpio_init(void)
> +{
> + if (of_register_platform_driver(&mpc52xx_wkup_gpiochip_driver))
> + printk(KERN_ERR "Unable to register wakeup GPIO driver\n");
> +
> + if (of_register_platform_driver(&mpc52xx_simple_gpiochip_driver))
> + printk(KERN_ERR "Unable to register simple GPIO driver\n");
> +
> + if (of_register_platform_driver(&mpc52xx_gpt_gpiochip_driver))
> + printk(KERN_ERR "Unable to register gpt GPIO driver\n");
> +
> + return 0;
> +}
> +
> +
> +/* Make sure we get initialised before anyone else tries to use us */
> +subsys_initcall(mpc52xx_gpio_init);
> +
> +/* No exit call at the moment as we cannot unregister of gpio chips */
> +
> +MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
> +MODULE_LICENSE("GPL v2");
> +
> Index: arch/powerpc/platforms/52xx/Kconfig
> ===================================================================
> --- a/arch/powerpc/platforms/52xx/Kconfig.orig
> +++ b/arch/powerpc/platforms/52xx/Kconfig
> @@ -44,3 +44,9 @@ config PPC_MPC5200_BUGFIX
>
> It is safe to say 'Y' here
>
> +config PPC_MPC5200_GPIO
> + bool "MPC5200 GPIO support"
> + depends on PPC_MPC52xx
> + select HAVE_GPIO_LIB
> + help
> + Enable gpiolib support for mpc5200 based boards
> Index: arch/powerpc/platforms/52xx/Makefile
> ===================================================================
> --- a/arch/powerpc/platforms/52xx/Makefile.orig
> +++ b/arch/powerpc/platforms/52xx/Makefile
> @@ -14,3 +14,5 @@ obj-$(CONFIG_PM) += mpc52xx_sleep.o mpc
> ifeq ($(CONFIG_PPC_LITE5200),y)
> obj-$(CONFIG_PM) += lite5200_sleep.o lite5200_pm.o
> endif
> +
> +obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
> \ No newline at end of file
>
> --
> Pengutronix e.K. - Linux Solutions for Science and Industry
> -----------------------------------------------------------
> Kontakt-Informationen finden Sie im Header dieser Mail oder
> auf der Webseite -> http://www.pengutronix.de/impressum/ <-
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 18:45 ` Grant Likely
@ 2008-04-25 10:53 ` Sascha Hauer
2008-04-25 12:32 ` Anton Vorontsov
2008-04-25 15:30 ` Matt Sealey
1 sibling, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2008-04-25 10:53 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
On Thu, Apr 24, 2008 at 12:45:49PM -0600, Grant Likely wrote:
> On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi all,
> >
> > Feel free to comment on this.
> >
> > Sascha
> >
> >
> > This patch adds gpiolib support for mpc5200 SOCs. I'm not sure
> > whether it's a good idea to make this optional via kconfig.
> > The gpt devices only support a single gpio. In the current of_gpio
> > implementation each chip consumes 32 GPIOs which leads to huge
> > gaps.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Looks pretty good. You've saved me the need to go write a driver
> myself. Comments below, but I'll pull it into my tree and give it a
> spin.
>
> I don't see any mechanism for setting the open drain state of the pin.
> That will either need to be done by platform code or encoded into the
> device tree. Does the OF gpio infrastructure provide any callback to
> the driver when something requests the pin? That would seem to be the
> ideal place to set the open drain state.
No, unfortunately not. The generic gpio stuff does not provide this
callback, so of gpio has no chance to do so.
This would also be a good place to catch the registration of reserved
pins, so maybe it's worth discussing this with the gpiolib maintainers.
>
> You'll also need to document the format of the gpio pin specifier for
> these devices (ie. first cell is GPIO number, second cell is ????).
I've taken the two-cell approach from booting-without-of.txt. There the
second cell is for flags. We could encode the open drain state into
this.
>
> As for the wide spans caused by gpt gpios, it is probably okay for
> now, but we can rework it to do something clever (like have a single
> registration for all gpt gpios) at a later date.
I would rather teach the of gpio infrastructure not to reserve 32 gpios
for each chip. This will bite us once we want to support gpio chips with
more than 32 gpios anyway.
> > + *
> > + */
> > +static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> > +{
> > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> > + unsigned int ret;
> > +
> > + ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
> > +
> > + pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
>
> dev_dbg maybe?
We would have to carry the device from the probe function to this
function just for the debugging output. I'm not sure if it's worth it.
>
> > +
> > + return ret;
> > +}
> > +
> > +static void mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > +{
> > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> > + unsigned int tmp;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&gpio_lock, flags);
> > +
> > + tmp = in_8(®s->wkup_dvo);
> > + if (val)
> > + tmp |= 1 << (7 - gpio);
> > + else
> > + tmp &= ~(1 << (7 - gpio));
> > + out_8(®s->wkup_dvo, tmp);
>
> Rather than read/modify/write of the device register; the function
> would probably be faster (one fewer barrier) if you used a shadow
> register of the pin state and the critical region would be
> shorter/simpler. Also, while this device doesn't have the side
> effects associated with shared input/output register, it might still
> be good form to use a shadow register just for the sake of clarity.
OK
>
> > +
> > + spin_unlock_irqrestore(&gpio_lock, flags);
> > +
> > + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> > +}
> > +
> > +static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > +{
> > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> > + unsigned int tmp;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&gpio_lock, flags);
> > +
> > + tmp = in_8(®s->wkup_ddr);
> > + tmp &= ~(1 << (7 - gpio));
> > + out_8(®s->wkup_ddr, tmp);
> > +
> > + spin_unlock_irqrestore(&gpio_lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> > +{
> > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> > + unsigned int tmp;
> > + unsigned long flags;
> > +
> > + /* First set initial value */
> > + mpc52xx_wkup_gpio_set(gc, gpio, val);
> > +
> > + spin_lock_irqsave(&gpio_lock, flags);
> > +
> > + /* Then set direction */
> > + tmp = in_8(®s->wkup_ddr);
> > + tmp |= 1 << (7 - gpio);
> > + out_8(®s->wkup_ddr, tmp);
> > +
> > + /* Finally enable the pin */
> > + tmp = in_8(®s->wkup_gpioe);
> > + tmp |= 1 << (7 - gpio);
> > + out_8(®s->wkup_gpioe, tmp);
>
> Do you want/need the cost of enabling the pin every time dir_out is
> called? Can it be done when the pin is requested instead? Or by the
> board firmware/platform code? Some drivers (for example the i2c
> bitbang driver for the clock signal; see i2c-gpio.c) change the state
> by changing the direction of the pin.
I changed this to use shadow registers aswell, so we have two fewer register
accesses. I don't have a good feeling about enabling the pins in the
platform code because at the moment we can change the gpio routing without
recompiling the kernel.
Just to know what we are talking about I made some measurements. As a
first improvement I added a lockless inline version of gpio_set so that
we do not have to aqcuire the spinlock twice. With this I get a maximum
toggle frequency using gpio_direction_output() of 760 KHz. Without pin
enabling this frequency increases to 870 KHz. Using gpio_set_value() we
have 3.3 MHz.
See my other mail for an updated patch
Sascha
--
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-25 10:53 ` Sascha Hauer
@ 2008-04-25 12:32 ` Anton Vorontsov
0 siblings, 0 replies; 12+ messages in thread
From: Anton Vorontsov @ 2008-04-25 12:32 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
On Fri, Apr 25, 2008 at 12:53:33PM +0200, Sascha Hauer wrote:
[...]
> >
> > As for the wide spans caused by gpt gpios, it is probably okay for
> > now, but we can rework it to do something clever (like have a single
> > registration for all gpt gpios) at a later date.
>
> I would rather teach the of gpio infrastructure not to reserve 32 gpios
> for each chip. This will bite us once we want to support gpio chips with
> more than 32 gpios anyway.
The patch already exists:
http://ozlabs.org/pipermail/linuxppc-dev/2008-March/052881.html
It is waiting for few patches that are in the -mm tree (Andrew said that
he'll push them into 2.6.26 at some point).
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 18:45 ` Grant Likely
2008-04-25 10:53 ` Sascha Hauer
@ 2008-04-25 15:30 ` Matt Sealey
2008-04-25 18:51 ` Sascha Hauer
1 sibling, 1 reply; 12+ messages in thread
From: Matt Sealey @ 2008-04-25 15:30 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
Can I make a suggestion for this chip support?
On certain 5200 boards these devices are not usable since they are not
connected. My concern is the Efika where we only have 2 wakeup and 1 simple
GPIO available on the board and maybe a few others with a bit of tweaking
and messing around.
Instead of having a block for GPIO which has 32 pins, and the ability to
use all of them just by asking for it (get will return a pin regardless)
I implemented a silly hackish "gpio-mask" property in the efika.forth device
tree so the gpiolib "allocate" style functions can tell if a pin is
actually usable or not.
Can we put some small thought into defining a standard for this, and possibly
moving the "number of gpios in the chip" into the device tree? It doesn't
seem particularly complete to just allow blanket use (which may be unsafe and
may change per-board) of pins just because you can twiddle a bit.. (or use
a pin when another driver may be using it).
This will tie into Grant's comments about optimizing the enabling of the
GPIO on every direction change etc. (it should be enabled on allocation
and then you have to make sure you free up your use of the pin, resource
tracking in the kernel is important.. flipping bits here and there without
something in the way in the API is kind of clumsy)
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
Grant Likely wrote:
> On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> Hi all,
>>
>> Feel free to comment on this.
>>
>> Sascha
>>
>>
>> This patch adds gpiolib support for mpc5200 SOCs. I'm not sure
>> whether it's a good idea to make this optional via kconfig.
>> The gpt devices only support a single gpio. In the current of_gpio
>> implementation each chip consumes 32 GPIOs which leads to huge
>> gaps.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Looks pretty good. You've saved me the need to go write a driver
> myself. Comments below, but I'll pull it into my tree and give it a
> spin.
>
> I don't see any mechanism for setting the open drain state of the pin.
> That will either need to be done by platform code or encoded into the
> device tree. Does the OF gpio infrastructure provide any callback to
> the driver when something requests the pin? That would seem to be the
> ideal place to set the open drain state.
>
> You'll also need to document the format of the gpio pin specifier for
> these devices (ie. first cell is GPIO number, second cell is ????).
>
> As for the wide spans caused by gpt gpios, it is probably okay for
> now, but we can rework it to do something clever (like have a single
> registration for all gpt gpios) at a later date.
>
> Cheers,
> g.
>
>> ---
>> arch/powerpc/platforms/52xx/Kconfig | 6
>> arch/powerpc/platforms/52xx/Makefile | 2
>> arch/powerpc/platforms/52xx/mpc52xx_gpio.c | 408 +++++++++++++++++++++++++++++
>> 3 files changed, 416 insertions(+)
>>
>> Index: arch/powerpc/platforms/52xx/mpc52xx_gpio.c
>> ===================================================================
>> --- /dev/null
>> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
>> @@ -0,0 +1,408 @@
>> +/*
>> + * MPC52xx gpio driver
>> + *
>> + * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +
>> +#include <linux/of.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/io.h>
>> +#include <asm/of_platform.h>
>> +#include <asm/prom.h>
>> +#include <asm/gpio.h>
>> +#include <asm/mpc52xx.h>
>> +#include <sysdev/fsl_soc.h>
>> +
>> +static DEFINE_SPINLOCK(gpio_lock);
>> +
>> +/*
>> + * GPIO LIB API implementation for wakeup GPIOs.
>> + *
>> + * There's a maximum of 8 wakeup GPIOs. Which of these are available
>> + * for use depends on your board setup.
>> + *
>> + * 0 -> GPIO_WKUP_7
>> + * 1 -> GPIO_WKUP_6
>> + * 2 -> PSC6_1
>> + * 3 -> PSC6_0
>> + * 4 -> ETH_17
>> + * 5 -> PSC3_9
>> + * 6 -> PSC2_4
>> + * 7 -> PSC1_4
>> + *
>> + */
>> +static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
>> + unsigned int ret;
>> +
>> + ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
>> +
>> + pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
>
> dev_dbg maybe?
>
>> +
>> + return ret;
>> +}
>> +
>> +static void mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + tmp = in_8(®s->wkup_dvo);
>> + if (val)
>> + tmp |= 1 << (7 - gpio);
>> + else
>> + tmp &= ~(1 << (7 - gpio));
>> + out_8(®s->wkup_dvo, tmp);
>
> Rather than read/modify/write of the device register; the function
> would probably be faster (one fewer barrier) if you used a shadow
> register of the pin state and the critical region would be
> shorter/simpler. Also, while this device doesn't have the side
> effects associated with shared input/output register, it might still
> be good form to use a shadow register just for the sake of clarity.
>
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +}
>> +
>> +static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + tmp = in_8(®s->wkup_ddr);
>> + tmp &= ~(1 << (7 - gpio));
>> + out_8(®s->wkup_ddr, tmp);
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + /* First set initial value */
>> + mpc52xx_wkup_gpio_set(gc, gpio, val);
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + /* Then set direction */
>> + tmp = in_8(®s->wkup_ddr);
>> + tmp |= 1 << (7 - gpio);
>> + out_8(®s->wkup_ddr, tmp);
>> +
>> + /* Finally enable the pin */
>> + tmp = in_8(®s->wkup_gpioe);
>> + tmp |= 1 << (7 - gpio);
>> + out_8(®s->wkup_gpioe, tmp);
>
> Do you want/need the cost of enabling the pin every time dir_out is
> called? Can it be done when the pin is requested instead? Or by the
> board firmware/platform code? Some drivers (for example the i2c
> bitbang driver for the clock signal; see i2c-gpio.c) change the state
> by changing the direction of the pin.
>
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devinit mpc52xx_wkup_gpiochip_probe(struct of_device *ofdev,
>> + const struct of_device_id *match)
>> +{
>> + struct of_mm_gpio_chip *mmchip;
>> + struct of_gpio_chip *chip;
>> +
>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>> + if (!mmchip)
>> + return -ENOMEM;
>> +
>> + chip = &mmchip->of_gc;
>> +
>> + chip->gpio_cells = 2;
>> + chip->gc.ngpio = 8;
>> + chip->gc.direction_input = mpc52xx_wkup_gpio_dir_in;
>> + chip->gc.direction_output = mpc52xx_wkup_gpio_dir_out;
>> + chip->gc.get = mpc52xx_wkup_gpio_get;
>> + chip->gc.set = mpc52xx_wkup_gpio_set;
>> +
>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>> +}
>> +
>> +static int mpc52xx_gpiochip_remove(struct of_device *ofdev)
>> +{
>> + return -EBUSY;
>> +}
>> +
>> +static struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
>> + {
>> + .compatible = "fsl,mpc5200-gpio-wkup",
>> + },
>> + {}
>> +};
>> +
>> +static struct of_platform_driver mpc52xx_wkup_gpiochip_driver = {
>> + .name = "gpio_wkup",
>> + .match_table = mpc52xx_wkup_gpiochip_match,
>> + .probe = mpc52xx_wkup_gpiochip_probe,
>> + .remove = mpc52xx_gpiochip_remove,
>> +};
>> +
>> +/*
>> + * GPIO LIB API implementation for simple GPIOs
>> + *
>> + * There's a maximum of 32 simple GPIOs. Which of these are available
>> + * for use depends on your board setup.
>> + * The numbering reflects the bit numbering in the port registers:
>> + *
>> + * 0..1 > reserved
>> + * 2..3 > IRDA
>> + * 4..7 > ETHR
>> + * 8..11 > reserved
>> + * 12..15 > USB
>> + * 16..17 > reserved
>> + * 18..23 > PSC3
>> + * 24..27 > PSC2
>> + * 28..31 > PSC1
>> + */
>> +static int mpc52xx_simple_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
>> + unsigned int ret;
>> +
>> + ret = (in_be32(®s->simple_ival) >> (31 - gpio)) & 1;
>> +
>> + return ret;
>> +}
>> +
>> +static void mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + tmp = in_be32(®s->simple_dvo);
>> + if (val)
>> + tmp |= 1 << (31 - gpio);
>> + else
>> + tmp &= ~(1 << (31 - gpio));
>> + out_be32(®s->simple_dvo, tmp);
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +}
>> +
>> +static int mpc52xx_simple_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + tmp = in_be32(®s->simple_ddr);
>> + tmp &= ~(1 << (31 - gpio));
>> + out_be32(®s->simple_ddr, tmp);
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int mpc52xx_simple_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpio *regs = mm_gc->regs;
>> + unsigned int tmp;
>> + unsigned long flags;
>> +
>> + /* First set initial value */
>> + mpc52xx_simple_gpio_set(gc, gpio, val);
>> +
>> + spin_lock_irqsave(&gpio_lock, flags);
>> +
>> + /* Then set direction */
>> + tmp = in_be32(®s->simple_ddr);
>> + tmp |= 1 << (31 - gpio);
>> + out_be32(®s->simple_ddr, tmp);
>> +
>> + /* Finally enable the pin */
>> + tmp = in_be32(®s->simple_gpioe);
>> + tmp |= 1 << (31 - gpio);
>> + out_be32(®s->simple_gpioe, tmp);
>
> ditto here
>
>> +
>> + spin_unlock_irqrestore(&gpio_lock, flags);
>> +
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devinit mpc52xx_simple_gpiochip_probe(struct of_device *ofdev,
>> + const struct of_device_id *match)
>> +{
>> + struct of_mm_gpio_chip *mmchip;
>> + struct of_gpio_chip *chip;
>> +
>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>> + if (!mmchip)
>> + return -ENOMEM;
>> +
>> + chip = &mmchip->of_gc;
>> +
>> + chip->gpio_cells = 2;
>> + chip->gc.ngpio = 32;
>> + chip->gc.direction_input = mpc52xx_simple_gpio_dir_in;
>> + chip->gc.direction_output = mpc52xx_simple_gpio_dir_out;
>> + chip->gc.get = mpc52xx_simple_gpio_get;
>> + chip->gc.set = mpc52xx_simple_gpio_set;
>> +
>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>> +}
>> +
>> +static struct of_device_id mpc52xx_simple_gpiochip_match[] = {
>> + {
>> + .compatible = "fsl,mpc5200-gpio",
>> + },
>> + {}
>> +};
>> +
>> +static struct of_platform_driver mpc52xx_simple_gpiochip_driver = {
>> + .name = "gpio",
>> + .match_table = mpc52xx_simple_gpiochip_match,
>> + .probe = mpc52xx_simple_gpiochip_probe,
>> + .remove = mpc52xx_gpiochip_remove,
>> +};
>> +
>> +/*
>> + * GPIO LIB API implementation for gpt GPIOs.
>> + *
>> + * Each gpt only has a single GPIO.
>> + */
>> +static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
>> + unsigned int ret;
>> +
>> + return (in_be32(®s->status) & (1 << (31 - 23))) ? 1 : 0;
>> +
>> + return ret;
>> +}
>> +
>> +static void mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
>> +
>> + if (val)
>> + out_be32(®s->mode, 0x34);
>> + else
>> + out_be32(®s->mode, 0x24);
>> +
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +}
>> +
>> +static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>> + struct mpc52xx_gpt *regs = mm_gc->regs;
>> +
>> + out_be32(®s->mode, 0x04);
>> +
>> + return 0;
>> +}
>> +
>> +static int mpc52xx_gpt_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>> +{
>> + mpc52xx_gpt_gpio_set(gc, gpio, val);
>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devinit mpc52xx_gpt_gpiochip_probe(struct of_device *ofdev,
>> + const struct of_device_id *match)
>> +{
>> + struct of_mm_gpio_chip *mmchip;
>> + struct of_gpio_chip *chip;
>> +
>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>> + if (!mmchip)
>> + return -ENOMEM;
>> +
>> + chip = &mmchip->of_gc;
>> +
>> + chip->gpio_cells = 2;
>> + chip->gc.ngpio = 1;
>> + chip->gc.direction_input = mpc52xx_gpt_gpio_dir_in;
>> + chip->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
>> + chip->gc.get = mpc52xx_gpt_gpio_get;
>> + chip->gc.set = mpc52xx_gpt_gpio_set;
>> +
>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>> +}
>> +
>> +static struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
>> + {
>> + .compatible = "fsl,mpc5200-gpt-gpio",
>> + },
>> + {}
>> +};
>> +
>> +static struct of_platform_driver mpc52xx_gpt_gpiochip_driver = {
>> + .name = "gpio_gpt",
>> + .match_table = mpc52xx_gpt_gpiochip_match,
>> + .probe = mpc52xx_gpt_gpiochip_probe,
>> + .remove = mpc52xx_gpiochip_remove,
>> +};
>> +
>> +static int __init mpc52xx_gpio_init(void)
>> +{
>> + if (of_register_platform_driver(&mpc52xx_wkup_gpiochip_driver))
>> + printk(KERN_ERR "Unable to register wakeup GPIO driver\n");
>> +
>> + if (of_register_platform_driver(&mpc52xx_simple_gpiochip_driver))
>> + printk(KERN_ERR "Unable to register simple GPIO driver\n");
>> +
>> + if (of_register_platform_driver(&mpc52xx_gpt_gpiochip_driver))
>> + printk(KERN_ERR "Unable to register gpt GPIO driver\n");
>> +
>> + return 0;
>> +}
>> +
>> +
>> +/* Make sure we get initialised before anyone else tries to use us */
>> +subsys_initcall(mpc52xx_gpio_init);
>> +
>> +/* No exit call at the moment as we cannot unregister of gpio chips */
>> +
>> +MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
>> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
>> +MODULE_LICENSE("GPL v2");
>> +
>> Index: arch/powerpc/platforms/52xx/Kconfig
>> ===================================================================
>> --- a/arch/powerpc/platforms/52xx/Kconfig.orig
>> +++ b/arch/powerpc/platforms/52xx/Kconfig
>> @@ -44,3 +44,9 @@ config PPC_MPC5200_BUGFIX
>>
>> It is safe to say 'Y' here
>>
>> +config PPC_MPC5200_GPIO
>> + bool "MPC5200 GPIO support"
>> + depends on PPC_MPC52xx
>> + select HAVE_GPIO_LIB
>> + help
>> + Enable gpiolib support for mpc5200 based boards
>> Index: arch/powerpc/platforms/52xx/Makefile
>> ===================================================================
>> --- a/arch/powerpc/platforms/52xx/Makefile.orig
>> +++ b/arch/powerpc/platforms/52xx/Makefile
>> @@ -14,3 +14,5 @@ obj-$(CONFIG_PM) += mpc52xx_sleep.o mpc
>> ifeq ($(CONFIG_PPC_LITE5200),y)
>> obj-$(CONFIG_PM) += lite5200_sleep.o lite5200_pm.o
>> endif
>> +
>> +obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
>> \ No newline at end of file
>>
>> --
>> Pengutronix e.K. - Linux Solutions for Science and Industry
>> -----------------------------------------------------------
>> Kontakt-Informationen finden Sie im Header dieser Mail oder
>> auf der Webseite -> http://www.pengutronix.de/impressum/ <-
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-25 15:30 ` Matt Sealey
@ 2008-04-25 18:51 ` Sascha Hauer
2008-04-25 19:30 ` Grant Likely
0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2008-04-25 18:51 UTC (permalink / raw)
To: Matt Sealey; +Cc: linuxppc-dev
On Fri, Apr 25, 2008 at 04:30:55PM +0100, Matt Sealey wrote:
> Can I make a suggestion for this chip support?
>
> On certain 5200 boards these devices are not usable since they are not
> connected. My concern is the Efika where we only have 2 wakeup and 1 simple
> GPIO available on the board and maybe a few others with a bit of tweaking
> and messing around.
This is probably true for most boards.
I haven't tested if this is true for all gpios, but the peripheral usage
settings seem to take precedence over the gpio settings. So at least you
can play with already for peripheral claimed gpios without bad things
happening. But ok, seems to be a bad idea to rely on this.
>
> Instead of having a block for GPIO which has 32 pins, and the ability to
> use all of them just by asking for it (get will return a pin regardless)
> I implemented a silly hackish "gpio-mask" property in the efika.forth device
> tree so the gpiolib "allocate" style functions can tell if a pin is
> actually usable or not.
I thought about the same thing, but we have to improve gpiolib first.
Currently there is no hook for gpio_request().
>
> Can we put some small thought into defining a standard for this, and possibly
> moving the "number of gpios in the chip" into the device tree?
How does the number of gpios help? the gpio-mask you mentioned above
seems to do better.
> It doesn't
> seem particularly complete to just allow blanket use (which may be unsafe and
> may change per-board) of pins just because you can twiddle a bit.. (or use
> a pin when another driver may be using it).
>
> This will tie into Grant's comments about optimizing the enabling of the
> GPIO on every direction change etc. (it should be enabled on allocation
> and then you have to make sure you free up your use of the pin, resource
> tracking in the kernel is important.. flipping bits here and there without
> something in the way in the API is kind of clumsy)
>
> --
> Matt Sealey <matt@genesi-usa.com>
> Genesi, Manager, Developer Relations
>
> Grant Likely wrote:
>> On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>> Hi all,
>>>
>>> Feel free to comment on this.
>>>
>>> Sascha
>>>
>>>
>>> This patch adds gpiolib support for mpc5200 SOCs. I'm not sure
>>> whether it's a good idea to make this optional via kconfig.
>>> The gpt devices only support a single gpio. In the current of_gpio
>>> implementation each chip consumes 32 GPIOs which leads to huge
>>> gaps.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> Looks pretty good. You've saved me the need to go write a driver
>> myself. Comments below, but I'll pull it into my tree and give it a
>> spin.
>>
>> I don't see any mechanism for setting the open drain state of the pin.
>> That will either need to be done by platform code or encoded into the
>> device tree. Does the OF gpio infrastructure provide any callback to
>> the driver when something requests the pin? That would seem to be the
>> ideal place to set the open drain state.
>>
>> You'll also need to document the format of the gpio pin specifier for
>> these devices (ie. first cell is GPIO number, second cell is ????).
>>
>> As for the wide spans caused by gpt gpios, it is probably okay for
>> now, but we can rework it to do something clever (like have a single
>> registration for all gpt gpios) at a later date.
>>
>> Cheers,
>> g.
>>
>>> ---
>>> arch/powerpc/platforms/52xx/Kconfig | 6
>>> arch/powerpc/platforms/52xx/Makefile | 2
>>> arch/powerpc/platforms/52xx/mpc52xx_gpio.c | 408 +++++++++++++++++++++++++++++
>>> 3 files changed, 416 insertions(+)
>>>
>>> Index: arch/powerpc/platforms/52xx/mpc52xx_gpio.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
>>> @@ -0,0 +1,408 @@
>>> +/*
>>> + * MPC52xx gpio driver
>>> + *
>>> + * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2
>>> + * as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>> + */
>>> +
>>> +#include <linux/of.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/of_gpio.h>
>>> +#include <linux/io.h>
>>> +#include <asm/of_platform.h>
>>> +#include <asm/prom.h>
>>> +#include <asm/gpio.h>
>>> +#include <asm/mpc52xx.h>
>>> +#include <sysdev/fsl_soc.h>
>>> +
>>> +static DEFINE_SPINLOCK(gpio_lock);
>>> +
>>> +/*
>>> + * GPIO LIB API implementation for wakeup GPIOs.
>>> + *
>>> + * There's a maximum of 8 wakeup GPIOs. Which of these are available
>>> + * for use depends on your board setup.
>>> + *
>>> + * 0 -> GPIO_WKUP_7
>>> + * 1 -> GPIO_WKUP_6
>>> + * 2 -> PSC6_1
>>> + * 3 -> PSC6_0
>>> + * 4 -> ETH_17
>>> + * 5 -> PSC3_9
>>> + * 6 -> PSC2_4
>>> + * 7 -> PSC1_4
>>> + *
>>> + */
>>> +static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
>>> + unsigned int ret;
>>> +
>>> + ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
>>> +
>>> + pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
>>
>> dev_dbg maybe?
>>
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + tmp = in_8(®s->wkup_dvo);
>>> + if (val)
>>> + tmp |= 1 << (7 - gpio);
>>> + else
>>> + tmp &= ~(1 << (7 - gpio));
>>> + out_8(®s->wkup_dvo, tmp);
>>
>> Rather than read/modify/write of the device register; the function
>> would probably be faster (one fewer barrier) if you used a shadow
>> register of the pin state and the critical region would be
>> shorter/simpler. Also, while this device doesn't have the side
>> effects associated with shared input/output register, it might still
>> be good form to use a shadow register just for the sake of clarity.
>>
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +}
>>> +
>>> +static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + tmp = in_8(®s->wkup_ddr);
>>> + tmp &= ~(1 << (7 - gpio));
>>> + out_8(®s->wkup_ddr, tmp);
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + /* First set initial value */
>>> + mpc52xx_wkup_gpio_set(gc, gpio, val);
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + /* Then set direction */
>>> + tmp = in_8(®s->wkup_ddr);
>>> + tmp |= 1 << (7 - gpio);
>>> + out_8(®s->wkup_ddr, tmp);
>>> +
>>> + /* Finally enable the pin */
>>> + tmp = in_8(®s->wkup_gpioe);
>>> + tmp |= 1 << (7 - gpio);
>>> + out_8(®s->wkup_gpioe, tmp);
>>
>> Do you want/need the cost of enabling the pin every time dir_out is
>> called? Can it be done when the pin is requested instead? Or by the
>> board firmware/platform code? Some drivers (for example the i2c
>> bitbang driver for the clock signal; see i2c-gpio.c) change the state
>> by changing the direction of the pin.
>>
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devinit mpc52xx_wkup_gpiochip_probe(struct of_device *ofdev,
>>> + const struct of_device_id *match)
>>> +{
>>> + struct of_mm_gpio_chip *mmchip;
>>> + struct of_gpio_chip *chip;
>>> +
>>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>>> + if (!mmchip)
>>> + return -ENOMEM;
>>> +
>>> + chip = &mmchip->of_gc;
>>> +
>>> + chip->gpio_cells = 2;
>>> + chip->gc.ngpio = 8;
>>> + chip->gc.direction_input = mpc52xx_wkup_gpio_dir_in;
>>> + chip->gc.direction_output = mpc52xx_wkup_gpio_dir_out;
>>> + chip->gc.get = mpc52xx_wkup_gpio_get;
>>> + chip->gc.set = mpc52xx_wkup_gpio_set;
>>> +
>>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>>> +}
>>> +
>>> +static int mpc52xx_gpiochip_remove(struct of_device *ofdev)
>>> +{
>>> + return -EBUSY;
>>> +}
>>> +
>>> +static struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
>>> + {
>>> + .compatible = "fsl,mpc5200-gpio-wkup",
>>> + },
>>> + {}
>>> +};
>>> +
>>> +static struct of_platform_driver mpc52xx_wkup_gpiochip_driver = {
>>> + .name = "gpio_wkup",
>>> + .match_table = mpc52xx_wkup_gpiochip_match,
>>> + .probe = mpc52xx_wkup_gpiochip_probe,
>>> + .remove = mpc52xx_gpiochip_remove,
>>> +};
>>> +
>>> +/*
>>> + * GPIO LIB API implementation for simple GPIOs
>>> + *
>>> + * There's a maximum of 32 simple GPIOs. Which of these are available
>>> + * for use depends on your board setup.
>>> + * The numbering reflects the bit numbering in the port registers:
>>> + *
>>> + * 0..1 > reserved
>>> + * 2..3 > IRDA
>>> + * 4..7 > ETHR
>>> + * 8..11 > reserved
>>> + * 12..15 > USB
>>> + * 16..17 > reserved
>>> + * 18..23 > PSC3
>>> + * 24..27 > PSC2
>>> + * 28..31 > PSC1
>>> + */
>>> +static int mpc52xx_simple_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
>>> + unsigned int ret;
>>> +
>>> + ret = (in_be32(®s->simple_ival) >> (31 - gpio)) & 1;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + tmp = in_be32(®s->simple_dvo);
>>> + if (val)
>>> + tmp |= 1 << (31 - gpio);
>>> + else
>>> + tmp &= ~(1 << (31 - gpio));
>>> + out_be32(®s->simple_dvo, tmp);
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +}
>>> +
>>> +static int mpc52xx_simple_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + tmp = in_be32(®s->simple_ddr);
>>> + tmp &= ~(1 << (31 - gpio));
>>> + out_be32(®s->simple_ddr, tmp);
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int mpc52xx_simple_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpio *regs = mm_gc->regs;
>>> + unsigned int tmp;
>>> + unsigned long flags;
>>> +
>>> + /* First set initial value */
>>> + mpc52xx_simple_gpio_set(gc, gpio, val);
>>> +
>>> + spin_lock_irqsave(&gpio_lock, flags);
>>> +
>>> + /* Then set direction */
>>> + tmp = in_be32(®s->simple_ddr);
>>> + tmp |= 1 << (31 - gpio);
>>> + out_be32(®s->simple_ddr, tmp);
>>> +
>>> + /* Finally enable the pin */
>>> + tmp = in_be32(®s->simple_gpioe);
>>> + tmp |= 1 << (31 - gpio);
>>> + out_be32(®s->simple_gpioe, tmp);
>>
>> ditto here
>>
>>> +
>>> + spin_unlock_irqrestore(&gpio_lock, flags);
>>> +
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devinit mpc52xx_simple_gpiochip_probe(struct of_device *ofdev,
>>> + const struct of_device_id *match)
>>> +{
>>> + struct of_mm_gpio_chip *mmchip;
>>> + struct of_gpio_chip *chip;
>>> +
>>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>>> + if (!mmchip)
>>> + return -ENOMEM;
>>> +
>>> + chip = &mmchip->of_gc;
>>> +
>>> + chip->gpio_cells = 2;
>>> + chip->gc.ngpio = 32;
>>> + chip->gc.direction_input = mpc52xx_simple_gpio_dir_in;
>>> + chip->gc.direction_output = mpc52xx_simple_gpio_dir_out;
>>> + chip->gc.get = mpc52xx_simple_gpio_get;
>>> + chip->gc.set = mpc52xx_simple_gpio_set;
>>> +
>>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>>> +}
>>> +
>>> +static struct of_device_id mpc52xx_simple_gpiochip_match[] = {
>>> + {
>>> + .compatible = "fsl,mpc5200-gpio",
>>> + },
>>> + {}
>>> +};
>>> +
>>> +static struct of_platform_driver mpc52xx_simple_gpiochip_driver = {
>>> + .name = "gpio",
>>> + .match_table = mpc52xx_simple_gpiochip_match,
>>> + .probe = mpc52xx_simple_gpiochip_probe,
>>> + .remove = mpc52xx_gpiochip_remove,
>>> +};
>>> +
>>> +/*
>>> + * GPIO LIB API implementation for gpt GPIOs.
>>> + *
>>> + * Each gpt only has a single GPIO.
>>> + */
>>> +static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
>>> + unsigned int ret;
>>> +
>>> + return (in_be32(®s->status) & (1 << (31 - 23))) ? 1 : 0;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
>>> +
>>> + if (val)
>>> + out_be32(®s->mode, 0x34);
>>> + else
>>> + out_be32(®s->mode, 0x24);
>>> +
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +}
>>> +
>>> +static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>>> +{
>>> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>> + struct mpc52xx_gpt *regs = mm_gc->regs;
>>> +
>>> + out_be32(®s->mode, 0x04);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int mpc52xx_gpt_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>>> +{
>>> + mpc52xx_gpt_gpio_set(gc, gpio, val);
>>> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devinit mpc52xx_gpt_gpiochip_probe(struct of_device *ofdev,
>>> + const struct of_device_id *match)
>>> +{
>>> + struct of_mm_gpio_chip *mmchip;
>>> + struct of_gpio_chip *chip;
>>> +
>>> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
>>> + if (!mmchip)
>>> + return -ENOMEM;
>>> +
>>> + chip = &mmchip->of_gc;
>>> +
>>> + chip->gpio_cells = 2;
>>> + chip->gc.ngpio = 1;
>>> + chip->gc.direction_input = mpc52xx_gpt_gpio_dir_in;
>>> + chip->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
>>> + chip->gc.get = mpc52xx_gpt_gpio_get;
>>> + chip->gc.set = mpc52xx_gpt_gpio_set;
>>> +
>>> + return of_mm_gpiochip_add(ofdev->node, mmchip);
>>> +}
>>> +
>>> +static struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
>>> + {
>>> + .compatible = "fsl,mpc5200-gpt-gpio",
>>> + },
>>> + {}
>>> +};
>>> +
>>> +static struct of_platform_driver mpc52xx_gpt_gpiochip_driver = {
>>> + .name = "gpio_gpt",
>>> + .match_table = mpc52xx_gpt_gpiochip_match,
>>> + .probe = mpc52xx_gpt_gpiochip_probe,
>>> + .remove = mpc52xx_gpiochip_remove,
>>> +};
>>> +
>>> +static int __init mpc52xx_gpio_init(void)
>>> +{
>>> + if (of_register_platform_driver(&mpc52xx_wkup_gpiochip_driver))
>>> + printk(KERN_ERR "Unable to register wakeup GPIO driver\n");
>>> +
>>> + if (of_register_platform_driver(&mpc52xx_simple_gpiochip_driver))
>>> + printk(KERN_ERR "Unable to register simple GPIO driver\n");
>>> +
>>> + if (of_register_platform_driver(&mpc52xx_gpt_gpiochip_driver))
>>> + printk(KERN_ERR "Unable to register gpt GPIO driver\n");
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +
>>> +/* Make sure we get initialised before anyone else tries to use us */
>>> +subsys_initcall(mpc52xx_gpio_init);
>>> +
>>> +/* No exit call at the moment as we cannot unregister of gpio chips */
>>> +
>>> +MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
>>> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
>>> +MODULE_LICENSE("GPL v2");
>>> +
>>> Index: arch/powerpc/platforms/52xx/Kconfig
>>> ===================================================================
>>> --- a/arch/powerpc/platforms/52xx/Kconfig.orig
>>> +++ b/arch/powerpc/platforms/52xx/Kconfig
>>> @@ -44,3 +44,9 @@ config PPC_MPC5200_BUGFIX
>>>
>>> It is safe to say 'Y' here
>>>
>>> +config PPC_MPC5200_GPIO
>>> + bool "MPC5200 GPIO support"
>>> + depends on PPC_MPC52xx
>>> + select HAVE_GPIO_LIB
>>> + help
>>> + Enable gpiolib support for mpc5200 based boards
>>> Index: arch/powerpc/platforms/52xx/Makefile
>>> ===================================================================
>>> --- a/arch/powerpc/platforms/52xx/Makefile.orig
>>> +++ b/arch/powerpc/platforms/52xx/Makefile
>>> @@ -14,3 +14,5 @@ obj-$(CONFIG_PM) += mpc52xx_sleep.o mpc
>>> ifeq ($(CONFIG_PPC_LITE5200),y)
>>> obj-$(CONFIG_PM) += lite5200_sleep.o lite5200_pm.o
>>> endif
>>> +
>>> +obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
>>> \ No newline at end of file
>>>
>>> --
>>> Pengutronix e.K. - Linux Solutions for Science and Industry
>>> -----------------------------------------------------------
>>> Kontakt-Informationen finden Sie im Header dieser Mail oder
>>> auf der Webseite -> http://www.pengutronix.de/impressum/ <-
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@ozlabs.org
>>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>>
>>
>>
>>
>
--
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-25 18:51 ` Sascha Hauer
@ 2008-04-25 19:30 ` Grant Likely
0 siblings, 0 replies; 12+ messages in thread
From: Grant Likely @ 2008-04-25 19:30 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
On Fri, Apr 25, 2008 at 12:51 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Apr 25, 2008 at 04:30:55PM +0100, Matt Sealey wrote:
> > Can I make a suggestion for this chip support?
> >
> > On certain 5200 boards these devices are not usable since they are not
> > connected. My concern is the Efika where we only have 2 wakeup and 1 simple
> > GPIO available on the board and maybe a few others with a bit of tweaking
> > and messing around.
>
> This is probably true for most boards.
> I haven't tested if this is true for all gpios, but the peripheral usage
> settings seem to take precedence over the gpio settings. So at least you
> can play with already for peripheral claimed gpios without bad things
> happening. But ok, seems to be a bad idea to rely on this.
There must also be some level of assumption that the device tree data
is accurate. If a pin is specified that is not enabled as a GPIO pin,
then it's not going to work.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 15:36 [PATCH] add gpiolib support for mpc5200 Sascha Hauer
2008-04-24 18:45 ` Grant Likely
@ 2008-04-24 19:11 ` Grant Likely
2008-04-25 8:22 ` Stephen Rothwell
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Grant Likely @ 2008-04-24 19:11 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi all,
>
> Feel free to comment on this.
>
> Sascha
>
>
> This patch adds gpiolib support for mpc5200 SOCs. I'm not sure
> whether it's a good idea to make this optional via kconfig.
> The gpt devices only support a single gpio. In the current of_gpio
> implementation each chip consumes 32 GPIOs which leads to huge
> gaps.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Looks pretty good. You've saved me the need to go write a driver
myself. Comments below, but I'll pull it into my tree and give it a
spin.
I don't see any mechanism for setting the open drain state of the pin.
That will either need to be done by platform code or encoded into the
device tree. Does the OF gpio infrastructure provide any callback to
the driver when something requests the pin? That would seem to be the
ideal place to set the open drain state.
You'll also need to document the format of the gpio pin specifier for
these devices (ie. first cell is GPIO number, second cell is ????).
As for the wide spans caused by gpt gpios, it is probably okay for
now, but we can rework it to do something clever (like have a single
registration for all gpt gpios) at a later date.
Cheers,
g.
>
> ---
> arch/powerpc/platforms/52xx/Kconfig | 6
> arch/powerpc/platforms/52xx/Makefile | 2
> arch/powerpc/platforms/52xx/mpc52xx_gpio.c | 408 +++++++++++++++++++++++++++++
> 3 files changed, 416 insertions(+)
>
> Index: arch/powerpc/platforms/52xx/mpc52xx_gpio.c
> ===================================================================
> --- /dev/null
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
> @@ -0,0 +1,408 @@
> +/*
> + * MPC52xx gpio driver
> + *
> + * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/of_gpio.h>
> +#include <linux/io.h>
> +#include <asm/of_platform.h>
> +#include <asm/prom.h>
> +#include <asm/gpio.h>
> +#include <asm/mpc52xx.h>
> +#include <sysdev/fsl_soc.h>
> +
> +static DEFINE_SPINLOCK(gpio_lock);
> +
> +/*
> + * GPIO LIB API implementation for wakeup GPIOs.
> + *
> + * There's a maximum of 8 wakeup GPIOs. Which of these are available
> + * for use depends on your board setup.
> + *
> + * 0 -> GPIO_WKUP_7
> + * 1 -> GPIO_WKUP_6
> + * 2 -> PSC6_1
> + * 3 -> PSC6_0
> + * 4 -> ETH_17
> + * 5 -> PSC3_9
> + * 6 -> PSC2_4
> + * 7 -> PSC1_4
> + *
> + */
> +static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
> +
> + pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
dev_dbg maybe?
> +
> + return ret;
> +}
> +
> +static void mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_8(®s->wkup_dvo);
> + if (val)
> + tmp |= 1 << (7 - gpio);
> + else
> + tmp &= ~(1 << (7 - gpio));
> + out_8(®s->wkup_dvo, tmp);
Rather than read/modify/write of the device register; the function
would probably be faster (one fewer barrier) if you used a shadow
register of the pin state and the critical region would be
shorter/simpler. Also, while this device doesn't have the side
effects associated with shared input/output register, it might still
be good form to use a shadow register just for the sake of clarity.
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_8(®s->wkup_ddr);
> + tmp &= ~(1 << (7 - gpio));
> + out_8(®s->wkup_ddr, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + /* First set initial value */
> + mpc52xx_wkup_gpio_set(gc, gpio, val);
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + /* Then set direction */
> + tmp = in_8(®s->wkup_ddr);
> + tmp |= 1 << (7 - gpio);
> + out_8(®s->wkup_ddr, tmp);
> +
> + /* Finally enable the pin */
> + tmp = in_8(®s->wkup_gpioe);
> + tmp |= 1 << (7 - gpio);
> + out_8(®s->wkup_gpioe, tmp);
Do you want/need the cost of enabling the pin every time dir_out is
called? Can it be done when the pin is requested instead? Or by the
board firmware/platform code? Some drivers (for example the i2c
bitbang driver for the clock signal; see i2c-gpio.c) change the state
by changing the direction of the pin.
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_wkup_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 8;
> + chip->gc.direction_input = mpc52xx_wkup_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_wkup_gpio_dir_out;
> + chip->gc.get = mpc52xx_wkup_gpio_get;
> + chip->gc.set = mpc52xx_wkup_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static int mpc52xx_gpiochip_remove(struct of_device *ofdev)
> +{
> + return -EBUSY;
> +}
> +
> +static struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpio-wkup",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_wkup_gpiochip_driver = {
> + .name = "gpio_wkup",
> + .match_table = mpc52xx_wkup_gpiochip_match,
> + .probe = mpc52xx_wkup_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +/*
> + * GPIO LIB API implementation for simple GPIOs
> + *
> + * There's a maximum of 32 simple GPIOs. Which of these are available
> + * for use depends on your board setup.
> + * The numbering reflects the bit numbering in the port registers:
> + *
> + * 0..1 > reserved
> + * 2..3 > IRDA
> + * 4..7 > ETHR
> + * 8..11 > reserved
> + * 12..15 > USB
> + * 16..17 > reserved
> + * 18..23 > PSC3
> + * 24..27 > PSC2
> + * 28..31 > PSC1
> + */
> +static int mpc52xx_simple_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + ret = (in_be32(®s->simple_ival) >> (31 - gpio)) & 1;
> +
> + return ret;
> +}
> +
> +static void mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_be32(®s->simple_dvo);
> + if (val)
> + tmp |= 1 << (31 - gpio);
> + else
> + tmp &= ~(1 << (31 - gpio));
> + out_be32(®s->simple_dvo, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_simple_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + tmp = in_be32(®s->simple_ddr);
> + tmp &= ~(1 << (31 - gpio));
> + out_be32(®s->simple_ddr, tmp);
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_simple_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpio *regs = mm_gc->regs;
> + unsigned int tmp;
> + unsigned long flags;
> +
> + /* First set initial value */
> + mpc52xx_simple_gpio_set(gc, gpio, val);
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + /* Then set direction */
> + tmp = in_be32(®s->simple_ddr);
> + tmp |= 1 << (31 - gpio);
> + out_be32(®s->simple_ddr, tmp);
> +
> + /* Finally enable the pin */
> + tmp = in_be32(®s->simple_gpioe);
> + tmp |= 1 << (31 - gpio);
> + out_be32(®s->simple_gpioe, tmp);
ditto here
> +
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_simple_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 32;
> + chip->gc.direction_input = mpc52xx_simple_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_simple_gpio_dir_out;
> + chip->gc.get = mpc52xx_simple_gpio_get;
> + chip->gc.set = mpc52xx_simple_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static struct of_device_id mpc52xx_simple_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpio",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_simple_gpiochip_driver = {
> + .name = "gpio",
> + .match_table = mpc52xx_simple_gpiochip_match,
> + .probe = mpc52xx_simple_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +/*
> + * GPIO LIB API implementation for gpt GPIOs.
> + *
> + * Each gpt only has a single GPIO.
> + */
> +static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
> + unsigned int ret;
> +
> + return (in_be32(®s->status) & (1 << (31 - 23))) ? 1 : 0;
> +
> + return ret;
> +}
> +
> +static void mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
> +
> + if (val)
> + out_be32(®s->mode, 0x34);
> + else
> + out_be32(®s->mode, 0x24);
> +
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> + struct mpc52xx_gpt *regs = mm_gc->regs;
> +
> + out_be32(®s->mode, 0x04);
> +
> + return 0;
> +}
> +
> +static int mpc52xx_gpt_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + mpc52xx_gpt_gpio_set(gc, gpio, val);
> + pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + return 0;
> +}
> +
> +static int __devinit mpc52xx_gpt_gpiochip_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + struct of_gpio_chip *chip;
> +
> + mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
> + if (!mmchip)
> + return -ENOMEM;
> +
> + chip = &mmchip->of_gc;
> +
> + chip->gpio_cells = 2;
> + chip->gc.ngpio = 1;
> + chip->gc.direction_input = mpc52xx_gpt_gpio_dir_in;
> + chip->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
> + chip->gc.get = mpc52xx_gpt_gpio_get;
> + chip->gc.set = mpc52xx_gpt_gpio_set;
> +
> + return of_mm_gpiochip_add(ofdev->node, mmchip);
> +}
> +
> +static struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
> + {
> + .compatible = "fsl,mpc5200-gpt-gpio",
> + },
> + {}
> +};
> +
> +static struct of_platform_driver mpc52xx_gpt_gpiochip_driver = {
> + .name = "gpio_gpt",
> + .match_table = mpc52xx_gpt_gpiochip_match,
> + .probe = mpc52xx_gpt_gpiochip_probe,
> + .remove = mpc52xx_gpiochip_remove,
> +};
> +
> +static int __init mpc52xx_gpio_init(void)
> +{
> + if (of_register_platform_driver(&mpc52xx_wkup_gpiochip_driver))
> + printk(KERN_ERR "Unable to register wakeup GPIO driver\n");
> +
> + if (of_register_platform_driver(&mpc52xx_simple_gpiochip_driver))
> + printk(KERN_ERR "Unable to register simple GPIO driver\n");
> +
> + if (of_register_platform_driver(&mpc52xx_gpt_gpiochip_driver))
> + printk(KERN_ERR "Unable to register gpt GPIO driver\n");
> +
> + return 0;
> +}
> +
> +
> +/* Make sure we get initialised before anyone else tries to use us */
> +subsys_initcall(mpc52xx_gpio_init);
> +
> +/* No exit call at the moment as we cannot unregister of gpio chips */
> +
> +MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
> +MODULE_LICENSE("GPL v2");
> +
> Index: arch/powerpc/platforms/52xx/Kconfig
> ===================================================================
> --- a/arch/powerpc/platforms/52xx/Kconfig.orig
> +++ b/arch/powerpc/platforms/52xx/Kconfig
> @@ -44,3 +44,9 @@ config PPC_MPC5200_BUGFIX
>
> It is safe to say 'Y' here
>
> +config PPC_MPC5200_GPIO
> + bool "MPC5200 GPIO support"
> + depends on PPC_MPC52xx
> + select HAVE_GPIO_LIB
> + help
> + Enable gpiolib support for mpc5200 based boards
> Index: arch/powerpc/platforms/52xx/Makefile
> ===================================================================
> --- a/arch/powerpc/platforms/52xx/Makefile.orig
> +++ b/arch/powerpc/platforms/52xx/Makefile
> @@ -14,3 +14,5 @@ obj-$(CONFIG_PM) += mpc52xx_sleep.o mpc
> ifeq ($(CONFIG_PPC_LITE5200),y)
> obj-$(CONFIG_PM) += lite5200_sleep.o lite5200_pm.o
> endif
> +
> +obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
> \ No newline at end of file
>
> --
> Pengutronix e.K. - Linux Solutions for Science and Industry
> -----------------------------------------------------------
> Kontakt-Informationen finden Sie im Header dieser Mail oder
> auf der Webseite -> http://www.pengutronix.de/impressum/ <-
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 15:36 [PATCH] add gpiolib support for mpc5200 Sascha Hauer
2008-04-24 18:45 ` Grant Likely
2008-04-24 19:11 ` Grant Likely
@ 2008-04-25 8:22 ` Stephen Rothwell
2008-04-25 10:56 ` Sascha Hauer
2008-04-25 13:07 ` Anton Vorontsov
4 siblings, 0 replies; 12+ messages in thread
From: Stephen Rothwell @ 2008-04-25 8:22 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 674 bytes --]
Hi Sascha,
One small comment.
On Thu, 24 Apr 2008 17:36:59 +0200 Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> +#include <asm/of_platform.h>
Never include <asm/of_platform.h>, use <linux/of_platform.h>
> +static struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
const, please.
OK, I lied about only one comment :-)
> +static struct of_device_id mpc52xx_simple_gpiochip_match[] = {
const, again.
> +static struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
Another one.
Also, I don't think you need to include <asm/prom.h>
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH] add gpiolib support for mpc5200
2008-04-24 15:36 [PATCH] add gpiolib support for mpc5200 Sascha Hauer
` (2 preceding siblings ...)
2008-04-25 8:22 ` Stephen Rothwell
@ 2008-04-25 10:56 ` Sascha Hauer
2008-04-25 13:07 ` Anton Vorontsov
4 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2008-04-25 10:56 UTC (permalink / raw)
To: linuxppc-dev
This patch adds gpiolib support for mpc5200 SOCs.
Changes since last submit:
- fixed checkpatch warnings
- use shadow variables for register accesses
- make match tables const
- Add documentation
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Documentation/powerpc/mpc52xx-device-tree-bindings.txt | 12
arch/powerpc/platforms/52xx/Kconfig | 6
arch/powerpc/platforms/52xx/Makefile | 2
arch/powerpc/platforms/52xx/mpc52xx_gpio.c | 465 +++++++++++++++++
4 files changed, 485 insertions(+)
Index: linux-2.6-powerpc/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
===================================================================
--- /dev/null
+++ linux-2.6-powerpc/arch/powerpc/platforms/52xx/mpc52xx_gpio.c
@@ -0,0 +1,465 @@
+/*
+ * MPC52xx gpio driver
+ *
+ * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/of_gpio.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+
+#include <asm/gpio.h>
+#include <asm/mpc52xx.h>
+#include <sysdev/fsl_soc.h>
+
+static DEFINE_SPINLOCK(gpio_lock);
+
+struct mpc52xx_gpiochip {
+ struct of_mm_gpio_chip mmchip;
+ unsigned int shadow_dvo;
+ unsigned int shadow_gpioe;
+ unsigned int shadow_ddr;
+};
+
+/*
+ * GPIO LIB API implementation for wakeup GPIOs.
+ *
+ * There's a maximum of 8 wakeup GPIOs. Which of these are available
+ * for use depends on your board setup.
+ *
+ * 0 -> GPIO_WKUP_7
+ * 1 -> GPIO_WKUP_6
+ * 2 -> PSC6_1
+ * 3 -> PSC6_0
+ * 4 -> ETH_17
+ * 5 -> PSC3_9
+ * 6 -> PSC2_4
+ * 7 -> PSC1_4
+ *
+ */
+static int mpc52xx_wkup_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
+ unsigned int ret;
+
+ ret = (in_8(®s->wkup_ival) >> (7 - gpio)) & 1;
+
+ pr_debug("%s: gpio: %d ret: %d\n", __func__, gpio, ret);
+
+ return ret;
+}
+
+static inline void
+__mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ struct mpc52xx_gpio_wkup __iomem *regs = mm_gc->regs;
+
+ if (val)
+ chip->shadow_dvo |= 1 << (7 - gpio);
+ else
+ chip->shadow_dvo &= ~(1 << (7 - gpio));
+
+ out_8(®s->wkup_dvo, chip->shadow_dvo);
+}
+
+static void
+mpc52xx_wkup_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ __mpc52xx_wkup_gpio_set(gc, gpio, val);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc52xx_wkup_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ /* set the direction */
+ chip->shadow_ddr &= ~(1 << (7 - gpio));
+ out_8(®s->wkup_ddr, chip->shadow_ddr);
+
+ /* and enable the pin */
+ chip->shadow_gpioe |= 1 << (7 - gpio);
+ out_8(®s->wkup_gpioe, chip->shadow_gpioe);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static int
+mpc52xx_wkup_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpio_wkup *regs = mm_gc->regs;
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ __mpc52xx_wkup_gpio_set(gc, gpio, val);
+
+ /* Then set direction */
+ chip->shadow_ddr |= 1 << (7 - gpio);
+ out_8(®s->wkup_ddr, chip->shadow_ddr);
+
+ /* Finally enable the pin */
+ chip->shadow_gpioe |= 1 << (7 - gpio);
+ out_8(®s->wkup_gpioe, chip->shadow_gpioe);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+ return 0;
+}
+
+static int __devinit mpc52xx_wkup_gpiochip_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct mpc52xx_gpiochip *chip;
+ struct mpc52xx_gpio_wkup *regs;
+ struct of_gpio_chip *ofchip;
+ int ret;
+
+ chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ ofchip = &chip->mmchip.of_gc;
+
+ ofchip->gpio_cells = 2;
+ ofchip->gc.ngpio = 8;
+ ofchip->gc.direction_input = mpc52xx_wkup_gpio_dir_in;
+ ofchip->gc.direction_output = mpc52xx_wkup_gpio_dir_out;
+ ofchip->gc.get = mpc52xx_wkup_gpio_get;
+ ofchip->gc.set = mpc52xx_wkup_gpio_set;
+
+ ret = of_mm_gpiochip_add(ofdev->node, &chip->mmchip);
+ if (ret)
+ return ret;
+
+ regs = chip->mmchip.regs;
+ chip->shadow_gpioe = in_8(®s->wkup_gpioe);
+ chip->shadow_ddr = in_8(®s->wkup_ddr);
+ chip->shadow_dvo = in_8(®s->wkup_dvo);
+
+ return 0;
+}
+
+static int mpc52xx_gpiochip_remove(struct of_device *ofdev)
+{
+ return -EBUSY;
+}
+
+static const struct of_device_id mpc52xx_wkup_gpiochip_match[] = {
+ {
+ .compatible = "fsl,mpc5200-gpio-wkup",
+ },
+ {}
+};
+
+static struct of_platform_driver mpc52xx_wkup_gpiochip_driver = {
+ .name = "gpio_wkup",
+ .match_table = mpc52xx_wkup_gpiochip_match,
+ .probe = mpc52xx_wkup_gpiochip_probe,
+ .remove = mpc52xx_gpiochip_remove,
+};
+
+/*
+ * GPIO LIB API implementation for simple GPIOs
+ *
+ * There's a maximum of 32 simple GPIOs. Which of these are available
+ * for use depends on your board setup.
+ * The numbering reflects the bit numbering in the port registers:
+ *
+ * 0..1 > reserved
+ * 2..3 > IRDA
+ * 4..7 > ETHR
+ * 8..11 > reserved
+ * 12..15 > USB
+ * 16..17 > reserved
+ * 18..23 > PSC3
+ * 24..27 > PSC2
+ * 28..31 > PSC1
+ */
+static int mpc52xx_simple_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
+ unsigned int ret;
+
+ ret = (in_be32(®s->simple_ival) >> (31 - gpio)) & 1;
+
+ return ret;
+}
+
+static inline void
+__mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ struct mpc52xx_gpio __iomem *regs = mm_gc->regs;
+
+ if (val)
+ chip->shadow_dvo |= 1 << (31 - gpio);
+ else
+ chip->shadow_dvo &= ~(1 << (31 - gpio));
+ out_be32(®s->simple_dvo, chip->shadow_dvo);
+}
+
+static void
+mpc52xx_simple_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ __mpc52xx_simple_gpio_set(gc, gpio, val);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc52xx_simple_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ struct mpc52xx_gpio *regs = mm_gc->regs;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ /* set the direction */
+ chip->shadow_ddr &= ~(1 << (31 - gpio));
+ out_be32(®s->simple_ddr, chip->shadow_ddr);
+
+ /* and enable the pin */
+ chip->shadow_gpioe |= 1 << (31 - gpio);
+ out_be32(®s->simple_gpioe, chip->shadow_gpioe);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static int
+mpc52xx_simple_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpiochip *chip = container_of(mm_gc,
+ struct mpc52xx_gpiochip, mmchip);
+ struct mpc52xx_gpio *regs = mm_gc->regs;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ /* First set initial value */
+ __mpc52xx_simple_gpio_set(gc, gpio, val);
+
+ /* Then set direction */
+ chip->shadow_ddr |= 1 << (31 - gpio);
+ out_be32(®s->simple_ddr, chip->shadow_ddr);
+
+ /* Finally enable the pin */
+ chip->shadow_gpioe |= 1 << (31 - gpio);
+ out_be32(®s->simple_gpioe, chip->shadow_gpioe);
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+ return 0;
+}
+
+static int __devinit mpc52xx_simple_gpiochip_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct mpc52xx_gpiochip *chip;
+ struct of_gpio_chip *ofchip;
+ struct mpc52xx_gpio *regs;
+ int ret;
+
+ chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ ofchip = &chip->mmchip.of_gc;
+
+ ofchip->gpio_cells = 2;
+ ofchip->gc.ngpio = 32;
+ ofchip->gc.direction_input = mpc52xx_simple_gpio_dir_in;
+ ofchip->gc.direction_output = mpc52xx_simple_gpio_dir_out;
+ ofchip->gc.get = mpc52xx_simple_gpio_get;
+ ofchip->gc.set = mpc52xx_simple_gpio_set;
+
+ ret = of_mm_gpiochip_add(ofdev->node, &chip->mmchip);
+ if (ret)
+ return ret;
+
+ regs = chip->mmchip.regs;
+ chip->shadow_gpioe = in_be32(®s->simple_gpioe);
+ chip->shadow_ddr = in_be32(®s->simple_ddr);
+ chip->shadow_dvo = in_be32(®s->simple_dvo);
+
+ return 0;
+}
+
+static const struct of_device_id mpc52xx_simple_gpiochip_match[] = {
+ {
+ .compatible = "fsl,mpc5200-gpio",
+ },
+ {}
+};
+
+static struct of_platform_driver mpc52xx_simple_gpiochip_driver = {
+ .name = "gpio",
+ .match_table = mpc52xx_simple_gpiochip_match,
+ .probe = mpc52xx_simple_gpiochip_probe,
+ .remove = mpc52xx_gpiochip_remove,
+};
+
+/*
+ * GPIO LIB API implementation for gpt GPIOs.
+ *
+ * Each gpt only has a single GPIO.
+ */
+static int mpc52xx_gpt_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
+ unsigned int ret;
+
+ return (in_be32(®s->status) & (1 << (31 - 23))) ? 1 : 0;
+
+ return ret;
+}
+
+static void
+mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpt __iomem *regs = mm_gc->regs;
+
+ if (val)
+ out_be32(®s->mode, 0x34);
+ else
+ out_be32(®s->mode, 0x24);
+
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct mpc52xx_gpt *regs = mm_gc->regs;
+
+ out_be32(®s->mode, 0x04);
+
+ return 0;
+}
+
+static int
+mpc52xx_gpt_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ mpc52xx_gpt_gpio_set(gc, gpio, val);
+ pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+ return 0;
+}
+
+static int __devinit mpc52xx_gpt_gpiochip_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct of_mm_gpio_chip *mmchip;
+ struct of_gpio_chip *chip;
+
+ mmchip = kzalloc(sizeof(*mmchip), GFP_KERNEL);
+ if (!mmchip)
+ return -ENOMEM;
+
+ chip = &mmchip->of_gc;
+
+ chip->gpio_cells = 2;
+ chip->gc.ngpio = 1;
+ chip->gc.direction_input = mpc52xx_gpt_gpio_dir_in;
+ chip->gc.direction_output = mpc52xx_gpt_gpio_dir_out;
+ chip->gc.get = mpc52xx_gpt_gpio_get;
+ chip->gc.set = mpc52xx_gpt_gpio_set;
+
+ return of_mm_gpiochip_add(ofdev->node, mmchip);
+}
+
+static const struct of_device_id mpc52xx_gpt_gpiochip_match[] = {
+ {
+ .compatible = "fsl,mpc5200-gpt-gpio",
+ },
+ {}
+};
+
+static struct of_platform_driver mpc52xx_gpt_gpiochip_driver = {
+ .name = "gpio_gpt",
+ .match_table = mpc52xx_gpt_gpiochip_match,
+ .probe = mpc52xx_gpt_gpiochip_probe,
+ .remove = mpc52xx_gpiochip_remove,
+};
+
+static int __init mpc52xx_gpio_init(void)
+{
+ if (of_register_platform_driver(&mpc52xx_wkup_gpiochip_driver))
+ printk(KERN_ERR "Unable to register wakeup GPIO driver\n");
+
+ if (of_register_platform_driver(&mpc52xx_simple_gpiochip_driver))
+ printk(KERN_ERR "Unable to register simple GPIO driver\n");
+
+ if (of_register_platform_driver(&mpc52xx_gpt_gpiochip_driver))
+ printk(KERN_ERR "Unable to register gpt GPIO driver\n");
+
+ return 0;
+}
+
+
+/* Make sure we get initialised before anyone else tries to use us */
+subsys_initcall(mpc52xx_gpio_init);
+
+/* No exit call at the moment as we cannot unregister of gpio chips */
+
+MODULE_DESCRIPTION("Freescale MPC52xx gpio driver");
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de");
+MODULE_LICENSE("GPL v2");
+
Index: linux-2.6-powerpc/arch/powerpc/platforms/52xx/Kconfig
===================================================================
--- linux-2.6-powerpc.orig/arch/powerpc/platforms/52xx/Kconfig
+++ linux-2.6-powerpc/arch/powerpc/platforms/52xx/Kconfig
@@ -44,3 +44,9 @@ config PPC_MPC5200_BUGFIX
It is safe to say 'Y' here
+config PPC_MPC5200_GPIO
+ bool "MPC5200 GPIO support"
+ depends on PPC_MPC52xx
+ select HAVE_GPIO_LIB
+ help
+ Enable gpiolib support for mpc5200 based boards
Index: linux-2.6-powerpc/arch/powerpc/platforms/52xx/Makefile
===================================================================
--- linux-2.6-powerpc.orig/arch/powerpc/platforms/52xx/Makefile
+++ linux-2.6-powerpc/arch/powerpc/platforms/52xx/Makefile
@@ -14,3 +14,5 @@ obj-$(CONFIG_PM) += mpc52xx_sleep.o mpc
ifeq ($(CONFIG_PPC_LITE5200),y)
obj-$(CONFIG_PM) += lite5200_sleep.o lite5200_pm.o
endif
+
+obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
\ No newline at end of file
Index: linux-2.6-powerpc/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
===================================================================
--- linux-2.6-powerpc.orig/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
+++ linux-2.6-powerpc/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
@@ -186,6 +186,12 @@ Recommended soc5200 child nodes; populat
name device_type compatible Description
---- ----------- ---------- -----------
gpt@<addr> gpt fsl,mpc5200-gpt General purpose timers
+gpt@<addr> gpt fsl,mpc5200-gpt-gpio General purpose
+ timers in GPIO mode
+gpio@<addr> fsl,mpc5200-gpio MPC5200 simple gpio
+ controller
+gpio@<addr> fsl,mpc5200-gpio-wkup MPC5200 wakeup gpio
+ controller
rtc@<addr> rtc mpc5200-rtc Real time clock
mscan@<addr> mscan mpc5200-mscan CAN bus controller
pci@<addr> pci mpc5200-pci PCI bridge
@@ -225,6 +231,12 @@ PSC in i2s mode: The mpc5200 and mpc520
i2s mode. An 'mpc5200b-psc-i2s' node cannot include 'mpc5200-psc-i2s' in the
compatible field.
+7) GPIO controller nodes
+Each GPIO controller node should have the empty property gpio-controller and
+#gpio-cells set to 2. First cell is the GPIO number which is interpreted
+according to the bit numbers in the GPIO control registers. The second cell
+is for flags which is currently unsused.
+
IV - Extra Notes
================
--
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-24 15:36 [PATCH] add gpiolib support for mpc5200 Sascha Hauer
` (3 preceding siblings ...)
2008-04-25 10:56 ` Sascha Hauer
@ 2008-04-25 13:07 ` Anton Vorontsov
2008-04-25 13:42 ` Sascha Hauer
4 siblings, 1 reply; 12+ messages in thread
From: Anton Vorontsov @ 2008-04-25 13:07 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linuxppc-dev
On Thu, Apr 24, 2008 at 05:36:59PM +0200, Sascha Hauer wrote:
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/of_gpio.h>
> +#include <linux/io.h>
> +#include <asm/of_platform.h>
> +#include <asm/prom.h>
> +#include <asm/gpio.h>
In the latest kernels the preferred way is to include <linux/gpio.h>,
the patch introducing it was merged just recently, so few people know
about it yet.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] add gpiolib support for mpc5200
2008-04-25 13:07 ` Anton Vorontsov
@ 2008-04-25 13:42 ` Sascha Hauer
0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2008-04-25 13:42 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
On Fri, Apr 25, 2008 at 05:07:37PM +0400, Anton Vorontsov wrote:
> On Thu, Apr 24, 2008 at 05:36:59PM +0200, Sascha Hauer wrote:
>
> > +#include <linux/of.h>
> > +#include <linux/kernel.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/io.h>
> > +#include <asm/of_platform.h>
> > +#include <asm/prom.h>
> > +#include <asm/gpio.h>
>
> In the latest kernels the preferred way is to include <linux/gpio.h>,
> the patch introducing it was merged just recently, so few people know
> about it yet.
Yes, checkpatch told me, but doing so resulted in a compilation error.
This only showed that a "select GENERIC_GPIO" was missing in my
patch. Will update.
Sascha
--
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-
^ permalink raw reply [flat|nested] 12+ messages in thread