| Message ID | 20170403160532.20282-13-thierry.reding@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
Sorry for taking so long before I get back to answering these important architecture questions. I am just overburdened by work in the merge window time frames. :( On Thu, Apr 13, 2017 at 5:19 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, Apr 13, 2017 at 02:38:44PM +0200, Linus Walleij wrote: >> > +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? I don't know honestly, the way I code is usually the most elegant solution pops out by itself when I work on the code. We have a bit of stuff in the gpiolib (I guess that is what you mean by the midlayer) for example the ability to mask off individual IRQ lines as non-requestable. I do not see a problem for gpiolib to keep a list of parents which may contain only one item, and special functions like gpiochip_set_chained_irqchip_multiparent() and gpiochip_set_nested_irqchip_multiparent() with gpiochip_set_chained_irqchip() just calling the former with a static inline passing a list of just one parent. And thus we would keep track of all parents inside of gpiolib. >> > +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. I understand that concern. But the problem I have as a maintainer, and I speak from experience doing the job, is that maintaining the core is causing very little problems because it solves problems for a lot of drivers in one single spot. What is causing me problems is a lot of duplicated or "almost-the-same" code in a lot of drivers, making similar bugs pop out and clog my inbox with similar semantic (and valid) patches generated by cocinelle because now the bugs are all over the place instead of confined to the gpiolib core. I do not think parametrized xlate functions for 8, 16 and 32 bits of MOD on the arg, so that interrupt parent A, B, C is assigned depending on whether arg MOD [8, 16, 32] is 0, 1, 2 is very extraordinary. It more seems to be what can be expected from every chained driver with multiple IRQ parents, and thus reusable. As soon as it is used by more than one driver, it is a win for me. The compiler will not include it in the final zImage if there are no users, so it is not a footprint issue. >> > +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. This concept: - The chip has multiple parents - The multiple parents correspond to multiple "ports" or actually registers, that are n bits wide, and map 1:1 between a parent and a "port". Is nothing unique at all. I have seen it dozens of times and I see an increasing influx of driver doing exactly this. Normally my gut reaction is to have one driver instance per "port" so that each port is its own device, but several times driver writers convinced me that they need to have it inside a parent device (like a pin controller or so), and that instead complicates these functions. But I think they are pretty much all the same and something can be done to abstract this. I am thinking something like bitmaps that map an GPIO interrupt line back to its parent IRQ. We need to think it through and look at some examples. For example: bcm2835_gpio_irq_handler in bcm/pinctrl-bcm2835.c iproc_gpio_irq_handler in bcm/pinctrl-iproc-gpio.c adi_gpio_handle_pint_irq in pinctrl-adi2.c atlas7_gpio_handle_irq in sirf/pinctrl-atlas7.c bcm_kona_gpio_irq_handler in gpio-bcm-kona.c mx2_gpio_irq_handler in gpio-mxc.c tegra_gpio_irq_handler in gpio-tegra.c xlp_gpio_generic_handler in gpio-xlp.c zynq_gpio_irqhandler in gpio-zynq.c They all do some bank-to-parent "magic" in their interrupt handlers. I'm suspecting that even pcs_irq_handle() in pinctrl-single.c does something similar. I strongly suspect they could all use some simple bitmap, hashmap or similar to associate banks to parent IRQs. It's not like I'm asking you to upfront clean up this mess (because it is a mess) but if you invented something that generalized this for just say tegra186 and gpio-tegra that you work on, I'd be a happy cow. (And you get to decide what everyone else has to do to map children to parents... yeah I can attack patch those drivers after that.) >> > + 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. It's fine if it is only a reference to an interrupt obtained with e.g. platform_get_irq() no problem with that. I am not expecting the core to *parse* IRQs, just keep track of them, and how they map to "banks". > 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. That is a good idea. > 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? I don't understand what you mean by "freeing it"? As those are mostly chained handler the drivers never e.g. request the IRQs in the first place, that is done by the consumers that just let the handler ripple down the chain. >> > + 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. Ah yeah I see. I coded that actually :D OK fine then, that is actually a design pattern we should encourage, it's very helpful to have default names and let the device tree override them if it can be more specific. > Putting these names into DT feels wrong because they can easily be > generated from the port and pin numbers. It's fine, keep it. > 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. We have those samples. Outlined above. gpio-tegra is one of them. > 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. We already have out-of-core handling with e.g. hierarchical irq domains. That is fine. But I feel that this driver is looking familiar to things I've seen before, and my grep operation above seems to confirm this. It is mapping IRQ sources in "banks" to parent IRQs, and that is being done all over the place, so we should generalize it. > 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. I kind of like the new struct and all. I just feel it doesn't add enough value unless we also deal with this bank-irq-to-parent mapping inside the core as well as part of this. 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
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");