[PATCH v2] powerpc: Basic GPIO support for GE Fanuc SBC610
Anton Vorontsov
avorontsov at ru.mvista.com
Fri Nov 7 02:17:23 EST 2008
On Thu, Nov 06, 2008 at 12:10:33PM +0000, Martyn Welch wrote:
> Basic support for the GPIO available on the SBC610 VPX Single Board Computer
> from GE Fanuc (PowerPC MPC8641D).
>
> This patch adds basic support for the GPIO in the devices I/O FPGA, the GPIO
> functionality is exposed through the AFIX pins on the backplane, unless used
> by an AFIX card.
>
> This code currently does not support switching between totem-pole and
> open-drain outputs (when used as outputs, GPIOs default to totem-pole).
> The interrupt capabilites of the GPIO lines is also not currently supported.
>
> Signed-off-by: Martyn Welch <martyn.welch at gefanuc.com>
> ---
Mostly looks good. Few comments below.
> Sorry for the quick jump to version 2 - A stray "i" crept in to the previous
> version as I was editing the email for submission.
>
> arch/powerpc/boot/dts/gef_sbc610.dts | 6 +
> arch/powerpc/platforms/86xx/Kconfig | 2
> arch/powerpc/platforms/86xx/Makefile | 3 -
> arch/powerpc/platforms/86xx/gef_gpio.c | 185 ++++++++++++++++++++++++++++++++
> arch/powerpc/platforms/86xx/gef_gpio.h | 10 ++
> 5 files changed, 205 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/gef_sbc610.dts b/arch/powerpc/boot/dts/gef_sbc610.dts
> index 6ed6083..963dd5b 100644
> --- a/arch/powerpc/boot/dts/gef_sbc610.dts
> +++ b/arch/powerpc/boot/dts/gef_sbc610.dts
> @@ -98,6 +98,12 @@
> interrupt-parent = <&mpic>;
>
> };
> + gef_gpio: gpio at 7,14000 {
> + #gpio-cells = <1>;
Don't use one-cell scheme. Two cells are convenient for GPIO flags
(active-low GPIO, for example).
> + compatible = "gef,fpga-gpio";
> + reg = <0x7 0x14000 0x24>;
> + gpio-controller;
> + };
> };
>
> soc at fef00000 {
> diff --git a/arch/powerpc/platforms/86xx/Kconfig b/arch/powerpc/platforms/86xx/Kconfig
> index 77dd797..8e56939 100644
> --- a/arch/powerpc/platforms/86xx/Kconfig
> +++ b/arch/powerpc/platforms/86xx/Kconfig
> @@ -34,6 +34,8 @@ config MPC8610_HPCD
> config GEF_SBC610
> bool "GE Fanuc SBC610"
> select DEFAULT_UIMAGE
> + select GENERIC_GPIO
> + select ARCH_REQUIRE_GPIOLIB
> select HAS_RAPIDIO
> help
> This option enables support for GE Fanuc's SBC610.
> diff --git a/arch/powerpc/platforms/86xx/Makefile b/arch/powerpc/platforms/86xx/Makefile
> index 4a56ff6..31e540c 100644
> --- a/arch/powerpc/platforms/86xx/Makefile
> +++ b/arch/powerpc/platforms/86xx/Makefile
> @@ -7,4 +7,5 @@ obj-$(CONFIG_SMP) += mpc86xx_smp.o
> obj-$(CONFIG_MPC8641_HPCN) += mpc86xx_hpcn.o
> obj-$(CONFIG_SBC8641D) += sbc8641d.o
> obj-$(CONFIG_MPC8610_HPCD) += mpc8610_hpcd.o
> -obj-$(CONFIG_GEF_SBC610) += gef_sbc610.o gef_pic.o
> +gef-gpio-$(CONFIG_GPIOLIB) += gef_gpio.o
> +obj-$(CONFIG_GEF_SBC610) += gef_sbc610.o gef_pic.o $(gef-gpio-y)
> diff --git a/arch/powerpc/platforms/86xx/gef_gpio.c b/arch/powerpc/platforms/86xx/gef_gpio.c
> new file mode 100644
> index 0000000..aafff1b
> --- /dev/null
> +++ b/arch/powerpc/platforms/86xx/gef_gpio.c
> @@ -0,0 +1,185 @@
> +/*
> + * Driver for GE Fanuc's FPGA based GPIO pins
> + *
> + * Author: Martyn Welch <martyn.welch at gefanuc.com>
> + *
> + * 2008 (c) GE Fanuc Intelligent Platforms Embedded Systems, Inc.
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +/* TODO
> + *
> + * Configuration of output modes (totem-pole/open-drain)
> + * Interrupt configuration - interrupts are always generated the FPGA relies on
> + * the I/O interrupt controllers mask to stop them propergating
> + */
#include <linux/compiler.h> for __iomem
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
> +
> +#include "gef_gpio.h"
> +
> +#define DEBUG
> +#undef DEBUG
> +
> +#ifdef DEBUG
> +#define DBG(fmt...) do { printk(KERN_DEBUG "gef_gpio: " fmt); } while (0)
> +#else
> +#define DBG(fmt...) do { } while (0)
> +#endif
There is pr_debug() function exists.
> +#define NUM_GPIO 19
> +
> +/* Let's create a common inlined function to commonise the code and so we don't
> + * have to resolve the chip structure multiple times.
> + */
> +inline void _gef_gpio_set(void __iomem *reg, unsigned int offset, int value)
'static' missing. + No need for inline, gcc can decide if inlined
function better or not.
> +{
> + unsigned int data;
> +
> + data = ioread32be(reg);
> + /* value: 0=low; 1=high */
> + DBG("Output Set, Pre:0x%8.8x, ", data);
> + if (value & 0x1) {
> + data = data | (0x1 << offset);
> + DBG("OR Mask:0x%8.8x, Post:0x%8.8x\n", (0x1 << offset), data);
> + } else {
> + data = data & ~(0x1 << offset);
> + DBG("AND Mask:0x%8.8x, Post:0x%8.8x\n", ~(0x1 << offset), data);
> + }
> + iowrite32be(data, reg);
> +}
> +
> +
> +static int gef_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + unsigned int data;
> +
> + /* Find memory mapped gpio chip structure from gpio_chip, this contains
> + * the mapped location of the GPIO controller */
Why not canonical-style comments?
/*
* Multi-line
* comment.
*/
> + mmchip = to_of_mm_gpio_chip(chip);
> +
> + data = ioread32be(mmchip->regs + GEF_GPIO_DIRECT);
> + DBG("Direction In, Pre:0x%8.8x, ", data);
> + data = data | (0x1 << offset);
> + DBG("OR Mask:0x%8.8x, Post:0x%8.8x\n", (0x1 << offset), data);
> + iowrite32be(data, mmchip->regs + GEF_GPIO_DIRECT);
> +
> + return 0;
> +}
> +
> +static int gef_gpio_dir_out(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + unsigned int data;
> +
> + /* Find memory mapped gpio chip structure from gpio_chip, this contains
> + * the mapped location of the GPIO controller */
> + mmchip = to_of_mm_gpio_chip(chip);
> +
> + /* Set direction before switching to input */
> + _gef_gpio_set(mmchip->regs + GEF_GPIO_OUT, offset, value);
> +
> + data = ioread32be(mmchip->regs + GEF_GPIO_DIRECT);
> + DBG("Direction Out, Pre:0x%8.8x, ", data);
> + data = data & ~(0x1 << offset);
> + DBG("AND Mask:0x%8.8x, Post:0x%8.8x\n", ~(0x1 << offset), data);
> + iowrite32be(data, mmchip->regs + GEF_GPIO_DIRECT);
> +
> + return 0;
> +}
> +
> +static int gef_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + struct of_mm_gpio_chip *mmchip;
> + unsigned int data;
> + int state = 0;
> +
> + /* Find memory mapped gpio chip structure from gpio_chip, this contains
> + * the mapped location of the GPIO controller */
> + mmchip = to_of_mm_gpio_chip(chip);
> +
> + data = ioread32be(mmchip->regs + GEF_GPIO_IN);
> + state = (int)((data >> offset) & 0x1);
> + DBG("Get Input, Data:0x%8.8x\n", data);
> +
> + return state;
> +}
> +
> +static void gef_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + struct of_mm_gpio_chip *mmchip;
> +
> + /* Find memory mapped gpio chip structure from gpio_chip, this contains
> + * the mapped location of the GPIO controller */
> + mmchip = to_of_mm_gpio_chip(chip);
> +
> + _gef_gpio_set(mmchip->regs + GEF_GPIO_OUT, offset, value);
> +
> +}
> +
> +static int __devinit gef_gpio_probe(struct of_device *dev,
> + const struct of_device_id *match)
> +{
> + int retval;
> + struct of_mm_gpio_chip *gef_gpio_chip;
> +
> + DBG("gef_gpio_probe() called for %s\n", dev->node->full_name);
> +
> + /* Allocate chip structure */
> + gef_gpio_chip = kzalloc(sizeof(*gef_gpio_chip), GFP_KERNEL);
> + if (!gef_gpio_chip) {
> + DBG("Unable to allocate GEF GPIO structure\n");
> + return -ENOMEM;
> + }
> +
> + /* Setup pointers to chip functions */
> + gef_gpio_chip->of_gc.gpio_cells = 1;
Two cells are preferred.
> + gef_gpio_chip->of_gc.gc.ngpio = NUM_GPIO;
> + gef_gpio_chip->of_gc.gc.direction_input = gef_gpio_dir_in;
> + gef_gpio_chip->of_gc.gc.direction_output = gef_gpio_dir_out;
> + gef_gpio_chip->of_gc.gc.get = gef_gpio_get;
> + gef_gpio_chip->of_gc.gc.set = gef_gpio_set;
> +
> + /* This function adds a memory mapped GPIO chip */
> + retval = of_mm_gpiochip_add(dev->node, gef_gpio_chip);
> + if (retval)
> + return retval;
gef_gpio_chip allocation leaked.
> +
> + return 0;
> +};
> +
> +static const struct of_device_id gef_gpio_ids[] = {
> + {
> + .compatible = "gef,fpga-gpio",
Isn't this too generic?
> + },
> + {},
> +};
> +
> +static struct of_platform_driver gef_gpio_driver = {
> + .name = "gef-of-gpio",
> + .match_table = gef_gpio_ids,
> + .probe = gef_gpio_probe,
> +};
> +
> +static __init int gef_gpio_init(void)
> +{
> + DBG("gef_gpio_init()\n");
> + return of_register_platform_driver(&gef_gpio_driver);
> +}
> +subsys_initcall(gef_gpio_init);
There is point in doing the of_platform_driver for this GPIO
controller. More than that, of_platform bus is probed at the
device_initcall time, so there is also no point in the
subsys_initcall for this driver.
I'd recommend to do the registration as in
arch/powerpc/sysdev/qe_lib/gpio.c.
> +MODULE_DESCRIPTION("GE Fanuc I/O FPGA GPIO driver");
> +MODULE_AUTHOR("Martyn Welch <martyn.welch at gefanuc.com");
> +MODULE_LICENSE("GPL");
> +
> diff --git a/arch/powerpc/platforms/86xx/gef_gpio.h b/arch/powerpc/platforms/86xx/gef_gpio.h
> new file mode 100755
> index 0000000..5f76a7c
> --- /dev/null
> +++ b/arch/powerpc/platforms/86xx/gef_gpio.h
> @@ -0,0 +1,10 @@
> +#define GEF_GPIO_DIRECT 0x00
> +#define GEF_GPIO_IN 0x04
> +#define GEF_GPIO_OUT 0x08
> +#define GEF_GPIO_TRIG 0x0C
> +#define GEF_GPIO_POLAR_A 0x10
> +#define GEF_GPIO_POLAR_B 0x14
> +#define GEF_GPIO_INT_STAT 0x18
> +#define GEF_GPIO_OVERRUN 0x1C
> +#define GEF_GPIO_MODE 0x20
No need for this header. Just place this into the .c file.
Thanks,
--
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2
More information about the Linuxppc-dev
mailing list