* [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 20:02 ` Ryan Mallon
2012-02-13 14:43 ` [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio Nicolas Ferre
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel,
Nicolas Ferre, rmallon
gpio_to_irq() implementation will be moved from a macro to a
plain function: we cannot use it in a structure initialization
anymore.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: rmallon@gmail.com
---
arch/arm/mach-at91/board-snapper9260.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
index 4770db0..3c2e3fc 100644
--- a/arch/arm/mach-at91/board-snapper9260.c
+++ b/arch/arm/mach-at91/board-snapper9260.c
@@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
/* Audio codec */
I2C_BOARD_INFO("tlv320aic23", 0x1a),
},
- {
+};
+
+static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
/* RTC */
I2C_BOARD_INFO("isl1208", 0x6f),
- .irq = gpio_to_irq(AT91_PIN_PA31),
- },
};
static void __init snapper9260_add_device_nand(void)
@@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
{
at91_add_device_i2c(snapper9260_i2c_devices,
ARRAY_SIZE(snapper9260_i2c_devices));
+
+ snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
+ i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
+
at91_add_device_serial();
at91_add_device_usbh(&snapper9260_usbh_data);
at91_add_device_udc(&snapper9260_udc_data);
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization
2012-02-13 14:43 ` [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization Nicolas Ferre
@ 2012-02-13 20:02 ` Ryan Mallon
2012-02-14 9:04 ` Nicolas Ferre
2012-02-14 9:48 ` Russell King - ARM Linux
0 siblings, 2 replies; 24+ messages in thread
From: Ryan Mallon @ 2012-02-13 20:02 UTC (permalink / raw)
To: Nicolas Ferre
Cc: plagnioj, linux-arm-kernel, grant.likely, rob.herring, tglx,
devicetree-discuss, avictor.za, linux-kernel
On 14/02/12 01:43, Nicolas Ferre wrote:
> gpio_to_irq() implementation will be moved from a macro to a
> plain function: we cannot use it in a structure initialization
> anymore.
What was the reason for the change? It was originally a macro for
exactly this reason.
Anyway,
Reviewed-by: Ryan Mallon <rmallon@gmail.com>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: rmallon@gmail.com
> ---
> arch/arm/mach-at91/board-snapper9260.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
> index 4770db0..3c2e3fc 100644
> --- a/arch/arm/mach-at91/board-snapper9260.c
> +++ b/arch/arm/mach-at91/board-snapper9260.c
> @@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
> /* Audio codec */
> I2C_BOARD_INFO("tlv320aic23", 0x1a),
> },
> - {
> +};
> +
> +static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
> /* RTC */
> I2C_BOARD_INFO("isl1208", 0x6f),
> - .irq = gpio_to_irq(AT91_PIN_PA31),
> - },
> };
>
> static void __init snapper9260_add_device_nand(void)
> @@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
> {
> at91_add_device_i2c(snapper9260_i2c_devices,
> ARRAY_SIZE(snapper9260_i2c_devices));
> +
> + snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
> + i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
> +
> at91_add_device_serial();
> at91_add_device_usbh(&snapper9260_usbh_data);
> at91_add_device_udc(&snapper9260_udc_data);
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization
2012-02-13 20:02 ` Ryan Mallon
@ 2012-02-14 9:04 ` Nicolas Ferre
2012-02-14 9:48 ` Russell King - ARM Linux
1 sibling, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-14 9:04 UTC (permalink / raw)
To: Ryan Mallon
Cc: plagnioj, linux-arm-kernel, grant.likely, rob.herring, tglx,
devicetree-discuss, avictor.za, linux-kernel
On 02/13/2012 09:02 PM, Ryan Mallon :
> On 14/02/12 01:43, Nicolas Ferre wrote:
>
>> gpio_to_irq() implementation will be moved from a macro to a
>> plain function: we cannot use it in a structure initialization
>> anymore.
>
> What was the reason for the change? It was originally a macro for
> exactly this reason.
Linux vIRQ numbers will be allocated dynamically when switching to
irqdomains. That will prevent the use of a static operation in a macro.
> Reviewed-by: Ryan Mallon <rmallon@gmail.com>
Thanks.
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> Cc: rmallon@gmail.com
>> ---
>> arch/arm/mach-at91/board-snapper9260.c | 10 +++++++---
>> 1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
>> index 4770db0..3c2e3fc 100644
>> --- a/arch/arm/mach-at91/board-snapper9260.c
>> +++ b/arch/arm/mach-at91/board-snapper9260.c
>> @@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
>> /* Audio codec */
>> I2C_BOARD_INFO("tlv320aic23", 0x1a),
>> },
>> - {
>> +};
>> +
>> +static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
>> /* RTC */
>> I2C_BOARD_INFO("isl1208", 0x6f),
>> - .irq = gpio_to_irq(AT91_PIN_PA31),
>> - },
>> };
>>
>> static void __init snapper9260_add_device_nand(void)
>> @@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
>> {
>> at91_add_device_i2c(snapper9260_i2c_devices,
>> ARRAY_SIZE(snapper9260_i2c_devices));
>> +
>> + snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
>> + i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
>> +
>> at91_add_device_serial();
>> at91_add_device_usbh(&snapper9260_usbh_data);
>> at91_add_device_udc(&snapper9260_udc_data);
>
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization
2012-02-13 20:02 ` Ryan Mallon
2012-02-14 9:04 ` Nicolas Ferre
@ 2012-02-14 9:48 ` Russell King - ARM Linux
1 sibling, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2012-02-14 9:48 UTC (permalink / raw)
To: Ryan Mallon
Cc: Nicolas Ferre, devicetree-discuss, linux-kernel, rob.herring,
grant.likely, avictor.za, tglx, plagnioj, linux-arm-kernel
On Tue, Feb 14, 2012 at 07:02:20AM +1100, Ryan Mallon wrote:
> On 14/02/12 01:43, Nicolas Ferre wrote:
>
> > gpio_to_irq() implementation will be moved from a macro to a
> > plain function: we cannot use it in a structure initialization
> > anymore.
>
> What was the reason for the change? It was originally a macro for
> exactly this reason.
gpio_to_irq() is supposed to work not only for built-in GPIOs, but also
GPIOs provided by other devices. Having it as a definition which only
works for the SoC GPIOs is broken.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
2012-02-13 14:43 ` [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-17 9:41 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH 4/9] ARM: at91/gpio: change comments and one variable name Nicolas Ferre
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel,
Nicolas Ferre, Thomas Petazzoni, linux-usb
irq_to_gpio() macro will be removed from AT91 GPIO interrupt
controller. So we replace it with the use of gpio_to_irq()
and a reworked test.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/ohci-at91.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 77afabc..8e855eb 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -448,10 +448,11 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
/* From the GPIO notifying the over-current situation, find
* out the corresponding port */
- gpio = irq_to_gpio(irq);
for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
- if (pdata->overcurrent_pin[port] == gpio)
+ if (gpio_to_irq(pdata->overcurrent_pin[port]) == irq) {
+ gpio = pdata->overcurrent_pin[port];
break;
+ }
}
if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio
2012-02-13 14:43 ` [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio Nicolas Ferre
@ 2012-02-17 9:41 ` Nicolas Ferre
2012-02-17 16:59 ` Greg Kroah-Hartman
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-17 9:41 UTC (permalink / raw)
To: linux-usb, Greg Kroah-Hartman
Cc: plagnioj, linux-arm-kernel, grant.likely, rob.herring, tglx,
devicetree-discuss, avictor.za, linux-kernel, Thomas Petazzoni
On 02/13/2012 03:43 PM, Nicolas Ferre :
> irq_to_gpio() macro will be removed from AT91 GPIO interrupt
> controller. So we replace it with the use of gpio_to_irq()
> and a reworked test.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: linux-usb@vger.kernel.org
Hi Greg,
I would like to queue this patch through arm-soc git tree (with all AT91
material of this patch series).
So, can I have your "Acked-by" on this one?
> ---
> drivers/usb/host/ohci-at91.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 77afabc..8e855eb 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -448,10 +448,11 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
>
> /* From the GPIO notifying the over-current situation, find
> * out the corresponding port */
> - gpio = irq_to_gpio(irq);
> for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
> - if (pdata->overcurrent_pin[port] == gpio)
> + if (gpio_to_irq(pdata->overcurrent_pin[port]) == irq) {
> + gpio = pdata->overcurrent_pin[port];
> break;
> + }
> }
>
> if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {
Thanks, best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio
2012-02-17 9:41 ` Nicolas Ferre
@ 2012-02-17 16:59 ` Greg Kroah-Hartman
0 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2012-02-17 16:59 UTC (permalink / raw)
To: Nicolas Ferre
Cc: linux-usb, plagnioj, linux-arm-kernel, grant.likely, rob.herring,
tglx, devicetree-discuss, avictor.za, linux-kernel,
Thomas Petazzoni
On Fri, Feb 17, 2012 at 10:41:32AM +0100, Nicolas Ferre wrote:
> On 02/13/2012 03:43 PM, Nicolas Ferre :
> > irq_to_gpio() macro will be removed from AT91 GPIO interrupt
> > controller. So we replace it with the use of gpio_to_irq()
> > and a reworked test.
> >
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: linux-usb@vger.kernel.org
>
> Hi Greg,
>
> I would like to queue this patch through arm-soc git tree (with all AT91
> material of this patch series).
>
> So, can I have your "Acked-by" on this one?
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/9] ARM: at91/gpio: change comments and one variable name
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
2012-02-13 14:43 ` [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization Nicolas Ferre
2012-02-13 14:43 ` [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH v5 5/9] ARM: at91/gpio: add irqdomain and DT support Nicolas Ferre
` (5 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
What was true only on at91sam9263 about the sharing of a single AIC
IRQ line for several GPIO banks is now used by several Atmel SoCs.
Change a variable name to allow better understanding while
introducing IRQ domains in following patches.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
arch/arm/mach-at91/gpio.c | 25 ++++++++++++++-----------
1 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 74d6783..b762afc 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -29,8 +29,8 @@
struct at91_gpio_chip {
struct gpio_chip chip;
struct at91_gpio_chip *next; /* Bank sharing same clock */
- int id; /* ID of register bank */
- void __iomem *regbase; /* Base of register bank */
+ int pioc_hwirq; /* PIO bank interrupt identifier on AIC */
+ void __iomem *regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
};
@@ -285,7 +285,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
else
wakeups[bank] &= ~mask;
- irq_set_irq_wake(gpio_chip[bank].id, state);
+ irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
return 0;
}
@@ -499,7 +499,7 @@ void __init at91_gpio_irq_setup(void)
for (pioc = 0, this = gpio_chip, prev = NULL;
pioc++ < gpio_banks;
prev = this, this++) {
- unsigned id = this->id;
+ unsigned pioc_hwirq = this->pioc_hwirq;
unsigned i;
__raw_writel(~0, this->regbase + PIO_IDR);
@@ -518,14 +518,14 @@ void __init at91_gpio_irq_setup(void)
}
/* The toplevel handler handles one bank of GPIOs, except
- * AT91SAM9263_ID_PIOCDE handles three... PIOC is first in
- * the list, so we only set up that handler.
+ * on some SoC it can handles up to three...
+ * We only set up the handler for the first of the list.
*/
if (prev && prev->next == this)
continue;
- irq_set_chip_data(id, this);
- irq_set_chained_handler(id, gpio_irq_handler);
+ irq_set_chip_data(pioc_hwirq, this);
+ irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
}
pr_info("AT91: %d gpio irqs in %d banks\n", irq - gpio_to_irq(0), gpio_banks);
}
@@ -615,7 +615,7 @@ void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
for (i = 0; i < nr_banks; i++) {
at91_gpio = &gpio_chip[i];
- at91_gpio->id = data[i].id;
+ at91_gpio->pioc_hwirq = data[i].pioc_hwirq;
at91_gpio->chip.base = i * 32;
at91_gpio->regbase = ioremap(data[i].regbase, 512);
@@ -633,8 +633,11 @@ void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
/* enable PIO controller's clock */
clk_enable(at91_gpio->clock);
- /* AT91SAM9263_ID_PIOCDE groups PIOC, PIOD, PIOE */
- if (last && last->id == at91_gpio->id)
+ /*
+ * GPIO controller are grouped on some SoC:
+ * PIOC, PIOD and PIOE can share the same IRQ line
+ */
+ if (last && last->pioc_hwirq == at91_gpio->pioc_hwirq)
last->next = at91_gpio;
last = at91_gpio;
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v5 5/9] ARM: at91/gpio: add irqdomain and DT support
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (2 preceding siblings ...)
2012-02-13 14:43 ` [PATCH 4/9] ARM: at91/gpio: change comments and one variable name Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH 6/9] ARM: at91/gpio: non-DT builds do not have gpio_chip.of_node field Nicolas Ferre
` (4 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
Add "legacy" type of irqdomain to preserve old-style numbering
and allow smooth transition for both DT and non-DT cases.
Original idea and code by Jean-Christophe Plagniol-Villard.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
v5: - merge irqdomain and device tree modifications
- use "legacy" type of irqdomain (call to irq_domain_add_legacy())
- use more explicit variable names
v4: - at91_gpio_setup_clk() returns an error code
v3: - Corrections proposed by Russell King:
- use of clk_prepare()/clk_unprepare()
- helper function to setup the clock for both DT/non DT init functions
v2: - no BUG_ON() anymore in case of an "interrupts" property missing
just stop configuring the gpio bank
- review error path in both of_at91_gpio_init_one() and
at91_gpio_init_one()
- use for_each_compatible_node() iterator instead of a homemade loop
.../devicetree/bindings/gpio/gpio_atmel.txt | 20 ++
arch/arm/boot/dts/at91sam9g20.dtsi | 30 +++
arch/arm/boot/dts/at91sam9g45.dtsi | 50 +++++
arch/arm/mach-at91/gpio.c | 233 ++++++++++++++++----
4 files changed, 287 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio_atmel.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt
new file mode 100644
index 0000000..a7bcaec
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt
@@ -0,0 +1,20 @@
+* Atmel GPIO controller (PIO)
+
+Required properties:
+- compatible: "atmel,at91rm9200-gpio"
+- reg: Should contain GPIO controller registers location and length
+- interrupts: Should be the port interrupt shared by all the pins.
+- #gpio-cells: Should be two. The first cell is the pin number and
+ the second cell is used to specify optional parameters (currently
+ unused).
+- gpio-controller: Marks the device node as a GPIO controller.
+
+Example:
+ pioA: gpio@fffff200 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff200 0x100>;
+ interrupts = <2 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ };
+
diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 9a0aee7..325989a 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -23,6 +23,9 @@
serial4 = &usart3;
serial5 = &usart4;
serial6 = &usart5;
+ gpio0 = &pioA;
+ gpio1 = &pioB;
+ gpio2 = &pioC;
};
cpus {
cpu@0 {
@@ -54,6 +57,33 @@
reg = <0xfffff000 0x200>;
};
+ pioA: gpio@fffff400 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff400 0x100>;
+ interrupts = <2 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioB: gpio@fffff600 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff600 0x100>;
+ interrupts = <3 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioC: gpio@fffff800 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff800 0x100>;
+ interrupts = <4 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
dbgu: serial@fffff200 {
compatible = "atmel,at91sam9260-usart";
reg = <0xfffff200 0x200>;
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 67f94d3..a9dbbb5 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -22,6 +22,11 @@
serial2 = &usart1;
serial3 = &usart2;
serial4 = &usart3;
+ gpio0 = &pioA;
+ gpio1 = &pioB;
+ gpio2 = &pioC;
+ gpio3 = &pioD;
+ gpio4 = &pioE;
};
cpus {
cpu@0 {
@@ -59,6 +64,51 @@
interrupts = <21 4>;
};
+ pioA: gpio@fffff200 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff200 0x100>;
+ interrupts = <2 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioB: gpio@fffff400 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff400 0x100>;
+ interrupts = <3 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioC: gpio@fffff600 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff600 0x100>;
+ interrupts = <4 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioD: gpio@fffff800 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffff800 0x100>;
+ interrupts = <5 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
+ pioE: gpio@fffffa00 {
+ compatible = "atmel,at91rm9200-gpio";
+ reg = <0xfffffa00 0x100>;
+ interrupts = <5 4>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ };
+
dbgu: serial@ffffee00 {
compatible = "atmel,at91sam9260-usart";
reg = <0xffffee00 0x200>;
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index b762afc..89e683a 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -20,6 +20,9 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/io.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
#include <mach/hardware.h>
#include <mach/at91_pio.h>
@@ -30,8 +33,10 @@ struct at91_gpio_chip {
struct gpio_chip chip;
struct at91_gpio_chip *next; /* Bank sharing same clock */
int pioc_hwirq; /* PIO bank interrupt identifier on AIC */
+ int pioc_idx; /* PIO bank index */
void __iomem *regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
+ struct irq_domain *domain; /* associated irq domain */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
@@ -273,9 +278,9 @@ static u32 backups[MAX_GPIO_BANKS];
static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
{
- unsigned pin = irq_to_gpio(d->irq);
- unsigned mask = pin_to_mask(pin);
- unsigned bank = pin / 32;
+ struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+ unsigned mask = 1 << d->hwirq;
+ unsigned bank = at91_gpio->pioc_idx;
if (unlikely(bank >= MAX_GPIO_BANKS))
return -EINVAL;
@@ -301,9 +306,10 @@ void at91_gpio_suspend(void)
__raw_writel(backups[i], pio + PIO_IDR);
__raw_writel(wakeups[i], pio + PIO_IER);
- if (!wakeups[i])
+ if (!wakeups[i]) {
+ clk_unprepare(gpio_chip[i].clock);
clk_disable(gpio_chip[i].clock);
- else {
+ } else {
#ifdef CONFIG_PM_DEBUG
printk(KERN_DEBUG "GPIO-%c may wake for %08x\n", 'A'+i, wakeups[i]);
#endif
@@ -318,8 +324,10 @@ void at91_gpio_resume(void)
for (i = 0; i < gpio_banks; i++) {
void __iomem *pio = gpio_chip[i].regbase;
- if (!wakeups[i])
- clk_enable(gpio_chip[i].clock);
+ if (!wakeups[i]) {
+ if (clk_prepare(gpio_chip[i].clock) == 0)
+ clk_enable(gpio_chip[i].clock);
+ }
__raw_writel(wakeups[i], pio + PIO_IDR);
__raw_writel(backups[i], pio + PIO_IER);
@@ -344,9 +352,9 @@ void at91_gpio_resume(void)
static void gpio_irq_mask(struct irq_data *d)
{
- unsigned pin = irq_to_gpio(d->irq);
- void __iomem *pio = pin_to_controller(pin);
- unsigned mask = pin_to_mask(pin);
+ struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << d->hwirq;
if (pio)
__raw_writel(mask, pio + PIO_IDR);
@@ -354,9 +362,9 @@ static void gpio_irq_mask(struct irq_data *d)
static void gpio_irq_unmask(struct irq_data *d)
{
- unsigned pin = irq_to_gpio(d->irq);
- void __iomem *pio = pin_to_controller(pin);
- unsigned mask = pin_to_mask(pin);
+ struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << d->hwirq;
if (pio)
__raw_writel(mask, pio + PIO_IER);
@@ -384,7 +392,7 @@ static struct irq_chip gpio_irqchip = {
static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
{
- unsigned irq_pin;
+ unsigned virq;
struct irq_data *idata = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(idata);
struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
@@ -407,12 +415,12 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
continue;
}
- irq_pin = gpio_to_irq(at91_gpio->chip.base);
+ virq = gpio_to_irq(at91_gpio->chip.base);
while (isr) {
if (isr & 1)
- generic_handle_irq(irq_pin);
- irq_pin++;
+ generic_handle_irq(virq);
+ virq++;
isr >>= 1;
}
}
@@ -483,6 +491,26 @@ postcore_initcall(at91_gpio_debugfs_init);
/*--------------------------------------------------------------------------*/
/*
+ * irqdomain initialization: pile up irqdomains on top of AIC range
+ */
+static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
+{
+ int irq_base;
+
+ irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
+ if (irq_base < 0)
+ panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
+ at91_gpio->pioc_idx, irq_base);
+ at91_gpio->domain = irq_domain_add_legacy(at91_gpio->chip.of_node,
+ at91_gpio->chip.ngpio,
+ irq_base, 0,
+ &irq_domain_simple_ops, NULL);
+ if (!at91_gpio->domain)
+ panic("at91_gpio.%d: couldn't allocate irq domain.\n",
+ at91_gpio->pioc_idx);
+}
+
+/*
* This lock class tells lockdep that GPIO irqs are in a different
* category than their parents, so it won't report false recursion.
*/
@@ -493,28 +521,35 @@ static struct lock_class_key gpio_lock_class;
*/
void __init at91_gpio_irq_setup(void)
{
- unsigned pioc, irq = gpio_to_irq(0);
+ unsigned pioc;
+ int gpio_irqnbr = 0;
struct at91_gpio_chip *this, *prev;
for (pioc = 0, this = gpio_chip, prev = NULL;
pioc++ < gpio_banks;
prev = this, this++) {
unsigned pioc_hwirq = this->pioc_hwirq;
- unsigned i;
+ int offset;
__raw_writel(~0, this->regbase + PIO_IDR);
- for (i = 0, irq = gpio_to_irq(this->chip.base); i < 32;
- i++, irq++) {
- irq_set_lockdep_class(irq, &gpio_lock_class);
+ /* setup irq domain for this GPIO controller */
+ at91_gpio_irqdomain(this);
+
+ for (offset = 0; offset < this->chip.ngpio; offset++) {
+ unsigned int virq = irq_find_mapping(this->domain, offset);
+ irq_set_lockdep_class(virq, &gpio_lock_class);
/*
* Can use the "simple" and not "edge" handler since it's
* shorter, and the AIC handles interrupts sanely.
*/
- irq_set_chip_and_handler(irq, &gpio_irqchip,
+ irq_set_chip_and_handler(virq, &gpio_irqchip,
handle_simple_irq);
- set_irq_flags(irq, IRQF_VALID);
+ set_irq_flags(virq, IRQF_VALID);
+ irq_set_chip_data(virq, this);
+
+ gpio_irqnbr++;
}
/* The toplevel handler handles one bank of GPIOs, except
@@ -527,7 +562,7 @@ void __init at91_gpio_irq_setup(void)
irq_set_chip_data(pioc_hwirq, this);
irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
}
- pr_info("AT91: %d gpio irqs in %d banks\n", irq - gpio_to_irq(0), gpio_banks);
+ pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
}
/* gpiolib support */
@@ -600,39 +635,145 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
}
}
+static int __init at91_gpio_setup_clk(int idx)
+{
+ struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
+
+ /* retreive PIO controller's clock */
+ at91_gpio->clock = clk_get_sys(NULL, at91_gpio->chip.label);
+ if (IS_ERR(at91_gpio->clock)) {
+ pr_err("at91_gpio.%d, failed to get clock, ignoring.\n", idx);
+ goto err;
+ }
+
+ if (clk_prepare(at91_gpio->clock))
+ goto clk_prep_err;
+
+ /* enable PIO controller's clock */
+ if (clk_enable(at91_gpio->clock)) {
+ pr_err("at91_gpio.%d, failed to enable clock, ignoring.\n", idx);
+ goto clk_err;
+ }
+
+ return 0;
+
+clk_err:
+ clk_unprepare(at91_gpio->clock);
+clk_prep_err:
+ clk_put(at91_gpio->clock);
+err:
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF_GPIO
+static void __init of_at91_gpio_init_one(struct device_node *np)
+{
+ int alias_idx;
+ struct at91_gpio_chip *at91_gpio;
+
+ if (!np)
+ return;
+
+ alias_idx = of_alias_get_id(np, "gpio");
+ if (alias_idx >= MAX_GPIO_BANKS) {
+ pr_err("at91_gpio, failed alias idx(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
+ alias_idx, MAX_GPIO_BANKS);
+ return;
+ }
+
+ at91_gpio = &gpio_chip[alias_idx];
+ at91_gpio->chip.base = alias_idx * at91_gpio->chip.ngpio;
+
+ at91_gpio->regbase = of_iomap(np, 0);
+ if (!at91_gpio->regbase) {
+ pr_err("at91_gpio.%d, failed to map registers, ignoring.\n",
+ alias_idx);
+ return;
+ }
+
+ /* Get the interrupts property */
+ if (of_property_read_u32(np, "interrupts", &at91_gpio->pioc_hwirq)) {
+ pr_err("at91_gpio.%d, failed to get interrupts property, ignoring.\n",
+ alias_idx);
+ goto ioremap_err;
+ }
+
+ /* Setup clock */
+ if (at91_gpio_setup_clk(alias_idx))
+ goto ioremap_err;
+
+ at91_gpio->chip.of_node = np;
+ gpio_banks = max(gpio_banks, alias_idx + 1);
+ at91_gpio->pioc_idx = alias_idx;
+ return;
+
+ioremap_err:
+ iounmap(at91_gpio->regbase);
+}
+
+static int __init of_at91_gpio_init(void)
+{
+ struct device_node *np = NULL;
+
+ /*
+ * This isn't ideal, but it gets things hooked up until this
+ * driver is converted into a platform_device
+ */
+ for_each_compatible_node(np, NULL, "atmel,at91rm9200-gpio")
+ of_at91_gpio_init_one(np);
+
+ return gpio_banks > 0 ? 0 : -EINVAL;
+}
+#else
+static int __init of_at91_gpio_init(void)
+{
+ return -EINVAL;
+}
+#endif
+
+static void __init at91_gpio_init_one(int idx, u32 regbase, int pioc_hwirq)
+{
+ struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
+
+ at91_gpio->chip.base = idx * at91_gpio->chip.ngpio;
+ at91_gpio->pioc_hwirq = pioc_hwirq;
+ at91_gpio->pioc_idx = idx;
+
+ at91_gpio->regbase = ioremap(regbase, 512);
+ if (!at91_gpio->regbase) {
+ pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", idx);
+ return;
+ }
+
+ if (at91_gpio_setup_clk(idx))
+ goto ioremap_err;
+
+ gpio_banks = max(gpio_banks, idx + 1);
+ return;
+
+ioremap_err:
+ iounmap(at91_gpio->regbase);
+}
+
/*
* Called from the processor-specific init to enable GPIO pin support.
*/
void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
{
- unsigned i;
+ unsigned i;
struct at91_gpio_chip *at91_gpio, *last = NULL;
BUG_ON(nr_banks > MAX_GPIO_BANKS);
- gpio_banks = nr_banks;
+ if (of_at91_gpio_init() < 0) {
+ /* No GPIO controller found in device tree */
+ for (i = 0; i < nr_banks; i++)
+ at91_gpio_init_one(i, data[i].regbase, data[i].id);
+ }
- for (i = 0; i < nr_banks; i++) {
+ for (i = 0; i < gpio_banks; i++) {
at91_gpio = &gpio_chip[i];
- at91_gpio->pioc_hwirq = data[i].pioc_hwirq;
- at91_gpio->chip.base = i * 32;
-
- at91_gpio->regbase = ioremap(data[i].regbase, 512);
- if (!at91_gpio->regbase) {
- pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", i);
- continue;
- }
-
- at91_gpio->clock = clk_get_sys(NULL, at91_gpio->chip.label);
- if (!at91_gpio->clock) {
- pr_err("at91_gpio.%d, failed to get clock, ignoring.\n", i);
- continue;
- }
-
- /* enable PIO controller's clock */
- clk_enable(at91_gpio->clock);
-
/*
* GPIO controller are grouped on some SoC:
* PIOC, PIOD and PIOE can share the same IRQ line
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 6/9] ARM: at91/gpio: non-DT builds do not have gpio_chip.of_node field
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (3 preceding siblings ...)
2012-02-13 14:43 ` [PATCH v5 5/9] ARM: at91/gpio: add irqdomain and DT support Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH 7/9] ARM: at91/gpio: add .to_irq gpio_chip handler Nicolas Ferre
` (3 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
Protect build failure in case of non-DT configuration: the
gpio_chip structure does not have a of_node field in case of
!CONFIG_OF_GPIO.
Keep this in a separate patch as it can be reverted if the
field is added for both DT/non-DT cases.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
arch/arm/mach-at91/gpio.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 89e683a..0dc3f5e 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -496,12 +496,17 @@ postcore_initcall(at91_gpio_debugfs_init);
static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
{
int irq_base;
+#if defined(CONFIG_OF)
+ struct device_node *of_node = at91_gpio->chip.of_node;
+#else
+ struct device_node *of_node = NULL;
+#endif
irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
if (irq_base < 0)
panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
at91_gpio->pioc_idx, irq_base);
- at91_gpio->domain = irq_domain_add_legacy(at91_gpio->chip.of_node,
+ at91_gpio->domain = irq_domain_add_legacy(of_node,
at91_gpio->chip.ngpio,
irq_base, 0,
&irq_domain_simple_ops, NULL);
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 7/9] ARM: at91/gpio: add .to_irq gpio_chip handler
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (4 preceding siblings ...)
2012-02-13 14:43 ` [PATCH 6/9] ARM: at91/gpio: non-DT builds do not have gpio_chip.of_node field Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH 8/9] ARM: at91/gpio: remove the static specification of gpio_chip.base Nicolas Ferre
` (2 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
Replace the gpio_to_irq() macro by a plain gpiolib .to_irq() handler.
This call is using the irqdomain to translate hardware to Linux
IRQ numbers.
The irq_to_gpio() macro is completely removed.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
arch/arm/mach-at91/gpio.c | 13 +++++++++++++
arch/arm/mach-at91/include/mach/gpio.h | 12 ------------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 0dc3f5e..e8f5831 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -11,6 +11,7 @@
#include <linux/clk.h>
#include <linux/errno.h>
+#include <linux/device.h>
#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
@@ -48,6 +49,7 @@ static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
unsigned offset);
+static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
#define AT91_GPIO_CHIP(name, base_gpio, nr_gpio) \
{ \
@@ -59,6 +61,7 @@ static int at91_gpiolib_direction_input(struct gpio_chip *chip,
.set = at91_gpiolib_set, \
.dbg_show = at91_gpiolib_dbg_show, \
.base = base_gpio, \
+ .to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
}, \
}
@@ -640,6 +643,16 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
}
}
+static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ int virq = irq_find_mapping(at91_gpio->domain, offset);
+
+ dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
+ chip->label, offset + chip->base, virq);
+ return virq;
+}
+
static int __init at91_gpio_setup_clk(int idx)
{
struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
diff --git a/arch/arm/mach-at91/include/mach/gpio.h b/arch/arm/mach-at91/include/mach/gpio.h
index e3fd225..7cf009b 100644
--- a/arch/arm/mach-at91/include/mach/gpio.h
+++ b/arch/arm/mach-at91/include/mach/gpio.h
@@ -204,18 +204,6 @@ extern int at91_get_gpio_value(unsigned pin);
extern void at91_gpio_suspend(void);
extern void at91_gpio_resume(void);
-/*-------------------------------------------------------------------------*/
-
-/* wrappers for "new style" GPIO calls. the old AT91-specific ones should
- * eventually be removed (along with this errno.h inclusion), and the
- * gpio request/free calls should probably be implemented.
- */
-
-#include <asm/errno.h>
-
-#define gpio_to_irq(gpio) (gpio + NR_AIC_IRQS)
-#define irq_to_gpio(irq) (irq - NR_AIC_IRQS)
-
#endif /* __ASSEMBLY__ */
#endif
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 8/9] ARM: at91/gpio: remove the static specification of gpio_chip.base
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (5 preceding siblings ...)
2012-02-13 14:43 ` [PATCH 7/9] ARM: at91/gpio: add .to_irq gpio_chip handler Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 14:43 ` [PATCH 9/9] ARM: at91/board-dt: remove AIC irq domain from board file Nicolas Ferre
2012-02-13 22:10 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Rob Herring
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
This value is determined at runtime using device tree or platform data
information.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
arch/arm/mach-at91/gpio.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index e8f5831..ecf6bbb 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -51,7 +51,7 @@ static int at91_gpiolib_direction_input(struct gpio_chip *chip,
unsigned offset);
static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
-#define AT91_GPIO_CHIP(name, base_gpio, nr_gpio) \
+#define AT91_GPIO_CHIP(name, nr_gpio) \
{ \
.chip = { \
.label = name, \
@@ -60,18 +60,17 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.get = at91_gpiolib_get, \
.set = at91_gpiolib_set, \
.dbg_show = at91_gpiolib_dbg_show, \
- .base = base_gpio, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
}, \
}
static struct at91_gpio_chip gpio_chip[] = {
- AT91_GPIO_CHIP("pioA", 0x00, 32),
- AT91_GPIO_CHIP("pioB", 0x20, 32),
- AT91_GPIO_CHIP("pioC", 0x40, 32),
- AT91_GPIO_CHIP("pioD", 0x60, 32),
- AT91_GPIO_CHIP("pioE", 0x80, 32),
+ AT91_GPIO_CHIP("pioA", 32),
+ AT91_GPIO_CHIP("pioB", 32),
+ AT91_GPIO_CHIP("pioC", 32),
+ AT91_GPIO_CHIP("pioD", 32),
+ AT91_GPIO_CHIP("pioE", 32),
};
static int gpio_banks;
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 9/9] ARM: at91/board-dt: remove AIC irq domain from board file
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (6 preceding siblings ...)
2012-02-13 14:43 ` [PATCH 8/9] ARM: at91/gpio: remove the static specification of gpio_chip.base Nicolas Ferre
@ 2012-02-13 14:43 ` Nicolas Ferre
2012-02-13 22:10 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Rob Herring
8 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-13 14:43 UTC (permalink / raw)
To: plagnioj, linux-arm-kernel, grant.likely, rob.herring
Cc: tglx, devicetree-discuss, avictor.za, linux-kernel, Nicolas Ferre
Adding of irqdomain in AIC code make the specification
of the irq domain in board file useless.
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
arch/arm/mach-at91/board-dt.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
index 0579315..6120978 100644
--- a/arch/arm/mach-at91/board-dt.c
+++ b/arch/arm/mach-at91/board-dt.c
@@ -15,8 +15,6 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/gpio.h>
-#include <linux/irqdomain.h>
-#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <mach/hardware.h>
@@ -88,14 +86,8 @@ static void __init ek_add_device_nand(void)
at91_add_device_nand(&ek_nand_data);
}
-static const struct of_device_id aic_of_match[] __initconst = {
- { .compatible = "atmel,at91rm9200-aic", },
- {},
-};
-
static void __init at91_dt_init_irq(void)
{
- irq_domain_generate_simple(aic_of_match, 0xfffff000, 0);
at91_init_irq_default();
}
--
1.7.9
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support
2012-02-13 14:43 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Nicolas Ferre
` (7 preceding siblings ...)
2012-02-13 14:43 ` [PATCH 9/9] ARM: at91/board-dt: remove AIC irq domain from board file Nicolas Ferre
@ 2012-02-13 22:10 ` Rob Herring
2012-02-14 10:24 ` Nicolas Ferre
8 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2012-02-13 22:10 UTC (permalink / raw)
To: Nicolas Ferre
Cc: plagnioj, linux-arm-kernel, grant.likely, rob.herring,
linux-kernel, tglx, devicetree-discuss, avictor.za
On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
> Add an irqdomain for the AIC interrupt controller.
> The device tree support is mapping the registers and
> is using the irq_domain_add_legacy() to manage hwirq
> translation.
> The documentation is describing the meaning of the
> two cells required for using this "interrupt-controller"
> in a device tree node.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
> patch series
> - use of irq_domain_add_legacy() API
>
> v4: - use irq_alloc_descs() to find irq_base
> - add a new constant AIC_BASE_IRQ that will allow to skip
> first interrupt numbers (in the future)
>
> v3: - change number of cells to define an AIC interrupt (irq trigger type)
> - change current .dtsi files to match specification
> - use irq_domain_simple_ops (for DT mapping)
>
> v2: - use of_irq_init() function for device tree probing
> - add documentation
> - use own simple struct irq_domain_ops
>
>
> .../devicetree/bindings/arm/atmel-aic.txt | 38 ++++++++++
> arch/arm/Kconfig | 1 +
> arch/arm/boot/dts/at91sam9g20.dtsi | 18 +++---
> arch/arm/boot/dts/at91sam9g45.dtsi | 16 ++--
> arch/arm/mach-at91/include/mach/irqs.h | 3 +-
> arch/arm/mach-at91/irq.c | 74 ++++++++++++++++----
> 6 files changed, 119 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/atmel-aic.txt b/Documentation/devicetree/bindings/arm/atmel-aic.txt
> new file mode 100644
> index 0000000..289feb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/atmel-aic.txt
> @@ -0,0 +1,38 @@
> +* Advanced Interrupt Controller (AIC)
> +
> +Required properties:
> +- compatible: Should be "atmel,<chip>-aic"
> +- interrupt-controller: Identifies the node as an interrupt controller.
> +- interrupt-parent: For single AIC system, it is an empty property.
> +- #interrupt-cells: The number of cells to define the interrupts. It sould be 2.
> + The first cell is the GPIO number.
s/GPIO/IRQ/
> + 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.
> + Default flag for internal sources should be set to 4 (active high).
> +- reg: Should contain AIC registers location and length
> +
> +Examples:
> + /*
> + * AIC
> + */
> + aic: interrupt-controller@fffff000 {
> + compatible = "atmel,at91rm9200-aic";
> + interrupt-controller;
> + interrupt-parent;
> + #interrupt-cells = <2>;
> + reg = <0xfffff000 0x200>;
> + };
> +
> + /*
> + * An interrupt generating device that is wired to an AIC.
> + */
> + dma: dma-controller@ffffec00 {
> + compatible = "atmel,at91sam9g45-dma";
> + reg = <0xffffec00 0x200>;
> + interrupts = <21 4>;
> + };
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 92c9c79..cabd8f5 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -322,6 +322,7 @@ config ARCH_AT91
> select ARCH_REQUIRE_GPIOLIB
> select HAVE_CLK
> select CLKDEV_LOOKUP
> + select IRQ_DOMAIN
This can likely go away assuming my change to select IRQ_DOMAIN for all
of ARM goes upstream.
> help
> This enables support for systems based on the Atmel AT91RM9200,
> AT91SAM9 processors.
> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> index 07603b8..9a0aee7 100644
> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> @@ -47,7 +47,7 @@
> ranges;
>
> aic: interrupt-controller@fffff000 {
> - #interrupt-cells = <1>;
> + #interrupt-cells = <2>;
> compatible = "atmel,at91rm9200-aic";
> interrupt-controller;
> interrupt-parent;
> @@ -57,14 +57,14 @@
> dbgu: serial@fffff200 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffff200 0x200>;
> - interrupts = <1>;
> + interrupts = <1 4>;
> status = "disabled";
> };
>
> usart0: serial@fffb0000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffb0000 0x200>;
> - interrupts = <6>;
> + interrupts = <6 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -73,7 +73,7 @@
> usart1: serial@fffb4000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffb4000 0x200>;
> - interrupts = <7>;
> + interrupts = <7 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -82,7 +82,7 @@
> usart2: serial@fffb8000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffb8000 0x200>;
> - interrupts = <8>;
> + interrupts = <8 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -91,7 +91,7 @@
> usart3: serial@fffd0000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffd0000 0x200>;
> - interrupts = <23>;
> + interrupts = <23 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -100,7 +100,7 @@
> usart4: serial@fffd4000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffd4000 0x200>;
> - interrupts = <24>;
> + interrupts = <24 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -109,7 +109,7 @@
> usart5: serial@fffd8000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfffd8000 0x200>;
> - interrupts = <25>;
> + interrupts = <25 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -118,7 +118,7 @@
> macb0: ethernet@fffc4000 {
> compatible = "cdns,at32ap7000-macb", "cdns,macb";
> reg = <0xfffc4000 0x100>;
> - interrupts = <21>;
> + interrupts = <21 4>;
> status = "disabled";
> };
> };
> diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
> index fffa005..67f94d3 100644
> --- a/arch/arm/boot/dts/at91sam9g45.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g45.dtsi
> @@ -46,7 +46,7 @@
> ranges;
>
> aic: interrupt-controller@fffff000 {
> - #interrupt-cells = <1>;
> + #interrupt-cells = <2>;
> compatible = "atmel,at91rm9200-aic";
> interrupt-controller;
> interrupt-parent;
> @@ -56,20 +56,20 @@
> dma: dma-controller@ffffec00 {
> compatible = "atmel,at91sam9g45-dma";
> reg = <0xffffec00 0x200>;
> - interrupts = <21>;
> + interrupts = <21 4>;
> };
>
> dbgu: serial@ffffee00 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xffffee00 0x200>;
> - interrupts = <1>;
> + interrupts = <1 4>;
> status = "disabled";
> };
>
> usart0: serial@fff8c000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfff8c000 0x200>;
> - interrupts = <7>;
> + interrupts = <7 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -78,7 +78,7 @@
> usart1: serial@fff90000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfff90000 0x200>;
> - interrupts = <8>;
> + interrupts = <8 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -87,7 +87,7 @@
> usart2: serial@fff94000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfff94000 0x200>;
> - interrupts = <9>;
> + interrupts = <9 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -96,7 +96,7 @@
> usart3: serial@fff98000 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0xfff98000 0x200>;
> - interrupts = <10>;
> + interrupts = <10 4>;
> atmel,use-dma-rx;
> atmel,use-dma-tx;
> status = "disabled";
> @@ -105,7 +105,7 @@
> macb0: ethernet@fffbc000 {
> compatible = "cdns,at32ap7000-macb", "cdns,macb";
> reg = <0xfffbc000 0x100>;
> - interrupts = <25>;
> + interrupts = <25 4>;
> status = "disabled";
> };
> };
> diff --git a/arch/arm/mach-at91/include/mach/irqs.h b/arch/arm/mach-at91/include/mach/irqs.h
> index ac8b7df..e3ee0fc 100644
> --- a/arch/arm/mach-at91/include/mach/irqs.h
> +++ b/arch/arm/mach-at91/include/mach/irqs.h
> @@ -25,6 +25,7 @@
> #include <mach/at91_aic.h>
>
> #define NR_AIC_IRQS 32
> +#define AIC_BASE_IRQ 0
>
>
> /*
> @@ -40,7 +41,7 @@
> * symbols in gpio.h for ones handled indirectly as GPIOs.
> * We make provision for 5 banks of GPIO.
> */
> -#define NR_IRQS (NR_AIC_IRQS + (5 * 32))
> +#define NR_IRQS (AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
Why? You're not changing anything.
You should turn on SPARSE_IRQ at some point and then this value goes away.
>
> /* FIQ is AIC source 0. */
> #define FIQ_START AT91_ID_FIQ
> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
> index be6b639..880da98 100644
> --- a/arch/arm/mach-at91/irq.c
> +++ b/arch/arm/mach-at91/irq.c
> @@ -24,6 +24,12 @@
> #include <linux/module.h>
> #include <linux/mm.h>
> #include <linux/types.h>
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/err.h>
>
> #include <mach/hardware.h>
> #include <asm/irq.h>
> @@ -34,22 +40,24 @@
> #include <asm/mach/map.h>
>
> void __iomem *at91_aic_base;
> +static struct irq_domain *at91_aic_domain;
> +static struct device_node *at91_aic_np;
>
> static void at91_aic_mask_irq(struct irq_data *d)
> {
> /* Disable interrupt on AIC */
> - at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
> + at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
> }
>
> static void at91_aic_unmask_irq(struct irq_data *d)
> {
> /* Enable interrupt on AIC */
> - at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
> + at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
> }
>
> unsigned int at91_extern_irq;
>
> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>
> static int at91_aic_set_type(struct irq_data *d, unsigned type)
> {
> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
> srctype = AT91_AIC_SRCTYPE_RISING;
> break;
> case IRQ_TYPE_LEVEL_LOW:
> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
> srctype = AT91_AIC_SRCTYPE_LOW;
> else
> return -EINVAL;
> break;
> case IRQ_TYPE_EDGE_FALLING:
> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
> srctype = AT91_AIC_SRCTYPE_FALLING;
> else
> return -EINVAL;
> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
> return -EINVAL;
> }
>
> - smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
> - at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
> + smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
> + at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
> return 0;
> }
>
> @@ -90,13 +98,13 @@ static u32 backups;
>
> static int at91_aic_set_wake(struct irq_data *d, unsigned value)
> {
> - if (unlikely(d->irq >= 32))
> + if (unlikely(d->hwirq >= NR_AIC_IRQS))
> return -EINVAL;
>
> if (value)
> - wakeups |= (1 << d->irq);
> + wakeups |= (1 << d->hwirq);
> else
> - wakeups &= ~(1 << d->irq);
> + wakeups &= ~(1 << d->hwirq);
>
> return 0;
> }
> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
> .irq_set_wake = at91_aic_set_wake,
> };
>
> +#if defined(CONFIG_OF)
> +static int __init __at91_aic_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + at91_aic_base = of_iomap(node, 0);
> + at91_aic_np = node;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aic_ids[] __initconst = {
> + { .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
> + { /*sentinel*/ }
> +};
> +
> +static void __init at91_aic_of_init(void)
> +{
> + of_irq_init(aic_ids);
This call and match str belongs in your board file (init_irq function)
and you should be doing all setup in __at91_aic_of_init.
> +}
> +#else
> +static void __init at91_aic_of_init(void) {}
> +#endif
> +
> /*
> * Initialize the AIC interrupt controller.
> */
> void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
Perhaps the priority should be a cell in your interrupts property?
> {
> unsigned int i;
> + int irq_base;
>
> - at91_aic_base = ioremap(AT91_AIC, 512);
> + if (of_have_populated_dt())
> + at91_aic_of_init();
You should get to this point because of_irq_init called you and you
should already have a node ptr at point.
> + else
> + at91_aic_base = ioremap(AT91_AIC, 512);
>
> if (!at91_aic_base)
> - panic("Impossible to ioremap AT91_AIC\n");
> + panic("Unable to ioremap AIC registers\n");
> +
> + /* Add irq domain for AIC */
> + irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
Really, you don't want to use irq0, but that would break your irq entry
code.
Rob
> + if (irq_base < 0) {
> + WARN(1, "Cannot allocate irq_descs, assuming pre-allocated\n");
> + irq_base = AIC_BASE_IRQ;
> + }
> + at91_aic_domain = irq_domain_add_legacy(at91_aic_np, NR_AIC_IRQS,
> + irq_base, 0,
> + &irq_domain_simple_ops, NULL);
> +
> + if (!at91_aic_domain)
> + panic("Unable to add AIC irq domain\n");
>
> /*
> * The IVR is used by macro get_irqnr_and_base to read and verify.
> * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
> */
> for (i = 0; i < NR_AIC_IRQS; i++) {
> - /* Put irq number in Source Vector Register: */
> + /* Put hardware irq number in Source Vector Register: */
> at91_aic_write(AT91_AIC_SVR(i), i);
> /* Active Low interrupt, with the specified priority */
> at91_aic_write(AT91_AIC_SMR(i), AT91_AIC_SRCTYPE_LOW | priority[i]);
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support
2012-02-13 22:10 ` [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support Rob Herring
@ 2012-02-14 10:24 ` Nicolas Ferre
2012-02-14 14:11 ` Rob Herring
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-14 10:24 UTC (permalink / raw)
To: Rob Herring, plagnioj
Cc: linux-arm-kernel, grant.likely, rob.herring, linux-kernel, tglx,
devicetree-discuss, avictor.za
On 02/13/2012 11:10 PM, Rob Herring :
> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>> Add an irqdomain for the AIC interrupt controller.
>> The device tree support is mapping the registers and
>> is using the irq_domain_add_legacy() to manage hwirq
>> translation.
>> The documentation is describing the meaning of the
>> two cells required for using this "interrupt-controller"
>> in a device tree node.
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> ---
>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>> patch series
>> - use of irq_domain_add_legacy() API
>>
>> v4: - use irq_alloc_descs() to find irq_base
>> - add a new constant AIC_BASE_IRQ that will allow to skip
>> first interrupt numbers (in the future)
>>
>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>> - change current .dtsi files to match specification
>> - use irq_domain_simple_ops (for DT mapping)
>>
>> v2: - use of_irq_init() function for device tree probing
>> - add documentation
>> - use own simple struct irq_domain_ops
>>
>>
>> .../devicetree/bindings/arm/atmel-aic.txt | 38 ++++++++++
>> arch/arm/Kconfig | 1 +
>> arch/arm/boot/dts/at91sam9g20.dtsi | 18 +++---
>> arch/arm/boot/dts/at91sam9g45.dtsi | 16 ++--
>> arch/arm/mach-at91/include/mach/irqs.h | 3 +-
>> arch/arm/mach-at91/irq.c | 74 ++++++++++++++++----
>> 6 files changed, 119 insertions(+), 31 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
[..]
>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>> @@ -25,6 +25,7 @@
>> #include <mach/at91_aic.h>
>>
>> #define NR_AIC_IRQS 32
>> +#define AIC_BASE_IRQ 0
>>
>>
>> /*
>> @@ -40,7 +41,7 @@
>> * symbols in gpio.h for ones handled indirectly as GPIOs.
>> * We make provision for 5 banks of GPIO.
>> */
>> -#define NR_IRQS (NR_AIC_IRQS + (5 * 32))
>> +#define NR_IRQS (AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
>
> Why? You're not changing anything.
>
> You should turn on SPARSE_IRQ at some point and then this value goes away.
Yes, my intention is to move to this step by step. I have tried hard to
rework completely the AIC driver without success so far. I plan:
- move the AIC_BASE_IRQ away from irq0
=> this is why I introduce a constant here
- use SPARSE_IRQ
- move to generic irq & irq domain helpers (move DT case to "linear")
*once* the code for handling all this is stabilized and integrated in
mainline.
This rework is designed for 3.4 and a lot of code depend on this for all
our at91 DT move.
>> /* FIQ is AIC source 0. */
>> #define FIQ_START AT91_ID_FIQ
>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>> index be6b639..880da98 100644
>> --- a/arch/arm/mach-at91/irq.c
>> +++ b/arch/arm/mach-at91/irq.c
>> @@ -24,6 +24,12 @@
>> #include <linux/module.h>
>> #include <linux/mm.h>
>> #include <linux/types.h>
>> +#include <linux/irq.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/err.h>
>>
>> #include <mach/hardware.h>
>> #include <asm/irq.h>
>> @@ -34,22 +40,24 @@
>> #include <asm/mach/map.h>
>>
>> void __iomem *at91_aic_base;
>> +static struct irq_domain *at91_aic_domain;
>> +static struct device_node *at91_aic_np;
>>
>> static void at91_aic_mask_irq(struct irq_data *d)
>> {
>> /* Disable interrupt on AIC */
>> - at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>> + at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>> }
>>
>> static void at91_aic_unmask_irq(struct irq_data *d)
>> {
>> /* Enable interrupt on AIC */
>> - at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>> + at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>> }
>>
>> unsigned int at91_extern_irq;
>>
>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>
>> static int at91_aic_set_type(struct irq_data *d, unsigned type)
>> {
>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>> srctype = AT91_AIC_SRCTYPE_RISING;
>> break;
>> case IRQ_TYPE_LEVEL_LOW:
>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>> srctype = AT91_AIC_SRCTYPE_LOW;
>> else
>> return -EINVAL;
>> break;
>> case IRQ_TYPE_EDGE_FALLING:
>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>> srctype = AT91_AIC_SRCTYPE_FALLING;
>> else
>> return -EINVAL;
>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>> return -EINVAL;
>> }
>>
>> - smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>> - at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>> + smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>> + at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>> return 0;
>> }
>>
>> @@ -90,13 +98,13 @@ static u32 backups;
>>
>> static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>> {
>> - if (unlikely(d->irq >= 32))
>> + if (unlikely(d->hwirq >= NR_AIC_IRQS))
>> return -EINVAL;
>>
>> if (value)
>> - wakeups |= (1 << d->irq);
>> + wakeups |= (1 << d->hwirq);
>> else
>> - wakeups &= ~(1 << d->irq);
>> + wakeups &= ~(1 << d->hwirq);
>>
>> return 0;
>> }
>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>> .irq_set_wake = at91_aic_set_wake,
>> };
>>
>> +#if defined(CONFIG_OF)
>> +static int __init __at91_aic_of_init(struct device_node *node,
>> + struct device_node *parent)
>> +{
>> + at91_aic_base = of_iomap(node, 0);
>> + at91_aic_np = node;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id aic_ids[] __initconst = {
>> + { .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>> + { /*sentinel*/ }
>> +};
>> +
>> +static void __init at91_aic_of_init(void)
>> +{
>> + of_irq_init(aic_ids);
>
> This call and match str belongs in your board file (init_irq function)
> and you should be doing all setup in __at91_aic_of_init.
It has already been discussed here:
http://article.gmane.org/gmane.linux.drivers.devicetree/9834
I feel that this self-contained driver is the best solution for the
moment. Once the priorities will be encoded in the device tree (and
moreover once they will be useful for threaded interrupts), we may
re-consider this.
>> +}
>> +#else
>> +static void __init at91_aic_of_init(void) {}
>> +#endif
>> +
>> /*
>> * Initialize the AIC interrupt controller.
>> */
>> void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>
> Perhaps the priority should be a cell in your interrupts property?
Yes, it is a good idea. But that will prevent me from using a standard
irq_domain simple xlate operation... Maybe we can think about this a bit
more and implement it in the future.
>> {
>> unsigned int i;
>> + int irq_base;
>>
>> - at91_aic_base = ioremap(AT91_AIC, 512);
>> + if (of_have_populated_dt())
>> + at91_aic_of_init();
>
> You should get to this point because of_irq_init called you and you
> should already have a node ptr at point.
>
>> + else
>> + at91_aic_base = ioremap(AT91_AIC, 512);
>>
>> if (!at91_aic_base)
>> - panic("Impossible to ioremap AT91_AIC\n");
>> + panic("Unable to ioremap AIC registers\n");
>> +
>> + /* Add irq domain for AIC */
>> + irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
>
> Really, you don't want to use irq0, but that would break your irq entry
> code.
Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
that will require a little modification of each board file. This
scattered modification is maybe a bit late for 3.4 inclusion...
Best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support
2012-02-14 10:24 ` Nicolas Ferre
@ 2012-02-14 14:11 ` Rob Herring
2012-02-17 9:26 ` Nicolas Ferre
0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2012-02-14 14:11 UTC (permalink / raw)
To: Nicolas Ferre
Cc: plagnioj, linux-arm-kernel, grant.likely, rob.herring,
linux-kernel, tglx, devicetree-discuss, avictor.za
On 02/14/2012 04:24 AM, Nicolas Ferre wrote:
> On 02/13/2012 11:10 PM, Rob Herring :
>> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>>> Add an irqdomain for the AIC interrupt controller.
>>> The device tree support is mapping the registers and
>>> is using the irq_domain_add_legacy() to manage hwirq
>>> translation.
>>> The documentation is describing the meaning of the
>>> two cells required for using this "interrupt-controller"
>>> in a device tree node.
>>>
>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> ---
>>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>>> patch series
>>> - use of irq_domain_add_legacy() API
>>>
>>> v4: - use irq_alloc_descs() to find irq_base
>>> - add a new constant AIC_BASE_IRQ that will allow to skip
>>> first interrupt numbers (in the future)
>>>
>>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>>> - change current .dtsi files to match specification
>>> - use irq_domain_simple_ops (for DT mapping)
>>>
>>> v2: - use of_irq_init() function for device tree probing
>>> - add documentation
>>> - use own simple struct irq_domain_ops
>>>
>>>
>>> .../devicetree/bindings/arm/atmel-aic.txt | 38 ++++++++++
>>> arch/arm/Kconfig | 1 +
>>> arch/arm/boot/dts/at91sam9g20.dtsi | 18 +++---
>>> arch/arm/boot/dts/at91sam9g45.dtsi | 16 ++--
>>> arch/arm/mach-at91/include/mach/irqs.h | 3 +-
>>> arch/arm/mach-at91/irq.c | 74 ++++++++++++++++----
>>> 6 files changed, 119 insertions(+), 31 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
>
> [..]
>
>>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>>> @@ -25,6 +25,7 @@
>>> #include <mach/at91_aic.h>
>>>
>>> #define NR_AIC_IRQS 32
>>> +#define AIC_BASE_IRQ 0
>>>
>>>
>>> /*
>>> @@ -40,7 +41,7 @@
>>> * symbols in gpio.h for ones handled indirectly as GPIOs.
>>> * We make provision for 5 banks of GPIO.
>>> */
>>> -#define NR_IRQS (NR_AIC_IRQS + (5 * 32))
>>> +#define NR_IRQS (AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
>>
>> Why? You're not changing anything.
>>
>> You should turn on SPARSE_IRQ at some point and then this value goes away.
>
> Yes, my intention is to move to this step by step. I have tried hard to
> rework completely the AIC driver without success so far. I plan:
> - move the AIC_BASE_IRQ away from irq0
> => this is why I introduce a constant here
> - use SPARSE_IRQ
> - move to generic irq & irq domain helpers (move DT case to "linear")
>
> *once* the code for handling all this is stabilized and integrated in
> mainline.
>
> This rework is designed for 3.4 and a lot of code depend on this for all
> our at91 DT move.
In that case, NR_IRQS will be removed. You will get the default of 16
(NR_LEGACY_IRQS) and they will get reserved so irq_alloc_descs will skip
over them. So there is no reason for you to have AIC_BASE_IRQ.
>
>
>>> /* FIQ is AIC source 0. */
>>> #define FIQ_START AT91_ID_FIQ
>>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>>> index be6b639..880da98 100644
>>> --- a/arch/arm/mach-at91/irq.c
>>> +++ b/arch/arm/mach-at91/irq.c
>>> @@ -24,6 +24,12 @@
>>> #include <linux/module.h>
>>> #include <linux/mm.h>
>>> #include <linux/types.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/err.h>
>>>
>>> #include <mach/hardware.h>
>>> #include <asm/irq.h>
>>> @@ -34,22 +40,24 @@
>>> #include <asm/mach/map.h>
>>>
>>> void __iomem *at91_aic_base;
>>> +static struct irq_domain *at91_aic_domain;
>>> +static struct device_node *at91_aic_np;
>>>
>>> static void at91_aic_mask_irq(struct irq_data *d)
>>> {
>>> /* Disable interrupt on AIC */
>>> - at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>>> + at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>>> }
>>>
>>> static void at91_aic_unmask_irq(struct irq_data *d)
>>> {
>>> /* Enable interrupt on AIC */
>>> - at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>>> + at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>>> }
>>>
>>> unsigned int at91_extern_irq;
>>>
>>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>>
>>> static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>> {
>>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>> srctype = AT91_AIC_SRCTYPE_RISING;
>>> break;
>>> case IRQ_TYPE_LEVEL_LOW:
>>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>>> srctype = AT91_AIC_SRCTYPE_LOW;
>>> else
>>> return -EINVAL;
>>> break;
>>> case IRQ_TYPE_EDGE_FALLING:
>>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>>> srctype = AT91_AIC_SRCTYPE_FALLING;
>>> else
>>> return -EINVAL;
>>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>> return -EINVAL;
>>> }
>>>
>>> - smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>>> - at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>>> + smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>>> + at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>>> return 0;
>>> }
>>>
>>> @@ -90,13 +98,13 @@ static u32 backups;
>>>
>>> static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>>> {
>>> - if (unlikely(d->irq >= 32))
>>> + if (unlikely(d->hwirq >= NR_AIC_IRQS))
>>> return -EINVAL;
>>>
>>> if (value)
>>> - wakeups |= (1 << d->irq);
>>> + wakeups |= (1 << d->hwirq);
>>> else
>>> - wakeups &= ~(1 << d->irq);
>>> + wakeups &= ~(1 << d->hwirq);
>>>
>>> return 0;
>>> }
>>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>>> .irq_set_wake = at91_aic_set_wake,
>>> };
>>>
>>> +#if defined(CONFIG_OF)
>>> +static int __init __at91_aic_of_init(struct device_node *node,
>>> + struct device_node *parent)
>>> +{
>>> + at91_aic_base = of_iomap(node, 0);
>>> + at91_aic_np = node;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id aic_ids[] __initconst = {
>>> + { .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>>> + { /*sentinel*/ }
>>> +};
>>> +
>>> +static void __init at91_aic_of_init(void)
>>> +{
>>> + of_irq_init(aic_ids);
>>
>> This call and match str belongs in your board file (init_irq function)
>> and you should be doing all setup in __at91_aic_of_init.
>
> It has already been discussed here:
> http://article.gmane.org/gmane.linux.drivers.devicetree/9834
>
> I feel that this self-contained driver is the best solution for the
> moment. Once the priorities will be encoded in the device tree (and
> moreover once they will be useful for threaded interrupts), we may
> re-consider this.
>
Can you setup the priorities after init with a separate function? Then
you can follow standard init flow and afterwards set the priorities.
BTW, We're probably going to start moving irqchips to a common
drivers/irqchips. So it's important that init flow be consistent with
other irqchips.
>>> +}
>>> +#else
>>> +static void __init at91_aic_of_init(void) {}
>>> +#endif
>>> +
>>> /*
>>> * Initialize the AIC interrupt controller.
>>> */
>>> void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>
>> Perhaps the priority should be a cell in your interrupts property?
>
> Yes, it is a good idea. But that will prevent me from using a standard
> irq_domain simple xlate operation... Maybe we can think about this a bit
> more and implement it in the future.
That's not a good argument. What Linux has or doesn't have should not
really influence your binding. What is the right thing to do?
You can put the priority into the upper half word of cell 2. The trigger
type is masked. But you would still need a custom translate function to
extract the priority and set it.
You could perhaps just put all the priorities as a property of the
interrupt controller node. Then you don't have to increase the cell size.
>>> {
>>> unsigned int i;
>>> + int irq_base;
>>>
>>> - at91_aic_base = ioremap(AT91_AIC, 512);
>>> + if (of_have_populated_dt())
>>> + at91_aic_of_init();
>>
>> You should get to this point because of_irq_init called you and you
>> should already have a node ptr at point.
>>
>>> + else
>>> + at91_aic_base = ioremap(AT91_AIC, 512);
>>>
>>> if (!at91_aic_base)
>>> - panic("Impossible to ioremap AT91_AIC\n");
>>> + panic("Unable to ioremap AIC registers\n");
>>> +
>>> + /* Add irq domain for AIC */
>>> + irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
>>
>> Really, you don't want to use irq0, but that would break your irq entry
>> code.
>
> Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
> that will require a little modification of each board file. This
> scattered modification is maybe a bit late for 3.4 inclusion...
>
Okay. It's fine for now. Just making sure you're aware of the issue.
Rob
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support
2012-02-14 14:11 ` Rob Herring
@ 2012-02-17 9:26 ` Nicolas Ferre
0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2012-02-17 9:26 UTC (permalink / raw)
To: Rob Herring, grant.likely, plagnioj, devicetree-discuss
Cc: linux-arm-kernel, rob.herring, linux-kernel, tglx, avictor.za
On 02/14/2012 03:11 PM, Rob Herring :
> On 02/14/2012 04:24 AM, Nicolas Ferre wrote:
>> On 02/13/2012 11:10 PM, Rob Herring :
>>> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>>>> Add an irqdomain for the AIC interrupt controller.
>>>> The device tree support is mapping the registers and
>>>> is using the irq_domain_add_legacy() to manage hwirq
>>>> translation.
>>>> The documentation is describing the meaning of the
>>>> two cells required for using this "interrupt-controller"
>>>> in a device tree node.
>>>>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>> ---
>>>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>>>> patch series
>>>> - use of irq_domain_add_legacy() API
>>>>
>>>> v4: - use irq_alloc_descs() to find irq_base
>>>> - add a new constant AIC_BASE_IRQ that will allow to skip
>>>> first interrupt numbers (in the future)
>>>>
>>>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>>>> - change current .dtsi files to match specification
>>>> - use irq_domain_simple_ops (for DT mapping)
>>>>
>>>> v2: - use of_irq_init() function for device tree probing
>>>> - add documentation
>>>> - use own simple struct irq_domain_ops
>>>>
>>>>
>>>> .../devicetree/bindings/arm/atmel-aic.txt | 38 ++++++++++
>>>> arch/arm/Kconfig | 1 +
>>>> arch/arm/boot/dts/at91sam9g20.dtsi | 18 +++---
>>>> arch/arm/boot/dts/at91sam9g45.dtsi | 16 ++--
>>>> arch/arm/mach-at91/include/mach/irqs.h | 3 +-
>>>> arch/arm/mach-at91/irq.c | 74 ++++++++++++++++----
>>>> 6 files changed, 119 insertions(+), 31 deletions(-)
>>>> create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
>>
>> [..]
>>
>>>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>>>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>>>> @@ -25,6 +25,7 @@
>>>> #include <mach/at91_aic.h>
>>>>
>>>> #define NR_AIC_IRQS 32
>>>> +#define AIC_BASE_IRQ 0
>>>>
>>>>
>>>> /*
>>>> @@ -40,7 +41,7 @@
>>>> * symbols in gpio.h for ones handled indirectly as GPIOs.
>>>> * We make provision for 5 banks of GPIO.
>>>> */
>>>> -#define NR_IRQS (NR_AIC_IRQS + (5 * 32))
>>>> +#define NR_IRQS (AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
>>>
>>> Why? You're not changing anything.
>>>
>>> You should turn on SPARSE_IRQ at some point and then this value goes away.
>>
>> Yes, my intention is to move to this step by step. I have tried hard to
>> rework completely the AIC driver without success so far. I plan:
>> - move the AIC_BASE_IRQ away from irq0
>> => this is why I introduce a constant here
>> - use SPARSE_IRQ
>> - move to generic irq & irq domain helpers (move DT case to "linear")
>>
>> *once* the code for handling all this is stabilized and integrated in
>> mainline.
>>
>> This rework is designed for 3.4 and a lot of code depend on this for all
>> our at91 DT move.
>
> In that case, NR_IRQS will be removed. You will get the default of 16
> (NR_LEGACY_IRQS) and they will get reserved so irq_alloc_descs will skip
> over them. So there is no reason for you to have AIC_BASE_IRQ.
Fair enough, that indeed makes a lot of sense: I remove it.
>>>> /* FIQ is AIC source 0. */
>>>> #define FIQ_START AT91_ID_FIQ
>>>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>>>> index be6b639..880da98 100644
>>>> --- a/arch/arm/mach-at91/irq.c
>>>> +++ b/arch/arm/mach-at91/irq.c
>>>> @@ -24,6 +24,12 @@
>>>> #include <linux/module.h>
>>>> #include <linux/mm.h>
>>>> #include <linux/types.h>
>>>> +#include <linux/irq.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/of_irq.h>
>>>> +#include <linux/irqdomain.h>
>>>> +#include <linux/err.h>
>>>>
>>>> #include <mach/hardware.h>
>>>> #include <asm/irq.h>
>>>> @@ -34,22 +40,24 @@
>>>> #include <asm/mach/map.h>
>>>>
>>>> void __iomem *at91_aic_base;
>>>> +static struct irq_domain *at91_aic_domain;
>>>> +static struct device_node *at91_aic_np;
>>>>
>>>> static void at91_aic_mask_irq(struct irq_data *d)
>>>> {
>>>> /* Disable interrupt on AIC */
>>>> - at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>>>> + at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>>>> }
>>>>
>>>> static void at91_aic_unmask_irq(struct irq_data *d)
>>>> {
>>>> /* Enable interrupt on AIC */
>>>> - at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>>>> + at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>>>> }
>>>>
>>>> unsigned int at91_extern_irq;
>>>>
>>>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>>>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>>>
>>>> static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>> {
>>>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>> srctype = AT91_AIC_SRCTYPE_RISING;
>>>> break;
>>>> case IRQ_TYPE_LEVEL_LOW:
>>>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>>>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>>>> srctype = AT91_AIC_SRCTYPE_LOW;
>>>> else
>>>> return -EINVAL;
>>>> break;
>>>> case IRQ_TYPE_EDGE_FALLING:
>>>> - if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq)) /* only supported on external interrupts */
>>>> + if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq)) /* only supported on external interrupts */
>>>> srctype = AT91_AIC_SRCTYPE_FALLING;
>>>> else
>>>> return -EINVAL;
>>>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>> return -EINVAL;
>>>> }
>>>>
>>>> - smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>>>> - at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>>>> + smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>>>> + at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>>>> return 0;
>>>> }
>>>>
>>>> @@ -90,13 +98,13 @@ static u32 backups;
>>>>
>>>> static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>>>> {
>>>> - if (unlikely(d->irq >= 32))
>>>> + if (unlikely(d->hwirq >= NR_AIC_IRQS))
>>>> return -EINVAL;
>>>>
>>>> if (value)
>>>> - wakeups |= (1 << d->irq);
>>>> + wakeups |= (1 << d->hwirq);
>>>> else
>>>> - wakeups &= ~(1 << d->irq);
>>>> + wakeups &= ~(1 << d->hwirq);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>>>> .irq_set_wake = at91_aic_set_wake,
>>>> };
>>>>
>>>> +#if defined(CONFIG_OF)
>>>> +static int __init __at91_aic_of_init(struct device_node *node,
>>>> + struct device_node *parent)
>>>> +{
>>>> + at91_aic_base = of_iomap(node, 0);
>>>> + at91_aic_np = node;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id aic_ids[] __initconst = {
>>>> + { .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>>>> + { /*sentinel*/ }
>>>> +};
>>>> +
>>>> +static void __init at91_aic_of_init(void)
>>>> +{
>>>> + of_irq_init(aic_ids);
>>>
>>> This call and match str belongs in your board file (init_irq function)
>>> and you should be doing all setup in __at91_aic_of_init.
>>
>> It has already been discussed here:
>> http://article.gmane.org/gmane.linux.drivers.devicetree/9834
>>
>> I feel that this self-contained driver is the best solution for the
>> moment. Once the priorities will be encoded in the device tree (and
>> moreover once they will be useful for threaded interrupts), we may
>> re-consider this.
>>
>
> Can you setup the priorities after init with a separate function? Then
> you can follow standard init flow and afterwards set the priorities.
>
> BTW, We're probably going to start moving irqchips to a common
> drivers/irqchips. So it's important that init flow be consistent with
> other irqchips.
That is indeed a strong argument.
I have made a bit of renewal of the initialization process which should
now match the standard flow. I post real-soon-now an additional patch
that goes on top of this series to address your advice.
In this new patch, the priorities are not managed at the moment but we
plan to use a "default" priority table which will be part of the AIC
node and, as you suggested, an additional optional cell in the
"interrupts" property itself.
Before implementing this, I would like to have your feedback on the
whole series (together with its brand-new additional patch).
>>>> +}
>>>> +#else
>>>> +static void __init at91_aic_of_init(void) {}
>>>> +#endif
>>>> +
>>>> /*
>>>> * Initialize the AIC interrupt controller.
>>>> */
>>>> void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>>
>>> Perhaps the priority should be a cell in your interrupts property?
>>
>> Yes, it is a good idea. But that will prevent me from using a standard
>> irq_domain simple xlate operation... Maybe we can think about this a bit
>> more and implement it in the future.
>
> That's not a good argument. What Linux has or doesn't have should not
> really influence your binding. What is the right thing to do?
>
> You can put the priority into the upper half word of cell 2. The trigger
> type is masked. But you would still need a custom translate function to
> extract the priority and set it.
>
> You could perhaps just put all the priorities as a property of the
> interrupt controller node. Then you don't have to increase the cell size.
Thank you for your thoughts about this: I will implement it soon (Cf.
above).
>>>> {
>>>> unsigned int i;
>>>> + int irq_base;
>>>>
>>>> - at91_aic_base = ioremap(AT91_AIC, 512);
>>>> + if (of_have_populated_dt())
>>>> + at91_aic_of_init();
>>>
>>> You should get to this point because of_irq_init called you and you
>>> should already have a node ptr at point.
>>>
>>>> + else
>>>> + at91_aic_base = ioremap(AT91_AIC, 512);
>>>>
>>>> if (!at91_aic_base)
>>>> - panic("Impossible to ioremap AT91_AIC\n");
>>>> + panic("Unable to ioremap AIC registers\n");
>>>> +
>>>> + /* Add irq domain for AIC */
>>>> + irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
>>>
>>> Really, you don't want to use irq0, but that would break your irq entry
>>> code.
>>
>> Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
>> that will require a little modification of each board file. This
>> scattered modification is maybe a bit late for 3.4 inclusion...
>>
>
> Okay. It's fine for now. Just making sure you're aware of the issue.
>
> Rob
>
Best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 24+ messages in thread