diff mbox series

[v4,2/4] gpio: visconti: Add Toshiba Visconti GPIO support

Message ID 20201211094138.2863677-3-nobuhiro1.iwamatsu@toshiba.co.jp (mailing list archive)
State New, archived
Headers show
Series gpio: visconti: Add Toshiba Visconti GPIO support | expand

Commit Message

Nobuhiro Iwamatsu Dec. 11, 2020, 9:41 a.m. UTC
Add the GPIO driver for Toshiba Visconti ARM SoCs.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 drivers/gpio/Kconfig                      |   9 +
 drivers/gpio/Makefile                     |   1 +
 drivers/gpio/gpio-visconti.c              | 230 ++++++++++++++++++++++
 drivers/pinctrl/visconti/pinctrl-common.c |  23 +++
 4 files changed, 263 insertions(+)
 create mode 100644 drivers/gpio/gpio-visconti.c

Comments

Linus Walleij Dec. 11, 2020, 11:20 p.m. UTC | #1
On Fri, Dec 11, 2020 at 1:43 AM Nobuhiro Iwamatsu
<nobuhiro1.iwamatsu@toshiba.co.jp> wrote:

This iteration is looking really good, but we are not quite there yet,
because now that the driver looks so much better I can see that it
is a hierarchical interrupt controller.

> Add the GPIO driver for Toshiba Visconti ARM SoCs.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
(...)

> +config GPIO_VISCONTI
> +       tristate "Toshiba Visconti GPIO support"
> +       depends on ARCH_VISCONTI || COMPILE_TEST
> +       depends on OF_GPIO
> +       select GPIOLIB_IRQCHIP
> +       select GPIO_GENERIC
> +       help
> +         Say yes here to support GPIO on Tohisba Visconti.

Add
select IRQ_DOMAIN_HIERARCHY

> +struct visconti_gpio {
> +       void __iomem *base;
> +       int *irq;

Don't keep these irqs around.

> +       ret = platform_irq_count(pdev);
> +       if (ret < 0)
> +               return ret;
> +
> +       num_irq = ret;
> +
> +       priv->irq = devm_kcalloc(dev, num_irq, sizeof(priv->irq), GFP_KERNEL);
> +       if (!priv->irq)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < num_irq; i++) {
> +               priv->irq[i] = platform_get_irq(pdev, i);
> +               if (priv->irq[i] < 0) {
> +                       dev_err(dev, "invalid IRQ[%d]\n", i);
> +                       return priv->irq[i];
> +               }
> +       }

Instead of doing this, look in for example
drivers/gpio/gpio-ixp4xx.c

You need:

> +       girq = &priv->gpio_chip.irq;
> +       girq->chip = irq_chip;

girq->fwnode = fwnode;
girq->parent_domain = parent;
girq->child_to_parent_hwirq = visconti_gpio_child_to_parent_hwirq;

The mapping function will be something like this:

static int visconti_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
                                             unsigned int child,
                                             unsigned int child_type,
                                             unsigned int *parent,
                                             unsigned int *parent_type)
{
        /* Interrupts 0..15 mapped to interrupts 24..39 on the GIC */
        if (child < 16) {
            /* All these interrupts are level high in the CPU */
            *parent_type = IRQ_TYPE_LEVEL_HIGH;
            *parent = child + 24;
            return 0;
        }
        return -EINVAL;
}

> +       priv->gpio_chip.irq.init_valid_mask = visconti_init_irq_valid_mask;

This will be set up by gpiolib when using hierarchical irqs.

> +       /* This will let us handle the parent IRQ in the driver */
> +       girq->parent_handler = NULL;
> +       girq->num_parents = 0;
> +       girq->parents = NULL;

You don't need this.

> +       girq->default_type = IRQ_TYPE_NONE;
> +       girq->handler = handle_level_irq;

But this stays.

> +       for (i = 0; i < num_irq; i++) {
> +               desc = irq_to_desc(priv->irq[i]);
> +               desc->status_use_accessors |= IRQ_NOAUTOEN;
> +               if (devm_request_irq(dev, priv->irq[i],
> +                                    visconti_gpio_irq_handler, 0, name, priv)) {
> +                       dev_err(dev, "failed to request IRQ[%d]\n", i);
> +                       return -ENOENT;
> +               }
> +       }

This should not be needed either when using hiearchical IRQs,
also the irqchip maintainers will beat us up for poking around in the
descs like this.

The rest looks solid!

Yours,
Linus Walleij
Nobuhiro Iwamatsu Dec. 16, 2020, 9:11 a.m. UTC | #2
Hi,

Thanks for your review.

On Sat, Dec 12, 2020 at 12:20:47AM +0100, Linus Walleij wrote:
> On Fri, Dec 11, 2020 at 1:43 AM Nobuhiro Iwamatsu
> <nobuhiro1.iwamatsu@toshiba.co.jp> wrote:
> 
> This iteration is looking really good, but we are not quite there yet,
> because now that the driver looks so much better I can see that it
> is a hierarchical interrupt controller.

As you pointed out, this GPIO interrupt is directly linked to the GIC
interrupt.
As a function of the GPIO driver, there is a function (IRQ_DOMAIN_HIERARCHY)
that can handle these as one-to-one, so it is pointed out that it is better
to use this. Is this correct?

> 
> > Add the GPIO driver for Toshiba Visconti ARM SoCs.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Reviewed-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> (...)
> 
> > +config GPIO_VISCONTI
> > +       tristate "Toshiba Visconti GPIO support"
> > +       depends on ARCH_VISCONTI || COMPILE_TEST
> > +       depends on OF_GPIO
> > +       select GPIOLIB_IRQCHIP
> > +       select GPIO_GENERIC
> > +       help
> > +         Say yes here to support GPIO on Tohisba Visconti.
> 
> Add
> select IRQ_DOMAIN_HIERARCHY

OK, I will add this.

> 
> > +struct visconti_gpio {
> > +       void __iomem *base;
> > +       int *irq;
> 
> Don't keep these irqs around.
>

OK, I will remove this
.
> > +       ret = platform_irq_count(pdev);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       num_irq = ret;
> > +
> > +       priv->irq = devm_kcalloc(dev, num_irq, sizeof(priv->irq), GFP_KERNEL);
> > +       if (!priv->irq)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0; i < num_irq; i++) {
> > +               priv->irq[i] = platform_get_irq(pdev, i);
> > +               if (priv->irq[i] < 0) {
> > +                       dev_err(dev, "invalid IRQ[%d]\n", i);
> > +                       return priv->irq[i];
> > +               }
> > +       }
> 
> Instead of doing this, look in for example
> drivers/gpio/gpio-ixp4xx.c
> 
> You need:
> 
> > +       girq = &priv->gpio_chip.irq;
> > +       girq->chip = irq_chip;
> 
> girq->fwnode = fwnode;
> girq->parent_domain = parent;
> girq->child_to_parent_hwirq = visconti_gpio_child_to_parent_hwirq;
> 


I understood that the irq_domain specified by girq->parent_domain will be the
GIC. Is this correct?


> The mapping function will be something like this:
> 
> static int visconti_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
>                                              unsigned int child,
>                                              unsigned int child_type,
>                                              unsigned int *parent,
>                                              unsigned int *parent_type)
> {
>         /* Interrupts 0..15 mapped to interrupts 24..39 on the GIC */
>         if (child < 16) {
>             /* All these interrupts are level high in the CPU */
>             *parent_type = IRQ_TYPE_LEVEL_HIGH;
>             *parent = child + 24;
>             return 0;
>         }
>         return -EINVAL;
> }
>

I see, I will add this function.

> > +       priv->gpio_chip.irq.init_valid_mask = visconti_init_irq_valid_mask;
> 
> This will be set up by gpiolib when using hierarchical irqs.
> 

OK.

> > +       /* This will let us handle the parent IRQ in the driver */
> > +       girq->parent_handler = NULL;
> > +       girq->num_parents = 0;
> > +       girq->parents = NULL;
> 
> You don't need this.
> 
> > +       girq->default_type = IRQ_TYPE_NONE;
> > +       girq->handler = handle_level_irq;
> 
> But this stays.
> 

OK, I will update to these.

> > +       for (i = 0; i < num_irq; i++) {
> > +               desc = irq_to_desc(priv->irq[i]);
> > +               desc->status_use_accessors |= IRQ_NOAUTOEN;
> > +               if (devm_request_irq(dev, priv->irq[i],
> > +                                    visconti_gpio_irq_handler, 0, name, priv)) {
> > +                       dev_err(dev, "failed to request IRQ[%d]\n", i);
> > +                       return -ENOENT;
> > +               }
> > +       }
> 
> This should not be needed either when using hiearchical IRQs,
> also the irqchip maintainers will beat us up for poking around in the
> descs like this.

I understand that the processing equivalent to request_irq() is processed
by the irqchip frame work (or GIC driver). Is this correct?


> 
> The rest looks solid!
> 

Thank you.
I will apply your point into the driver.

Best regards,
  Nobuhiro
Marc Zyngier Dec. 16, 2020, 9:36 a.m. UTC | #3
On 2020-12-16 09:11, Nobuhiro Iwamatsu wrote:

[...]

>> > +       for (i = 0; i < num_irq; i++) {
>> > +               desc = irq_to_desc(priv->irq[i]);
>> > +               desc->status_use_accessors |= IRQ_NOAUTOEN;
>> > +               if (devm_request_irq(dev, priv->irq[i],
>> > +                                    visconti_gpio_irq_handler, 0, name, priv)) {
>> > +                       dev_err(dev, "failed to request IRQ[%d]\n", i);
>> > +                       return -ENOENT;
>> > +               }
>> > +       }
>> 
>> This should not be needed either when using hiearchical IRQs,
>> also the irqchip maintainers will beat us up for poking around in the
>> descs like this.
> 
> I understand that the processing equivalent to request_irq() is 
> processed
> by the irqchip frame work (or GIC driver). Is this correct?

request_irq() is reserved to endpoint drivers (the driver for the device
driving the IRQ line). If this is indeed a hierarchical irqchip, the
line allocation will be driven from the GPIO framework, and 
request_irq()
will perform the activation. There isn't anything that this driver 
should
do directly other than configuring its own state and passing the request
along to the parent controller.

And yes, mucking with the irq descriptor will get you in massive 
trouble,
never do that.

Thanks,

         M.
Nobuhiro Iwamatsu Dec. 17, 2020, 5:10 a.m. UTC | #4
Hi,

On Wed, Dec 16, 2020 at 09:36:17AM +0000, Marc Zyngier wrote:
> On 2020-12-16 09:11, Nobuhiro Iwamatsu wrote:
> 
> [...]
> 
> > > > +       for (i = 0; i < num_irq; i++) {
> > > > +               desc = irq_to_desc(priv->irq[i]);
> > > > +               desc->status_use_accessors |= IRQ_NOAUTOEN;
> > > > +               if (devm_request_irq(dev, priv->irq[i],
> > > > +                                    visconti_gpio_irq_handler, 0, name, priv)) {
> > > > +                       dev_err(dev, "failed to request IRQ[%d]\n", i);
> > > > +                       return -ENOENT;
> > > > +               }
> > > > +       }
> > > 
> > > This should not be needed either when using hiearchical IRQs,
> > > also the irqchip maintainers will beat us up for poking around in the
> > > descs like this.
> > 
> > I understand that the processing equivalent to request_irq() is
> > processed
> > by the irqchip frame work (or GIC driver). Is this correct?
> 
> request_irq() is reserved to endpoint drivers (the driver for the device
> driving the IRQ line). If this is indeed a hierarchical irqchip, the
> line allocation will be driven from the GPIO framework, and request_irq()
> will perform the activation. There isn't anything that this driver should
> do directly other than configuring its own state and passing the request
> along to the parent controller.
> 
> And yes, mucking with the irq descriptor will get you in massive trouble,
> never do that.
> 

I see. Thank you for the explanation.
I got a better understanding.

Best regards,
  Nobuhiro

> -- 
> Jazz is not dead. It just smells funny...
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5d4de5cd6759..85d0f4499d63 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -631,6 +631,15 @@  config GPIO_VF610
 	help
 	  Say yes here to support Vybrid vf610 GPIOs.
 
+config GPIO_VISCONTI
+	tristate "Toshiba Visconti GPIO support"
+	depends on ARCH_VISCONTI || COMPILE_TEST
+	depends on OF_GPIO
+	select GPIOLIB_IRQCHIP
+	select GPIO_GENERIC
+	help
+	  Say yes here to support GPIO on Tohisba Visconti.
+
 config GPIO_VR41XX
 	tristate "NEC VR4100 series General-purpose I/O Uint support"
 	depends on CPU_VR41XX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 09dada80ac34..02c9d8d83a54 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -162,6 +162,7 @@  obj-$(CONFIG_GPIO_UCB1400)		+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_UNIPHIER)		+= gpio-uniphier.o
 obj-$(CONFIG_GPIO_VF610)		+= gpio-vf610.o
 obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
+obj-$(CONFIG_GPIO_VISCONTI)		+= gpio-visconti.o
 obj-$(CONFIG_GPIO_VR41XX)		+= gpio-vr41xx.o
 obj-$(CONFIG_GPIO_VX855)		+= gpio-vx855.o
 obj-$(CONFIG_GPIO_WCD934X)		+= gpio-wcd934x.o
diff --git a/drivers/gpio/gpio-visconti.c b/drivers/gpio/gpio-visconti.c
new file mode 100644
index 000000000000..8a3081bf67cf
--- /dev/null
+++ b/drivers/gpio/gpio-visconti.c
@@ -0,0 +1,230 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Toshiba Visconti GPIO Support
+ *
+ * (C) Copyright 2020 Toshiba Electronic Devices & Storage Corporation
+ * (C) Copyright 2020 TOSHIBA CORPORATION
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
+#include <linux/of.h>
+#include <linux/bitops.h>
+
+/* register offset */
+#define GPIO_DIR	0x00
+#define GPIO_IDATA	0x08
+#define GPIO_ODATA	0x10
+#define GPIO_OSET	0x18
+#define GPIO_OCLR	0x20
+#define GPIO_INTMODE	0x30
+
+struct visconti_gpio {
+	void __iomem *base;
+	int *irq;
+	spinlock_t lock; /* protect gpio register */
+	struct device *dev;
+	struct gpio_chip gpio_chip;
+	struct irq_chip irq_chip;
+};
+
+static void visconti_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct visconti_gpio *priv = gpiochip_get_data(gc);
+
+	disable_irq_nosync(priv->irq[irqd_to_hwirq(d)]);
+}
+
+static void visconti_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct visconti_gpio *priv = gpiochip_get_data(gc);
+
+	enable_irq(priv->irq[irqd_to_hwirq(d)]);
+}
+
+static int visconti_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct visconti_gpio *priv = gpiochip_get_data(gc);
+	u32 offset = irqd_to_hwirq(d);
+	u32 bit = BIT(offset);
+	u32 intc_type = IRQ_TYPE_EDGE_RISING;
+	u32 intmode, odata;
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	odata = readl(priv->base + GPIO_ODATA);
+	intmode = readl(priv->base + GPIO_INTMODE);
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		odata &= ~bit;
+		intmode &= ~bit;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		odata |= bit;
+		intmode &= ~bit;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		intmode |= bit;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		intc_type = IRQ_TYPE_LEVEL_HIGH;
+		odata &= ~bit;
+		intmode &= ~bit;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		intc_type = IRQ_TYPE_LEVEL_HIGH;
+		odata |= bit;
+		intmode &= ~bit;
+		break;
+	default:
+		ret = -EINVAL;
+		goto err;
+	}
+
+	writel(odata, priv->base + GPIO_ODATA);
+	writel(intmode, priv->base + GPIO_INTMODE);
+	irq_set_irq_type(priv->irq[offset], intc_type);
+err:
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return ret;
+}
+
+static irqreturn_t visconti_gpio_irq_handler(int irq, void *dev_id)
+{
+	struct visconti_gpio *priv = dev_id;
+	u32 offset = irq - priv->irq[0];
+
+	generic_handle_irq(irq_find_mapping(priv->gpio_chip.irq.domain, offset));
+
+	return IRQ_HANDLED;
+}
+
+static void visconti_init_irq_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask,
+					 unsigned int ngpios)
+{
+	int i;
+
+	/* Exclude GPIO pins 16-31 from irq */
+	for (i = 16; i < ngpios; i++)
+		clear_bit(i, valid_mask);
+}
+
+static int visconti_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct visconti_gpio *priv;
+	struct irq_chip *irq_chip;
+	struct irq_desc *desc;
+	struct gpio_irq_chip *girq;
+	const char *name = dev_name(dev);
+	int i, ret, num_irq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	spin_lock_init(&priv->lock);
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	ret = platform_irq_count(pdev);
+	if (ret < 0)
+		return ret;
+
+	num_irq = ret;
+
+	priv->irq = devm_kcalloc(dev, num_irq, sizeof(priv->irq), GFP_KERNEL);
+	if (!priv->irq)
+		return -ENOMEM;
+
+	for (i = 0; i < num_irq; i++) {
+		priv->irq[i] = platform_get_irq(pdev, i);
+		if (priv->irq[i] < 0) {
+			dev_err(dev, "invalid IRQ[%d]\n", i);
+			return priv->irq[i];
+		}
+	}
+
+	ret = bgpio_init(&priv->gpio_chip, dev, 4,
+			 priv->base + GPIO_IDATA,
+			 priv->base + GPIO_OSET,
+			 priv->base + GPIO_OCLR,
+			 priv->base + GPIO_DIR,
+			 NULL,
+			 0);
+	if (ret) {
+		dev_err(dev, "unable to init generic GPIO\n");
+		return ret;
+	}
+
+	priv->gpio_chip.irq.init_valid_mask = visconti_init_irq_valid_mask;
+
+	irq_chip = &priv->irq_chip;
+	irq_chip->name = "gpio-visconti";
+	irq_chip->irq_mask = visconti_gpio_irq_mask;
+	irq_chip->irq_unmask = visconti_gpio_irq_unmask;
+	irq_chip->irq_set_type = visconti_gpio_irq_set_type;
+	irq_chip->flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_MASK_ON_SUSPEND;
+
+	girq = &priv->gpio_chip.irq;
+	girq->chip = irq_chip;
+	/* This will let us handle the parent IRQ in the driver */
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+
+	ret = devm_gpiochip_add_data(dev, &priv->gpio_chip, priv);
+	if (ret) {
+		dev_err(dev, "failed to add GPIO chip\n");
+		return ret;
+	}
+
+	for (i = 0; i < num_irq; i++) {
+		desc = irq_to_desc(priv->irq[i]);
+		desc->status_use_accessors |= IRQ_NOAUTOEN;
+		if (devm_request_irq(dev, priv->irq[i],
+				     visconti_gpio_irq_handler, 0, name, priv)) {
+			dev_err(dev, "failed to request IRQ[%d]\n", i);
+			return -ENOENT;
+		}
+	}
+
+	return ret;
+}
+
+static const struct of_device_id visconti_gpio_of_match[] = {
+	{ .compatible = "toshiba,gpio-tmpv7708", },
+	{ /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, visconti_gpio_of_match);
+
+static struct platform_driver visconti_gpio_driver = {
+	.probe		= visconti_gpio_probe,
+	.driver		= {
+		.name	= "visconti_gpio",
+		.of_match_table = of_match_ptr(visconti_gpio_of_match),
+	}
+};
+module_platform_driver(visconti_gpio_driver);
+
+MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>");
+MODULE_DESCRIPTION("Toshiba Visconti GPIO Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pinctrl/visconti/pinctrl-common.c b/drivers/pinctrl/visconti/pinctrl-common.c
index 0cb10b7b4430..21c7e0d18fea 100644
--- a/drivers/pinctrl/visconti/pinctrl-common.c
+++ b/drivers/pinctrl/visconti/pinctrl-common.c
@@ -245,11 +245,34 @@  static int visconti_set_mux(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static int visconti_gpio_request_enable(struct pinctrl_dev *pctldev,
+				      struct pinctrl_gpio_range *range,
+				      unsigned int pin)
+{
+	struct visconti_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+	const struct visconti_mux *gpio_mux = &priv->devdata->gpio_mux[pin];
+	unsigned long flags;
+	unsigned int val;
+
+	dev_dbg(priv->dev, "%s: pin = %d\n", __func__, pin);
+
+	/* update mux */
+	spin_lock_irqsave(&priv->lock, flags);
+	val = readl(priv->base + gpio_mux->offset);
+	val &= ~gpio_mux->mask;
+	val |= gpio_mux->val;
+	writel(val, priv->base + gpio_mux->offset);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
 static const struct pinmux_ops visconti_pinmux_ops = {
 	.get_functions_count	= visconti_get_functions_count,
 	.get_function_name	= visconti_get_function_name,
 	.get_function_groups	= visconti_get_function_groups,
 	.set_mux		= visconti_set_mux,
+	.gpio_request_enable	= visconti_gpio_request_enable,
 	.strict			= true,
 };