[PATCH] IBM GPIO driver for PowerPC 4xx is back from the dead
Arnd Bergmann
arnd.bergmann at de.ibm.com
Fri Sep 29 19:06:19 EST 2006
On Friday 29 September 2006 10:29, Jean-Baptiste Maneyrol wrote:
> diff -Naur linux-2.6.18_gpio/drivers/char/ibm_gpio.c tlgate_gpio/drivers/char/ibm_gpio.c
> --- linux-2.6.18_gpio/drivers/char/ibm_gpio.c 1970-01-01 01:00:00.000000000 +0100
> +++ tlgate_gpio/drivers/char/ibm_gpio.c 2006-09-28 16:46:42.000000000 +0200
> @@ -0,0 +1,349 @@
> +/*
> + * FILE NAME ibm_gpio.c
<insert usual comment about removing the file name from the file>
> + * BRIEF MODULE DESCRIPTION
> + * API for IBM PowerPC 4xx GPIO device.
> + * Driver for IBM PowerPC 4xx GPIO device.
> + *
> + * Armin Kuster akuster at pacbell.net
> + * Sept, 2001
> + *
> + * Orignial driver
> + * Author: MontaVista Software, Inc. <source at mvista.com>
> + * Frank Rowand <frank_rowand at mvista.com>
> + * Debbie Chu <debbie_chu at mvista.com>
> + *
> + * Copyright 2000,2001,2002 MontaVista Software Inc.
Any new copyright year?
> + * TODO: devfs
surely not.
> + * Version: 02/01/12 - Armin
> + * converted to ocp and using ioremap
> + *
> + * 1.2 02/21/01 - Armin
> + * minor compiler warning fixes
> + *
> + * 1.3 02/22/01 - Armin
> + * added apm
> + *
> + * 1.4 05/07/02 - Armin/David Mueller
> + * coverted to core_ocp[];
> + *
> + * 1.5 05/25/02 - Armin
> + * name change from *_driver to *_dev
> + *
> + * 1.6 06/04/02 - Matt Porter
> + * ioremap paddr. Comment as 4xx generic driver.
> + * Fix header to be userland safe and locate in
> + * an accessible area. Add ioctl to configure
> + * multiplexed GPIO pins.
> + *
> + * 1.7 07/25/02 - Armin
> + * added CPM to enable/disable in init/exit
kill that changelog
> + */
> +
> +#define VUFX "07.25.02"
should be MODULE_VERSION()
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/init.h>
> +#include <linux/ioctl.h>
> +#include <linux/fs.h>
> +#include <asm/ibm_gpio.h>
> +#include <asm/uaccess.h>
> +#include <asm/io.h>
> +#include <asm/machdep.h>
> +#include <asm/ocp.h>
> +#include <asm/ibm4xx.h>
> +
> +struct miscdevice ibm_gpio_miscdev;
> +static struct gpio_regs *gpiop;
> +
> +int
> +ibm_gpio_config(__u32 device, __u32 mask, __u32 data)
any symbols should be static. Function arguments are not
visible to users, so they should be 'u32' instead of '__u32'.
> +{
> + u32 cfg_reg;
> +
> + if (device != 0)
> + return -ENXIO;
> +
> +#ifdef CONFIG_40x
> +#ifdef DCRN_CHCR0
> + /*
> + * PPC405 uses CPC0_CR0 to select multiplexed GPIO pins.
> + */
> + cfg_reg = mfdcr(DCRN_CHCR0);
> + cfg_reg = (cfg_reg & ~mask) | (data & mask);
> + mtdcr(DCRN_CHCR0, cfg_reg);
> +#endif
> +#elif CONFIG_440GP
> + /*
> + * PPC440GP uses CPC0_GPIO to select multiplexed GPIO pins.
> + */
> + cfg_reg = mfdcr(DCRN_CPC0_GPIO);
> + cfg_reg = (cfg_reg & ~mask) | (data & mask);
> + mtdcr(DCRN_CPC0_GPIO, cfg_reg);
> +#elif CONFIG_440GX
> + /*
> + * PPC440GX uses SDR0_PFC0 to select multiplexed GPIO pins
> + */
> + cfg_reg = SDR_READ(DCRN_SDR_PFC0);
> + cfg_reg = (cfg_reg & ~mask) | (data & mask);
> + SDR_WRITE(DCRN_SDR_PFC0, cfg_reg);
> +#else
> +#error This driver is only supported on PPC40x and PPC440 CPUs
> +#endif
This prevents building a single kernel for multiple 440 version.
Please use a run-time check, or better get the necessary information
from the device tree.
> +int
> +ibm_gpio_in(__u32 device, __u32 mask, volatile __u32 * data)
> +{
> + if (device != 0)
> + return -ENXIO;
> + gpiop->tcr = gpiop->tcr & ~mask;
> + eieio();
> +
> + /*
> + ** If the previous state was OUT, and gpiop->ir is read once, then the
> + ** data that was being OUTput will be read. One way to get the right
> + ** data is to read gpiop->ir twice.
> + */
> +
> + *data = gpiop->ir;
> + *data = gpiop->ir & mask;
> + eieio();
> + return 0;
> +}
Don't just assign *data, make that an __iomem pointer and use
an appropriate accessor function (out_be32 or such).
> +
> +static int
> +ibm_gpio_open(struct inode *inode, struct file *file)
> +{
> + return 0;
> +}
> +
> +static int
> +ibm_gpio_release(struct inode *inode, struct file *file)
> +{
> + return 0;
> +}
You don't need these.
> +static int
> +ibm_gpio_ioctl(struct inode *inode, struct file *file,
> + unsigned int cmd, unsigned long arg)
> +{
> + static struct ibm_gpio_ioctl_data ioctl_data;
> + int status;
> +
> + switch (cmd) {
> + case IBMGPIO_IN:
> + if (copy_from_user(&ioctl_data,
> + (struct ibm_gpio_ioctl_data *) arg,
> + sizeof (ioctl_data))) {
> + return -EFAULT;
> + }
Just do the copy_from_user() once, in the beginning here, instead
of each case.
> +
> +static int __init
> +ibm_gpio_probe(struct ocp_device *ocp)
> +{
> + ibm_gpio_miscdev.minor = GPIO_MINOR;
> + ibm_gpio_miscdev.name = ocp->name;
> + ibm_gpio_miscdev.fops = &ibm_gpio_fops;
> + misc_register(&ibm_gpio_miscdev);
> +
> + if (!request_mem_region(ocp->def->paddr, sizeof(struct gpio_regs), "ibm_gpio"))
> + return -EBUSY;
> +
> + gpiop = (struct gpio_regs *) ioremap(ocp->def->paddr,
> + sizeof(struct gpio_regs));
> + if (!gpiop) {
> + release_mem_region(ocp->def->paddr, sizeof(struct gpio_regs));
> + return -ENOMEM;
> + }
> +
> + printk("GPIO #%d at 0x%lx\n", ocp->def->index,
> + (unsigned long) gpiop);
> +
> + return 0;
> +}
> +
> +static void __exit
> +ibm_gpio_remove(struct ocp_device *ocp)
> +{
> + misc_deregister(&ibm_gpio_miscdev);
> +
> + iounmap(gpiop);
> + gpiop = NULL;
> +
> + release_mem_region(ocp->def->paddr, sizeof(struct gpio_regs));
> +}
> +
> +static struct ocp_device_id ibm_gpio_ids[] __devinitdata =
> +{
> + { .vendor = OCP_VENDOR_IBM, .function = OCP_FUNC_GPIO },
> + { .vendor = OCP_VENDOR_INVALID }
> +};
> +
> +MODULE_DEVICE_TABLE(ocp, ibm_gpio_ids);
> +
> +static struct ocp_driver ibm_gpio_driver =
> +{
> + .name = "ibm_gpio",
> + .id_table = ibm_gpio_ids,
> + .probe = ibm_gpio_probe,
> + .remove = __devexit_p(ibm_gpio_remove),
> +#if defined(CONFIG_PM)
> + .suspend = NULL,
> + .resume = NULL,
> +#endif
> +};
> +
> +static int __init
> +ibm_gpio_init(void)
> +{
> + printk("IBM GPIO driver version %s\n", VUFX);
> + return ocp_register_driver(&ibm_gpio_driver);
> +}
> +
> +static void __exit
> +ibm_gpio_exit(void)
> +{
> + ocp_unregister_driver(&ibm_gpio_driver);
> +}
ocp will die a painful death soon, when 440 has been converted over
to use the device tree and of_device.
> +EXPORT_SYMBOL(ibm_gpio_tristate);
> +EXPORT_SYMBOL(ibm_gpio_open_drain);
> +EXPORT_SYMBOL(ibm_gpio_in);
> +EXPORT_SYMBOL(ibm_gpio_out);
Why export these at all? They don't seem to be used elsewhere?
If you need to export them, make it EXPORT_SYMBOL_GPL and put that
line directly under the functions.
> +
> +struct ibm_gpio_ioctl_data {
> + __u32 device;
> + __u32 mask;
> + __u32 data;
> +};
> +
> +#define IBMGPIO_IOCTL_BASE 'Z'
> +
> +#define IBMGPIO_IN _IOWR(IBMGPIO_IOCTL_BASE, 0, struct ibm_gpio_ioctl_data)
> +#define IBMGPIO_OUT _IOW (IBMGPIO_IOCTL_BASE, 1, struct ibm_gpio_ioctl_data)
> +#define IBMGPIO_OPEN_DRAIN _IOW (IBMGPIO_IOCTL_BASE, 2, struct ibm_gpio_ioctl_data)
> +#define IBMGPIO_TRISTATE _IOW (IBMGPIO_IOCTL_BASE, 3, struct ibm_gpio_ioctl_data)
> +#define IBMGPIO_CFG _IOW (IBMGPIO_IOCTL_BASE, 4, struct ibm_gpio_ioctl_data)
Is that the same ioctl interface as for the other gpio drivers?
> +#endif
More information about the Linuxppc-embedded
mailing list