* [PATCH 1/2] irq: Add dummy irq_domain_add_simple_when not defined @ 2011-11-30 1:32 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 1:32 UTC (permalink / raw) To: Olof Johansson, Colin Cross Cc: Peter De Schrijver, Rob Herring, Grant Likely, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-tegra-u79uwXL29TY76Z2rM5mHXA, Stephen Warren This prevents users of this API from having to ifdef it. Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- include/linux/irqdomain.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index 99834e5..77b1751 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -99,6 +99,8 @@ extern void irq_domain_add_simple(struct device_node *controller, int irq_base); extern void irq_domain_generate_simple(const struct of_device_id *match, u64 phys_base, unsigned int irq_start); #else /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */ +static inline void irq_domain_add_simple(struct device_node *controller, + int irq_base) { } static inline void irq_domain_generate_simple(const struct of_device_id *match, u64 phys_base, unsigned int irq_start) { } #endif /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] irq: Add dummy irq_domain_add_simple_when not defined @ 2011-11-30 1:32 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 1:32 UTC (permalink / raw) To: linux-arm-kernel This prevents users of this API from having to ifdef it. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- include/linux/irqdomain.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index 99834e5..77b1751 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -99,6 +99,8 @@ extern void irq_domain_add_simple(struct device_node *controller, int irq_base); extern void irq_domain_generate_simple(const struct of_device_id *match, u64 phys_base, unsigned int irq_start); #else /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */ +static inline void irq_domain_add_simple(struct device_node *controller, + int irq_base) { } static inline void irq_domain_generate_simple(const struct of_device_id *match, u64 phys_base, unsigned int irq_start) { } #endif /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1322616743-31985-1-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller 2011-11-30 1:32 ` Stephen Warren @ 2011-11-30 1:32 ` Stephen Warren -1 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 1:32 UTC (permalink / raw) To: Olof Johansson, Colin Cross Cc: Peter De Schrijver, Rob Herring, Grant Likely, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-tegra-u79uwXL29TY76Z2rM5mHXA, Stephen Warren Fix the DT binding documentation to describe interrupt-related properties, and the contents of "child" node interrupts property. Update tegra20.dtsi to specify the required interrupt-related properties. Fix the driver to creating an IRQ domain for itself, so that child node interrupts properties are correctly parsed. Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- This patch depends on "arm/tegra: convert tegra20 to GIC devicetree binding", at least for context, and probably functionally (and of course patch 1 in this series). I've tested that this patch causes the WM8903 to receive a valid IRQ when instantiated from device-tree (although the patches to instantiate the WM8903 from DT are local to me for now). .../devicetree/bindings/gpio/gpio_nvidia.txt | 10 ++++++++++ arch/arm/boot/dts/tegra20.dtsi | 2 ++ drivers/gpio/gpio-tegra.c | 4 ++++ 3 files changed, 16 insertions(+), 0 deletions(-) diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt index eb4b530..ecdf19c 100644 --- a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +++ b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt @@ -6,3 +6,13 @@ Required properties: second cell is used to specify optional parameters: - bit 0 specifies polarity (0 for normal, 1 for inverted) - gpio-controller : Marks the device node as a GPIO controller. +- #interrupt-cells : Should be 3. + The first cell is the GPIO number. + The second cell is used to specify flags: + bits[3:0] trigger type and level flags: + 1 = low-to-high edge triggered. + 2 = high-to-low edge triggered. + 4 = active high level-sensitive. + 8 = active low level-sensitive. + Valid combinations are 1, 2, 3, 4, 8. +- interrupt-controller : Marks the device node as an interrupt controller. diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi index db6f562..e25f4a6 100644 --- a/arch/arm/boot/dts/tegra20.dtsi +++ b/arch/arm/boot/dts/tegra20.dtsi @@ -79,6 +79,8 @@ 0 55 0x04 0 87 0x04 0 89 0x04 >; + interrupt-controller; + #interrupt-cells = <2>; #gpio-cells = <2>; gpio-controller; }; diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 61044c8..26b0c85 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -25,6 +25,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> +#include <linux/irqdomain.h> #include <asm/mach/irq.h> @@ -410,6 +411,9 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) spin_lock_init(&bank->lvl_lock[j]); } + if (pdev->dev.of_node) + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); + return 0; } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller @ 2011-11-30 1:32 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 1:32 UTC (permalink / raw) To: linux-arm-kernel Fix the DT binding documentation to describe interrupt-related properties, and the contents of "child" node interrupts property. Update tegra20.dtsi to specify the required interrupt-related properties. Fix the driver to creating an IRQ domain for itself, so that child node interrupts properties are correctly parsed. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- This patch depends on "arm/tegra: convert tegra20 to GIC devicetree binding", at least for context, and probably functionally (and of course patch 1 in this series). I've tested that this patch causes the WM8903 to receive a valid IRQ when instantiated from device-tree (although the patches to instantiate the WM8903 from DT are local to me for now). .../devicetree/bindings/gpio/gpio_nvidia.txt | 10 ++++++++++ arch/arm/boot/dts/tegra20.dtsi | 2 ++ drivers/gpio/gpio-tegra.c | 4 ++++ 3 files changed, 16 insertions(+), 0 deletions(-) diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt index eb4b530..ecdf19c 100644 --- a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +++ b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt @@ -6,3 +6,13 @@ Required properties: second cell is used to specify optional parameters: - bit 0 specifies polarity (0 for normal, 1 for inverted) - gpio-controller : Marks the device node as a GPIO controller. +- #interrupt-cells : Should be 3. + The first cell is the GPIO number. + The second cell is used to specify flags: + bits[3:0] trigger type and level flags: + 1 = low-to-high edge triggered. + 2 = high-to-low edge triggered. + 4 = active high level-sensitive. + 8 = active low level-sensitive. + Valid combinations are 1, 2, 3, 4, 8. +- interrupt-controller : Marks the device node as an interrupt controller. diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi index db6f562..e25f4a6 100644 --- a/arch/arm/boot/dts/tegra20.dtsi +++ b/arch/arm/boot/dts/tegra20.dtsi @@ -79,6 +79,8 @@ 0 55 0x04 0 87 0x04 0 89 0x04 >; + interrupt-controller; + #interrupt-cells = <2>; #gpio-cells = <2>; gpio-controller; }; diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 61044c8..26b0c85 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -25,6 +25,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> +#include <linux/irqdomain.h> #include <asm/mach/irq.h> @@ -410,6 +411,9 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) spin_lock_init(&bank->lvl_lock[j]); } + if (pdev->dev.of_node) + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); + return 0; } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1322616743-31985-2-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller 2011-11-30 1:32 ` Stephen Warren @ 2011-11-30 3:28 ` Rob Herring -1 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2011-11-30 3:28 UTC (permalink / raw) To: Stephen Warren Cc: Olof Johansson, Colin Cross, Peter De Schrijver, Grant Likely, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-tegra-u79uwXL29TY76Z2rM5mHXA On 11/29/2011 07:32 PM, Stephen Warren wrote: > Fix the DT binding documentation to describe interrupt-related properties, > and the contents of "child" node interrupts property. > > Update tegra20.dtsi to specify the required interrupt-related properties. > > Fix the driver to creating an IRQ domain for itself, so that child node > interrupts properties are correctly parsed. > > Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > This patch depends on "arm/tegra: convert tegra20 to GIC devicetree binding", > at least for context, and probably functionally (and of course patch 1 in > this series). > > I've tested that this patch causes the WM8903 to receive a valid IRQ when > instantiated from device-tree (although the patches to instantiate the > WM8903 from DT are local to me for now). > > .../devicetree/bindings/gpio/gpio_nvidia.txt | 10 ++++++++++ > arch/arm/boot/dts/tegra20.dtsi | 2 ++ > drivers/gpio/gpio-tegra.c | 4 ++++ > 3 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > index eb4b530..ecdf19c 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > @@ -6,3 +6,13 @@ Required properties: > second cell is used to specify optional parameters: > - bit 0 specifies polarity (0 for normal, 1 for inverted) > - gpio-controller : Marks the device node as a GPIO controller. > +- #interrupt-cells : Should be 3. Should be 2?? > + The first cell is the GPIO number. > + The second cell is used to specify flags: > + bits[3:0] trigger type and level flags: > + 1 = low-to-high edge triggered. > + 2 = high-to-low edge triggered. > + 4 = active high level-sensitive. > + 8 = active low level-sensitive. > + Valid combinations are 1, 2, 3, 4, 8. > +- interrupt-controller : Marks the device node as an interrupt controller. > diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi > index db6f562..e25f4a6 100644 > --- a/arch/arm/boot/dts/tegra20.dtsi > +++ b/arch/arm/boot/dts/tegra20.dtsi > @@ -79,6 +79,8 @@ > 0 55 0x04 > 0 87 0x04 > 0 89 0x04 >; > + interrupt-controller; > + #interrupt-cells = <2>; > #gpio-cells = <2>; > gpio-controller; > }; > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > index 61044c8..26b0c85 100644 > --- a/drivers/gpio/gpio-tegra.c > +++ b/drivers/gpio/gpio-tegra.c > @@ -25,6 +25,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/module.h> > +#include <linux/irqdomain.h> > > #include <asm/mach/irq.h> > > @@ -410,6 +411,9 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) > spin_lock_init(&bank->lvl_lock[j]); > } > > + if (pdev->dev.of_node) > + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); > + This is a bit of a temporary solution. Really, you want to end up with something like this to eliminate INT_GPIO_BASE and dynamically assign the irqbase: diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 61044c8..a146a20 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -139,7 +139,7 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset, static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { - return TEGRA_GPIO_TO_IRQ(offset); + return irq_domain_to_irq(chip->domain, offset); >>> This is a bit wrong. You need a tegra_gpio_chip containing a domain and gpio_chip. } static struct gpio_chip tegra_gpio_chip = { @@ -155,28 +155,28 @@ static struct gpio_chip tegra_gpio_chip = { static void tegra_gpio_irq_ack(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_writel(1 << GPIO_BIT(gpio), GPIO_INT_CLR(gpio)); } static void tegra_gpio_irq_mask(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_mask_write(GPIO_MSK_INT_ENB(gpio), gpio, 0); } static void tegra_gpio_irq_unmask(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_mask_write(GPIO_MSK_INT_ENB(gpio), gpio, 1); } static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d); int port = GPIO_PORT(gpio); int lvl_type; @@ -385,6 +385,7 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) tegra_gpio_chip.of_node = pdev->dev.of_node; #endif + domain.irq_base = irq_alloc_descs(...) gpiochip_add(&tegra_gpio_chip); for (gpio = 0; gpio < TEGRA_NR_GPIOS; gpio++) { Rob ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller @ 2011-11-30 3:28 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2011-11-30 3:28 UTC (permalink / raw) To: linux-arm-kernel On 11/29/2011 07:32 PM, Stephen Warren wrote: > Fix the DT binding documentation to describe interrupt-related properties, > and the contents of "child" node interrupts property. > > Update tegra20.dtsi to specify the required interrupt-related properties. > > Fix the driver to creating an IRQ domain for itself, so that child node > interrupts properties are correctly parsed. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > This patch depends on "arm/tegra: convert tegra20 to GIC devicetree binding", > at least for context, and probably functionally (and of course patch 1 in > this series). > > I've tested that this patch causes the WM8903 to receive a valid IRQ when > instantiated from device-tree (although the patches to instantiate the > WM8903 from DT are local to me for now). > > .../devicetree/bindings/gpio/gpio_nvidia.txt | 10 ++++++++++ > arch/arm/boot/dts/tegra20.dtsi | 2 ++ > drivers/gpio/gpio-tegra.c | 4 ++++ > 3 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > index eb4b530..ecdf19c 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > @@ -6,3 +6,13 @@ Required properties: > second cell is used to specify optional parameters: > - bit 0 specifies polarity (0 for normal, 1 for inverted) > - gpio-controller : Marks the device node as a GPIO controller. > +- #interrupt-cells : Should be 3. Should be 2?? > + The first cell is the GPIO number. > + The second cell is used to specify flags: > + bits[3:0] trigger type and level flags: > + 1 = low-to-high edge triggered. > + 2 = high-to-low edge triggered. > + 4 = active high level-sensitive. > + 8 = active low level-sensitive. > + Valid combinations are 1, 2, 3, 4, 8. > +- interrupt-controller : Marks the device node as an interrupt controller. > diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi > index db6f562..e25f4a6 100644 > --- a/arch/arm/boot/dts/tegra20.dtsi > +++ b/arch/arm/boot/dts/tegra20.dtsi > @@ -79,6 +79,8 @@ > 0 55 0x04 > 0 87 0x04 > 0 89 0x04 >; > + interrupt-controller; > + #interrupt-cells = <2>; > #gpio-cells = <2>; > gpio-controller; > }; > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > index 61044c8..26b0c85 100644 > --- a/drivers/gpio/gpio-tegra.c > +++ b/drivers/gpio/gpio-tegra.c > @@ -25,6 +25,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/module.h> > +#include <linux/irqdomain.h> > > #include <asm/mach/irq.h> > > @@ -410,6 +411,9 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) > spin_lock_init(&bank->lvl_lock[j]); > } > > + if (pdev->dev.of_node) > + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); > + This is a bit of a temporary solution. Really, you want to end up with something like this to eliminate INT_GPIO_BASE and dynamically assign the irqbase: diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 61044c8..a146a20 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -139,7 +139,7 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset, static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { - return TEGRA_GPIO_TO_IRQ(offset); + return irq_domain_to_irq(chip->domain, offset); >>> This is a bit wrong. You need a tegra_gpio_chip containing a domain and gpio_chip. } static struct gpio_chip tegra_gpio_chip = { @@ -155,28 +155,28 @@ static struct gpio_chip tegra_gpio_chip = { static void tegra_gpio_irq_ack(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_writel(1 << GPIO_BIT(gpio), GPIO_INT_CLR(gpio)); } static void tegra_gpio_irq_mask(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_mask_write(GPIO_MSK_INT_ENB(gpio), gpio, 0); } static void tegra_gpio_irq_unmask(struct irq_data *d) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; tegra_gpio_mask_write(GPIO_MSK_INT_ENB(gpio), gpio, 1); } static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) { - int gpio = d->irq - INT_GPIO_BASE; + int gpio = d->hwirq; struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d); int port = GPIO_PORT(gpio); int lvl_type; @@ -385,6 +385,7 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) tegra_gpio_chip.of_node = pdev->dev.of_node; #endif + domain.irq_base = irq_alloc_descs(...) gpiochip_add(&tegra_gpio_chip); for (gpio = 0; gpio < TEGRA_NR_GPIOS; gpio++) { Rob ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <4ED5A2CB.8020905-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* RE: [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller 2011-11-30 3:28 ` Rob Herring @ 2011-11-30 17:19 ` Stephen Warren -1 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 17:19 UTC (permalink / raw) To: Rob Herring Cc: Peter De Schrijver, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Colin Cross, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM: > On 11/29/2011 07:32 PM, Stephen Warren wrote: > > Fix the DT binding documentation to describe interrupt-related properties, > > and the contents of "child" node interrupts property. > > > > Update tegra20.dtsi to specify the required interrupt-related properties. > > > > Fix the driver to creating an IRQ domain for itself, so that child node > > interrupts properties are correctly parsed. > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt ... > > +- #interrupt-cells : Should be 3. > > Should be 2?? Yes, typo. > > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > > + if (pdev->dev.of_node) > > + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); > > + > > This is a bit of a temporary solution. Really, you want to end up with > something like this to eliminate INT_GPIO_BASE and dynamically assign > the irqbase: > static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) > { > - return TEGRA_GPIO_TO_IRQ(offset); > + return irq_domain_to_irq(chip->domain, offset); > > >>> This is a bit wrong. You need a tegra_gpio_chip containing a domain > and gpio_chip. That implies there's always an IRQ domain for this chip. Right now, my patch only creates an IRQ domain when the GPIO controller is instantiated from DT; there is no domain when instantiated as a platform device from a board file. I suppose that really should change, but: Some board files initialize platform devices as follows: ./arch/arm/mach-tegra/board-seaboard.c:162: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_ISL29018_IRQ), ./arch/arm/mach-tegra/board-seaboard.c:186: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), ./arch/arm/mach-tegra/board-harmony.c:104: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), Is there a way to make that work if there is no static IRQ numbering and hence the compile-time macro TEGRA_GPIO_TO_IRQ() doesn't exist? For Tegra, IRQ domain support (CONFIG_IRQ_DOMAIN) is only enabled via CONFIG_USE_OF, so for non-DT builds isn't enabled. I guess we could easily make CONFIG_ARCH_TEGRA select CONFIG_IRQ_DOMAIN though. The sooner we switch to DT-only the better! > static void tegra_gpio_irq_ack(struct irq_data *d) > { > - int gpio = d->irq - INT_GPIO_BASE; > + int gpio = d->hwirq; Is the hwirq field valid when there isn't an IRQ domain? Thanks for the feedback. -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller @ 2011-11-30 17:19 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2011-11-30 17:19 UTC (permalink / raw) To: linux-arm-kernel Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM: > On 11/29/2011 07:32 PM, Stephen Warren wrote: > > Fix the DT binding documentation to describe interrupt-related properties, > > and the contents of "child" node interrupts property. > > > > Update tegra20.dtsi to specify the required interrupt-related properties. > > > > Fix the driver to creating an IRQ domain for itself, so that child node > > interrupts properties are correctly parsed. > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt ... > > +- #interrupt-cells : Should be 3. > > Should be 2?? Yes, typo. > > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > > + if (pdev->dev.of_node) > > + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); > > + > > This is a bit of a temporary solution. Really, you want to end up with > something like this to eliminate INT_GPIO_BASE and dynamically assign > the irqbase: > static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) > { > - return TEGRA_GPIO_TO_IRQ(offset); > + return irq_domain_to_irq(chip->domain, offset); > > >>> This is a bit wrong. You need a tegra_gpio_chip containing a domain > and gpio_chip. That implies there's always an IRQ domain for this chip. Right now, my patch only creates an IRQ domain when the GPIO controller is instantiated from DT; there is no domain when instantiated as a platform device from a board file. I suppose that really should change, but: Some board files initialize platform devices as follows: ./arch/arm/mach-tegra/board-seaboard.c:162: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_ISL29018_IRQ), ./arch/arm/mach-tegra/board-seaboard.c:186: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), ./arch/arm/mach-tegra/board-harmony.c:104: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), Is there a way to make that work if there is no static IRQ numbering and hence the compile-time macro TEGRA_GPIO_TO_IRQ() doesn't exist? For Tegra, IRQ domain support (CONFIG_IRQ_DOMAIN) is only enabled via CONFIG_USE_OF, so for non-DT builds isn't enabled. I guess we could easily make CONFIG_ARCH_TEGRA select CONFIG_IRQ_DOMAIN though. The sooner we switch to DT-only the better! > static void tegra_gpio_irq_ack(struct irq_data *d) > { > - int gpio = d->irq - INT_GPIO_BASE; > + int gpio = d->hwirq; Is the hwirq field valid when there isn't an IRQ domain? Thanks for the feedback. -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <74CDBE0F657A3D45AFBB94109FB122FF174FDAFE4D-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>]
* Re: [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller 2011-11-30 17:19 ` Stephen Warren @ 2011-11-30 19:34 ` Rob Herring -1 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2011-11-30 19:34 UTC (permalink / raw) To: Stephen Warren Cc: Olof Johansson, Colin Cross, Peter De Schrijver, Grant Likely, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11/30/2011 11:19 AM, Stephen Warren wrote: > Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM: >> On 11/29/2011 07:32 PM, Stephen Warren wrote: >>> Fix the DT binding documentation to describe interrupt-related properties, >>> and the contents of "child" node interrupts property. >>> >>> Update tegra20.dtsi to specify the required interrupt-related properties. >>> >>> Fix the driver to creating an IRQ domain for itself, so that child node >>> interrupts properties are correctly parsed. > >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > ... >>> +- #interrupt-cells : Should be 3. >> >> Should be 2?? > > Yes, typo. > >>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > >>> + if (pdev->dev.of_node) >>> + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); >>> + >> >> This is a bit of a temporary solution. Really, you want to end up with >> something like this to eliminate INT_GPIO_BASE and dynamically assign >> the irqbase: > >> static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) >> { >> - return TEGRA_GPIO_TO_IRQ(offset); >> + return irq_domain_to_irq(chip->domain, offset); >> >>>>> This is a bit wrong. You need a tegra_gpio_chip containing a domain >> and gpio_chip. > > That implies there's always an IRQ domain for this chip. Right now, my > patch only creates an IRQ domain when the GPIO controller is instantiated > from DT; there is no domain when instantiated as a platform device from a > board file. I suppose that really should change, but: > > Some board files initialize platform devices as follows: > > ./arch/arm/mach-tegra/board-seaboard.c:162: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_ISL29018_IRQ), > ./arch/arm/mach-tegra/board-seaboard.c:186: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), > ./arch/arm/mach-tegra/board-harmony.c:104: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), > > Is there a way to make that work if there is no static IRQ numbering and > hence the compile-time macro TEGRA_GPIO_TO_IRQ() doesn't exist? > > For Tegra, IRQ domain support (CONFIG_IRQ_DOMAIN) is only enabled via > CONFIG_USE_OF, so for non-DT builds isn't enabled. I guess we could > easily make CONFIG_ARCH_TEGRA select CONFIG_IRQ_DOMAIN though. > No, the GIC selects IRQ_DOMAIN now. > The sooner we switch to DT-only the better! Dynamic irq assignment doesn't require DT. If you can change the .irq init above to run-time, I think you can just call gpio_to_irq. You could have some init ordering issues though. You could leave the base hard-coded for now, but limit it's use to domain.irqbase. That's at least much better INT_GPIO_BASE used all over the place. > >> static void tegra_gpio_irq_ack(struct irq_data *d) >> { >> - int gpio = d->irq - INT_GPIO_BASE; >> + int gpio = d->hwirq; > > Is the hwirq field valid when there isn't an IRQ domain? No. irq_domain_add sets it up. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller @ 2011-11-30 19:34 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2011-11-30 19:34 UTC (permalink / raw) To: linux-arm-kernel On 11/30/2011 11:19 AM, Stephen Warren wrote: > Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM: >> On 11/29/2011 07:32 PM, Stephen Warren wrote: >>> Fix the DT binding documentation to describe interrupt-related properties, >>> and the contents of "child" node interrupts property. >>> >>> Update tegra20.dtsi to specify the required interrupt-related properties. >>> >>> Fix the driver to creating an IRQ domain for itself, so that child node >>> interrupts properties are correctly parsed. > >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt b/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt > ... >>> +- #interrupt-cells : Should be 3. >> >> Should be 2?? > > Yes, typo. > >>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > >>> + if (pdev->dev.of_node) >>> + irq_domain_add_simple(pdev->dev.of_node, TEGRA_GPIO_TO_IRQ(0)); >>> + >> >> This is a bit of a temporary solution. Really, you want to end up with >> something like this to eliminate INT_GPIO_BASE and dynamically assign >> the irqbase: > >> static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) >> { >> - return TEGRA_GPIO_TO_IRQ(offset); >> + return irq_domain_to_irq(chip->domain, offset); >> >>>>> This is a bit wrong. You need a tegra_gpio_chip containing a domain >> and gpio_chip. > > That implies there's always an IRQ domain for this chip. Right now, my > patch only creates an IRQ domain when the GPIO controller is instantiated > from DT; there is no domain when instantiated as a platform device from a > board file. I suppose that really should change, but: > > Some board files initialize platform devices as follows: > > ./arch/arm/mach-tegra/board-seaboard.c:162: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_ISL29018_IRQ), > ./arch/arm/mach-tegra/board-seaboard.c:186: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), > ./arch/arm/mach-tegra/board-harmony.c:104: .irq = TEGRA_GPIO_TO_IRQ(TEGRA_GPIO_CDC_IRQ), > > Is there a way to make that work if there is no static IRQ numbering and > hence the compile-time macro TEGRA_GPIO_TO_IRQ() doesn't exist? > > For Tegra, IRQ domain support (CONFIG_IRQ_DOMAIN) is only enabled via > CONFIG_USE_OF, so for non-DT builds isn't enabled. I guess we could > easily make CONFIG_ARCH_TEGRA select CONFIG_IRQ_DOMAIN though. > No, the GIC selects IRQ_DOMAIN now. > The sooner we switch to DT-only the better! Dynamic irq assignment doesn't require DT. If you can change the .irq init above to run-time, I think you can just call gpio_to_irq. You could have some init ordering issues though. You could leave the base hard-coded for now, but limit it's use to domain.irqbase. That's at least much better INT_GPIO_BASE used all over the place. > >> static void tegra_gpio_irq_ack(struct irq_data *d) >> { >> - int gpio = d->irq - INT_GPIO_BASE; >> + int gpio = d->hwirq; > > Is the hwirq field valid when there isn't an IRQ domain? No. irq_domain_add sets it up. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-11-30 19:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-30 1:32 [PATCH 1/2] irq: Add dummy irq_domain_add_simple_when not defined Stephen Warren
2011-11-30 1:32 ` Stephen Warren
[not found] ` <1322616743-31985-1-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2011-11-30 1:32 ` [PATCH 2/2] gpio/tegra: Make it a DT interrupt controller Stephen Warren
2011-11-30 1:32 ` Stephen Warren
[not found] ` <1322616743-31985-2-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2011-11-30 3:28 ` Rob Herring
2011-11-30 3:28 ` Rob Herring
[not found] ` <4ED5A2CB.8020905-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2011-11-30 17:19 ` Stephen Warren
2011-11-30 17:19 ` Stephen Warren
[not found] ` <74CDBE0F657A3D45AFBB94109FB122FF174FDAFE4D-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2011-11-30 19:34 ` Rob Herring
2011-11-30 19:34 ` Rob Herring
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.