powerpc/mpc512x: Add gpio driver
PATCH 1/1 powerpc/mpc512x: Add gpio driver
--- arch/powerpc/include/asm/mpc512x_gpio.h | 30 +++++ arch/powerpc/platforms/Kconfig | 9 ++ arch/powerpc/sysdev/Makefile | 1 + arch/powerpc/sysdev/mpc512x_gpio.c | 179 +++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/include/asm/mpc512x_gpio.h create mode 100644 arch/powerpc/sysdev/mpc512x_gpio.c
arch/powerpc/include/asm/mpc512x_gpio.h new file +30 -0
@@ -0,0 +1,30 @@ +/* +* Copyright (C) 2010 Matthias Fuchs <[email protected]>, esd gmbh +* +* 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 +*/ +#ifndef MPC512X_GPIO_H +#define MPC512X_GPIO_H + +struct mpc512x_gpio_regs { +u32 gpdir; +u32 gpodr; +u32 gpdat; +u32 gpier; +u32 gpimr; +u32 gpicr1; +u32 gpicr2; +}; + +#endif /* MPC512X_GPIO_H */
arch/powerpc/platforms/Kconfig +9 -0
@@ -312,6 +312,15 @@ config MPC8xxx_GPIO Say Y here if you're going to use hardware that connects to the MPC831x/834x/837x/8572/8610 GPIOs. +config MPC512x_GPIO + bool "MPC512x GPIO support" + depends on PPC_MPC512x + select GENERIC_GPIO + select ARCH_REQUIRE_GPIOLIB + help + Say Y here if you're going to use hardware that connects to the + MPC512x GPIOs. + config SIMPLE_GPIO bool "Support for simple, memory-mapped GPIO controllers" depends on PPC
arch/powerpc/sysdev/Makefile +1 -0
@@ -20,6 +20,7 @@ obj-$(CONFIG_FSL_PMC) += fsl_pmc.o obj-$(CONFIG_FSL_LBC) += fsl_lbc.o obj-$(CONFIG_FSL_GTM) += fsl_gtm.o obj-$(CONFIG_MPC8xxx_GPIO) += mpc8xxx_gpio.o +obj-$(CONFIG_MPC512x_GPIO) += mpc512x_gpio.o obj-$(CONFIG_SIMPLE_GPIO) += simple_gpio.o obj-$(CONFIG_RAPIDIO) += fsl_rio.o obj-$(CONFIG_TSI108_BRIDGE) += tsi108_pci.o tsi108_dev.o
arch/powerpc/sysdev/mpc512x_gpio.c new file +179 -0
179 lines not on this page · raw message
Signature · 1 line
1.5.6.3
Hi Matthias. Thanks for the patch. Some comments below...
Quoted 1 line · Matthias Fuchs
Signed-off-by Matthias Fuchs <[email protected]>
Could use a commit log. It is useful to give some details about what this driver has been developed/tested on.
Quoted 5 lines
---
=A0arch/powerpc/include/asm/mpc512x_gpio.h | =A0 30 +++++ =A0arch/powerpc/platforms/Kconfig =A0 =A0 =A0 =A0 =A0| =A0 =A09 ++ =A0arch/powerpc/sysdev/Makefile =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A01 + =A0arch/powerpc/sysdev/mpc512x_gpio.c =A0 =A0 =A0| =A0179 +++++++++++++++=
++++++++++++++++
Move mpc512x_gpio.c to the arch/powerpc/platforms/512x directory. Also put the Kconfig changes in arch/powerpc/platform/512x/Kconfig.
arch/powerpc/inclu=
Quoted 5 lines
=A04 files changed, 219 insertions(+), 0 deletions(-) =A0create mode 100644
arch/powerpc/include/asm/mpc512x_gpio.h=A0create mode 100644arch/powerpc/sysdev/mpc512x_gpio.carch/powerpc/inclu= renamed from arch/powerpc/include/asm/mpc512x_gpio.h +0
-0
de/asm/mpc512x_gpio.h
Quoted 10 lines
new file mode 100644
index 0000000..8b6922darch/powerpc/include/asm/mpc512x_gpio.h new file +5
-0@@ -0,0 +1,30 @@ +/* +* Copyright (C) 2010 Matthias Fuchs <[email protected]>, esd gmbh +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2
arch/powerpc/include/asm/mpc512x_gpio.h line 15
Quoted 10 lines
arch/powerpc/include/asm/mpc512x_gpio.h new file +10
-0+* 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. =A0See 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 =A002111-130=
7 =A0USA
arch/powerpc/include/asm/mpc512x_gpio.h line 28
Quoted 13 lines
arch/powerpc/include/asm/mpc512x_gpio.h new file +13
-0+*/ +#ifndef MPC512X_GPIO_H +#define MPC512X_GPIO_H + +struct mpc512x_gpio_regs { + =A0 =A0 =A0 u32 gpdir; + =A0 =A0 =A0 u32 gpodr; + =A0 =A0 =A0 u32 gpdat; + =A0 =A0 =A0 u32 gpier; + =A0 =A0 =A0 u32 gpimr; + =A0 =A0 =A0 u32 gpicr1; + =A0 =A0 =A0 u32 gpicr2; +};
Currently this is only used by the GPIO driver. Move the structure definition into the .c file.
arch/powerpc/sysdev/mpc=
Quoted 3 lines
arch/powerpc/include/asm/mpc512x_gpio.h new file +2
-0+ +#endif /* MPC512X_GPIO_H */ arch/powerpc/sysdev/mpc= renamed from arch/powerpc/sysdev/mpc512x_gpio.c +0
-0
512x_gpio.c
Quoted 20 lines
new file mode 100644
index 0000000..09f075barch/powerpc/sysdev/mpc512x_gpio.c new file +15
-0@@ -0,0 +1,179 @@ +/* +* MPC512x gpio driver +* +* Copyright (c) 2010 Matthias Fuchs <[email protected]>, esd gmbh +* +* derived from ppc4xx gpio driver +* +* Copyright (c) 2008 Harris Corporation +* Copyright (c) 2008 Sascha Hauer <[email protected]>, Pengutronix +* Copyright (c) MontaVista Software, Inc. 2008. +* +* Author: Steve Falco <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2
arch/powerpc/sysdev/mpc512x_gpio.c line 25
Quoted 10 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +10
-0+* 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. =A0See 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 =A002111-130=
7 =A0USA
arch/powerpc/sysdev/mpc512x_gpio.c
Quoted 13 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +13
-0+*/ + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/spinlock.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/gpio.h> +#include <linux/types.h> +#include <asm/mpc512x_gpio.h> + +#define GPIO_MASK (gpio) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(0x80000000 >> (g=
pio))
For clarity, this local define should also be prefixed with MPC5121_ so it is not confused with core gpio code.
Quoted 25 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +25
-0+ +struct mpc512x_chip { + =A0 =A0 =A0 struct of_mm_gpio_chip mm_gc; + =A0 =A0 =A0 spinlock_t lock; +}; + +/* +* GPIO LIB API implementation for GPIOs +* +* There are a maximum of 32 gpios in each gpio controller. +*/ +static inline struct mpc512x_chip * +to_mpc512x_gpiochip (struct of_mm_gpio_chip *mm_gc) +{ + =A0 =A0 =A0 return container_of(mm_gc, struct mpc512x_chip, mm_gc); +} + +static int mpc512x_gpio_get (struct gpio_chip *gc,unsigned int gpio) +{ + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc); + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs; + + =A0 =A0 =A0 return in_be32(®s->gpdat) & GPIO_MASK(gpio); +} +
arch/powerpc/sysdev/mpc512x_gpio.c
Quoted 10 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +10
-0+static inline void +__mpc512x_gpio_set (struct gpio_chip *gc,unsigned int gpio,int val) +{ + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc); + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs; + + =A0 =A0 =A0 if (val) + =A0 =A0 =A0 =A0 =A0 =A0 =A0 setbits32(®s->gpdat, GPIO_MASK(gpio)); + =A0 =A0 =A0 else + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrbits32(®s->gpdat, GPIO_MASK(gpio));
Rather than the read/modify/write cycle you're using here, you should consider maintaining a shadow register for the GPIO output register. Even though it is the SoC local bus, bus cycles can be expensive when a shadow reg may already be in the cache.
Also, this may be a bug. If you read the data register, modify just the bit you care about and then write it back, then you are also setting the output state of pins currently marked for input. That doesn't sound bad, but some drivers change the output state by switching back and forth between input and output mode. Using a read/modify/write on the data register will break it.
Quoted 66 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +66
-0+} + +static void +mpc512x_gpio_set (struct gpio_chip *gc,unsigned int gpio,int val) +{ + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc); + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc); + =A0 =A0 =A0 unsigned long flags; + + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags); + + =A0 =A0 =A0 __mpc512x_gpio_set(gc, gpio, val); + + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags); + + =A0 =A0 =A0 pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val); +} + +static int mpc512x_gpio_dir_in (struct gpio_chip *gc,unsigned int gpio) +{ + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc); + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc); + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs; + =A0 =A0 =A0 unsigned long flags; + + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags); + + =A0 =A0 =A0 /* Disable open-drain function */ + =A0 =A0 =A0 clrbits32(®s->gpodr, GPIO_MASK(gpio)); + + =A0 =A0 =A0 /* Float the pin */ + =A0 =A0 =A0 clrbits32(®s->gpdir, GPIO_MASK(gpio)); + + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags); + + =A0 =A0 =A0 return 0; +} + +static int +mpc512x_gpio_dir_out (struct gpio_chip *gc,unsigned int gpio,int val) +{ + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc); + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc); + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs; + =A0 =A0 =A0 unsigned long flags; + + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags); + + =A0 =A0 =A0 /* First set initial value */ + =A0 =A0 =A0 __mpc512x_gpio_set(gc, gpio, val); + + =A0 =A0 =A0 /* Disable open-drain function */ + =A0 =A0 =A0 clrbits32(®s->gpodr, GPIO_MASK(gpio)); + + =A0 =A0 =A0 /* Drive the pin */ + =A0 =A0 =A0 setbits32(®s->gpdir, GPIO_MASK(gpio)); + + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags); + + =A0 =A0 =A0 pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val); + + =A0 =A0 =A0 return 0; +} + +static int __init mpc512x_add_gpiochips (void ) +{
arch/powerpc/sysdev/mpc512x_gpio.c line 149
Quoted 10 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +10
-0+ =A0 =A0 =A0 struct device_node *np; + + =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio") { + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int ret; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct mpc512x_chip *mpc512x_gc; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct of_gpio_chip *of_gc; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct gpio_chip *gc; + + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc512x_gc =3D kzalloc(sizeof(*mpc512x_gc),=
GFP_KERNEL);
arch/powerpc/sysdev/mpc512x_gpio.c line 162
Quoted 13 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +13
-0+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!mpc512x_gc) { + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D -ENOMEM; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } + + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_init(&mpc512x_gc->lock); + + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mm_gc =3D &mpc512x_gc->mm_gc; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 of_gc =3D &mm_gc->of_gc; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc =3D &of_gc->gc; + + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->ngpio =3D 32; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->direction_input =3D mpc512x_gpio_dir_in=
;
arch/powerpc/sysdev/mpc512x_gpio.c line 163
Quoted 1 line
arch/powerpc/sysdev/mpc512x_gpio.c new file +1
-0+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->direction_output =3D mpc512x_gpio_dir_o=
ut;
arch/powerpc/sysdev/mpc512x_gpio.c line 172
Quoted 9 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +9
-0+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->get =3D mpc512x_gpio_get; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->set =3D mpc512x_gpio_set; + + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D of_mm_gpiochip_add(np, mm_gc); + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (ret) + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err; + =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; +err: + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("%s: registration failed with status=
%d\n",
arch/powerpc/sysdev/mpc512x_gpio.c line 179
Quoted 7 lines
arch/powerpc/sysdev/mpc512x_gpio.c new file +7
-0+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0np->full_name, ret); + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(mpc512x_gc); + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* try others anyway */ + =A0 =A0 =A0 } + =A0 =A0 =A0 return 0; +} +arch_initcall (mpc512x_add_gpiochips);
Don't do this. Either make this a proper of_platform device driver, or call mpc512x_add_gpiochips() explicitly from the arch platform setup code. Otherwise, if the kernel is built for multiplatform, this function will get executed regardless of the platform.
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
Hi Grant,
thanks for comments. I will post an updated version soon.
Quoted 8 lines · Grant Likely
Review
Quoted 3 lines
+ =A0 =A0 =A0 return 0;
(quoted diff) +2
-0+} +arch_initcall(mpc512x_add_gpiochips);=20
Don't do this. Either make this a properof_platformdevice driver, or callmpc512x_add_gpiochips()explicitly from the arch platform setup code. Otherwise, if the kernel is built for multiplatform, this function will get executed regardless of the platform.
In this case I prefer moving the call to the platform code.
I tested the driver on one of our 5123 boards. Shall I add it=20 to mpc5121_ads.c:mpc5121_ads_setup_arch() also? So it's at least called=20 on the reference board? I not sure if there are some LEDs or buttons that could use it.
Matthias
Quoted 14 lines · Matthias Fuchs
Hi Grant,
thanks for comments. I will post an updated version soon.
Quoted 8 lines · Grant Likely
Review
Quoted 3 lines
+ =A0 =A0 =A0 return 0;
(quoted diff) +2
-0+} +arch_initcall(mpc512x_add_gpiochips);Don't do this. =A0Either make this a proper
of_platformdevice driver, or callmpc512x_add_gpiochips()explicitly from the arch platform setup code. =A0Otherwise, if the kernel is built for multiplatform, this function will get executed regardless of the platform.
Quoted 5 lines
In this case I prefer moving the call to the platform code. I tested the driver on one of our 5123 boards. Shall I add it to mpc5121_ads.c:mpc5121_ads_setup_arch() also? So it's at least called on the reference board? I not sure if there are some LEDs or buttons that could use it.
Yes, make sure all 5121 platforms call it.
Thanks,
g.