[v3,12/12] gpio: Add Tegra186 support

Submitted by Thierry Reding on April 3, 2017, 4:05 p.m.

Details

Message ID 20170403160532.20282-13-thierry.reding@gmail.com
State New
Headers show

Commit Message

Thierry Reding April 3, 2017, 4:05 p.m.
From: Thierry Reding <treding@nvidia.com>

Tegra186 has two GPIO controllers that are largely register compatible
between one another but are completely different from the controller
found on earlier generations.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- make use of GPIOLIB_IRQCHIP for IRQ chip handling

Changes in v2:
- add pin names to allow easy lookup using the chardev interface
- distinguish AON and main GPIO controllers by label
- use gpiochip_get_data() instead of container_of()
- use C99 initializers

 drivers/gpio/Kconfig         |   9 +
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-tegra186.c | 625 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 635 insertions(+)
 create mode 100644 drivers/gpio/gpio-tegra186.c

Comments

Linus Walleij April 13, 2017, 12:38 p.m.
On Mon, Apr 3, 2017 at 6:05 PM, Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> Tegra186 has two GPIO controllers that are largely register compatible
> between one another but are completely different from the controller
> found on earlier generations.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v3:
> - make use of GPIOLIB_IRQCHIP for IRQ chip handling

So this is the interesting patch where you make use of the new
tightly integrated IRQchip.

> +struct tegra_gpio {
> +       struct gpio_chip gpio;
> +       struct irq_chip intc;
> +       unsigned int num_irq;
> +       unsigned int *irq;

So I find it a bit strange that the array of irq parents is still kept around
when you add so much infrastructure. Could this not just go into the core
so that is natively able to handle multiple parents cleanly?

> +static int tegra186_gpio_of_xlate(struct gpio_chip *chip,
> +                                 const struct of_phandle_args *spec,
> +                                 u32 *flags)
> +{
> +       struct tegra_gpio *gpio = gpiochip_get_data(chip);
> +       unsigned int port, pin, i, offset = 0;
> +
> +       if (WARN_ON(chip->of_gpio_n_cells < 2))
> +               return -EINVAL;
> +
> +       if (WARN_ON(spec->args_count < chip->of_gpio_n_cells))
> +               return -EINVAL;
> +
> +       port = spec->args[0] / 8;
> +       pin = spec->args[0] % 8;

Actually modular ports/banks is something we will also see again and
have seen before. Maybe this should be a new core xlate function
just taking some MOD argument.

> +static void tegra186_gpio_irq(struct irq_desc *desc)
> +{
> +       struct tegra_gpio *gpio = irq_desc_get_handler_data(desc);
> +       struct irq_domain *domain = gpio->gpio.irq.domain;
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +       unsigned int parent = irq_desc_get_irq(desc);
> +       unsigned int i, offset = 0;
> +
> +       chained_irq_enter(chip, desc);
> +
> +       for (i = 0; i < gpio->soc->num_ports; i++) {
> +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> +               void __iomem *base = gpio->base + port->offset;
> +               unsigned int pin, irq;
> +               unsigned long value;
> +
> +               /* skip ports that are not associated with this controller */
> +               if (parent != gpio->irq[port->irq])
> +                       goto skip;

This parent-to-irq lookup I think the core should handle.

> +static int tegra186_gpio_irq_domain_xlate(struct irq_domain *domain,
> +                                         struct device_node *np,
> +                                         const u32 *spec, unsigned int size,
> +                                         unsigned long *hwirq,
> +                                         unsigned int *type)
> +{
> +       struct tegra_gpio *gpio = gpiochip_get_data(domain->host_data);
> +       unsigned int port, pin, i, offset = 0;
> +
> +       if (size < 2)
> +               return -EINVAL;
> +
> +       port = spec[0] / 8;
> +       pin = spec[0] % 8;

Here is this MOD business again. This is clearly a pattern.

> +       err = of_irq_count(pdev->dev.of_node);
> +       if (err < 0)
> +               return err;
> +
> +       gpio->num_irq = err;

This variable (err) has a confusing name. This is clearly not an error
at this point. Can you rename it to "ret" or something?

> +       gpio->irq = devm_kcalloc(&pdev->dev, gpio->num_irq, sizeof(*gpio->irq),
> +                                GFP_KERNEL);
> +       if (!gpio->irq)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < gpio->num_irq; i++) {
> +               err = platform_get_irq(pdev, i);
> +               if (err < 0)
> +                       return err;
> +
> +               gpio->irq[i] = err;
> +       }

So this array of parent IRQs I think should be stored in the core
gpio irqchip, not here locally in the driver.

> +       for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
> +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> +               char *name;
> +
> +               for (j = 0; j < port->pins; j++) {
> +                       name = devm_kasprintf(gpio->gpio.parent, GFP_KERNEL,
> +                                             "P%s.%02x", port->name, j);
> +                       if (!name)
> +                               return -ENOMEM;
> +
> +                       names[offset + j] = name;
> +               }
> +
> +               offset += port->pins;
> +       }

Interesting way of using the names array. This will override any names
from the device tree I think, don't you want to use gpio-line-names = ""..
in the device tree instead?

> +       irq = &gpio->gpio.irq;
> +       irq->chip = &gpio->intc;
> +       irq->first = 0;
> +       irq->domain_ops = &tegra186_gpio_irq_domain_ops;
> +       irq->handler = handle_simple_irq;
> +       irq->lock_key = &tegra186_gpio_lock_class;
> +       irq->default_type = IRQ_TYPE_NONE;
> +       irq->parent_handler = tegra186_gpio_irq;
> +       irq->parent_handler_data = gpio;
> +       irq->num_parents = gpio->num_irq;
> +       irq->parents = gpio->irq;
> +
> +       irq->map = devm_kcalloc(&pdev->dev, gpio->gpio.ngpio,
> +                               sizeof(*irq->map), GFP_KERNEL);
> +       if (!irq->map)
> +               return -ENOMEM;
> +
> +       for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
> +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> +
> +               for (j = 0; j < port->pins; j++)
> +                       irq->map[offset + j] = irq->parents[port->irq];
> +
> +               offset += port->pins;
> +       }

So this is the core API use. But setting up the map in the driver, that
seems too much to implement over and over again for all drivers
don't you think? Also to have local irqdomain operations. All that
should be in the core and be reusable IMO.

I would imagine that the core keeps track of the parent IRQs and
will make sure to free up all maps of all IRQs when the GPIOchip
is removed.

Maybe I'm missing something about this, I was simply expecting
a full multi-parent handling and tracking in the core, moving the
drivers that now loose track of their parents over to that as well.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding April 13, 2017, 3:19 p.m.
On Thu, Apr 13, 2017 at 02:38:44PM +0200, Linus Walleij wrote:
> On Mon, Apr 3, 2017 at 6:05 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Tegra186 has two GPIO controllers that are largely register compatible
> > between one another but are completely different from the controller
> > found on earlier generations.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v3:
> > - make use of GPIOLIB_IRQCHIP for IRQ chip handling
> 
> So this is the interesting patch where you make use of the new
> tightly integrated IRQchip.
> 
> > +struct tegra_gpio {
> > +       struct gpio_chip gpio;
> > +       struct irq_chip intc;
> > +       unsigned int num_irq;
> > +       unsigned int *irq;
> 
> So I find it a bit strange that the array of irq parents is still kept around
> when you add so much infrastructure. Could this not just go into the core
> so that is natively able to handle multiple parents cleanly?

I did this on purpose because I'd like to avoid making this new
infrastructure into too much of a midlayer. The mechanism to parse
interrupts may depend on the driver. Perhaps a good middle ground
would be to provide generic helpers that will parse this, while
keeping the possibility to let the driver specify the list?

> > +static int tegra186_gpio_of_xlate(struct gpio_chip *chip,
> > +                                 const struct of_phandle_args *spec,
> > +                                 u32 *flags)
> > +{
> > +       struct tegra_gpio *gpio = gpiochip_get_data(chip);
> > +       unsigned int port, pin, i, offset = 0;
> > +
> > +       if (WARN_ON(chip->of_gpio_n_cells < 2))
> > +               return -EINVAL;
> > +
> > +       if (WARN_ON(spec->args_count < chip->of_gpio_n_cells))
> > +               return -EINVAL;
> > +
> > +       port = spec->args[0] / 8;
> > +       pin = spec->args[0] % 8;
> 
> Actually modular ports/banks is something we will also see again and
> have seen before. Maybe this should be a new core xlate function
> just taking some MOD argument.

This xlate function uses driver-specific data to compute the hwirq
number of the specified GPIO. That's not something I think we can easily
turn into a generic helper, not without adding a lot more infrastructure
than we already have. One of the problems that I see with adding too
much infrastructure is that the core will become completely unwieldy
because it has to take care of too many special cases.

> > +static void tegra186_gpio_irq(struct irq_desc *desc)
> > +{
> > +       struct tegra_gpio *gpio = irq_desc_get_handler_data(desc);
> > +       struct irq_domain *domain = gpio->gpio.irq.domain;
> > +       struct irq_chip *chip = irq_desc_get_chip(desc);
> > +       unsigned int parent = irq_desc_get_irq(desc);
> > +       unsigned int i, offset = 0;
> > +
> > +       chained_irq_enter(chip, desc);
> > +
> > +       for (i = 0; i < gpio->soc->num_ports; i++) {
> > +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> > +               void __iomem *base = gpio->base + port->offset;
> > +               unsigned int pin, irq;
> > +               unsigned long value;
> > +
> > +               /* skip ports that are not associated with this controller */
> > +               if (parent != gpio->irq[port->irq])
> > +                       goto skip;
> 
> This parent-to-irq lookup I think the core should handle.

Again, this is based heavily on data that is highly Tegra-specific. If
we wanted to support something like this in the core, we'd have to add
some generic version of struct tegra_gpio_port, which has the
disadvantage of making it more difficult to support for drivers that
have only a single parent interrupt.

> > +static int tegra186_gpio_irq_domain_xlate(struct irq_domain *domain,
> > +                                         struct device_node *np,
> > +                                         const u32 *spec, unsigned int size,
> > +                                         unsigned long *hwirq,
> > +                                         unsigned int *type)
> > +{
> > +       struct tegra_gpio *gpio = gpiochip_get_data(domain->host_data);
> > +       unsigned int port, pin, i, offset = 0;
> > +
> > +       if (size < 2)
> > +               return -EINVAL;
> > +
> > +       port = spec[0] / 8;
> > +       pin = spec[0] % 8;
> 
> Here is this MOD business again. This is clearly a pattern.

The same as above applies. Yes, we could support this in the core but at
the expense of making it difficult to write simple drivers.

> > +       err = of_irq_count(pdev->dev.of_node);
> > +       if (err < 0)
> > +               return err;
> > +
> > +       gpio->num_irq = err;
> 
> This variable (err) has a confusing name. This is clearly not an error
> at this point. Can you rename it to "ret" or something?

Yeah, why not.

> > +       gpio->irq = devm_kcalloc(&pdev->dev, gpio->num_irq, sizeof(*gpio->irq),
> > +                                GFP_KERNEL);
> > +       if (!gpio->irq)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0; i < gpio->num_irq; i++) {
> > +               err = platform_get_irq(pdev, i);
> > +               if (err < 0)
> > +                       return err;
> > +
> > +               gpio->irq[i] = err;
> > +       }
> 
> So this array of parent IRQs I think should be stored in the core
> gpio irqchip, not here locally in the driver.

Well, it's already stored in the struct gpio_irq_chip, but it is the
driver that allocates it, because the mechanism to construct it is
driver-specific, hence I prefer it to be in the driver to make it
explicit that it is the driver that owns it. That way the array of
interrupts stored in struct gpio_irq_chip can be defined as merely a
reference, so it is very clear that the core shouldn't touch it. In
fact I had thought about making it even more explicit by making the
struct gpio_irq_chip carry a const unsigned int * for these.

If we only store it in the core, it becomes ambiguous who owns it.
Should the core be responsible for freeing it? How does the core decide
that it is responsible for managing it?

> > +       for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
> > +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> > +               char *name;
> > +
> > +               for (j = 0; j < port->pins; j++) {
> > +                       name = devm_kasprintf(gpio->gpio.parent, GFP_KERNEL,
> > +                                             "P%s.%02x", port->name, j);
> > +                       if (!name)
> > +                               return -ENOMEM;
> > +
> > +                       names[offset + j] = name;
> > +               }
> > +
> > +               offset += port->pins;
> > +       }
> 
> Interesting way of using the names array. This will override any names
> from the device tree I think, don't you want to use gpio-line-names = ""..
> in the device tree instead?

I don't think this overrides any names provided in DT. The names are
picked up from this arry in gpiochip_set_desc_names(), which is called
before of_gpiochip_add() where the names are read from DT. So this will
effectively make the driver set default names for the pins, but allow
these names to be overridden by DT.

Putting these names into DT feels wrong because they can easily be
generated from the port and pin numbers.

> > +       irq = &gpio->gpio.irq;
> > +       irq->chip = &gpio->intc;
> > +       irq->first = 0;
> > +       irq->domain_ops = &tegra186_gpio_irq_domain_ops;
> > +       irq->handler = handle_simple_irq;
> > +       irq->lock_key = &tegra186_gpio_lock_class;
> > +       irq->default_type = IRQ_TYPE_NONE;
> > +       irq->parent_handler = tegra186_gpio_irq;
> > +       irq->parent_handler_data = gpio;
> > +       irq->num_parents = gpio->num_irq;
> > +       irq->parents = gpio->irq;
> > +
> > +       irq->map = devm_kcalloc(&pdev->dev, gpio->gpio.ngpio,
> > +                               sizeof(*irq->map), GFP_KERNEL);
> > +       if (!irq->map)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
> > +               const struct tegra_gpio_port *port = &gpio->soc->ports[i];
> > +
> > +               for (j = 0; j < port->pins; j++)
> > +                       irq->map[offset + j] = irq->parents[port->irq];
> > +
> > +               offset += port->pins;
> > +       }
> 
> So this is the core API use. But setting up the map in the driver, that
> seems too much to implement over and over again for all drivers
> don't you think? Also to have local irqdomain operations. All that
> should be in the core and be reusable IMO.

Like I said above, the mapping is highly specific to Tegra. The only way
to move this into the core is to provide even more infrastructure. I'm
not sure that would be wise at this point. Maybe that can be added at a
later point when we have more samples.

Similarly I think having the driver specify IRQ domain operations is
useful because it increases the flexibility to do what drivers need,
rather than restricting the drivers to what the midlayer defines. If we
wrap all of this into the core, we're likely going to face situations
where we either have to rework or add quirks to the core because some
drivers need some extra flexibility.

The above tries to implement the infrastructure as a library of helpers
which has the advantage that for the common cases the core can provide
helpers, but drivers can also substitute their own if they need to.

> I would imagine that the core keeps track of the parent IRQs and
> will make sure to free up all maps of all IRQs when the GPIOchip
> is removed.
> 
> Maybe I'm missing something about this, I was simply expecting
> a full multi-parent handling and tracking in the core, moving the
> drivers that now loose track of their parents over to that as well.

There are two pieces to this: this series prepares the infrastructure to
deal with multiple parent interrupts better and uses that infrastructure
in the new Tegra driver. But it also keeps the driver-specific bits in
the driver. Certainly there's a possibility to refactor some of that
later on, but I think it's premature to do that right away. We'd need to
convert a slew of other drivers first to see if the same pattern appears
or if these are chip-specific after all.

Thierry

Patch hide | download patch | download mbox

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 63ceed246b6f..196c0431b54d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -430,6 +430,15 @@  config GPIO_TEGRA
 	help
 	  Say yes here to support GPIO pins on NVIDIA Tegra SoCs.
 
+config GPIO_TEGRA186
+	tristate "NVIDIA Tegra186 GPIO support"
+	default ARCH_TEGRA_186_SOC
+	depends on ARCH_TEGRA_186_SOC || COMPILE_TEST
+	depends on OF_GPIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support GPIO pins on NVIDIA Tegra186 SoCs.
+
 config GPIO_TS4800
 	tristate "TS-4800 DIO blocks and compatibles"
 	depends on OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 095598e856ca..c6f53208becd 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -111,6 +111,7 @@  obj-$(CONFIG_GPIO_SYSCON)	+= gpio-syscon.o
 obj-$(CONFIG_GPIO_TB10X)	+= gpio-tb10x.o
 obj-$(CONFIG_GPIO_TC3589X)	+= gpio-tc3589x.o
 obj-$(CONFIG_GPIO_TEGRA)	+= gpio-tegra.o
+obj-$(CONFIG_GPIO_TEGRA186)	+= gpio-tegra186.o
 obj-$(CONFIG_GPIO_TIMBERDALE)	+= gpio-timberdale.o
 obj-$(CONFIG_GPIO_PALMAS)	+= gpio-palmas.o
 obj-$(CONFIG_GPIO_TPIC2810)	+= gpio-tpic2810.o
diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
new file mode 100644
index 000000000000..e14beca596f6
--- /dev/null
+++ b/drivers/gpio/gpio-tegra186.c
@@ -0,0 +1,625 @@ 
+/*
+ * Copyright (c) 2016-2017 NVIDIA Corporation
+ *
+ * Author: Thierry Reding <treding@nvidia.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/gpio/tegra186-gpio.h>
+
+#define TEGRA186_GPIO_ENABLE_CONFIG 0x00
+#define  TEGRA186_GPIO_ENABLE_CONFIG_ENABLE BIT(0)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_OUT BIT(1)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_NONE (0x0 << 2)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_LEVEL (0x1 << 2)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_SINGLE_EDGE (0x2 << 2)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_DOUBLE_EDGE (0x3 << 2)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_MASK (0x3 << 2)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_LEVEL BIT(4)
+#define  TEGRA186_GPIO_ENABLE_CONFIG_INTERRUPT BIT(6)
+
+#define TEGRA186_GPIO_DEBOUNCE_CONTROL 0x04
+#define  TEGRA186_GPIO_DEBOUNCE_CONTROL_THRESHOLD(x) ((x) & 0xff)
+
+#define TEGRA186_GPIO_INPUT 0x08
+#define  TEGRA186_GPIO_INPUT_HIGH BIT(0)
+
+#define TEGRA186_GPIO_OUTPUT_CONTROL 0x0c
+#define  TEGRA186_GPIO_OUTPUT_CONTROL_FLOATED BIT(0)
+
+#define TEGRA186_GPIO_OUTPUT_VALUE 0x10
+#define  TEGRA186_GPIO_OUTPUT_VALUE_HIGH BIT(0)
+
+#define TEGRA186_GPIO_INTERRUPT_CLEAR 0x14
+
+#define TEGRA186_GPIO_INTERRUPT_STATUS(x) (0x100 + (x) * 4)
+
+struct tegra_gpio_port {
+	const char *name;
+	unsigned int offset;
+	unsigned int pins;
+	unsigned int irq;
+};
+
+struct tegra_gpio_soc {
+	const struct tegra_gpio_port *ports;
+	unsigned int num_ports;
+	const char *name;
+};
+
+struct tegra_gpio {
+	struct gpio_chip gpio;
+	struct irq_chip intc;
+	unsigned int num_irq;
+	unsigned int *irq;
+
+	const struct tegra_gpio_soc *soc;
+
+	void __iomem *base;
+};
+
+static const struct tegra_gpio_port *
+tegra186_gpio_get_port(struct tegra_gpio *gpio, unsigned int *pin)
+{
+	unsigned int start = 0, i;
+
+	for (i = 0; i < gpio->soc->num_ports; i++) {
+		const struct tegra_gpio_port *port = &gpio->soc->ports[i];
+
+		if (*pin >= start && *pin < start + port->pins) {
+			*pin -= start;
+			return port;
+		}
+
+		start += port->pins;
+	}
+
+	return NULL;
+}
+
+static void __iomem *tegra186_gpio_get_base(struct tegra_gpio *gpio,
+					    unsigned int pin)
+{
+	const struct tegra_gpio_port *port;
+
+	port = tegra186_gpio_get_port(gpio, &pin);
+	if (!port)
+		return NULL;
+
+	return gpio->base + port->offset + pin * 0x20;
+}
+
+static int tegra186_gpio_get_direction(struct gpio_chip *chip,
+				       unsigned int offset)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, offset);
+	if (WARN_ON(base == NULL))
+		return -ENODEV;
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	if (value & TEGRA186_GPIO_ENABLE_CONFIG_OUT)
+		return 0;
+
+	return 1;
+}
+
+static int tegra186_gpio_direction_input(struct gpio_chip *chip,
+					 unsigned int offset)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, offset);
+	if (WARN_ON(base == NULL))
+		return -ENODEV;
+
+	value = readl(base + TEGRA186_GPIO_OUTPUT_CONTROL);
+	value |= TEGRA186_GPIO_OUTPUT_CONTROL_FLOATED;
+	writel(value, base + TEGRA186_GPIO_OUTPUT_CONTROL);
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	value |= TEGRA186_GPIO_ENABLE_CONFIG_ENABLE;
+	value &= ~TEGRA186_GPIO_ENABLE_CONFIG_OUT;
+	writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
+
+	return 0;
+}
+
+static int tegra186_gpio_direction_output(struct gpio_chip *chip,
+					  unsigned int offset, int level)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	void __iomem *base;
+	u32 value;
+
+	/* configure output level first */
+	chip->set(chip, offset, level);
+
+	base = tegra186_gpio_get_base(gpio, offset);
+	if (WARN_ON(base == NULL))
+		return -EINVAL;
+
+	/* set the direction */
+	value = readl(base + TEGRA186_GPIO_OUTPUT_CONTROL);
+	value &= ~TEGRA186_GPIO_OUTPUT_CONTROL_FLOATED;
+	writel(value, base + TEGRA186_GPIO_OUTPUT_CONTROL);
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	value |= TEGRA186_GPIO_ENABLE_CONFIG_ENABLE;
+	value |= TEGRA186_GPIO_ENABLE_CONFIG_OUT;
+	writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
+
+	return 0;
+}
+
+static int tegra186_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, offset);
+	if (WARN_ON(base == NULL))
+		return -ENODEV;
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	if (value & TEGRA186_GPIO_ENABLE_CONFIG_OUT)
+		value = readl(base + TEGRA186_GPIO_OUTPUT_VALUE);
+	else
+		value = readl(base + TEGRA186_GPIO_INPUT);
+
+	return value & BIT(0);
+}
+
+static void tegra186_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			      int level)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, offset);
+	if (WARN_ON(base == NULL))
+		return;
+
+	value = readl(base + TEGRA186_GPIO_OUTPUT_VALUE);
+	if (level == 0)
+		value &= ~TEGRA186_GPIO_OUTPUT_VALUE_HIGH;
+	else
+		value |= TEGRA186_GPIO_OUTPUT_VALUE_HIGH;
+
+	writel(value, base + TEGRA186_GPIO_OUTPUT_VALUE);
+}
+
+static int tegra186_gpio_of_xlate(struct gpio_chip *chip,
+				  const struct of_phandle_args *spec,
+				  u32 *flags)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	unsigned int port, pin, i, offset = 0;
+
+	if (WARN_ON(chip->of_gpio_n_cells < 2))
+		return -EINVAL;
+
+	if (WARN_ON(spec->args_count < chip->of_gpio_n_cells))
+		return -EINVAL;
+
+	port = spec->args[0] / 8;
+	pin = spec->args[0] % 8;
+
+	if (port >= gpio->soc->num_ports) {
+		dev_err(chip->parent, "invalid port number: %u\n", port);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < port; i++)
+		offset += gpio->soc->ports[i].pins;
+
+	if (flags)
+		*flags = spec->args[1];
+
+	return offset + pin;
+}
+
+static void tegra186_irq_ack(struct irq_data *data)
+{
+	struct tegra_gpio *gpio = irq_data_get_irq_chip_data(data);
+	void __iomem *base;
+
+	base = tegra186_gpio_get_base(gpio, data->hwirq);
+	if (WARN_ON(base == NULL))
+		return;
+
+	writel(1, base + TEGRA186_GPIO_INTERRUPT_CLEAR);
+}
+
+static void tegra186_irq_mask(struct irq_data *data)
+{
+	struct tegra_gpio *gpio = irq_data_get_irq_chip_data(data);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, data->hwirq);
+	if (WARN_ON(base == NULL))
+		return;
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	value &= ~TEGRA186_GPIO_ENABLE_CONFIG_INTERRUPT;
+	writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
+}
+
+static void tegra186_irq_unmask(struct irq_data *data)
+{
+	struct tegra_gpio *gpio = irq_data_get_irq_chip_data(data);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, data->hwirq);
+	if (WARN_ON(base == NULL))
+		return;
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	value |= TEGRA186_GPIO_ENABLE_CONFIG_INTERRUPT;
+	writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
+}
+
+static int tegra186_irq_set_type(struct irq_data *data, unsigned int flow)
+{
+	struct tegra_gpio *gpio = irq_data_get_irq_chip_data(data);
+	void __iomem *base;
+	u32 value;
+
+	base = tegra186_gpio_get_base(gpio, data->hwirq);
+	if (WARN_ON(base == NULL))
+		return -ENODEV;
+
+	value = readl(base + TEGRA186_GPIO_ENABLE_CONFIG);
+	value &= ~TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_MASK;
+	value &= ~TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_LEVEL;
+
+	switch (flow & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_NONE:
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_SINGLE_EDGE;
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_LEVEL;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_SINGLE_EDGE;
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_DOUBLE_EDGE;
+		break;
+
+	case IRQ_TYPE_LEVEL_HIGH:
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_LEVEL;
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_LEVEL;
+		break;
+
+	case IRQ_TYPE_LEVEL_LOW:
+		value |= TEGRA186_GPIO_ENABLE_CONFIG_TRIGGER_TYPE_LEVEL;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	writel(value, base + TEGRA186_GPIO_ENABLE_CONFIG);
+
+	if ((flow & IRQ_TYPE_EDGE_BOTH) == 0)
+		irq_set_handler_locked(data, handle_level_irq);
+	else
+		irq_set_handler_locked(data, handle_edge_irq);
+
+	return 0;
+}
+
+static void tegra186_gpio_irq(struct irq_desc *desc)
+{
+	struct tegra_gpio *gpio = irq_desc_get_handler_data(desc);
+	struct irq_domain *domain = gpio->gpio.irq.domain;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned int parent = irq_desc_get_irq(desc);
+	unsigned int i, offset = 0;
+
+	chained_irq_enter(chip, desc);
+
+	for (i = 0; i < gpio->soc->num_ports; i++) {
+		const struct tegra_gpio_port *port = &gpio->soc->ports[i];
+		void __iomem *base = gpio->base + port->offset;
+		unsigned int pin, irq;
+		unsigned long value;
+
+		/* skip ports that are not associated with this controller */
+		if (parent != gpio->irq[port->irq])
+			goto skip;
+
+		value = readl(base + TEGRA186_GPIO_INTERRUPT_STATUS(1));
+
+		for_each_set_bit(pin, &value, port->pins) {
+			irq = irq_find_mapping(domain, offset + pin);
+			if (WARN_ON(irq == 0))
+				continue;
+
+			generic_handle_irq(irq);
+		}
+
+skip:
+		offset += port->pins;
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static int tegra186_gpio_irq_domain_xlate(struct irq_domain *domain,
+					  struct device_node *np,
+					  const u32 *spec, unsigned int size,
+					  unsigned long *hwirq,
+					  unsigned int *type)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(domain->host_data);
+	unsigned int port, pin, i, offset = 0;
+
+	if (size < 2)
+		return -EINVAL;
+
+	port = spec[0] / 8;
+	pin = spec[0] % 8;
+
+	if (port >= gpio->soc->num_ports) {
+		dev_err(gpio->gpio.parent, "invalid port number: %u\n", port);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < port; i++)
+		offset += gpio->soc->ports[i].pins;
+
+	*type = spec[1] & IRQ_TYPE_SENSE_MASK;
+	*hwirq = offset + pin;
+
+	return 0;
+}
+
+static const struct irq_domain_ops tegra186_gpio_irq_domain_ops = {
+	.map = gpiochip_irq_map,
+	.unmap = gpiochip_irq_unmap,
+	.xlate = tegra186_gpio_irq_domain_xlate,
+};
+
+static struct lock_class_key tegra186_gpio_lock_class;
+
+static int tegra186_gpio_probe(struct platform_device *pdev)
+{
+	unsigned int i, j, offset;
+	struct gpio_irq_chip *irq;
+	struct tegra_gpio *gpio;
+	struct resource *res;
+	char **names;
+	int err;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->soc = of_device_get_match_data(&pdev->dev);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gpio");
+	gpio->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(gpio->base))
+		return PTR_ERR(gpio->base);
+
+	err = of_irq_count(pdev->dev.of_node);
+	if (err < 0)
+		return err;
+
+	gpio->num_irq = err;
+
+	gpio->irq = devm_kcalloc(&pdev->dev, gpio->num_irq, sizeof(*gpio->irq),
+				 GFP_KERNEL);
+	if (!gpio->irq)
+		return -ENOMEM;
+
+	for (i = 0; i < gpio->num_irq; i++) {
+		err = platform_get_irq(pdev, i);
+		if (err < 0)
+			return err;
+
+		gpio->irq[i] = err;
+	}
+
+	gpio->gpio.label = gpio->soc->name;
+	gpio->gpio.parent = &pdev->dev;
+
+	gpio->gpio.get_direction = tegra186_gpio_get_direction;
+	gpio->gpio.direction_input = tegra186_gpio_direction_input;
+	gpio->gpio.direction_output = tegra186_gpio_direction_output;
+	gpio->gpio.get = tegra186_gpio_get,
+	gpio->gpio.set = tegra186_gpio_set;
+
+	gpio->gpio.base = -1;
+
+	for (i = 0; i < gpio->soc->num_ports; i++)
+		gpio->gpio.ngpio += gpio->soc->ports[i].pins;
+
+	names = devm_kcalloc(gpio->gpio.parent, gpio->gpio.ngpio,
+			     sizeof(*names), GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+
+	for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
+		const struct tegra_gpio_port *port = &gpio->soc->ports[i];
+		char *name;
+
+		for (j = 0; j < port->pins; j++) {
+			name = devm_kasprintf(gpio->gpio.parent, GFP_KERNEL,
+					      "P%s.%02x", port->name, j);
+			if (!name)
+				return -ENOMEM;
+
+			names[offset + j] = name;
+		}
+
+		offset += port->pins;
+	}
+
+	gpio->gpio.names = (const char * const *)names;
+
+	gpio->gpio.of_node = pdev->dev.of_node;
+	gpio->gpio.of_gpio_n_cells = 2;
+	gpio->gpio.of_xlate = tegra186_gpio_of_xlate;
+
+	gpio->intc.name = pdev->dev.of_node->name;
+	gpio->intc.irq_ack = tegra186_irq_ack;
+	gpio->intc.irq_mask = tegra186_irq_mask;
+	gpio->intc.irq_unmask = tegra186_irq_unmask;
+	gpio->intc.irq_set_type = tegra186_irq_set_type;
+
+	irq = &gpio->gpio.irq;
+	irq->chip = &gpio->intc;
+	irq->first = 0;
+	irq->domain_ops = &tegra186_gpio_irq_domain_ops;
+	irq->handler = handle_simple_irq;
+	irq->lock_key = &tegra186_gpio_lock_class;
+	irq->default_type = IRQ_TYPE_NONE;
+	irq->parent_handler = tegra186_gpio_irq;
+	irq->parent_handler_data = gpio;
+	irq->num_parents = gpio->num_irq;
+	irq->parents = gpio->irq;
+
+	irq->map = devm_kcalloc(&pdev->dev, gpio->gpio.ngpio,
+				sizeof(*irq->map), GFP_KERNEL);
+	if (!irq->map)
+		return -ENOMEM;
+
+	for (i = 0, offset = 0; i < gpio->soc->num_ports; i++) {
+		const struct tegra_gpio_port *port = &gpio->soc->ports[i];
+
+		for (j = 0; j < port->pins; j++)
+			irq->map[offset + j] = irq->parents[port->irq];
+
+		offset += port->pins;
+	}
+
+	platform_set_drvdata(pdev, gpio);
+
+	err = devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int tegra186_gpio_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+#define TEGRA_MAIN_GPIO_PORT(port, base, count, controller)	\
+	[TEGRA_MAIN_GPIO_PORT_##port] = {			\
+		.name = #port,					\
+		.offset = base,					\
+		.pins = count,					\
+		.irq = controller,				\
+	}
+
+static const struct tegra_gpio_port tegra186_main_ports[] = {
+	TEGRA_MAIN_GPIO_PORT( A, 0x2000, 7, 2),
+	TEGRA_MAIN_GPIO_PORT( B, 0x3000, 7, 3),
+	TEGRA_MAIN_GPIO_PORT( C, 0x3200, 7, 3),
+	TEGRA_MAIN_GPIO_PORT( D, 0x3400, 6, 3),
+	TEGRA_MAIN_GPIO_PORT( E, 0x2200, 8, 2),
+	TEGRA_MAIN_GPIO_PORT( F, 0x2400, 6, 2),
+	TEGRA_MAIN_GPIO_PORT( G, 0x4200, 6, 4),
+	TEGRA_MAIN_GPIO_PORT( H, 0x1000, 7, 1),
+	TEGRA_MAIN_GPIO_PORT( I, 0x0800, 8, 0),
+	TEGRA_MAIN_GPIO_PORT( J, 0x5000, 8, 5),
+	TEGRA_MAIN_GPIO_PORT( K, 0x5200, 1, 5),
+	TEGRA_MAIN_GPIO_PORT( L, 0x1200, 8, 1),
+	TEGRA_MAIN_GPIO_PORT( M, 0x5600, 6, 5),
+	TEGRA_MAIN_GPIO_PORT( N, 0x0000, 7, 0),
+	TEGRA_MAIN_GPIO_PORT( O, 0x0200, 4, 0),
+	TEGRA_MAIN_GPIO_PORT( P, 0x4000, 7, 4),
+	TEGRA_MAIN_GPIO_PORT( Q, 0x0400, 6, 0),
+	TEGRA_MAIN_GPIO_PORT( R, 0x0a00, 6, 0),
+	TEGRA_MAIN_GPIO_PORT( T, 0x0600, 4, 0),
+	TEGRA_MAIN_GPIO_PORT( X, 0x1400, 8, 1),
+	TEGRA_MAIN_GPIO_PORT( Y, 0x1600, 7, 1),
+	TEGRA_MAIN_GPIO_PORT(BB, 0x2600, 2, 2),
+	TEGRA_MAIN_GPIO_PORT(CC, 0x5400, 4, 5),
+};
+
+static const struct tegra_gpio_soc tegra186_main_soc = {
+	.num_ports = ARRAY_SIZE(tegra186_main_ports),
+	.ports = tegra186_main_ports,
+	.name = "tegra186-gpio",
+};
+
+#define TEGRA_AON_GPIO_PORT(port, base, count, controller)	\
+	[TEGRA_AON_GPIO_PORT_##port] = {			\
+		.name = #port,					\
+		.offset = base,					\
+		.pins = count,					\
+		.irq = controller,				\
+	}
+
+static const struct tegra_gpio_port tegra186_aon_ports[] = {
+	TEGRA_AON_GPIO_PORT( S, 0x0200, 5, 0),
+	TEGRA_AON_GPIO_PORT( U, 0x0400, 6, 0),
+	TEGRA_AON_GPIO_PORT( V, 0x0800, 8, 0),
+	TEGRA_AON_GPIO_PORT( W, 0x0a00, 8, 0),
+	TEGRA_AON_GPIO_PORT( Z, 0x0e00, 4, 0),
+	TEGRA_AON_GPIO_PORT(AA, 0x0c00, 8, 0),
+	TEGRA_AON_GPIO_PORT(EE, 0x0600, 3, 0),
+	TEGRA_AON_GPIO_PORT(FF, 0x0000, 5, 0),
+};
+
+static const struct tegra_gpio_soc tegra186_aon_soc = {
+	.num_ports = ARRAY_SIZE(tegra186_aon_ports),
+	.ports = tegra186_aon_ports,
+	.name = "tegra186-gpio-aon",
+};
+
+static const struct of_device_id tegra186_gpio_of_match[] = {
+	{
+		.compatible = "nvidia,tegra186-gpio",
+		.data = &tegra186_main_soc
+	}, {
+		.compatible = "nvidia,tegra186-gpio-aon",
+		.data = &tegra186_aon_soc
+	}, {
+		/* sentinel */
+	}
+};
+
+static struct platform_driver tegra186_gpio_driver = {
+	.driver = {
+		.name = "tegra186-gpio",
+		.of_match_table = tegra186_gpio_of_match,
+	},
+	.probe = tegra186_gpio_probe,
+	.remove = tegra186_gpio_remove,
+};
+module_platform_driver(tegra186_gpio_driver);
+
+MODULE_DESCRIPTION("NVIDIA Tegra186 GPIO controller driver");
+MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
+MODULE_LICENSE("GPL v2");