irq: Add dummy irq_domain_add_simple_when not defined
Subsystems (4)
PATCH 1/2 irq: Add dummy irq_domain_add_simple_when not defined
This prevents users of this API from having to ifdef it.
--- include/linux/irqdomain.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/irqdomain.h +2 -0
@@ -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 */
Signature · 1 line
1.7.0.4
PATCH 2/2 gpio/tegra: Make it a DT interrupt controller
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.
---
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(-)
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +10 -0
@@ -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.
arch/arm/boot/dts/tegra20.dtsi +2 -0
@@ -79,6 +79,8 @@ 0 55 0x04 0 87 0x04 0 89 0x04 >; + interrupt-controller; + #interrupt-cells = <2>; #gpio-cells = <2>; gpio-controller; };
drivers/gpio/gpio-tegra.c +4 -0
@@ -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 ; }
Signature · 1 line
1.7.0.4
PATCH 2/2 gpio/tegra: Make it a DT interrupt controller
Quoted 28 lines · Stephen Warren
Fix the DT binding documentation to describe interrupt-related properties, and the contents of "child" node interrupts property.
Update
tegra20.dtsito 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 <[email protected]>---
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(-)Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +0
-0
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt Required properties: · line 9
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +1
-0@@ -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??
Quoted 35 lines
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +9
-0+ 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.arch/arm/boot/dts/tegra20.dtsi +2
-0@@ -79,6 +79,8 @@ 0 55 0x04 0 87 0x04 0 89 0x04 >; + interrupt-controller; + #interrupt-cells = <2>; #gpio-cells = <2>; gpio-controller; };drivers/gpio/gpio-tegra.c +1
-0@@ -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>
drivers/gpio/gpio-tegra.c static int __devinit tegra_gpio_probe(struct platform_device *pdev) · line 416
Quoted 6 lines
drivers/gpio/gpio-tegra.c +3
-0@@ -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:
drivers/gpio/gpio-tegra.c +0 -0
@@ -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);
Quoted 1 line
Quoted 1 line
Quoted 1 line
This is a bit wrong. You need a
tegra_gpio_chipcontaining 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
PATCH 2/2 gpio/tegra: Make it a DT interrupt controller
Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM:
Quoted 10 lines · Stephen Warren
Quoted 9 lines
Fix the DT binding documentation to describe interrupt-related properties, and the contents of "child" node interrupts property.
Update
tegra20.dtsito 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.
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +0
-0
...
Quoted 3 lines
Quoted 1 line
+- #interrupt-cells : Should be 3.
Should be 2??
Yes, typo.
Quoted 15 lines
drivers/gpio/gpio-tegra.c
Quoted 5 lines
drivers/gpio/gpio-tegra.c +3
-0+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_BASEand dynamically assign the irqbase:static int
tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset){(quoted diff) +1
-1- return TEGRA_GPIO_TO_IRQ(offset); + return irq_domain_to_irq(chip->domain, offset);
Quoted 2 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
This is a bit wrong. You need a
tegra_gpio_chipcontaining a domainand
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!
Review
Quoted 4 lines
static void
tegra_gpio_irq_ack(struct irq_data *d){(quoted diff) +1
-1- 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.
Signature · 1 line
nvpublic
PATCH 2/2 gpio/tegra: Make it a DT interrupt controller
Quoted 47 lines · Stephen Warren
Rob Herring wrote at Tuesday, November 29, 2011 8:28 PM:
Quoted 10 lines · Stephen Warren
Quoted 9 lines
Fix the DT binding documentation to describe interrupt-related properties, and the contents of "child" node interrupts property.
Update
tegra20.dtsito 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.
Documentation/devicetree/bindings/gpio/gpio_nvidia.txt +0
-0...
Quoted 3 lines
Quoted 1 line
+- #interrupt-cells : Should be 3.
Should be 2??
Yes, typo.
Quoted 15 lines
drivers/gpio/gpio-tegra.c
Quoted 5 lines
drivers/gpio/gpio-tegra.c +3
-0+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_BASEand dynamically assign the irqbase:static int
tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset){(quoted diff) +1
-1- return TEGRA_GPIO_TO_IRQ(offset); + return irq_domain_to_irq(chip->domain, offset);Quoted 2 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
This is a bit wrong. You need a
tegra_gpio_chipcontaining a domainand
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),
Quoted 7 lines
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 viaCONFIG_USE_OF, so for non-DT builds isn't enabled. I guess we could easily makeCONFIG_ARCH_TEGRAselectCONFIG_IRQ_DOMAINthough.
No, the GIC selects IRQ_DOMAIN now.
Quoted 1 line
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.
Quoted 7 lines
Review
Quoted 5 lines
static void
tegra_gpio_irq_ack(struct irq_data *d){(quoted diff) +1
-1- 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