* [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller @ 2016-10-19 15:21 Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 1/9] irqchip: meson: add support for " Jerome Brunet ` (8 more replies) 0 siblings, 9 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree This patch series adds support for the GPIO interrupt controller found on Amlogic's meson SoC families. Unlike what the name suggests, this controller is not part of the SoC GPIO subsystem. It's an indepedent controller which can watch almost all pad of the SoC and generate and interrupt from it. Some pins, which are not part of the public datasheet, don't seem to have this capability though. Hardware wise, the controller is a 256 to 8 multiplexer. It can take up to 256 input pads and route them to any of 8 GIC's interrupts. There is also a filter block in the middle to select the appropriate edge or level. The number of interrupt declared by the irqchip is lowered from 256 to the actual number of signal routed to the controller on each SoC family. As we have access to only 8 GIC’s interrupts, these are allocated when an interrupt is requested from the controller, on a first come, first served basis. This series has been tested on Amlogic S905-P200 board with the front panel power button. Directly passing an IRQ or using gpio_to_irq both work with this driver. This work is derived from the previous work of Carlo Caione [1]. Changes since RFC : [2] * Remove interrupt property in device tree: the controller cannot generate interrupts on its own and is merely routing the interrupt to the GIC, therefore it should not use the interrupt property. This data is now stored directly in the driver, same as the pinctrl data. * Improve compatibility checking of meson pinctrl on its interrupt parent to activate gpio_to_irq callback * Drop IRQ_BOTH hack. Need more work to have an acceptable solution for this Changes since v1 : [3] * Correct mistake in patch 4 when no compatible controller is found. Sorry for the inconvenience. [1] : http://lkml.kernel.org/r/1448987062-31225-1-git-send-email-carlo@caione.org [2] : http://lkml.kernel.org/r/1475593708-10526-1-git-send-email-jbrunet@baylibre.com [3] : http://lkml.kernel.org/r/1476871709-8359-1-git-send-email-jbrunet@baylibre.com Jerome Brunet (9): irqchip: meson: add support for gpio interrupt controller dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller pinctrl: meson: update pinctrl data with gpio irq data pinctrl: meson: allow gpio to request irq dt-bindings: pinctrl: meson: update gpio dt-bindings ARM64: meson: enable MESON_IRQ_GPIO in Kconfig ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 ARM64: dts: amlogic: enable gpio interrupt controller on gxbb ARM: dts: amlogic: enable gpio interrupt controller on meson8 .../amlogic,meson-gpio-intc.txt | 31 ++ .../devicetree/bindings/pinctrl/meson,pinctrl.txt | 4 + arch/arm/boot/dts/meson8.dtsi | 11 + arch/arm/boot/dts/meson8b.dtsi | 11 + arch/arm/mach-meson/Kconfig | 2 + arch/arm64/Kconfig.platforms | 1 + arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 9 + drivers/irqchip/Kconfig | 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-meson-gpio.c | 423 +++++++++++++++++++++ drivers/pinctrl/Kconfig | 2 + drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 24 +- drivers/pinctrl/meson/pinctrl-meson.c | 77 +++- drivers/pinctrl/meson/pinctrl-meson.h | 17 +- drivers/pinctrl/meson/pinctrl-meson8.c | 22 +- drivers/pinctrl/meson/pinctrl-meson8b.c | 34 +- 16 files changed, 641 insertions(+), 37 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt create mode 100644 drivers/irqchip/irq-meson-gpio.c -- 2.7.4 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-20 16:33 ` Marc Zyngier 2016-10-19 15:21 ` [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO " Jerome Brunet ` (7 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King Add support for the interrupt gpio controller found on Amlogic's meson SoC family. Unlike what the IP name suggest, it is not directly linked to the gpio subsystem. It is actually an independent IP that is able to spy on the SoC pad. For that purpose, it can mux and filter (edge or level and polarity) any single SoC pad to one of the 8 GIC's interrupts it owns. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/irqchip/Kconfig | 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-meson-gpio.c | 423 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 433 insertions(+) create mode 100644 drivers/irqchip/irq-meson-gpio.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 82b0b5daf3f5..168837263e80 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -279,3 +279,12 @@ config EZNPS_GIC config STM32_EXTI bool select IRQ_DOMAIN + +config MESON_GPIO_IRQ + bool "Meson GPIO Interrupt Multiplexer" + depends on ARCH_MESON || COMPILE_TEST + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Support Meson SoC Family GPIO Interrupt Multiplexer + diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index e4dbfc85abdb..33f913d037d0 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o +obj-$(CONFIG_MESON_GPIO_IRQ) += irq-meson-gpio.o diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c new file mode 100644 index 000000000000..869b4df8c483 --- /dev/null +++ b/drivers/irqchip/irq-meson-gpio.c @@ -0,0 +1,423 @@ +/* + * Copyright (c) 2015 Endless Mobile, Inc. + * Author: Carlo Caione <carlo@endlessm.com> + * Copyright (c) 2016 BayLibre, SAS. + * Author: Jerome Brunet <jbrunet@baylibre.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + * The full GNU General Public License is included in this distribution + * in the file called COPYING. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/irqchip.h> +#include <linux/of.h> +#include <linux/of_address.h> + +#define IRQ_FREE (-1) + +#define REG_EDGE_POL 0x00 +#define REG_PIN_03_SEL 0x04 +#define REG_PIN_47_SEL 0x08 +#define REG_FILTER_SEL 0x0c + +#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x))) +#define REG_EDGE_POL_EDGE(x) BIT(x) +#define REG_EDGE_POL_LOW(x) BIT(16 + (x)) +#define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8) +#define REG_FILTER_SEL_SHIFT(x) ((x) * 4) + +struct meson_gpio_irq_params { + unsigned int nhwirq; + irq_hw_number_t *source; + int nsource; +}; + +struct meson_gpio_irq_domain { + void __iomem *base; + int *map; + const struct meson_gpio_irq_params *params; +}; + +struct meson_gpio_irq_chip_data { + void __iomem *base; + int index; +}; + +static irq_hw_number_t meson_parent_hwirqs[] = { + 64, 65, 66, 67, 68, 69, 70, 71, +}; + +static const struct meson_gpio_irq_params meson8_params = { + .nhwirq = 134, + .source = meson_parent_hwirqs, + .nsource = ARRAY_SIZE(meson_parent_hwirqs), +}; + +static const struct meson_gpio_irq_params meson8b_params = { + .nhwirq = 119, + .source = meson_parent_hwirqs, + .nsource = ARRAY_SIZE(meson_parent_hwirqs), +}; + +static const struct meson_gpio_irq_params meson_gxbb_params = { + .nhwirq = 133, + .source = meson_parent_hwirqs, + .nsource = ARRAY_SIZE(meson_parent_hwirqs), +}; + +static const struct of_device_id meson_irq_gpio_matches[] = { + { + .compatible = "amlogic,meson8-gpio-intc", + .data = &meson8_params + }, + { + .compatible = "amlogic,meson8b-gpio-intc", + .data = &meson8b_params + }, + { + .compatible = "amlogic,meson-gxbb-gpio-intc", + .data = &meson_gxbb_params + }, + {} +}; + +static void meson_gpio_irq_update_bits(void __iomem *base, unsigned int reg, + u32 mask, u32 val) +{ + u32 tmp; + + tmp = readl(base + reg); + tmp &= ~mask; + tmp |= val; + + writel(tmp, base + reg); +} + +static int meson_gpio_irq_get_index(struct meson_gpio_irq_domain *domain_data, + int hwirq) +{ + int i; + + for (i = 0; i < domain_data->params->nsource; i++) { + if (domain_data->map[i] == hwirq) + return i; + } + + return -1; +} + +static int mesion_gpio_irq_map_source(struct meson_gpio_irq_domain *domain_data, + irq_hw_number_t hwirq, + irq_hw_number_t *source) +{ + int index; + unsigned int reg; + + index = meson_gpio_irq_get_index(domain_data, IRQ_FREE); + if (index < 0) { + pr_err("No irq available\n"); + return -ENOSPC; + } + + domain_data->map[index] = hwirq; + + reg = (index < 4) ? REG_PIN_03_SEL : REG_PIN_47_SEL; + meson_gpio_irq_update_bits(domain_data->base, reg, + 0xff << REG_PIN_SEL_SHIFT(index), + hwirq << REG_PIN_SEL_SHIFT(index)); + + *source = domain_data->params->source[index]; + + pr_debug("hwirq %lu assigned to channel %d - source %lu\n", + hwirq, index, *source); + + return index; +} + +static int meson_gpio_irq_type_setup(unsigned int type, void __iomem *base, + int index) +{ + u32 val = 0; + + type &= IRQ_TYPE_SENSE_MASK; + + if (type == IRQ_TYPE_EDGE_BOTH) + return -EINVAL; + + if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) + val |= REG_EDGE_POL_EDGE(index); + + if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) + val |= REG_EDGE_POL_LOW(index); + + meson_gpio_irq_update_bits(base, REG_EDGE_POL, + REG_EDGE_POL_MASK(index), val); + + return 0; +} + +static unsigned int meson_gpio_irq_type_output(unsigned int type) +{ + unsigned int sense = type & IRQ_TYPE_SENSE_MASK; + + type &= ~IRQ_TYPE_SENSE_MASK; + + /* + * If the polarity of interrupt is low, the controller will + * invert the signal for gic + */ + if (sense & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) + type |= IRQ_TYPE_LEVEL_HIGH; + else if (sense & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) + type |= IRQ_TYPE_EDGE_RISING; + + return type; +} + +static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type) +{ + struct meson_gpio_irq_chip_data *cd = irq_data_get_irq_chip_data(data); + int ret; + + pr_debug("set type of hwirq %lu to %u\n", data->hwirq, type); + + ret = meson_gpio_irq_type_setup(type, cd->base, cd->index); + if (ret) + return ret; + + return irq_chip_set_type_parent(data, + meson_gpio_irq_type_output(type)); +} + +static struct irq_chip meson_gpio_irq_chip = { + .name = "meson-gpio-irqchip", + .irq_mask = irq_chip_mask_parent, + .irq_unmask = irq_chip_unmask_parent, + .irq_eoi = irq_chip_eoi_parent, + .irq_set_type = meson_gpio_irq_set_type, + .irq_retrigger = irq_chip_retrigger_hierarchy, +#ifdef CONFIG_SMP + .irq_set_affinity = irq_chip_set_affinity_parent, +#endif +}; + +static int meson_gpio_irq_domain_translate(struct irq_domain *domain, + struct irq_fwspec *fwspec, + unsigned long *hwirq, + unsigned int *type) +{ + if (is_of_node(fwspec->fwnode)) { + if (fwspec->param_count != 2) + return -EINVAL; + + *hwirq = fwspec->param[0]; + *type = fwspec->param[1]; + + return 0; + } + + return -EINVAL; +} + +static int meson_gpio_irq_allocate_gic_irq(struct irq_domain *domain, + unsigned int virq, + irq_hw_number_t source, + unsigned int type) +{ + struct irq_fwspec fwspec; + + if (!irq_domain_get_of_node(domain->parent)) + return -EINVAL; + + fwspec.fwnode = domain->parent->fwnode; + fwspec.param_count = 3; + fwspec.param[0] = 0; /* SPI */ + fwspec.param[1] = source; + fwspec.param[2] = meson_gpio_irq_type_output(type); + + return irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec); +} + +static int meson_gpio_irq_domain_alloc(struct irq_domain *domain, + unsigned int virq, + unsigned int nr_irqs, + void *data) +{ + struct irq_fwspec *fwspec = data; + struct meson_gpio_irq_domain *domain_data = domain->host_data; + struct meson_gpio_irq_chip_data *cd; + unsigned long hwirq, source; + unsigned int type; + int i, index, ret; + + ret = meson_gpio_irq_domain_translate(domain, fwspec, &hwirq, &type); + if (ret) + return ret; + + pr_debug("irq %d, nr_irqs %d, hwirqs %lu\n", virq, nr_irqs, hwirq); + + for (i = 0; i < nr_irqs; i++) { + index = mesion_gpio_irq_map_source(domain_data, hwirq + i, + &source); + if (index < 0) + return index; + + ret = meson_gpio_irq_type_setup(type, domain_data->base, + index); + if (ret) + return ret; + + cd = kzalloc(sizeof(*cd), GFP_KERNEL); + if (!cd) + return -ENOMEM; + + cd->base = domain_data->base; + cd->index = index; + + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, + &meson_gpio_irq_chip, cd); + + ret = meson_gpio_irq_allocate_gic_irq(domain, virq + i, + source, type); + if (ret < 0) + return ret; + } + + return 0; +} + +static void meson_gpio_irq_domain_free(struct irq_domain *domain, + unsigned int virq, + unsigned int nr_irqs) +{ + struct meson_gpio_irq_domain *domain_data = domain->host_data; + struct meson_gpio_irq_chip_data *cd; + struct irq_data *irq_data; + int i; + + for (i = 0; i < nr_irqs; i++) { + irq_data = irq_domain_get_irq_data(domain, virq + i); + cd = irq_data_get_irq_chip_data(irq_data); + + domain_data->map[cd->index] = IRQ_FREE; + kfree(cd); + } + + irq_domain_free_irqs_parent(domain, virq, nr_irqs); + +} + +static const struct irq_domain_ops meson_gpio_irq_domain_ops = { + .alloc = meson_gpio_irq_domain_alloc, + .free = meson_gpio_irq_domain_free, + .translate = meson_gpio_irq_domain_translate, +}; + +static int __init +meson_gpio_irq_init_domain(struct device_node *node, + struct meson_gpio_irq_domain *domain_data, + const struct meson_gpio_irq_params *params) +{ + int i; + int nsource = params->nsource; + int *map; + + map = kcalloc(nsource, sizeof(*map), GFP_KERNEL); + if (!map) + return -ENOMEM; + + for (i = 0; i < nsource; i++) + map[i] = IRQ_FREE; + + domain_data->map = map; + domain_data->params = params; + + return 0; +} + +static int __init meson_gpio_irq_of_init(struct device_node *node, + struct device_node *parent) +{ + struct irq_domain *domain, *parent_domain; + const struct of_device_id *match; + const struct meson_gpio_irq_params *params; + struct meson_gpio_irq_domain *domain_data; + int ret; + + match = of_match_node(meson_irq_gpio_matches, node); + if (!match) + return -ENODEV; + params = match->data; + + if (!parent) { + pr_err("missing parent interrupt node\n"); + return -ENODEV; + } + + parent_domain = irq_find_host(parent); + if (!parent_domain) { + pr_err("unable to obtain parent domain\n"); + return -ENXIO; + } + + domain_data = kzalloc(sizeof(*domain_data), GFP_KERNEL); + if (!domain_data) + return -ENOMEM; + + domain_data->base = of_iomap(node, 0); + if (!domain_data->base) { + ret = -ENOMEM; + goto out_free_dev; + } + + ret = meson_gpio_irq_init_domain(node, domain_data, params); + if (ret < 0) + goto out_free_dev_content; + + domain = irq_domain_add_hierarchy(parent_domain, 0, params->nhwirq, + node, &meson_gpio_irq_domain_ops, + domain_data); + + if (!domain) { + pr_err("failed to allocated domain\n"); + ret = -ENOMEM; + goto out_free_dev_content; + } + + pr_info("%d to %d gpio interrupt mux initialized\n", + params->nhwirq, params->nsource); + + return 0; + +out_free_dev_content: + kfree(domain_data->map); + iounmap(domain_data->base); + +out_free_dev: + kfree(domain_data); + + return ret; +} + +IRQCHIP_DECLARE(meson8_gpio_intc, "amlogic,meson8-gpio-intc", + meson_gpio_irq_of_init); +IRQCHIP_DECLARE(meson8b_gpio_intc, "amlogic,meson8b-gpio-intc", + meson_gpio_irq_of_init); +IRQCHIP_DECLARE(gxbb_gpio_intc, "amlogic,meson-gxbb-gpio-intc", + meson_gpio_irq_of_init); -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-19 15:21 ` [PATCH v2 1/9] irqchip: meson: add support for " Jerome Brunet @ 2016-10-20 16:33 ` Marc Zyngier 2016-10-21 8:49 ` Jerome Brunet 0 siblings, 1 reply; 20+ messages in thread From: Marc Zyngier @ 2016-10-20 16:33 UTC (permalink / raw) To: Jerome Brunet, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper Cc: linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King, Mark Rutland Jerome, On 19/10/16 16:21, Jerome Brunet wrote: > Add support for the interrupt gpio controller found on Amlogic's meson > SoC family. > > Unlike what the IP name suggest, it is not directly linked to the gpio > subsystem. It is actually an independent IP that is able to spy on the > SoC pad. For that purpose, it can mux and filter (edge or level and > polarity) any single SoC pad to one of the 8 GIC's interrupts it owns. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/irqchip/Kconfig | 9 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-meson-gpio.c | 423 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 433 insertions(+) > create mode 100644 drivers/irqchip/irq-meson-gpio.c > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index 82b0b5daf3f5..168837263e80 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -279,3 +279,12 @@ config EZNPS_GIC > config STM32_EXTI > bool > select IRQ_DOMAIN > + > +config MESON_GPIO_IRQ > + bool "Meson GPIO Interrupt Multiplexer" > + depends on ARCH_MESON || COMPILE_TEST > + select IRQ_DOMAIN > + select IRQ_DOMAIN_HIERARCHY > + help > + Support Meson SoC Family GPIO Interrupt Multiplexer > + > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index e4dbfc85abdb..33f913d037d0 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o > obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o > obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o > +obj-$(CONFIG_MESON_GPIO_IRQ) += irq-meson-gpio.o > diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c > new file mode 100644 > index 000000000000..869b4df8c483 > --- /dev/null > +++ b/drivers/irqchip/irq-meson-gpio.c > @@ -0,0 +1,423 @@ > +/* > + * Copyright (c) 2015 Endless Mobile, Inc. > + * Author: Carlo Caione <carlo@endlessm.com> > + * Copyright (c) 2016 BayLibre, SAS. > + * Author: Jerome Brunet <jbrunet@baylibre.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of version 2 of the GNU General Public License as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + * The full GNU General Public License is included in this distribution > + * in the file called COPYING. > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/irqchip.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > + > +#define IRQ_FREE (-1) > + > +#define REG_EDGE_POL 0x00 > +#define REG_PIN_03_SEL 0x04 > +#define REG_PIN_47_SEL 0x08 > +#define REG_FILTER_SEL 0x0c > + > +#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x))) > +#define REG_EDGE_POL_EDGE(x) BIT(x) > +#define REG_EDGE_POL_LOW(x) BIT(16 + (x)) > +#define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8) > +#define REG_FILTER_SEL_SHIFT(x) ((x) * 4) > + > +struct meson_gpio_irq_params { > + unsigned int nhwirq; > + irq_hw_number_t *source; > + int nsource; > +}; > + > +struct meson_gpio_irq_domain { The name of that structure is utterly confusing, as it doesn't contain anything related to an IRQ domain. Can you please clarify its usage? > + void __iomem *base; > + int *map; > + const struct meson_gpio_irq_params *params; > +}; > + > +struct meson_gpio_irq_chip_data { > + void __iomem *base; > + int index; > +}; > + > +static irq_hw_number_t meson_parent_hwirqs[] = { > + 64, 65, 66, 67, 68, 69, 70, 71, > +}; If that a guarantee that these numbers will always represent the parent interrupt? It feels a bit odd not to get that information directly from the device tree, in the form of a device specific property. Something like: upstream-interrupts = <64 65 66 ... >; or even as a base/range: upstream-interrupts = <64 8>; Also, how does it work if you have more than a single device like this? This driver seems a do a great deal of dynamic allocation, and yet its core resource is completely static... Please pick a side! ;-) > + > +static const struct meson_gpio_irq_params meson8_params = { > + .nhwirq = 134, > + .source = meson_parent_hwirqs, > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), I find it utterly confusing that you are calling source something that is an output for this controller. It makes my brain melt, and I don't like the feeling. > +}; > + > +static const struct meson_gpio_irq_params meson8b_params = { > + .nhwirq = 119, > + .source = meson_parent_hwirqs, > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > +}; > + > +static const struct meson_gpio_irq_params meson_gxbb_params = { > + .nhwirq = 133, > + .source = meson_parent_hwirqs, > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > +}; Same thing. How big is the variability of these structures? Are we going to see more of those? or is that now set into stone? +Mark: what's the policy to describe this kind of things? > + > +static const struct of_device_id meson_irq_gpio_matches[] = { > + { > + .compatible = "amlogic,meson8-gpio-intc", If it's an independent IP (as described in the commit message), shouldn't in be rescribed in a more SoC-independent way? > + .data = &meson8_params > + }, > + { > + .compatible = "amlogic,meson8b-gpio-intc", > + .data = &meson8b_params > + }, > + { > + .compatible = "amlogic,meson-gxbb-gpio-intc", > + .data = &meson_gxbb_params > + }, > + {} > +}; > + > +static void meson_gpio_irq_update_bits(void __iomem *base, unsigned int reg, > + u32 mask, u32 val) > +{ > + u32 tmp; > + > + tmp = readl(base + reg); > + tmp &= ~mask; > + tmp |= val; > + > + writel(tmp, base + reg); Can't you use the relaxed accessors? > +} > + > +static int meson_gpio_irq_get_index(struct meson_gpio_irq_domain *domain_data, > + int hwirq) > +{ > + int i; > + > + for (i = 0; i < domain_data->params->nsource; i++) { > + if (domain_data->map[i] == hwirq) > + return i; > + } > + > + return -1; I'm a bit worried by this function. If you need an allocator, then having a simple bitmap is much better than iterating over an array. If you're using this to go from a hwirq to the structure describing your interrupt, this is wrong. You should never have to look-up something based on a hwirq, because that's what irq domains are for. > +} > + > +static int mesion_gpio_irq_map_source(struct meson_gpio_irq_domain *domain_data, > + irq_hw_number_t hwirq, > + irq_hw_number_t *source) > +{ > + int index; > + unsigned int reg; > + > + index = meson_gpio_irq_get_index(domain_data, IRQ_FREE); So assuming you turn this into a proper bitmap driven allocator... > + if (index < 0) { > + pr_err("No irq available\n"); > + return -ENOSPC; > + } > + > + domain_data->map[index] = hwirq; this can go away, as there is hardly any point in tracking the hwirq on its own. Actually, the whole map[] array looks totally useless. > + > + reg = (index < 4) ? REG_PIN_03_SEL : REG_PIN_47_SEL; > + meson_gpio_irq_update_bits(domain_data->base, reg, > + 0xff << REG_PIN_SEL_SHIFT(index), > + hwirq << REG_PIN_SEL_SHIFT(index)); > + > + *source = domain_data->params->source[index]; > + > + pr_debug("hwirq %lu assigned to channel %d - source %lu\n", > + hwirq, index, *source); > + > + return index; > +} > + > +static int meson_gpio_irq_type_setup(unsigned int type, void __iomem *base, > + int index) > +{ > + u32 val = 0; > + > + type &= IRQ_TYPE_SENSE_MASK; > + > + if (type == IRQ_TYPE_EDGE_BOTH) > + return -EINVAL; > + > + if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) > + val |= REG_EDGE_POL_EDGE(index); > + > + if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) > + val |= REG_EDGE_POL_LOW(index); > + > + meson_gpio_irq_update_bits(base, REG_EDGE_POL, > + REG_EDGE_POL_MASK(index), val); > + > + return 0; > +} > + > +static unsigned int meson_gpio_irq_type_output(unsigned int type) > +{ > + unsigned int sense = type & IRQ_TYPE_SENSE_MASK; > + > + type &= ~IRQ_TYPE_SENSE_MASK; > + > + /* > + * If the polarity of interrupt is low, the controller will > + * invert the signal for gic > + */ > + if (sense & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) > + type |= IRQ_TYPE_LEVEL_HIGH; > + else if (sense & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) > + type |= IRQ_TYPE_EDGE_RISING; > + > + return type; > +} > + > +static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type) > +{ > + struct meson_gpio_irq_chip_data *cd = irq_data_get_irq_chip_data(data); > + int ret; > + > + pr_debug("set type of hwirq %lu to %u\n", data->hwirq, type); > + > + ret = meson_gpio_irq_type_setup(type, cd->base, cd->index); > + if (ret) > + return ret; > + > + return irq_chip_set_type_parent(data, > + meson_gpio_irq_type_output(type)); > +} > + > +static struct irq_chip meson_gpio_irq_chip = { > + .name = "meson-gpio-irqchip", > + .irq_mask = irq_chip_mask_parent, > + .irq_unmask = irq_chip_unmask_parent, > + .irq_eoi = irq_chip_eoi_parent, > + .irq_set_type = meson_gpio_irq_set_type, > + .irq_retrigger = irq_chip_retrigger_hierarchy, > +#ifdef CONFIG_SMP > + .irq_set_affinity = irq_chip_set_affinity_parent, > +#endif > +}; > + > +static int meson_gpio_irq_domain_translate(struct irq_domain *domain, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, > + unsigned int *type) > +{ > + if (is_of_node(fwspec->fwnode)) { > + if (fwspec->param_count != 2) > + return -EINVAL; You can write this as: if (is_of_node() && fwspec->... == 2) { > + > + *hwirq = fwspec->param[0]; > + *type = fwspec->param[1]; > + > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int meson_gpio_irq_allocate_gic_irq(struct irq_domain *domain, > + unsigned int virq, > + irq_hw_number_t source, > + unsigned int type) > +{ > + struct irq_fwspec fwspec; > + > + if (!irq_domain_get_of_node(domain->parent)) > + return -EINVAL; How can you end-up here if the translate operation has failed? > + > + fwspec.fwnode = domain->parent->fwnode; > + fwspec.param_count = 3; > + fwspec.param[0] = 0; /* SPI */ > + fwspec.param[1] = source; > + fwspec.param[2] = meson_gpio_irq_type_output(type); > + > + return irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec); > +} > + > +static int meson_gpio_irq_domain_alloc(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs, > + void *data) > +{ > + struct irq_fwspec *fwspec = data; > + struct meson_gpio_irq_domain *domain_data = domain->host_data; > + struct meson_gpio_irq_chip_data *cd; > + unsigned long hwirq, source; > + unsigned int type; > + int i, index, ret; > + > + ret = meson_gpio_irq_domain_translate(domain, fwspec, &hwirq, &type); > + if (ret) > + return ret; > + > + pr_debug("irq %d, nr_irqs %d, hwirqs %lu\n", virq, nr_irqs, hwirq); > + > + for (i = 0; i < nr_irqs; i++) { This is a pattern that gets repeated over and over, for no good reason. The only reason we have this nr_irqs thing is to handle things like multi-MSI, where we have to *guarantee* that the hwirqs are allocated in a contiguous manner. Here, you don't enforce that guarantee, so you'd rather have a big fat: if (WARN_ON(nr_irqs != 1)) return -EINVAL; and get rid of that loop, because I cannot imagine a case where you'd allocate more than a single interrupt at a time. > + index = mesion_gpio_irq_map_source(domain_data, hwirq + i, > + &source); > + if (index < 0) > + return index; > + > + ret = meson_gpio_irq_type_setup(type, domain_data->base, > + index); > + if (ret) > + return ret; Why do you have to to this here? This should be handled by the core code already. > + > + cd = kzalloc(sizeof(*cd), GFP_KERNEL); > + if (!cd) > + return -ENOMEM; > + > + cd->base = domain_data->base; > + cd->index = index; So what is the exact purpose of this structure? The base address is essentially a global, or could be directly derived from the domain. The index is only used in set_type, and is the index of the pin connected to the GIC. It looks to me like you could have: irq_hw_number_t *out_line = &meson_parent_hwirqs[index]; > + > + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, > + &meson_gpio_irq_chip, cd); and this written as irq_domain_set_hwirq_and_chip(domain, virq, hwirq, out_line); In your set_type function, you just compute the index back: irq_hw_number_t *out_line = irq_data_get_irq_chip_data(data); irq_hw_number_t index = out_line - meson_parent_hwirqs; and you're done. > + > + ret = meson_gpio_irq_allocate_gic_irq(domain, virq + i, > + source, type); Resource leak on error. > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +static void meson_gpio_irq_domain_free(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs) > +{ > + struct meson_gpio_irq_domain *domain_data = domain->host_data; > + struct meson_gpio_irq_chip_data *cd; > + struct irq_data *irq_data; > + int i; > + > + for (i = 0; i < nr_irqs; i++) { Same comment as above. > + irq_data = irq_domain_get_irq_data(domain, virq + i); > + cd = irq_data_get_irq_chip_data(irq_data); > + > + domain_data->map[cd->index] = IRQ_FREE; > + kfree(cd); > + } > + > + irq_domain_free_irqs_parent(domain, virq, nr_irqs); > + > +} > + > +static const struct irq_domain_ops meson_gpio_irq_domain_ops = { > + .alloc = meson_gpio_irq_domain_alloc, > + .free = meson_gpio_irq_domain_free, > + .translate = meson_gpio_irq_domain_translate, > +}; > + > +static int __init > +meson_gpio_irq_init_domain(struct device_node *node, > + struct meson_gpio_irq_domain *domain_data, > + const struct meson_gpio_irq_params *params) > +{ > + int i; > + int nsource = params->nsource; > + int *map; > + > + map = kcalloc(nsource, sizeof(*map), GFP_KERNEL); > + if (!map) > + return -ENOMEM; > + > + for (i = 0; i < nsource; i++) > + map[i] = IRQ_FREE; > + > + domain_data->map = map; You should now be able to kill most or all of this. > + domain_data->params = params; > + > + return 0; > +} > + > +static int __init meson_gpio_irq_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct irq_domain *domain, *parent_domain; > + const struct of_device_id *match; > + const struct meson_gpio_irq_params *params; > + struct meson_gpio_irq_domain *domain_data; > + int ret; > + > + match = of_match_node(meson_irq_gpio_matches, node); > + if (!match) > + return -ENODEV; > + params = match->data; > + > + if (!parent) { > + pr_err("missing parent interrupt node\n"); > + return -ENODEV; > + } > + > + parent_domain = irq_find_host(parent); > + if (!parent_domain) { > + pr_err("unable to obtain parent domain\n"); > + return -ENXIO; > + } > + > + domain_data = kzalloc(sizeof(*domain_data), GFP_KERNEL); > + if (!domain_data) > + return -ENOMEM; > + > + domain_data->base = of_iomap(node, 0); > + if (!domain_data->base) { > + ret = -ENOMEM; > + goto out_free_dev; > + } > + > + ret = meson_gpio_irq_init_domain(node, domain_data, params); > + if (ret < 0) > + goto out_free_dev_content; > + > + domain = irq_domain_add_hierarchy(parent_domain, 0, params->nhwirq, > + node, &meson_gpio_irq_domain_ops, > + domain_data); Please be consistent in using the fwnode API instead of the of_node one. > + > + if (!domain) { > + pr_err("failed to allocated domain\n"); > + ret = -ENOMEM; > + goto out_free_dev_content; > + } > + > + pr_info("%d to %d gpio interrupt mux initialized\n", > + params->nhwirq, params->nsource); > + > + return 0; > + > +out_free_dev_content: > + kfree(domain_data->map); > + iounmap(domain_data->base); > + > +out_free_dev: > + kfree(domain_data); > + > + return ret; > +} > + > +IRQCHIP_DECLARE(meson8_gpio_intc, "amlogic,meson8-gpio-intc", > + meson_gpio_irq_of_init); > +IRQCHIP_DECLARE(meson8b_gpio_intc, "amlogic,meson8b-gpio-intc", > + meson_gpio_irq_of_init); > +IRQCHIP_DECLARE(gxbb_gpio_intc, "amlogic,meson-gxbb-gpio-intc", > + meson_gpio_irq_of_init); > Overall, this driver is a bit of a mess. Tons of structures that don't make much sense, and a false sense of being able to support multiple instances of the device. I'll let Mark comment on the DT side of things. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-20 16:33 ` Marc Zyngier @ 2016-10-21 8:49 ` Jerome Brunet 2016-10-21 10:10 ` Mark Rutland 2016-10-21 10:28 ` Marc Zyngier 0 siblings, 2 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-21 8:49 UTC (permalink / raw) To: Marc Zyngier, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper Cc: linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King, Mark Rutland On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote: > Jerome, > > On 19/10/16 16:21, Jerome Brunet wrote: > > > > Add support for the interrupt gpio controller found on Amlogic's > > meson > > SoC family. > > > > Unlike what the IP name suggest, it is not directly linked to the > > gpio > > subsystem. It is actually an independent IP that is able to spy on > > the > > SoC pad. For that purpose, it can mux and filter (edge or level and > > polarity) any single SoC pad to one of the 8 GIC's interrupts it > > owns. > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/irqchip/Kconfig | 9 + > > drivers/irqchip/Makefile | 1 + > > drivers/irqchip/irq-meson-gpio.c | 423 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 433 insertions(+) > > create mode 100644 drivers/irqchip/irq-meson-gpio.c > > > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > > index 82b0b5daf3f5..168837263e80 100644 > > --- a/drivers/irqchip/Kconfig > > +++ b/drivers/irqchip/Kconfig > > @@ -279,3 +279,12 @@ config EZNPS_GIC > > config STM32_EXTI > > bool > > select IRQ_DOMAIN > > + > > +config MESON_GPIO_IRQ > > + bool "Meson GPIO Interrupt Multiplexer" > > + depends on ARCH_MESON || COMPILE_TEST > > + select IRQ_DOMAIN > > + select IRQ_DOMAIN_HIERARCHY > > + help > > + Support Meson SoC Family GPIO Interrupt Multiplexer > > + > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > > index e4dbfc85abdb..33f913d037d0 100644 > > --- a/drivers/irqchip/Makefile > > +++ b/drivers/irqchip/Makefile > > @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq- > > ls-scfg-msi.o > > obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o > > obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o > > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o > > +obj-$(CONFIG_MESON_GPIO_IRQ) += irq-meson-gpio.o > > diff --git a/drivers/irqchip/irq-meson-gpio.c > > b/drivers/irqchip/irq-meson-gpio.c > > new file mode 100644 > > index 000000000000..869b4df8c483 > > --- /dev/null > > +++ b/drivers/irqchip/irq-meson-gpio.c > > @@ -0,0 +1,423 @@ > > +/* > > + * Copyright (c) 2015 Endless Mobile, Inc. > > + * Author: Carlo Caione <carlo@endlessm.com> > > + * Copyright (c) 2016 BayLibre, SAS. > > + * Author: Jerome Brunet <jbrunet@baylibre.com> > > + * > > + * This program is free software; you can redistribute it and/or > > modify > > + * it under the terms of version 2 of the GNU General Public > > License as > > + * published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > but > > + * WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > GNU > > + * General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program; if not, see <http://www.gnu.org/licens > > es/>. > > + * The full GNU General Public License is included in this > > distribution > > + * in the file called COPYING. > > + */ > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include <linux/io.h> > > +#include <linux/module.h> > > +#include <linux/irq.h> > > +#include <linux/irqdomain.h> > > +#include <linux/irqchip.h> > > +#include <linux/of.h> > > +#include <linux/of_address.h> > > + > > +#define IRQ_FREE (-1) > > + > > +#define REG_EDGE_POL 0x00 > > +#define REG_PIN_03_SEL 0x04 > > +#define REG_PIN_47_SEL 0x08 > > +#define REG_FILTER_SEL 0x0c > > + > > +#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x))) > > +#define REG_EDGE_POL_EDGE(x) BIT(x) > > +#define REG_EDGE_POL_LOW(x) BIT(16 + (x)) > > +#define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8) > > +#define REG_FILTER_SEL_SHIFT(x) ((x) * 4) > > + > > +struct meson_gpio_irq_params { > > + unsigned int nhwirq; > > + irq_hw_number_t *source; > > + int nsource; > > +}; > > + > > +struct meson_gpio_irq_domain { > > The name of that structure is utterly confusing, as it doesn't > contain > anything related to an IRQ domain. Can you please clarify its usage? This structure is holding the data of the controller. The name is indeed misleading, I will change this. What about 'meson_gpio_irq_pdata' or 'meson_gpio_irq_controller' ? > > > > > + void __iomem *base; > > + int *map; > > + const struct meson_gpio_irq_params *params; > > +}; > > + > > +struct meson_gpio_irq_chip_data { > > + void __iomem *base; > > + int index; > > +}; > > + > > +static irq_hw_number_t meson_parent_hwirqs[] = { > > + 64, 65, 66, 67, 68, 69, 70, 71, > > +}; > > If that a guarantee that these numbers will always represent the > parent interrupt? At the moment, the 3 supported SoC use these parent interrupts, but we have absolutely no idea (or guarantee) that is will remain the same, or even contiguous, in the upcoming SoC (like the GXM or GXL) I reckon, it is likely that manufacturer will keep on using these parent irqs for a while but I would prefer not make an assumption about it in the driver. If a SoC get a different set of interrupts I would have added a new table like this and passed it to the appropriate params : static irq_hw_number_t meson_new_parent_hwirqs[] = { 143, 144, 150, 151, 152, 173, 178, 179, }; > It feels a bit odd not to get that information directly from > the device tree, in the form of a device specific property. Something > like: > > upstream-interrupts = <64 65 66 ... >; > I wondered about putting this information in DT or in the driver for a while. Maybe DT would be a more suitable place holder for these data (parent irq and number of provided hwirq) but I was under the understanding that we should now put these information in the driver and use the compatible property to get the appropriate parameters. I'd love to get the view of the DT guys on this. Also, since we already need to put some information about the pin the pinctrl driver (to get the appropriate mux value of each pad), I thought It would make sense to have the same approach here. If there is some kind of policy saying this should be in DT, I'd be happy to make the change. > or even as a base/range: > > upstream-interrupts = <64 8>; I would prefer to avoid using ranges as we have no guarantee the manufacturer will keep the parent irqs contiguous in the future. > > Also, how does it work if you have more than a single device like > this? > This driver seems a do a great deal of dynamic allocation, and yet > its > core resource is completely static... Please pick a side! ;-) I don't think we could have more than one instance per device and I certainly did not have this case in mind while writing the code. If it was the case someday, I guess having two compatibles would do the trick :) > > > > > + > > +static const struct meson_gpio_irq_params meson8_params = { > > + .nhwirq = 134, > > + .source = meson_parent_hwirqs, > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > I find it utterly confusing that you are calling source something > that is an output for this controller. I had the call sequence (GIC->GPIO_IRQ->handler) in mind while writing it. I see your point and it is indeed confusing, I'll find something better > It makes my brain melt, and I don't like the feeling. Ohhhh !! it's Halloween season and you don't need a costume anymore ;) > > > > > +}; > > + > > +static const struct meson_gpio_irq_params meson8b_params = { > > + .nhwirq = 119, > > + .source = meson_parent_hwirqs, > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > +}; > > + > > +static const struct meson_gpio_irq_params meson_gxbb_params = { > > + .nhwirq = 133, > > + .source = meson_parent_hwirqs, > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > +}; > > Same thing. How big is the variability of these structures? Are we > going to see more of those? or is that now set into stone? The number of pad mapped to the controller seems to change with every SoC version. The parent irqs have not changed so far, but as explained above, there is no guarantee it will keep on being this way. So i'd say probably more of those ... > > +Mark: what's the policy to describe this kind of things? Very interested about this question as well. > > > > > + > > +static const struct of_device_id meson_irq_gpio_matches[] = { > > + { > > + .compatible = "amlogic,meson8-gpio-intc", > > If it's an independent IP (as described in the commit message), > shouldn't in be rescribed in a more SoC-independent way? Ok, I was probably not very clear (again). What I meant in the cover letter is that the interrupt gpio controller is independent for the pad/gpio controller. For example, it could work even you did not setup anything in pinmux or you did not instantiate pinctrl at all. But, at least from my point of view, it is SoC dependent since the number of pad routed to it is changing with every SoC version. > > > > > + .data = &meson8_params > > + }, > > + { > > + .compatible = "amlogic,meson8b-gpio-intc", > > + .data = &meson8b_params > > + }, > > + { > > + .compatible = "amlogic,meson-gxbb-gpio-intc", > > + .data = &meson_gxbb_params > > + }, > > + {} > > +}; > > + > > +static void meson_gpio_irq_update_bits(void __iomem *base, > > unsigned int reg, > > + u32 mask, u32 val) > > +{ > > + u32 tmp; > > + > > + tmp = readl(base + reg); > > + tmp &= ~mask; > > + tmp |= val; > > + > > + writel(tmp, base + reg); > > Can't you use the relaxed accessors? Indeed, this will be fixed. > > > > > +} > > + > > +static int meson_gpio_irq_get_index(struct meson_gpio_irq_domain > > *domain_data, > > + int hwirq) > > +{ > > + int i; > > + > > + for (i = 0; i < domain_data->params->nsource; i++) { > > + if (domain_data->map[i] == hwirq) > > + return i; > > + } > > + > > + return -1; > > I'm a bit worried by this function. If you need an allocator, then > having a simple bitmap is much better than iterating over an array. > > If you're using this to go from a hwirq to the structure describing > your > interrupt, this is wrong. You should never have to look-up something > based on a hwirq, because that's what irq domains are for. OK > > > > > +} > > + > > +static int mesion_gpio_irq_map_source(struct meson_gpio_irq_domain > > *domain_data, > > + irq_hw_number_t hwirq, > > + irq_hw_number_t *source) > > +{ > > + int index; > > + unsigned int reg; > > + > > + index = meson_gpio_irq_get_index(domain_data, IRQ_FREE); > > So assuming you turn this into a proper bitmap driven allocator... > > > > > + if (index < 0) { > > + pr_err("No irq available\n"); > > + return -ENOSPC; > > + } > > + > > + domain_data->map[index] = hwirq; > > this can go away, as there is hardly any point in tracking the hwirq > on > its own. Actually, the whole map[] array looks totally useless. > > > > > + > > + reg = (index < 4) ? REG_PIN_03_SEL : REG_PIN_47_SEL; > > + meson_gpio_irq_update_bits(domain_data->base, reg, > > + 0xff << > > REG_PIN_SEL_SHIFT(index), > > + hwirq << > > REG_PIN_SEL_SHIFT(index)); > > + > > + *source = domain_data->params->source[index]; > > + > > + pr_debug("hwirq %lu assigned to channel %d - source > > %lu\n", > > + hwirq, index, *source); > > + > > + return index; > > +} > > + > > +static int meson_gpio_irq_type_setup(unsigned int type, void > > __iomem *base, > > + int index) > > +{ > > + u32 val = 0; > > + > > + type &= IRQ_TYPE_SENSE_MASK; > > + > > + if (type == IRQ_TYPE_EDGE_BOTH) > > + return -EINVAL; > > + > > + if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) > > + val |= REG_EDGE_POL_EDGE(index); > > + > > + if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) > > + val |= REG_EDGE_POL_LOW(index); > > + > > + meson_gpio_irq_update_bits(base, REG_EDGE_POL, > > + REG_EDGE_POL_MASK(index), val); > > + > > + return 0; > > +} > > + > > +static unsigned int meson_gpio_irq_type_output(unsigned int type) > > +{ > > + unsigned int sense = type & IRQ_TYPE_SENSE_MASK; > > + > > + type &= ~IRQ_TYPE_SENSE_MASK; > > + > > + /* > > + * If the polarity of interrupt is low, the controller > > will > > + * invert the signal for gic > > + */ > > + if (sense & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) > > + type |= IRQ_TYPE_LEVEL_HIGH; > > + else if (sense & (IRQ_TYPE_EDGE_RISING | > > IRQ_TYPE_EDGE_FALLING)) > > + type |= IRQ_TYPE_EDGE_RISING; > > + > > + return type; > > +} > > + > > +static int meson_gpio_irq_set_type(struct irq_data *data, unsigned > > int type) > > +{ > > + struct meson_gpio_irq_chip_data *cd = > > irq_data_get_irq_chip_data(data); > > + int ret; > > + > > + pr_debug("set type of hwirq %lu to %u\n", data->hwirq, > > type); > > + > > + ret = meson_gpio_irq_type_setup(type, cd->base, cd- > > >index); > > + if (ret) > > + return ret; > > + > > + return irq_chip_set_type_parent(data, > > + meson_gpio_irq_type_output > > (type)); > > +} > > + > > +static struct irq_chip meson_gpio_irq_chip = { > > + .name = "meson-gpio-irqchip", > > + .irq_mask = irq_chip_mask_parent, > > + .irq_unmask = irq_chip_unmask_parent, > > + .irq_eoi = irq_chip_eoi_parent, > > + .irq_set_type = meson_gpio_irq_set_type, > > + .irq_retrigger = > > irq_chip_retrigger_hierarchy, > > +#ifdef CONFIG_SMP > > + .irq_set_affinity = irq_chip_set_affinity_parent, > > +#endif > > +}; > > + > > +static int meson_gpio_irq_domain_translate(struct irq_domain > > *domain, > > + struct irq_fwspec > > *fwspec, > > + unsigned long *hwirq, > > + unsigned int *type) > > +{ > > + if (is_of_node(fwspec->fwnode)) { > > + if (fwspec->param_count != 2) > > + return -EINVAL; > > You can write this as: > > if (is_of_node() && fwspec->... == 2) { OK > > > > > + > > + *hwirq = fwspec->param[0]; > > + *type = fwspec->param[1]; > > + > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int meson_gpio_irq_allocate_gic_irq(struct irq_domain > > *domain, > > + unsigned int virq, > > + irq_hw_number_t source, > > + unsigned int type) > > +{ > > + struct irq_fwspec fwspec; > > + > > + if (!irq_domain_get_of_node(domain->parent)) > > + return -EINVAL; > > How can you end-up here if the translate operation has failed? You can't, will be removed > > > > > + > > + fwspec.fwnode = domain->parent->fwnode; > > + fwspec.param_count = 3; > > + fwspec.param[0] = 0; /* SPI */ > > + fwspec.param[1] = source; > > + fwspec.param[2] = meson_gpio_irq_type_output(type); > > + > > + return irq_domain_alloc_irqs_parent(domain, virq, 1, > > &fwspec); > > +} > > + > > +static int meson_gpio_irq_domain_alloc(struct irq_domain *domain, > > + unsigned int virq, > > + unsigned int nr_irqs, > > + void *data) > > +{ > > + struct irq_fwspec *fwspec = data; > > + struct meson_gpio_irq_domain *domain_data = domain- > > >host_data; > > + struct meson_gpio_irq_chip_data *cd; > > + unsigned long hwirq, source; > > + unsigned int type; > > + int i, index, ret; > > + > > + ret = meson_gpio_irq_domain_translate(domain, fwspec, > > &hwirq, &type); > > + if (ret) > > + return ret; > > + > > + pr_debug("irq %d, nr_irqs %d, hwirqs %lu\n", virq, > > nr_irqs, hwirq); > > + > > + for (i = 0; i < nr_irqs; i++) { > > This is a pattern that gets repeated over and over, for no good > reason. > The only reason we have this nr_irqs thing is to handle things like > multi-MSI, where we have to *guarantee* that the hwirqs are allocated > in > a contiguous manner. > > Here, you don't enforce that guarantee, so you'd rather have a big > fat: > > if (WARN_ON(nr_irqs != 1)) > return -EINVAL; > > and get rid of that loop, because I cannot imagine a case where you'd > allocate more than a single interrupt at a time. Thanks for this clarification. I was actually very confused about getting a single fwspec and this 'nr_irqs' parameters. I could not figure a case where one would want multiple irqs in one call, and looking at the other irqchip drivers did not really help. In the end, I tried to implement the API the best I could, thinking that somebody probably had a very good reason for this. I'm perfectly happy using your solution, it makes more sense. > > > > > + index = mesion_gpio_irq_map_source(domain_data, > > hwirq + i, > > + &source); > > + if (index < 0) > > + return index; > > + > > + ret = meson_gpio_irq_type_setup(type, domain_data- > > >base, > > + index); > > + if (ret) > > + return ret; > > Why do you have to to this here? This should be handled by the core > code already. OK > > > > > + > > + cd = kzalloc(sizeof(*cd), GFP_KERNEL); > > + if (!cd) > > + return -ENOMEM; > > + > > + cd->base = domain_data->base; > > + cd->index = index; > > So what is the exact purpose of this structure? The base address is > essentially a global, or could be directly derived from the domain. > The > index is only used in set_type, and is the index of the pin connected > to > the GIC. It looks to me like you could have: > > irq_hw_number_t *out_line = > &meson_parent_hwirqs[index]; meson_parent_hwirq is only declared this way to avoid writing the parent irqs 3 times (in each SoC params). Using it this way would make it global and imply it is the same whatever the SoC. This something I can't guarantee and I would prefer to avoid that. > > > > > + > > + irq_domain_set_hwirq_and_chip(domain, virq + i, > > hwirq + i, > > + &meson_gpio_irq_chip > > , cd); > > and this written as > > irq_domain_set_hwirq_and_chip(domain, virq, hwirq, > out_line); > > In your set_type function, you just compute the index back: > > irq_hw_number_t *out_line = irq_data_get_irq_chip_data(data); > irq_hw_number_t index = out_line - meson_parent_hwirqs; > > and you're done. > Since I can derive the base address, I only need the index in set_type. I'll rework this to get rid of this structure. > > > > + > > + ret = meson_gpio_irq_allocate_gic_irq(domain, virq > > + i, > > + source, > > type); > > Resource leak on error. Ok, Thx. > > > > > + if (ret < 0) > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static void meson_gpio_irq_domain_free(struct irq_domain *domain, > > + unsigned int virq, > > + unsigned int nr_irqs) > > +{ > > + struct meson_gpio_irq_domain *domain_data = domain- > > >host_data; > > + struct meson_gpio_irq_chip_data *cd; > > + struct irq_data *irq_data; > > + int i; > > + > > + for (i = 0; i < nr_irqs; i++) { > > Same comment as above. OK > > > > > + irq_data = irq_domain_get_irq_data(domain, virq + > > i); > > + cd = irq_data_get_irq_chip_data(irq_data); > > + > > + domain_data->map[cd->index] = IRQ_FREE; > > + kfree(cd); > > + } > > + > > + irq_domain_free_irqs_parent(domain, virq, nr_irqs); > > + > > +} > > + > > +static const struct irq_domain_ops meson_gpio_irq_domain_ops = { > > + .alloc = meson_gpio_irq_domain_alloc, > > + .free = meson_gpio_irq_domain_free, > > + .translate = meson_gpio_irq_domain_translate, > > +}; > > + > > +static int __init > > +meson_gpio_irq_init_domain(struct device_node *node, > > + struct meson_gpio_irq_domain > > *domain_data, > > + const struct meson_gpio_irq_params > > *params) > > +{ > > + int i; > > + int nsource = params->nsource; > > + int *map; > > + > > + map = kcalloc(nsource, sizeof(*map), GFP_KERNEL); > > + if (!map) > > + return -ENOMEM; > > + > > + for (i = 0; i < nsource; i++) > > + map[i] = IRQ_FREE; > > + > > + domain_data->map = map; > > You should now be able to kill most or all of this. > > > > > + domain_data->params = params; > > + > > + return 0; > > +} > > + > > +static int __init meson_gpio_irq_of_init(struct device_node *node, > > + struct device_node > > *parent) > > +{ > > + struct irq_domain *domain, *parent_domain; > > + const struct of_device_id *match; > > + const struct meson_gpio_irq_params *params; > > + struct meson_gpio_irq_domain *domain_data; > > + int ret; > > + > > + match = of_match_node(meson_irq_gpio_matches, node); > > + if (!match) > > + return -ENODEV; > > + params = match->data; > > + > > + if (!parent) { > > + pr_err("missing parent interrupt node\n"); > > + return -ENODEV; > > + } > > + > > + parent_domain = irq_find_host(parent); > > + if (!parent_domain) { > > + pr_err("unable to obtain parent domain\n"); > > + return -ENXIO; > > + } > > + > > + domain_data = kzalloc(sizeof(*domain_data), GFP_KERNEL); > > + if (!domain_data) > > + return -ENOMEM; > > + > > + domain_data->base = of_iomap(node, 0); > > + if (!domain_data->base) { > > + ret = -ENOMEM; > > + goto out_free_dev; > > + } > > + > > + ret = meson_gpio_irq_init_domain(node, domain_data, > > params); > > + if (ret < 0) > > + goto out_free_dev_content; > > + > > + domain = irq_domain_add_hierarchy(parent_domain, 0, > > params->nhwirq, > > + node, > > &meson_gpio_irq_domain_ops, > > + domain_data); > > Please be consistent in using the fwnode API instead of the of_node > one. OK > > > > > + > > + if (!domain) { > > + pr_err("failed to allocated domain\n"); > > + ret = -ENOMEM; > > + goto out_free_dev_content; > > + } > > + > > + pr_info("%d to %d gpio interrupt mux initialized\n", > > + params->nhwirq, params->nsource); > > + > > + return 0; > > + > > +out_free_dev_content: > > + kfree(domain_data->map); > > + iounmap(domain_data->base); > > + > > +out_free_dev: > > + kfree(domain_data); > > + > > + return ret; > > +} > > + > > +IRQCHIP_DECLARE(meson8_gpio_intc, "amlogic,meson8-gpio-intc", > > + meson_gpio_irq_of_init); > > +IRQCHIP_DECLARE(meson8b_gpio_intc, "amlogic,meson8b-gpio-intc", > > + meson_gpio_irq_of_init); > > +IRQCHIP_DECLARE(gxbb_gpio_intc, "amlogic,meson-gxbb-gpio-intc", > > + meson_gpio_irq_of_init); > > > > Overall, this driver is a bit of a mess. Tons of structures that > don't > make much sense, and a false sense of being able to support multiple > instances of the device. > > I'll let Mark comment on the DT side of things. > > Thanks, > > M. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-21 8:49 ` Jerome Brunet @ 2016-10-21 10:10 ` Mark Rutland 2016-10-21 10:17 ` Jerome Brunet 2016-10-21 10:28 ` Marc Zyngier 1 sibling, 1 reply; 20+ messages in thread From: Mark Rutland @ 2016-10-21 10:10 UTC (permalink / raw) To: Jerome Brunet Cc: Marc Zyngier, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King On Fri, Oct 21, 2016 at 10:49:11AM +0200, Jerome Brunet wrote: > On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote: > > On 19/10/16 16:21, Jerome Brunet wrote: > > > +struct meson_gpio_irq_chip_data { > > > + void __iomem *base; > > > + int index; > > > +}; > > > + > > > +static irq_hw_number_t meson_parent_hwirqs[] = { > > > + 64, 65, 66, 67, 68, 69, 70, 71, > > > +}; > > > > If that a guarantee that these numbers will always represent the > > parent interrupt? > > At the moment, the 3 supported SoC use these parent interrupts, but we > have absolutely no idea (or guarantee) that is will remain the same, or > even contiguous, in the upcoming SoC (like the GXM or GXL) > > I reckon, it is likely that manufacturer will keep on using these > parent irqs for a while but I would prefer not make an assumption about > it in the driver. > > If a SoC get a different set of interrupts I would have added a new > table like this and passed it to the appropriate params : > > static irq_hw_number_t meson_new_parent_hwirqs[] = { > 143, 144, 150, 151, 152, 173, 178, 179, > }; > > > It feels a bit odd not to get that information directly from > > the device tree, in the form of a device specific property. Something > > like: > > > > upstream-interrupts = <64 65 66 ... >; > > > > I wondered about putting this information in DT or in the driver for a > while. Maybe DT would be a more suitable place holder for these data > (parent irq and number of provided hwirq) but I was under the > understanding that we should now put these information in the driver > and use the compatible property to get the appropriate parameters. > > I'd love to get the view of the DT guys on this. Please describe inter-device relationships in DT when you are aware of them. The SoC-specific compatible string is more of a future-proofing thing / last restort for things we realise too late. To be clear, we should *also* have an soc-specific compatible string, but for differences we already know about, we should use DT properties. > > > +static const struct meson_gpio_irq_params meson8b_params = { > > > + .nhwirq = 119, > > > + .source = meson_parent_hwirqs, > > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > > +}; > > > + > > > +static const struct meson_gpio_irq_params meson_gxbb_params = { > > > + .nhwirq = 133, > > > + .source = meson_parent_hwirqs, > > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > > +}; > > > > Same thing. How big is the variability of these structures? Are we > > going to see more of those? or is that now set into stone? > > The number of pad mapped to the controller seems to change with every > SoC version. The parent irqs have not changed so far, but as explained > above, there is no guarantee it will keep on being this way. > > So i'd say probably more of those ... > > > +Mark: what's the policy to describe this kind of things? Generally, I'd prefer that we describe this in DT rather than accumulating a set of string -> number mappings in the driver. Thanks, Mark. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-21 10:10 ` Mark Rutland @ 2016-10-21 10:17 ` Jerome Brunet 0 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-21 10:17 UTC (permalink / raw) To: Mark Rutland Cc: Marc Zyngier, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King On Fri, 2016-10-21 at 11:10 +0100, Mark Rutland wrote: > On Fri, Oct 21, 2016 at 10:49:11AM +0200, Jerome Brunet wrote: > > > > On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote: > > > > > > On 19/10/16 16:21, Jerome Brunet wrote: > > > > > > > > +struct meson_gpio_irq_chip_data { > > > > + void __iomem *base; > > > > + int index; > > > > +}; > > > > + > > > > +static irq_hw_number_t meson_parent_hwirqs[] = { > > > > + 64, 65, 66, 67, 68, 69, 70, 71, > > > > +}; > > > > > > If that a guarantee that these numbers will always represent the > > > parent interrupt? > > > > At the moment, the 3 supported SoC use these parent interrupts, but > > we > > have absolutely no idea (or guarantee) that is will remain the > > same, or > > even contiguous, in the upcoming SoC (like the GXM or GXL) > > > > I reckon, it is likely that manufacturer will keep on using these > > parent irqs for a while but I would prefer not make an assumption > > about > > it in the driver. > > > > If a SoC get a different set of interrupts I would have added a new > > table like this and passed it to the appropriate params : > > > > static irq_hw_number_t meson_new_parent_hwirqs[] = { > > 143, 144, 150, 151, 152, 173, 178, 179, > > }; > > > > > > > > It feels a bit odd not to get that information directly from > > > the device tree, in the form of a device specific property. > > > Something > > > like: > > > > > > upstream-interrupts = <64 65 66 ... >; > > > > > > > I wondered about putting this information in DT or in the driver > > for a > > while. Maybe DT would be a more suitable place holder for these > > data > > (parent irq and number of provided hwirq) but I was under the > > understanding that we should now put these information in the > > driver > > and use the compatible property to get the appropriate parameters. > > > > I'd love to get the view of the DT guys on this. > > Please describe inter-device relationships in DT when you are aware > of > them. The SoC-specific compatible string is more of a future-proofing > thing / last restort for things we realise too late. > > To be clear, we should *also* have an soc-specific compatible string, > but for differences we already know about, we should use DT > properties. > > > > > > > > > > > > > > +static const struct meson_gpio_irq_params meson8b_params = { > > > > + .nhwirq = 119, > > > > + .source = meson_parent_hwirqs, > > > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > > > +}; > > > > + > > > > +static const struct meson_gpio_irq_params meson_gxbb_params = > > > > { > > > > + .nhwirq = 133, > > > > + .source = meson_parent_hwirqs, > > > > + .nsource = ARRAY_SIZE(meson_parent_hwirqs), > > > > +}; > > > > > > Same thing. How big is the variability of these structures? Are > > > we > > > going to see more of those? or is that now set into stone? > > > > The number of pad mapped to the controller seems to change with > > every > > SoC version. The parent irqs have not changed so far, but as > > explained > > above, there is no guarantee it will keep on being this way. > > > > So i'd say probably more of those ... > > > > > > > > +Mark: what's the policy to describe this kind of things? > > Generally, I'd prefer that we describe this in DT rather than > accumulating a set of string -> number mappings in the driver. Thx Marc. I will change it. > > Thanks, > Mark. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller 2016-10-21 8:49 ` Jerome Brunet 2016-10-21 10:10 ` Mark Rutland @ 2016-10-21 10:28 ` Marc Zyngier 1 sibling, 0 replies; 20+ messages in thread From: Marc Zyngier @ 2016-10-21 10:28 UTC (permalink / raw) To: Jerome Brunet, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper Cc: linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Rob Herring, Linus Walleij, Catalin Marinas, Will Deacon, Russell King, Mark Rutland On 21/10/16 09:49, Jerome Brunet wrote: > On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote: >> Jerome, >> >> On 19/10/16 16:21, Jerome Brunet wrote: >>> >>> Add support for the interrupt gpio controller found on Amlogic's >>> meson >>> SoC family. >>> >>> Unlike what the IP name suggest, it is not directly linked to the >>> gpio >>> subsystem. It is actually an independent IP that is able to spy on >>> the >>> SoC pad. For that purpose, it can mux and filter (edge or level and >>> polarity) any single SoC pad to one of the 8 GIC's interrupts it >>> owns. >>> >>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> >>> --- >>> drivers/irqchip/Kconfig | 9 + >>> drivers/irqchip/Makefile | 1 + >>> drivers/irqchip/irq-meson-gpio.c | 423 >>> +++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 433 insertions(+) >>> create mode 100644 drivers/irqchip/irq-meson-gpio.c >>> >>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >>> index 82b0b5daf3f5..168837263e80 100644 >>> --- a/drivers/irqchip/Kconfig >>> +++ b/drivers/irqchip/Kconfig >>> @@ -279,3 +279,12 @@ config EZNPS_GIC >>> config STM32_EXTI >>> bool >>> select IRQ_DOMAIN >>> + >>> +config MESON_GPIO_IRQ >>> + bool "Meson GPIO Interrupt Multiplexer" >>> + depends on ARCH_MESON || COMPILE_TEST >>> + select IRQ_DOMAIN >>> + select IRQ_DOMAIN_HIERARCHY >>> + help >>> + Support Meson SoC Family GPIO Interrupt Multiplexer >>> + >>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile >>> index e4dbfc85abdb..33f913d037d0 100644 >>> --- a/drivers/irqchip/Makefile >>> +++ b/drivers/irqchip/Makefile >>> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq- >>> ls-scfg-msi.o >>> obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o >>> obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o >>> obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o >>> +obj-$(CONFIG_MESON_GPIO_IRQ) += irq-meson-gpio.o >>> diff --git a/drivers/irqchip/irq-meson-gpio.c >>> b/drivers/irqchip/irq-meson-gpio.c >>> new file mode 100644 >>> index 000000000000..869b4df8c483 >>> --- /dev/null >>> +++ b/drivers/irqchip/irq-meson-gpio.c >>> @@ -0,0 +1,423 @@ >>> +/* >>> + * Copyright (c) 2015 Endless Mobile, Inc. >>> + * Author: Carlo Caione <carlo@endlessm.com> >>> + * Copyright (c) 2016 BayLibre, SAS. >>> + * Author: Jerome Brunet <jbrunet@baylibre.com> >>> + * >>> + * This program is free software; you can redistribute it and/or >>> modify >>> + * it under the terms of version 2 of the GNU General Public >>> License as >>> + * published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> but >>> + * WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> GNU >>> + * General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU General Public >>> License >>> + * along with this program; if not, see <http://www.gnu.org/licens >>> es/>. >>> + * The full GNU General Public License is included in this >>> distribution >>> + * in the file called COPYING. >>> + */ >>> + >>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >>> + >>> +#include <linux/io.h> >>> +#include <linux/module.h> >>> +#include <linux/irq.h> >>> +#include <linux/irqdomain.h> >>> +#include <linux/irqchip.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> + >>> +#define IRQ_FREE (-1) >>> + >>> +#define REG_EDGE_POL 0x00 >>> +#define REG_PIN_03_SEL 0x04 >>> +#define REG_PIN_47_SEL 0x08 >>> +#define REG_FILTER_SEL 0x0c >>> + >>> +#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x))) >>> +#define REG_EDGE_POL_EDGE(x) BIT(x) >>> +#define REG_EDGE_POL_LOW(x) BIT(16 + (x)) >>> +#define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8) >>> +#define REG_FILTER_SEL_SHIFT(x) ((x) * 4) >>> + >>> +struct meson_gpio_irq_params { >>> + unsigned int nhwirq; >>> + irq_hw_number_t *source; >>> + int nsource; >>> +}; >>> + >>> +struct meson_gpio_irq_domain { >> >> The name of that structure is utterly confusing, as it doesn't >> contain >> anything related to an IRQ domain. Can you please clarify its usage? > > This structure is holding the data of the controller. The name is > indeed misleading, I will change this. What about > 'meson_gpio_irq_pdata' or 'meson_gpio_irq_controller' ? > >> >>> >>> + void __iomem *base; >>> + int *map; >>> + const struct meson_gpio_irq_params *params; >>> +}; >>> + >>> +struct meson_gpio_irq_chip_data { >>> + void __iomem *base; >>> + int index; >>> +}; >>> + >>> +static irq_hw_number_t meson_parent_hwirqs[] = { >>> + 64, 65, 66, 67, 68, 69, 70, 71, >>> +}; >> >> If that a guarantee that these numbers will always represent the >> parent interrupt? > > At the moment, the 3 supported SoC use these parent interrupts, but we > have absolutely no idea (or guarantee) that is will remain the same, or > even contiguous, in the upcoming SoC (like the GXM or GXL) > > I reckon, it is likely that manufacturer will keep on using these > parent irqs for a while but I would prefer not make an assumption about > it in the driver. > > If a SoC get a different set of interrupts I would have added a new > table like this and passed it to the appropriate params : > > static irq_hw_number_t meson_new_parent_hwirqs[] = { > 143, 144, 150, 151, 152, 173, 178, 179, > }; > >> It feels a bit odd not to get that information directly from >> the device tree, in the form of a device specific property. Something >> like: >> >> upstream-interrupts = <64 65 66 ... >; >> > > I wondered about putting this information in DT or in the driver for a > while. Maybe DT would be a more suitable place holder for these data > (parent irq and number of provided hwirq) but I was under the > understanding that we should now put these information in the driver > and use the compatible property to get the appropriate parameters. > > I'd love to get the view of the DT guys on this. > > Also, since we already need to put some information about the pin the > pinctrl driver (to get the appropriate mux value of each pad), I > thought It would make sense to have the same approach here. > > If there is some kind of policy saying this should be in DT, I'd be > happy to make the change. > >> or even as a base/range: >> >> upstream-interrupts = <64 8>; > > I would prefer to avoid using ranges as we have no guarantee the > manufacturer will keep the parent irqs contiguous in the future. > >> >> Also, how does it work if you have more than a single device like >> this? >> This driver seems a do a great deal of dynamic allocation, and yet >> its >> core resource is completely static... Please pick a side! ;-) > > I don't think we could have more than one instance per device and I > certainly did not have this case in mind while writing the code. If it > was the case someday, I guess having two compatibles would do the trick > :) This has nothing to do with compatible strings. Your current approach of defining a static list of interrupts and then dynamically allocating things that point directly to it feels wrong. just define a second instance of this IP to be convinced of it. So either you support something that is fully dynamic (supporting multiple instances at every level), or you only support *one*, and everything becomes mostly static. > >> >>> >>> + >>> +static const struct meson_gpio_irq_params meson8_params = { >>> + .nhwirq = 134, >>> + .source = meson_parent_hwirqs, >>> + .nsource = ARRAY_SIZE(meson_parent_hwirqs), >> >> I find it utterly confusing that you are calling source something >> that is an output for this controller. > > I had the call sequence (GIC->GPIO_IRQ->handler) in mind while writing > it. I see your point and it is indeed confusing, I'll find something > better It infinitely better to describe the signal path rather than the call stack (which is actually GIC->GPIO_IRQ->GIC->handler). [...] >>> >>> + >>> + cd = kzalloc(sizeof(*cd), GFP_KERNEL); >>> + if (!cd) >>> + return -ENOMEM; >>> + >>> + cd->base = domain_data->base; >>> + cd->index = index; >> >> So what is the exact purpose of this structure? The base address is >> essentially a global, or could be directly derived from the domain. >> The >> index is only used in set_type, and is the index of the pin connected >> to >> the GIC. It looks to me like you could have: >> >> irq_hw_number_t *out_line = >> &meson_parent_hwirqs[index]; > > meson_parent_hwirq is only declared this way to avoid writing the > parent irqs 3 times (in each SoC params). > Using it this way would make it global and imply it is the same > whatever the SoC. This something I can't guarantee and I would prefer > to avoid that. Well, let's face it: It is global (see my earlier rant). And if you device to support multiple instances, you can attach the parent array at the irq domain level, and still perform that same index calculation. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 1/9] irqchip: meson: add support for " Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-26 21:42 ` Rob Herring 2016-10-19 15:21 ` [PATCH v2 3/9] pinctrl: meson: update pinctrl data with gpio irq data Jerome Brunet ` (6 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Linus Walleij, Catalin Marinas, Will Deacon, Russell King This commit adds the device tree bindings description for Amlogic's GPIO interrupt controller available on the meson8, meson8b and gxbb SoC families Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- Rob, I did not include the Ack you gave for the RFC as bindings have slightly changed. Only the interrupt property has be removed following a discussion I had with Marc .../amlogic,meson-gpio-intc.txt | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt new file mode 100644 index 000000000000..2464d9a0865d --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt @@ -0,0 +1,31 @@ +Amlogic meson GPIO interrupt controller + +Meson SoCs contains an interrupt controller which is able watch the SoC pads +and generate an interrupt on edges or level. The controller is essentially a +256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge +or level and polarity. We don’t expose all 256 mux inputs because the +documentation shows that upper part is not mapped to any pad. The actual number +of interrupt exposed depends on the SoC. + +Required properties: + +- compatible : should be either + "amlogic,meson8-gpio-intc” for meson8 SoCs (AML7826MX) or + “amlogic,meson8b-gpio-intc” for meson8b SoCs (S805) or + “amlogic,meson-gxbb-gpio-intc” for GXBB SoCs (S905) +- interrupt-parent : a phandle to the GIC the interrupts are routed to. + Usually this is provided at the root level of the device tree as it is + common to most of the SoC +- reg : Specifies base physical address and size of the registers. +- interrupt-controller : Identifies the node as an interrupt controller. +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The value must be 2. + +Example: + +gpio_interrupt: interrupt-controller@9880 { + compatible = "amlogic,meson-gxbb-gpio-intc"; + reg = <0x0 0x9880 0x0 0x10>; + interrupt-controller; + #interrupt-cells = <2>; +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller 2016-10-19 15:21 ` [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO " Jerome Brunet @ 2016-10-26 21:42 ` Rob Herring 2016-10-27 9:32 ` Mark Rutland 0 siblings, 1 reply; 20+ messages in thread From: Rob Herring @ 2016-10-26 21:42 UTC (permalink / raw) To: Jerome Brunet Cc: Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Linus Walleij, Catalin Marinas, Will Deacon, Russell King On Wed, Oct 19, 2016 at 05:21:13PM +0200, Jerome Brunet wrote: > > This commit adds the device tree bindings description for Amlogic's GPIO > interrupt controller available on the meson8, meson8b and gxbb SoC families > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > Rob, I did not include the Ack you gave for the RFC as bindings have slightly > changed. Only the interrupt property has be removed following a discussion I > had with Marc As Mark R already said, you should keep the interrupts property. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller 2016-10-26 21:42 ` Rob Herring @ 2016-10-27 9:32 ` Mark Rutland 2016-10-27 9:40 ` Jerome Brunet 0 siblings, 1 reply; 20+ messages in thread From: Mark Rutland @ 2016-10-27 9:32 UTC (permalink / raw) To: Rob Herring Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Linus Walleij, Catalin Marinas, Will Deacon, Russell King On Wed, Oct 26, 2016 at 04:42:35PM -0500, Rob Herring wrote: > On Wed, Oct 19, 2016 at 05:21:13PM +0200, Jerome Brunet wrote: > > > > This commit adds the device tree bindings description for Amlogic's GPIO > > interrupt controller available on the meson8, meson8b and gxbb SoC families > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > Rob, I did not include the Ack you gave for the RFC as bindings have slightly > > changed. Only the interrupt property has be removed following a discussion I > > had with Marc > > As Mark R already said, you should keep the interrupts property. To be clear, the interrupt routing should be described *somehow*, though I don't think the generic interrupts property is correct, as this is an interrupt *router*, i.e. this device doesn't own the interrupt, it just joins two parts of the line together (and so flags are meaningless). Thanks, Mark. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller 2016-10-27 9:32 ` Mark Rutland @ 2016-10-27 9:40 ` Jerome Brunet 0 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-27 9:40 UTC (permalink / raw) To: Mark Rutland, Rob Herring Cc: Carlo Caione, Kevin Hilman, Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Linus Walleij, Catalin Marinas, Will Deacon, Russell King On Thu, 2016-10-27 at 10:32 +0100, Mark Rutland wrote: > On Wed, Oct 26, 2016 at 04:42:35PM -0500, Rob Herring wrote: > > > > On Wed, Oct 19, 2016 at 05:21:13PM +0200, Jerome Brunet wrote: > > > > > > > > > This commit adds the device tree bindings description for > > > Amlogic's GPIO > > > interrupt controller available on the meson8, meson8b and gxbb > > > SoC families > > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > > --- > > > Rob, I did not include the Ack you gave for the RFC as bindings > > > have slightly > > > changed. Only the interrupt property has be removed following a > > > discussion I > > > had with Marc > > > > As Mark R already said, you should keep the interrupts property. > > To be clear, the interrupt routing should be described *somehow*, > though > I don't think the generic interrupts property is correct, as this is > an > interrupt *router*, i.e. this device doesn't own the interrupt, it > just > joins two parts of the line together (and so flags are meaningless). > > Thanks, > Mark. Indeed Mark, I already rewritten the driver taking Marc's comment into account. There will be 2 properties added to the DT: - meson,upstream-interrupts : an array with the 8 gic interrupt used as output of the controller - meson,num-input-mux : the number of inputs (pads) available to the muxes I'm looking for solution to the "create mapping" issue we have in pinctrl (patch 4) before posting a v3. If you think these properties should be different, feel free to tell me now. Thx Jerome ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/9] pinctrl: meson: update pinctrl data with gpio irq data 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 1/9] irqchip: meson: add support for " Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO " Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 4/9] pinctrl: meson: allow gpio to request irq Jerome Brunet ` (5 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Linus Walleij Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Catalin Marinas, Will Deacon, Russell King This patch extends meson's pinctrl SoC specific data by adding the gpio irq base number and the compatible controller for each SoC. This will allow gpios to request an interrupt on the gpio interrupt controller using this base irq number the pin offset inbank Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 24 +++++++++++---------- drivers/pinctrl/meson/pinctrl-meson.h | 16 +++++++++----- drivers/pinctrl/meson/pinctrl-meson8.c | 22 ++++++++++--------- drivers/pinctrl/meson/pinctrl-meson8b.c | 34 +++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 36 deletions(-) diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c index c3928aa3fefa..ed8a1222de3b 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c +++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c @@ -715,24 +715,25 @@ static struct meson_pmx_func meson_gxbb_aobus_functions[] = { }; static struct meson_bank meson_gxbb_periphs_banks[] = { - /* name first last pullen pull dir out in */ - BANK("X", PIN(GPIOX_0, EE_OFF), PIN(GPIOX_22, EE_OFF), 4, 0, 4, 0, 12, 0, 13, 0, 14, 0), - BANK("Y", PIN(GPIOY_0, EE_OFF), PIN(GPIOY_16, EE_OFF), 1, 0, 1, 0, 3, 0, 4, 0, 5, 0), - BANK("DV", PIN(GPIODV_0, EE_OFF), PIN(GPIODV_29, EE_OFF), 0, 0, 0, 0, 0, 0, 1, 0, 2, 0), - BANK("H", PIN(GPIOH_0, EE_OFF), PIN(GPIOH_3, EE_OFF), 1, 20, 1, 20, 3, 20, 4, 20, 5, 20), - BANK("Z", PIN(GPIOZ_0, EE_OFF), PIN(GPIOZ_15, EE_OFF), 3, 0, 3, 0, 9, 0, 10, 0, 11, 0), - BANK("CARD", PIN(CARD_0, EE_OFF), PIN(CARD_6, EE_OFF), 2, 20, 2, 20, 6, 20, 7, 20, 8, 20), - BANK("BOOT", PIN(BOOT_0, EE_OFF), PIN(BOOT_17, EE_OFF), 2, 0, 2, 0, 6, 0, 7, 0, 8, 0), - BANK("CLK", PIN(GPIOCLK_0, EE_OFF), PIN(GPIOCLK_3, EE_OFF), 3, 28, 3, 28, 9, 28, 10, 28, 11, 28), + /* name first last irq pullen pull dir out in */ + BANK("X", PIN(GPIOX_0, EE_OFF), PIN(GPIOX_22, EE_OFF), 106, 128, 4, 0, 4, 0, 12, 0, 13, 0, 14, 0), + BANK("Y", PIN(GPIOY_0, EE_OFF), PIN(GPIOY_16, EE_OFF), 89, 105, 1, 0, 1, 0, 3, 0, 4, 0, 5, 0), + BANK("DV", PIN(GPIODV_0, EE_OFF), PIN(GPIODV_29, EE_OFF), 59, 88, 0, 0, 0, 0, 0, 0, 1, 0, 2, 0), + BANK("H", PIN(GPIOH_0, EE_OFF), PIN(GPIOH_3, EE_OFF), 30, 33, 1, 20, 1, 20, 3, 20, 4, 20, 5, 20), + BANK("Z", PIN(GPIOZ_0, EE_OFF), PIN(GPIOZ_15, EE_OFF), 14, 29, 3, 0, 3, 0, 9, 0, 10, 0, 11, 0), + BANK("CARD", PIN(CARD_0, EE_OFF), PIN(CARD_6, EE_OFF), 52, 58, 2, 20, 2, 20, 6, 20, 7, 20, 8, 20), + BANK("BOOT", PIN(BOOT_0, EE_OFF), PIN(BOOT_17, EE_OFF), 34, 51, 2, 0, 2, 0, 6, 0, 7, 0, 8, 0), + BANK("CLK", PIN(GPIOCLK_0, EE_OFF), PIN(GPIOCLK_3, EE_OFF), 129, 132, 3, 28, 3, 28, 9, 28, 10, 28, 11, 28), }; static struct meson_bank meson_gxbb_aobus_banks[] = { - /* name first last pullen pull dir out in */ - BANK("AO", PIN(GPIOAO_0, 0), PIN(GPIOAO_13, 0), 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), + /* name first last irq pullen pull dir out in */ + BANK("AO", PIN(GPIOAO_0, 0), PIN(GPIOAO_13, 0), 0, 13, 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), }; struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = { .name = "periphs-banks", + .irq_compat = "amlogic,meson-gxbb-gpio-intc", .pin_base = 14, .pins = meson_gxbb_periphs_pins, .groups = meson_gxbb_periphs_groups, @@ -746,6 +747,7 @@ struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = { struct meson_pinctrl_data meson_gxbb_aobus_pinctrl_data = { .name = "aobus-banks", + .irq_compat = "amlogic,meson-gxbb-gpio-intc", .pin_base = 0, .pins = meson_gxbb_aobus_pins, .groups = meson_gxbb_aobus_groups, diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h index 98b5080650c1..b90d69e366df 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.h +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -81,6 +81,7 @@ enum meson_reg_type { * @name: bank name * @first: first pin of the bank * @last: last pin of the bank + * @irq: hwirq base number of the bank * @regs: array of register descriptors * * A bank represents a set of pins controlled by a contiguous set of @@ -92,11 +93,14 @@ struct meson_bank { const char *name; unsigned int first; unsigned int last; + int irq_first; + int irq_last; struct meson_reg_desc regs[NUM_REG]; }; struct meson_pinctrl_data { const char *name; + const char *irq_compat; const struct pinctrl_pin_desc *pins; struct meson_pmx_group *groups; struct meson_pmx_func *funcs; @@ -147,12 +151,14 @@ struct meson_pinctrl { .num_groups = ARRAY_SIZE(fn ## _groups), \ } -#define BANK(n, f, l, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ +#define BANK(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ { \ - .name = n, \ - .first = f, \ - .last = l, \ - .regs = { \ + .name = n, \ + .first = f, \ + .last = l, \ + .irq_first = fi, \ + .irq_last = li, \ + .regs = { \ [REG_PULLEN] = { per, peb }, \ [REG_PULL] = { pr, pb }, \ [REG_DIR] = { dr, db }, \ diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c index 07f1cb21c1b8..eddab1091408 100644 --- a/drivers/pinctrl/meson/pinctrl-meson8.c +++ b/drivers/pinctrl/meson/pinctrl-meson8.c @@ -916,23 +916,24 @@ static struct meson_pmx_func meson8_aobus_functions[] = { }; static struct meson_bank meson8_cbus_banks[] = { - /* name first last pullen pull dir out in */ - BANK("X", PIN(GPIOX_0, 0), PIN(GPIOX_21, 0), 4, 0, 4, 0, 0, 0, 1, 0, 2, 0), - BANK("Y", PIN(GPIOY_0, 0), PIN(GPIOY_16, 0), 3, 0, 3, 0, 3, 0, 4, 0, 5, 0), - BANK("DV", PIN(GPIODV_0, 0), PIN(GPIODV_29, 0), 0, 0, 0, 0, 7, 0, 8, 0, 9, 0), - BANK("H", PIN(GPIOH_0, 0), PIN(GPIOH_9, 0), 1, 16, 1, 16, 9, 19, 10, 19, 11, 19), - BANK("Z", PIN(GPIOZ_0, 0), PIN(GPIOZ_14, 0), 1, 0, 1, 0, 3, 17, 4, 17, 5, 17), - BANK("CARD", PIN(CARD_0, 0), PIN(CARD_6, 0), 2, 20, 2, 20, 0, 22, 1, 22, 2, 22), - BANK("BOOT", PIN(BOOT_0, 0), PIN(BOOT_18, 0), 2, 0, 2, 0, 9, 0, 10, 0, 11, 0), + /* name first last irq pullen pull dir out in */ + BANK("X", PIN(GPIOX_0, 0), PIN(GPIOX_21, 0), 112, 133, 4, 0, 4, 0, 0, 0, 1, 0, 2, 0), + BANK("Y", PIN(GPIOY_0, 0), PIN(GPIOY_16, 0), 95, 111, 3, 0, 3, 0, 3, 0, 4, 0, 5, 0), + BANK("DV", PIN(GPIODV_0, 0), PIN(GPIODV_29, 0), 65, 94, 0, 0, 0, 0, 7, 0, 8, 0, 9, 0), + BANK("H", PIN(GPIOH_0, 0), PIN(GPIOH_9, 0), 29, 38, 1, 16, 1, 16, 9, 19, 10, 19, 11, 19), + BANK("Z", PIN(GPIOZ_0, 0), PIN(GPIOZ_14, 0), 14, 28, 1, 0, 1, 0, 3, 17, 4, 17, 5, 17), + BANK("CARD", PIN(CARD_0, 0), PIN(CARD_6, 0), 58, 64, 2, 20, 2, 20, 0, 22, 1, 22, 2, 22), + BANK("BOOT", PIN(BOOT_0, 0), PIN(BOOT_18, 0), 39, 57, 2, 0, 2, 0, 9, 0, 10, 0, 11, 0), }; static struct meson_bank meson8_aobus_banks[] = { - /* name first last pullen pull dir out in */ - BANK("AO", PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), + /* name first last irq pullen pull dir out in */ + BANK("AO", PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0, 13, 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), }; struct meson_pinctrl_data meson8_cbus_pinctrl_data = { .name = "cbus-banks", + .irq_compat = "amlogic,meson8-gpio-intc", .pin_base = 0, .pins = meson8_cbus_pins, .groups = meson8_cbus_groups, @@ -946,6 +947,7 @@ struct meson_pinctrl_data meson8_cbus_pinctrl_data = { struct meson_pinctrl_data meson8_aobus_pinctrl_data = { .name = "ao-bank", + .irq_compat = "amlogic,meson8-gpio-intc", .pin_base = 120, .pins = meson8_aobus_pins, .groups = meson8_aobus_groups, diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c index 76f077f18193..d7505f492639 100644 --- a/drivers/pinctrl/meson/pinctrl-meson8b.c +++ b/drivers/pinctrl/meson/pinctrl-meson8b.c @@ -124,6 +124,12 @@ static const struct pinctrl_pin_desc meson8b_aobus_pins[] = { MESON_PIN(GPIOAO_11, AO_OFF), MESON_PIN(GPIOAO_12, AO_OFF), MESON_PIN(GPIOAO_13, AO_OFF), + + /* + * The following 2 pins are not mentionned in the public datasheet + * According to this datasheet, they can't be used with the gpio + * interrupt controller + */ MESON_PIN(GPIO_BSD_EN, AO_OFF), MESON_PIN(GPIO_TEST_N, AO_OFF), }; @@ -881,23 +887,30 @@ static struct meson_pmx_func meson8b_aobus_functions[] = { }; static struct meson_bank meson8b_cbus_banks[] = { - /* name first last pullen pull dir out in */ - BANK("X", PIN(GPIOX_0, 0), PIN(GPIOX_21, 0), 4, 0, 4, 0, 0, 0, 1, 0, 2, 0), - BANK("Y", PIN(GPIOY_0, 0), PIN(GPIOY_14, 0), 3, 0, 3, 0, 3, 0, 4, 0, 5, 0), - BANK("DV", PIN(GPIODV_9, 0), PIN(GPIODV_29, 0), 0, 0, 0, 0, 7, 0, 8, 0, 9, 0), - BANK("H", PIN(GPIOH_0, 0), PIN(GPIOH_9, 0), 1, 16, 1, 16, 9, 19, 10, 19, 11, 19), - BANK("CARD", PIN(CARD_0, 0), PIN(CARD_6, 0), 2, 20, 2, 20, 0, 22, 1, 22, 2, 22), - BANK("BOOT", PIN(BOOT_0, 0), PIN(BOOT_18, 0), 2, 0, 2, 0, 9, 0, 10, 0, 11, 0), - BANK("DIF", PIN(DIF_0_P, 0), PIN(DIF_4_N, 0), 5, 8, 5, 8, 12, 12, 13, 12, 14, 12), + /* name first last irq pullen pull dir out in */ + BANK("X", PIN(GPIOX_0, 0), PIN(GPIOX_21, 0), 97, 118, 4, 0, 4, 0, 0, 0, 1, 0, 2, 0), + BANK("Y", PIN(GPIOY_0, 0), PIN(GPIOY_14, 0), 80, 96, 3, 0, 3, 0, 3, 0, 4, 0, 5, 0), + BANK("DV", PIN(GPIODV_9, 0), PIN(GPIODV_29, 0), 59, 79, 0, 0, 0, 0, 7, 0, 8, 0, 9, 0), + BANK("H", PIN(GPIOH_0, 0), PIN(GPIOH_9, 0), 14, 23, 1, 16, 1, 16, 9, 19, 10, 19, 11, 19), + BANK("CARD", PIN(CARD_0, 0), PIN(CARD_6, 0), 43, 49, 2, 20, 2, 20, 0, 22, 1, 22, 2, 22), + BANK("BOOT", PIN(BOOT_0, 0), PIN(BOOT_18, 0), 24, 42, 2, 0, 2, 0, 9, 0, 10, 0, 11, 0), + + /* + * The following bank is not mentionned in the public datasheet + * There is no information whether it can be used with the gpio + * interrupt controller + */ + BANK("DIF", PIN(DIF_0_P, 0), PIN(DIF_4_N, 0), -1, -1, 5, 8, 5, 8, 12, 12, 13, 12, 14, 12), }; static struct meson_bank meson8b_aobus_banks[] = { - /* name first last pullen pull dir out in */ - BANK("AO", PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), + /* name first last irq pullen pull dir out in */ + BANK("AO", PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0, 13, 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), }; struct meson_pinctrl_data meson8b_cbus_pinctrl_data = { .name = "cbus-banks", + .irq_compat = "amlogic,meson8b-gpio-intc", .pin_base = 0, .pins = meson8b_cbus_pins, .groups = meson8b_cbus_groups, @@ -911,6 +924,7 @@ struct meson_pinctrl_data meson8b_cbus_pinctrl_data = { struct meson_pinctrl_data meson8b_aobus_pinctrl_data = { .name = "aobus-banks", + .irq_compat = "amlogic,meson8b-gpio-intc", .pin_base = 130, .pins = meson8b_aobus_pins, .groups = meson8b_aobus_groups, -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/9] pinctrl: meson: allow gpio to request irq 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (2 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 3/9] pinctrl: meson: update pinctrl data with gpio irq data Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-19 15:37 ` [RESEND PATCH " Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 5/9] dt-bindings: pinctrl: meson: update gpio dt-bindings Jerome Brunet ` (4 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Linus Walleij Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Catalin Marinas, Will Deacon, Russell King Add the ability for gpio to request irq from the gpio interrupt controller if present. We have to specificaly that the parent interrupt controller is the gpio interrupt controller because gpio on meson SoCs can't generate interrupt directly on the GIC. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/pinctrl/Kconfig | 2 + drivers/pinctrl/meson/pinctrl-meson.c | 77 ++++++++++++++++++++++++++++++++++- drivers/pinctrl/meson/pinctrl-meson.h | 1 + 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94972ba..d5bfbfcddab0 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -126,7 +126,9 @@ config PINCTRL_MESON select PINCONF select GENERIC_PINCONF select GPIOLIB + select IRQ_DOMAIN select OF_GPIO + select OF_IRQ select REGMAP_MMIO config PINCTRL_OXNAS diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c index 57122eda155a..fd3c1d44978b 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.c +++ b/drivers/pinctrl/meson/pinctrl-meson.c @@ -50,6 +50,7 @@ #include <linux/io.h> #include <linux/of.h> #include <linux/of_address.h> +#include <linux/of_irq.h> #include <linux/pinctrl/pinconf-generic.h> #include <linux/pinctrl/pinconf.h> #include <linux/pinctrl/pinctrl.h> @@ -481,6 +482,58 @@ static void meson_gpio_set(struct gpio_chip *chip, unsigned gpio, int value) value ? BIT(bit) : 0); } +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned int offset) +{ + unsigned int hwirq; + + if (bank->irq_first < 0) + /* this bank cannot generate irqs */ + return -1; + + hwirq = offset - bank->first + bank->irq_first; + + if (hwirq > bank->irq_last) + /* this pin cannot generate irqs */ + return -1; + + return hwirq; +} + +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) +{ + struct meson_pinctrl *pc = gpiochip_get_data(chip); + struct meson_bank *bank; + struct irq_fwspec fwspec; + unsigned int hwirq; + int ret; + + ret = meson_get_bank(pc, offset, &bank); + if (ret) + return ret; + + /* + * The interrupt controller might be missing, in such case we can't + * provide an interrupt for a pin + */ + if (is_fwnode_irqchip(pc->fwnode)) { + dev_info(pc->dev, "interrupt controller not found\n"); + return 0; + } + + hwirq = meson_gpio_to_hwirq(bank, offset); + if (hwirq < 0) { + dev_dbg(pc->dev, "no interrupt for pin %u\n", offset); + return 0; + } + + fwspec.fwnode = pc->fwnode; + fwspec.param_count = 2; + fwspec.param[0] = hwirq; + fwspec.param[1] = IRQ_TYPE_NONE; + + return irq_create_fwspec_mapping(&fwspec); +} + static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) { struct meson_pinctrl *pc = gpiochip_get_data(chip); @@ -539,6 +592,7 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc) pc->chip.direction_output = meson_gpio_direction_output; pc->chip.get = meson_gpio_get; pc->chip.set = meson_gpio_set; + pc->chip.to_irq = meson_gpio_to_irq; pc->chip.base = pc->data->pin_base; pc->chip.ngpio = pc->data->num_pins; pc->chip.can_sleep = false; @@ -598,6 +652,27 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc, return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config); } +static int meson_pinctrl_get_irq_gpio_intc(struct meson_pinctrl *pc, + struct device_node *node) +{ + struct device_node *np; + + np = of_irq_find_parent(node); + if (unlikely(!np)) { + dev_err(pc->dev, "interrupt parent not found\n"); + return -EINVAL; + } + + if (!of_device_is_compatible(np, pc->data->irq_compat)) { + dev_info(pc->dev, "gpio interrupt disabled\n"); + pc->fwnode = NULL; + } + + pc->fwnode = of_node_to_fwnode(np); + + return 0; +} + static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc, struct device_node *node) { @@ -643,7 +718,7 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc, return PTR_ERR(pc->reg_gpio); } - return 0; + return meson_pinctrl_get_irq_gpio_intc(pc, gpio_np); } static int meson_pinctrl_probe(struct platform_device *pdev) diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h index b90d69e366df..2e6c83adbd1f 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.h +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -123,6 +123,7 @@ struct meson_pinctrl { struct regmap *reg_gpio; struct gpio_chip chip; struct device_node *of_node; + struct fwnode_handle *fwnode; }; #define PIN(x, b) (b + x) -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RESEND PATCH v2 4/9] pinctrl: meson: allow gpio to request irq 2016-10-19 15:21 ` [PATCH v2 4/9] pinctrl: meson: allow gpio to request irq Jerome Brunet @ 2016-10-19 15:37 ` Jerome Brunet 0 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:37 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Linus Walleij Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Catalin Marinas, Will Deacon, Russell King Add the ability for gpio to request irq from the gpio interrupt controller if present. We have to specificaly that the parent interrupt controller is the gpio interrupt controller because gpio on meson SoCs can't generate interrupt directly on the GIC. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- I messed up in v2 and actually sent the v1 again. Here is the actual v2 with the fix. Again, sorry for the inconvenience. drivers/pinctrl/Kconfig | 2 + drivers/pinctrl/meson/pinctrl-meson.c | 69 +++++++++++++++++++++++++++++++++++ drivers/pinctrl/meson/pinctrl-meson.h | 1 + 3 files changed, 72 insertions(+) diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94972ba..d5bfbfcddab0 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -126,7 +126,9 @@ config PINCTRL_MESON select PINCONF select GENERIC_PINCONF select GPIOLIB + select IRQ_DOMAIN select OF_GPIO + select OF_IRQ select REGMAP_MMIO config PINCTRL_OXNAS diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c index 57122eda155a..e3f5241f337f 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.c +++ b/drivers/pinctrl/meson/pinctrl-meson.c @@ -50,6 +50,7 @@ #include <linux/io.h> #include <linux/of.h> #include <linux/of_address.h> +#include <linux/of_irq.h> #include <linux/pinctrl/pinconf-generic.h> #include <linux/pinctrl/pinconf.h> #include <linux/pinctrl/pinctrl.h> @@ -481,6 +482,58 @@ static void meson_gpio_set(struct gpio_chip *chip, unsigned gpio, int value) value ? BIT(bit) : 0); } +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned int offset) +{ + unsigned int hwirq; + + if (bank->irq_first < 0) + /* this bank cannot generate irqs */ + return -1; + + hwirq = offset - bank->first + bank->irq_first; + + if (hwirq > bank->irq_last) + /* this pin cannot generate irqs */ + return -1; + + return hwirq; +} + +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) +{ + struct meson_pinctrl *pc = gpiochip_get_data(chip); + struct meson_bank *bank; + struct irq_fwspec fwspec; + unsigned int hwirq; + int ret; + + ret = meson_get_bank(pc, offset, &bank); + if (ret) + return ret; + + /* + * The interrupt controller might be missing, in such case we can't + * provide an interrupt for a pin + */ + if (is_fwnode_irqchip(pc->fwnode)) { + dev_info(pc->dev, "interrupt controller not found\n"); + return 0; + } + + hwirq = meson_gpio_to_hwirq(bank, offset); + if (hwirq < 0) { + dev_dbg(pc->dev, "no interrupt for pin %u\n", offset); + return 0; + } + + fwspec.fwnode = pc->fwnode; + fwspec.param_count = 2; + fwspec.param[0] = hwirq; + fwspec.param[1] = IRQ_TYPE_NONE; + + return irq_create_fwspec_mapping(&fwspec); +} + static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) { struct meson_pinctrl *pc = gpiochip_get_data(chip); @@ -539,6 +592,7 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc) pc->chip.direction_output = meson_gpio_direction_output; pc->chip.get = meson_gpio_get; pc->chip.set = meson_gpio_set; + pc->chip.to_irq = meson_gpio_to_irq; pc->chip.base = pc->data->pin_base; pc->chip.ngpio = pc->data->num_pins; pc->chip.can_sleep = false; @@ -598,6 +652,19 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc, return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config); } +static void meson_pinctrl_get_irq_gpio_intc(struct meson_pinctrl *pc, + struct device_node *node) +{ + struct device_node *np = of_irq_find_parent(node); + + if (!np || !of_device_is_compatible(np, pc->data->irq_compat)) { + dev_info(pc->dev, "gpio interrupt disabled\n"); + pc->fwnode = NULL; + } else { + pc->fwnode = of_node_to_fwnode(np); + } +} + static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc, struct device_node *node) { @@ -643,6 +710,8 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc, return PTR_ERR(pc->reg_gpio); } + meson_pinctrl_get_irq_gpio_intc(pc, gpio_np); + return 0; } diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h index b90d69e366df..2e6c83adbd1f 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.h +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -123,6 +123,7 @@ struct meson_pinctrl { struct regmap *reg_gpio; struct gpio_chip chip; struct device_node *of_node; + struct fwnode_handle *fwnode; }; #define PIN(x, b) (b + x) -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/9] dt-bindings: pinctrl: meson: update gpio dt-bindings 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (3 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 4/9] pinctrl: meson: allow gpio to request irq Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig Jerome Brunet ` (3 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Rob Herring Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Linus Walleij, Catalin Marinas, Will Deacon, Russell King Add description for the interrupt-parent property of the gpio sub-node If provided here, this property must be a phandle to an interrupt controller suitable for meson pinctrl, like the meson gpio interrupt controller. Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt index fe7fe0b03cfb..39932c4dfb32 100644 --- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt @@ -23,6 +23,10 @@ Required properties for sub-nodes are: - gpio-controller: identifies the node as a gpio controller - #gpio-cells: must be 2 +Optional property for sub-nodes is: + - interrupt-parent: must be a phandle to the meson gpio interrupt controller. + if this property is provided, enables gpio ability to generate interrupts + === Other sub-nodes === Child nodes without the "gpio-controller" represent some desired -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (4 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 5/9] dt-bindings: pinctrl: meson: update gpio dt-bindings Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-20 16:34 ` Marc Zyngier 2016-10-19 15:21 ` [PATCH v2 7/9] ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 Jerome Brunet ` (2 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Catalin Marinas, Will Deacon Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Russell King, Linus Walleij Add select MESON_IRQ_GPIO in Kconfig for Amlogic's meson SoC family Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- arch/arm64/Kconfig.platforms | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index cfbdf02ef566..846479d4492d 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -95,6 +95,7 @@ config ARCH_MESON select PINCTRL_MESON select COMMON_CLK_AMLOGIC select COMMON_CLK_GXBB + select MESON_GPIO_IRQ help This enables support for the Amlogic S905 SoCs. -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig 2016-10-19 15:21 ` [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig Jerome Brunet @ 2016-10-20 16:34 ` Marc Zyngier 0 siblings, 0 replies; 20+ messages in thread From: Marc Zyngier @ 2016-10-20 16:34 UTC (permalink / raw) To: Jerome Brunet, Carlo Caione, Kevin Hilman, Catalin Marinas, Will Deacon Cc: linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Rob Herring, Russell King, Linus Walleij On 19/10/16 16:21, Jerome Brunet wrote: > Add select MESON_IRQ_GPIO in Kconfig for Amlogic's meson SoC family > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > arch/arm64/Kconfig.platforms | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index cfbdf02ef566..846479d4492d 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -95,6 +95,7 @@ config ARCH_MESON > select PINCTRL_MESON > select COMMON_CLK_AMLOGIC > select COMMON_CLK_GXBB > + select MESON_GPIO_IRQ MESON_IRQ_GPIO? > help > This enables support for the Amlogic S905 SoCs. > > -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 7/9] ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (5 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 8/9] ARM64: dts: amlogic: enable gpio interrupt controller on gxbb Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 9/9] ARM: dts: amlogic: enable gpio interrupt controller on meson8 Jerome Brunet 8 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman, Russell King Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Catalin Marinas, Will Deacon, Linus Walleij Add select MESON_IRQ_GPIO in Kconfig for Amlogic's meson8 and meson8b SoC Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- arch/arm/mach-meson/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/mach-meson/Kconfig b/arch/arm/mach-meson/Kconfig index b6e3acc63e14..63157295cd9d 100644 --- a/arch/arm/mach-meson/Kconfig +++ b/arch/arm/mach-meson/Kconfig @@ -21,11 +21,13 @@ config MACH_MESON8 bool "Amlogic Meson8 SoCs support" default ARCH_MESON select MESON6_TIMER + select MESON_IRQ_GPIO config MACH_MESON8B bool "Amlogic Meson8b SoCs support" default ARCH_MESON select MESON6_TIMER select COMMON_CLK_MESON8B + select MESON_IRQ_GPIO endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 8/9] ARM64: dts: amlogic: enable gpio interrupt controller on gxbb 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (6 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 7/9] ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 9/9] ARM: dts: amlogic: enable gpio interrupt controller on meson8 Jerome Brunet 8 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Russell King, Linus Walleij, Catalin Marinas, Will Deacon Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index aad639ab0112..5208cb80b55e 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi @@ -141,6 +141,13 @@ #reset-cells = <1>; }; + gpio_interrupt: interrupt-controller@9880 { + compatible = "amlogic,meson-gxbb-gpio-intc"; + reg = <0x0 0x9880 0x0 0x10>; + interrupt-controller; + #interrupt-cells = <2>; + }; + uart_B: serial@84dc { compatible = "amlogic,meson-uart"; reg = <0x0 0x84dc 0x0 0x14>; @@ -238,6 +245,7 @@ reg-names = "mux", "pull", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; uart_ao_a_pins: uart_ao_a { @@ -343,6 +351,7 @@ reg-names = "mux", "pull", "pull-enable", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; emmc_pins: emmc { -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 9/9] ARM: dts: amlogic: enable gpio interrupt controller on meson8 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet ` (7 preceding siblings ...) 2016-10-19 15:21 ` [PATCH v2 8/9] ARM64: dts: amlogic: enable gpio interrupt controller on gxbb Jerome Brunet @ 2016-10-19 15:21 ` Jerome Brunet 8 siblings, 0 replies; 20+ messages in thread From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw) To: Carlo Caione, Kevin Hilman Cc: Jerome Brunet, linux-amlogic, linux-arm-kernel, linux-gpio, linux-kernel, devicetree, Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Russell King, Linus Walleij, Catalin Marinas, Will Deacon Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- arch/arm/boot/dts/meson8.dtsi | 11 +++++++++++ arch/arm/boot/dts/meson8b.dtsi | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi index 45619f6162c5..713a22456ff1 100644 --- a/arch/arm/boot/dts/meson8.dtsi +++ b/arch/arm/boot/dts/meson8.dtsi @@ -43,6 +43,8 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/gpio/meson8-gpio.h> /include/ "meson.dtsi" @@ -91,6 +93,13 @@ clock-frequency = <141666666>; }; + gpio_interrupt: interrupt-controller@c1109880 { + compatible = "amlogic,meson8-gpio-intc"; + reg = <0xc1109880 0x10>; + interrupt-controller; + #interrupt-cells = <2>; + }; + pinctrl_cbus: pinctrl@c1109880 { compatible = "amlogic,meson8-cbus-pinctrl"; reg = <0xc1109880 0x10>; @@ -106,6 +115,7 @@ reg-names = "mux", "pull", "pull-enable", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; spi_nor_pins: nor { @@ -148,6 +158,7 @@ reg-names = "mux", "pull", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; uart_ao_a_pins: uart_ao_a { diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi index 41fd53671859..36a239a645f5 100644 --- a/arch/arm/boot/dts/meson8b.dtsi +++ b/arch/arm/boot/dts/meson8b.dtsi @@ -44,6 +44,8 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/clock/meson8b-clkc.h> #include <dt-bindings/gpio/meson8b-gpio.h> #include <dt-bindings/reset/amlogic,meson8b-reset.h> @@ -183,6 +185,13 @@ status = "disabled"; }; + gpio_interrupt: interrupt-controller@c1109880 { + compatible = "amlogic,meson8b-gpio-intc"; + reg = <0xc1109880 0x10>; + interrupt-controller; + #interrupt-cells = <2>; + }; + pinctrl_cbus: pinctrl@c1109880 { compatible = "amlogic,meson8b-cbus-pinctrl"; reg = <0xc1109880 0x10>; @@ -198,6 +207,7 @@ reg-names = "mux", "pull", "pull-enable", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; }; @@ -215,6 +225,7 @@ reg-names = "mux", "pull", "gpio"; gpio-controller; #gpio-cells = <2>; + interrupt-parent = <&gpio_interrupt>; }; uart_ao_a_pins: uart_ao_a { -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-10-27 15:01 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-19 15:21 [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 1/9] irqchip: meson: add support for " Jerome Brunet 2016-10-20 16:33 ` Marc Zyngier 2016-10-21 8:49 ` Jerome Brunet 2016-10-21 10:10 ` Mark Rutland 2016-10-21 10:17 ` Jerome Brunet 2016-10-21 10:28 ` Marc Zyngier 2016-10-19 15:21 ` [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO " Jerome Brunet 2016-10-26 21:42 ` Rob Herring 2016-10-27 9:32 ` Mark Rutland 2016-10-27 9:40 ` Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 3/9] pinctrl: meson: update pinctrl data with gpio irq data Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 4/9] pinctrl: meson: allow gpio to request irq Jerome Brunet 2016-10-19 15:37 ` [RESEND PATCH " Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 5/9] dt-bindings: pinctrl: meson: update gpio dt-bindings Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig Jerome Brunet 2016-10-20 16:34 ` Marc Zyngier 2016-10-19 15:21 ` [PATCH v2 7/9] ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 8/9] ARM64: dts: amlogic: enable gpio interrupt controller on gxbb Jerome Brunet 2016-10-19 15:21 ` [PATCH v2 9/9] ARM: dts: amlogic: enable gpio interrupt controller on meson8 Jerome Brunet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).