Re: [PATCH] gpio: add MSIC gpio driver

From: Grant Likely
Date: Fri May 11 2012 - 15:20:53 EST

Next message: Rhyland Klein: "Re: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree"
Previous message: Christoph Lameter: "Re: [PATCH v2 04/29] slub: always get the cache from its page inkfree"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, 24 Apr 2012 11:02:49 +0100, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
> From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>
> Add gpio support for Intel MSIC chips found in Intel Medfield platforms.
> MSIC supports totally 24 GPIOs with 16 low voltage and 8 high voltage pins.
> Driver uses MSIC mfd interface for MSIC access.
>
> (Updated comment to indicate why locking is actually safe)
>
> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>

I've got a comment, but since this is a new driver I don't have any
problem applying it. Any possible changes can be handled in a
follow-on patch, so I've merged it.

My main question is what is responsible for allocating the irq_descs
uses by this driver? I don't see a call to irq_alloc_descs() in this
function.

Also, I'm pushing for gpio controllers that handle irqs to use an
irq_domain for mapping to a Linux irq. It makes sense on the embedded
platforms, but I haven't looked enough at x86 to say if it should be
done there too (aside from when using DT).

g.

> ---
>
> drivers/gpio/Kconfig | 8 +
> drivers/gpio/Makefile | 1
> drivers/gpio/gpio-msic.c | 339 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 348 insertions(+), 0 deletions(-)
> create mode 100644 drivers/gpio/gpio-msic.c
>
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 0833ed5..b686db8 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -532,4 +532,12 @@ config GPIO_TPS65910
> help
> Select this option to enable GPIO driver for the TPS65910
> chip family.
> +
> +config GPIO_MSIC
> + bool "Intel MSIC mixed signal gpio support"
> + depends on MFD_INTEL_MSIC
> + help
> + Enable support for GPIO on intel MSIC controllers found in
> + intel MID devices
> +
> endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index ad963811..fe8d182 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_MCP23S08) += gpio-mcp23s08.o
> obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
> obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
> obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
> +obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
> obj-$(CONFIG_GPIO_MSM_V1) += gpio-msm-v1.o
> obj-$(CONFIG_GPIO_MSM_V2) += gpio-msm-v2.o
> obj-$(CONFIG_GPIO_MXC) += gpio-mxc.o
> diff --git a/drivers/gpio/gpio-msic.c b/drivers/gpio/gpio-msic.c
> new file mode 100644
> index 0000000..71a838f
> --- /dev/null
> +++ b/drivers/gpio/gpio-msic.c
> @@ -0,0 +1,339 @@
> +/*
> + * Intel Medfield MSIC GPIO driver>
> + * Copyright (c) 2011, Intel Corporation.
> + *
> + * Author: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> + * Based on intel_pmic_gpio.c
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.,
> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/init.h>
> +#include <linux/gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/intel_msic.h>
> +
> +/* the offset for the mapping of global gpio pin to irq */
> +#define MSIC_GPIO_IRQ_OFFSET 0x100
> +
> +#define MSIC_GPIO_DIR_IN 0
> +#define MSIC_GPIO_DIR_OUT BIT(5)
> +#define MSIC_GPIO_TRIG_FALL BIT(1)
> +#define MSIC_GPIO_TRIG_RISE BIT(2)
> +
> +/* masks for msic gpio output GPIOxxxxCTLO registers */
> +#define MSIC_GPIO_DIR_MASK BIT(5)
> +#define MSIC_GPIO_DRV_MASK BIT(4)
> +#define MSIC_GPIO_REN_MASK BIT(3)
> +#define MSIC_GPIO_RVAL_MASK (BIT(2) | BIT(1))
> +#define MSIC_GPIO_DOUT_MASK BIT(0)
> +
> +/* masks for msic gpio input GPIOxxxxCTLI registers */
> +#define MSIC_GPIO_GLBYP_MASK BIT(5)
> +#define MSIC_GPIO_DBNC_MASK (BIT(4) | BIT(3))
> +#define MSIC_GPIO_INTCNT_MASK (BIT(2) | BIT(1))
> +#define MSIC_GPIO_DIN_MASK BIT(0)
> +
> +#define MSIC_NUM_GPIO 24
> +
> +struct msic_gpio {
> + struct platform_device *pdev;
> + struct mutex buslock;
> + struct gpio_chip chip;
> + int irq;
> + unsigned irq_base;
> + unsigned long trig_change_mask;
> + unsigned trig_type;
> +};
> +
> +/*
> + * MSIC has 24 gpios, 16 low voltage (1.2-1.8v) and 8 high voltage (3v).
> + * Both the high and low voltage gpios are divided in two banks.
> + * GPIOs are numbered with GPIO0LV0 as gpio_base in the following order:
> + * GPIO0LV0..GPIO0LV7: low voltage, bank 0, gpio_base
> + * GPIO1LV0..GPIO1LV7: low voltage, bank 1, gpio_base + 8
> + * GPIO0HV0..GPIO0HV3: high voltage, bank 0, gpio_base + 16
> + * GPIO1HV0..GPIO1HV3: high voltage, bank 1, gpio_base + 20
> + */
> +
> +static int msic_gpio_to_ireg(unsigned offset)
> +{
> + if (offset >= MSIC_NUM_GPIO)
> + return -EINVAL;
> +
> + if (offset < 8)
> + return INTEL_MSIC_GPIO0LV0CTLI - offset;
> + if (offset < 16)
> + return INTEL_MSIC_GPIO1LV0CTLI - offset + 8;
> + if (offset < 20)
> + return INTEL_MSIC_GPIO0HV0CTLI - offset + 16;
> +
> + return INTEL_MSIC_GPIO1HV0CTLI - offset + 20;
> +}
> +
> +static int msic_gpio_to_oreg(unsigned offset)
> +{
> + if (offset >= MSIC_NUM_GPIO)
> + return -EINVAL;
> +
> + if (offset < 8)
> + return INTEL_MSIC_GPIO0LV0CTLO - offset;
> + if (offset < 16)
> + return INTEL_MSIC_GPIO1LV0CTLO - offset + 8;
> + if (offset < 20)
> + return INTEL_MSIC_GPIO0HV0CTLO - offset + 16;
> +
> + return INTEL_MSIC_GPIO1HV0CTLO + offset + 20;
> +}
> +
> +static int msic_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + int reg;
> +
> + reg = msic_gpio_to_oreg(offset);
> + if (reg < 0)
> + return reg;
> +
> + return intel_msic_reg_update(reg, MSIC_GPIO_DIR_IN, MSIC_GPIO_DIR_MASK);
> +}
> +
> +static int msic_gpio_direction_output(struct gpio_chip *chip,
> + unsigned offset, int value)
> +{
> + int reg;
> + unsigned mask;
> +
> + value = (!!value) | MSIC_GPIO_DIR_OUT;
> + mask = MSIC_GPIO_DIR_MASK | MSIC_GPIO_DOUT_MASK;
> +
> + reg = msic_gpio_to_oreg(offset);
> + if (reg < 0)
> + return reg;
> +
> + return intel_msic_reg_update(reg, value, mask);
> +}
> +
> +static int msic_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + u8 r;
> + int ret;
> + int reg;
> +
> + reg = msic_gpio_to_ireg(offset);
> + if (reg < 0)
> + return reg;
> +
> + ret = intel_msic_reg_read(reg, &r);
> + if (ret < 0)
> + return ret;
> +
> + return r & MSIC_GPIO_DIN_MASK;
> +}
> +
> +static void msic_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + int reg;
> +
> + reg = msic_gpio_to_oreg(offset);
> + if (reg < 0)
> + return;
> +
> + intel_msic_reg_update(reg, !!value , MSIC_GPIO_DOUT_MASK);
> +}
> +
> +/*
> + * This is called from genirq with mg->buslock locked and
> + * irq_desc->lock held. We can not access the scu bus here, so we
> + * store the change and update in the bus_sync_unlock() function below
> + */
> +static int msic_irq_type(struct irq_data *data, unsigned type)
> +{
> + struct msic_gpio *mg = irq_data_get_irq_chip_data(data);
> + u32 gpio = data->irq - mg->irq_base;
> +
> + if (gpio >= mg->chip.ngpio)
> + return -EINVAL;
> +
> + /* mark for which gpio the trigger changed, protected by buslock */
> + mg->trig_change_mask |= (1 << gpio);
> + mg->trig_type = type;
> +
> + return 0;
> +}
> +
> +static int msic_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + struct msic_gpio *mg = container_of(chip, struct msic_gpio, chip);
> + return mg->irq_base + offset;
> +}
> +
> +static void msic_bus_lock(struct irq_data *data)
> +{
> + struct msic_gpio *mg = irq_data_get_irq_chip_data(data);
> + mutex_lock(&mg->buslock);
> +}
> +
> +static void msic_bus_sync_unlock(struct irq_data *data)
> +{
> + struct msic_gpio *mg = irq_data_get_irq_chip_data(data);
> + int offset;
> + int reg;
> + u8 trig = 0;
> +
> + /* We can only get one change at a time as the buslock covers the
> + entire transaction. The irq_desc->lock is dropped before we are
> + called but that is fine */
> + if (mg->trig_change_mask) {
> + offset = __ffs(mg->trig_change_mask);
> +
> + reg = msic_gpio_to_ireg(offset);
> + if (reg < 0)
> + goto out;
> +
> + if (mg->trig_type & IRQ_TYPE_EDGE_RISING)
> + trig |= MSIC_GPIO_TRIG_RISE;
> + if (mg->trig_type & IRQ_TYPE_EDGE_FALLING)
> + trig |= MSIC_GPIO_TRIG_FALL;
> +
> + intel_msic_reg_update(reg, trig, MSIC_GPIO_INTCNT_MASK);
> + mg->trig_change_mask = 0;
> + }
> +out:
> + mutex_unlock(&mg->buslock);
> +}
> +
> +/* Firmware does all the masking and unmasking for us, no masking here. */
> +static void msic_irq_unmask(struct irq_data *data) { }
> +
> +static void msic_irq_mask(struct irq_data *data) { }
> +
> +static struct irq_chip msic_irqchip = {
> + .name = "MSIC-GPIO",
> + .irq_mask = msic_irq_mask,
> + .irq_unmask = msic_irq_unmask,
> + .irq_set_type = msic_irq_type,
> + .irq_bus_lock = msic_bus_lock,
> + .irq_bus_sync_unlock = msic_bus_sync_unlock,
> +};
> +
> +static void msic_gpio_irq_handler(unsigned irq, struct irq_desc *desc)
> +{
> + struct irq_data *data = irq_desc_get_irq_data(desc);
> + struct msic_gpio *mg = irq_data_get_irq_handler_data(data);
> + struct irq_chip *chip = irq_data_get_irq_chip(data);
> + struct intel_msic *msic = pdev_to_intel_msic(mg->pdev);
> + int i;
> + int bitnr;
> + u8 pin;
> + unsigned long pending = 0;
> +
> + for (i = 0; i < (mg->chip.ngpio / BITS_PER_BYTE); i++) {
> + intel_msic_irq_read(msic, INTEL_MSIC_GPIO0LVIRQ + i, &pin);
> + pending = pin;
> +
> + if (pending) {
> + for_each_set_bit(bitnr, &pending, BITS_PER_BYTE)
> + generic_handle_irq(mg->irq_base +
> + (i * BITS_PER_BYTE) + bitnr);
> + }
> + }
> + chip->irq_eoi(data);
> +}
> +
> +static int __devinit platform_msic_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_msic_gpio_pdata *pdata = dev->platform_data;
> + struct msic_gpio *mg;
> + int irq = platform_get_irq(pdev, 0);
> + int retval;
> + int i;
> +
> + if (irq < 0) {
> + dev_err(dev, "no IRQ line\n");
> + return -EINVAL;
> + }
> +
> + if (!pdata || !pdata->gpio_base) {
> + dev_err(dev, "incorrect or missing platform data\n");
> + return -EINVAL;
> + }
> +
> + mg = kzalloc(sizeof(*mg), GFP_KERNEL);
> + if (!mg)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, mg);
> +
> + mg->pdev = pdev;
> + mg->irq = irq;
> + mg->irq_base = pdata->gpio_base + MSIC_GPIO_IRQ_OFFSET;
> + mg->chip.label = "msic_gpio";
> + mg->chip.direction_input = msic_gpio_direction_input;
> + mg->chip.direction_output = msic_gpio_direction_output;
> + mg->chip.get = msic_gpio_get;
> + mg->chip.set = msic_gpio_set;
> + mg->chip.to_irq = msic_gpio_to_irq;
> + mg->chip.base = pdata->gpio_base;
> + mg->chip.ngpio = MSIC_NUM_GPIO;
> + mg->chip.can_sleep = 1;
> + mg->chip.dev = dev;
> +
> + mutex_init(&mg->buslock);
> +
> + retval = gpiochip_add(&mg->chip);
> + if (retval) {
> + dev_err(dev, "Adding MSIC gpio chip failed\n");
> + goto err;
> + }
> +
> + for (i = 0; i < mg->chip.ngpio; i++) {
> + irq_set_chip_data(i + mg->irq_base, mg);
> + irq_set_chip_and_handler_name(i + mg->irq_base,
> + &msic_irqchip,
> + handle_simple_irq,
> + "demux");
> + }
> + irq_set_chained_handler(mg->irq, msic_gpio_irq_handler);
> + irq_set_handler_data(mg->irq, mg);
> +
> + return 0;
> +err:
> + kfree(mg);
> + return retval;
> +}
> +
> +static struct platform_driver platform_msic_gpio_driver = {
> + .driver = {
> + .name = "msic_gpio",
> + .owner = THIS_MODULE,
> + },
> + .probe = platform_msic_gpio_probe,
> +};
> +
> +static int __init platform_msic_gpio_init(void)
> +{
> + return platform_driver_register(&platform_msic_gpio_driver);
> +}
> +
> +subsys_initcall(platform_msic_gpio_init);
> +
> +MODULE_AUTHOR("Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Intel Medfield MSIC GPIO driver");
> +MODULE_LICENSE("GPL v2");
>

--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Rhyland Klein: "Re: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree"
Previous message: Christoph Lameter: "Re: [PATCH v2 04/29] slub: always get the cache from its page inkfree"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]