Re: [PATCH] add gpiolib support for mpc5200
Hi Sascha,
One small comment.
On Thu, 24 Apr 2008 17:36:59 +0200 Sascha Hauer [EMAIL PROTECTED] 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[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgp33FHfMuPwH.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] add gpiolib support for mpc5200
On Thu, Apr 24, 2008 at 12:45:49PM -0600, Grant Likely wrote:
On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
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(regs-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(regs-wkup_dvo);
+ if (val)
+ tmp |= 1 (7 - gpio);
+ else
+ tmp = ~(1 (7 - gpio));
+ out_8(regs-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(regs-wkup_ddr);
+ tmp = ~(1 (7 - gpio));
+ out_8(regs-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(regs-wkup_ddr);
+ tmp |= 1 (7 - gpio);
+ out_8(regs-wkup_ddr, tmp);
+
+ /* Finally enable the pin */
+ tmp = in_8(regs-wkup_gpioe);
+ tmp |= 1 (7 - gpio);
+ out_8(regs-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?
[PATCH] add gpiolib support for mpc5200
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 [EMAIL PROTECTED]
---
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 [EMAIL PROTECTED], 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(regs-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(regs-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(regs-wkup_ddr, chip-shadow_ddr);
+
+ /* and enable the pin */
+ chip-shadow_gpioe |= 1 (7 - gpio);
+ out_8(regs-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(regs-wkup_ddr, chip-shadow_ddr);
+
+ /* Finally enable the pin */
+ chip-shadow_gpioe |= 1 (7 - gpio);
+ out_8(regs-wkup_gpioe, chip-shadow_gpioe);
+
+
Re: [PATCH] add gpiolib support for mpc5200
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: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] add gpiolib support for mpc5200
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: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] add gpiolib support for mpc5200
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 [EMAIL PROTECTED]
Genesi, Manager, Developer Relations
Grant Likely wrote:
On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
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 [EMAIL PROTECTED], 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(regs-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 =
Re: [PATCH] add gpiolib support for mpc5200
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 [EMAIL PROTECTED] Genesi, Manager, Developer Relations Grant Likely wrote: On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED], 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 -
Re: [PATCH] add gpiolib support for mpc5200
On Fri, Apr 25, 2008 at 12:51 PM, Sascha Hauer [EMAIL PROTECTED] 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. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] add gpiolib support for mpc5200
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 [EMAIL PROTECTED]
---
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 [EMAIL PROTECTED], 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(regs-wkup_ival) (7 - gpio)) 1;
+
+ pr_debug(%s: gpio: %d ret: %d\n, __func__, gpio, ret);
+
+ 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(regs-wkup_dvo);
+ if (val)
+ tmp |= 1 (7 - gpio);
+ else
+ tmp = ~(1 (7 - gpio));
+ out_8(regs-wkup_dvo, tmp);
+
+ 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(regs-wkup_ddr);
+ tmp = ~(1 (7 - gpio));
+ out_8(regs-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(regs-wkup_ddr);
+ tmp |= 1 (7 - gpio);
+ out_8(regs-wkup_ddr, tmp);
+
+ /* Finally enable the pin */
+ tmp = in_8(regs-wkup_gpioe);
+ tmp |= 1 (7 - gpio);
+ out_8(regs-wkup_gpioe, tmp);
+
+ 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
Re: [PATCH] add gpiolib support for mpc5200
On Thu, Apr 24, 2008 at 9:36 AM, Sascha Hauer [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
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 [EMAIL PROTECTED], 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(regs-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(regs-wkup_dvo);
+ if (val)
+ tmp |= 1 (7 - gpio);
+ else
+ tmp = ~(1 (7 - gpio));
+ out_8(regs-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(regs-wkup_ddr);
+ tmp = ~(1 (7 - gpio));
+
