Hi,
This v5 fixes bad copy/paste inside stm32exti driver (spot by Jason), and remove
GPIOLIB_IRQCHIP config for stm32 pinctrl driver (Linus).
The series adds support to EXTI interrupt controller and GPIO IRQ support in
STM32 pinctrl driver.
The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.
Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, fallin or both edges. Each line can also
be masked independently.
Regards
Alex
Changes since v4:
-----------------
- Fix bad copy/paste in stm32 exti driver
- Remove GPIOLIB_IRQCHIP config in stm32 pinctrl driver
Changes since v3:
-----------------
- Review domain dealloc/free irq in stm32 pinctrl driver
- Review domain dealloc/free irq in stm32 exti driver
- Fix remarks on coding style
Changes since v2:
-----------------
- Define irq_chip for GPIO banks
- Use hierarchical domain for GPIO banks
- Improve search loop inside stm32_exti handler
- Rebased on top of v4.8-rc1
Changes since v1:
-----------------
- Rebased on top of v4.6-rc1
- Change variable name from virq to irq (Linus W.)
Alexandre TORGUE (9):
Documentation: dt-bindings: Document STM32 EXTI controller bindings
drivers: irqchip: Add STM32 external interrupts support
ARM: STM32: Select external interrupts controller
ARM: dts: Add EXTI controller node to stm32f429
Documentation: dt-bindings: Add IRQ related properties of STM32
pinctrl
pinctrl: Add IRQ support to STM32 gpios
ARM: dts: Add GPIO irq support to STM2F429
ARM: dts: Declare push button as GPIO key on stm32f429 boards
ARM: config: Enable GPIO Key driver in stm32_defconfig
.../interrupt-controller/st,stm32-exti.txt | 20 ++
.../bindings/pinctrl/st,stm32-pinctrl.txt | 3 +
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/stm32429i-eval.dts | 18 ++
arch/arm/boot/dts/stm32f429-disco.dts | 13 ++
arch/arm/boot/dts/stm32f429.dtsi | 10 +
arch/arm/configs/stm32_defconfig | 6 +-
drivers/irqchip/Kconfig | 4 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-stm32-exti.c | 202 +++++++++++++++++++++
drivers/pinctrl/stm32/Kconfig | 1 +
drivers/pinctrl/stm32/pinctrl-stm32.c | 163 ++++++++++++++++-
12 files changed, 440 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
create mode 100644 drivers/irqchip/irq-stm32-exti.c
--
1.9.1
@@ -0,0 +1,20 @@+STM32 External Interrupt Controller++Required properties:++- compatible: Should be "st,stm32-exti"+- reg: Specifies base physical address and size of the registers+- interrupt-controller: Indentifies the node as an interrupt controller+- #interrupt-cells: Specifies the number of cells to encode an interrupt+ specifier, shall be 2+- interrupts: interrupts references to primary interrupt controller++Example:++exti: interrupt-controller@40013c00 {+ compatible = "st,stm32-exti";+ interrupt-controller;+ #interrupt-cells = <2>;+ reg = <0x40013C00 0x400>;+ interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;+};
This patch adds IRQ support to STM32 gpios.
The EXTI controller has 16 lines dedicated to GPIOs.
EXTI line n can be connected to only line n of one of the GPIO ports, for
example EXTI0 can be connected to either PA0, or PB0, or PC0...
This port selection is done by specifying the port number into System
Config registers.
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <redacted>
@@ -38,7 +38,11 @@ CONFIG_DEVTMPFS_MOUNT=y # CONFIG_FW_LOADER is not set # CONFIG_BLK_DEV is not set CONFIG_EEPROM_93CX6=y-# CONFIG_INPUT is not set+# CONFIG_INPUT_LEDS is not set+# CONFIG_INPUT_MOUSEDEV is not set+# CONFIG_KEYBOARD_ATKBD is not set+CONFIG_KEYBOARD_GPIO=y+# CONFIG_INPUT_MOUSE is not set # CONFIG_SERIO is not set # CONFIG_VT is not set # CONFIG_UNIX98_PTYS is not set
@@ -14,6 +14,9 @@ Required properies: - #size-cells : The value of this property must be 1 - ranges : defines mapping between pin controller node (parent) to gpio-bank node (children).+ - interrupt-parent: phandle of the interrupt parent to which the external+ GPIO interrupts are forwarded to.+ - st,syscfg: phandle of the syscfg node used for IRQ mux selection. - pins-are-numbered: Specify the subnodes are using numbered pinmux to specify pins.
The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.
Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, falling or both edges. Each line can also
be masked independently.
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Signed-off-by: Alexandre TORGUE <redacted>
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 9, 2016 at 4:42 PM, Alexandre TORGUE
[off-list ref] wrote:
Signed-off-by: Maxime Coquelin <redacted>
Acked-by: Rob Herring <redacted>
Signed-off-by: Alexandre TORGUE <redacted>
Patch applied to the pinctrl tree.
I am happy with this solution.
As I see it, it doesn't hurt to add this into the pinctrl
tree while the irqchip stuff and DTS stuff get merged
into different trees orthogonally.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 9, 2016 at 4:42 PM, Alexandre TORGUE
[off-list ref] wrote:
This patch adds IRQ support to STM32 gpios.
The EXTI controller has 16 lines dedicated to GPIOs.
EXTI line n can be connected to only line n of one of the GPIO ports, for
example EXTI0 can be connected to either PA0, or PB0, or PC0...
This port selection is done by specifying the port number into System
Config registers.
Signed-off-by: Maxime Coquelin <redacted>
Signed-off-by: Alexandre TORGUE <redacted>
Nicely isolated from the other irq stuff and abstracted.
It's complex but this is as clean as we can make it,
thanks for your efforts.
Patch applied.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -14,6 +14,9 @@ Required properies: - #size-cells : The value of this property must be 1 - ranges : defines mapping between pin controller node (parent) to gpio-bank node (children).+ - interrupt-parent: phandle of the interrupt parent to which the external+ GPIO interrupts are forwarded to.+ - st,syscfg: phandle of the syscfg node used for IRQ mux selection.
Actually this doc is incomplete.
This is a phandle + offset, not just a phandle.
It is a small detail so I don't care much, either send a patch to
fix up this doc (I have already merged it) or patch the driver
to not retrieve the offset and instead use
#define SYSCFG_OFFSET 0x08
or something...
Yours,
Linus Walleij
From: Marc Zyngier <hidden> Date: 2016-09-13 08:19:08
On 09/09/16 15:41, Alexandre TORGUE wrote:
The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.
Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, falling or both edges. Each line can also
be masked independently.
Signed-off-by: Maxime Coquelin <redacted>
Signed-off-by: Alexandre TORGUE <redacted>
Acked-by: Marc Zyngier <redacted>
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -14,6 +14,9 @@ Required properies: - #size-cells : The value of this property must be 1 - ranges : defines mapping between pin controller node (parent) to gpio-bank node (children).+ - interrupt-parent: phandle of the interrupt parent to which the external+ GPIO interrupts are forwarded to.+ - st,syscfg: phandle of the syscfg node used for IRQ mux selection.
Actually this doc is incomplete.
This is a phandle + offset, not just a phandle.
It is a small detail so I don't care much, either send a patch to
fix up this doc (I have already merged it) or patch the driver
to not retrieve the offset and instead use
#define SYSCFG_OFFSET 0x08
or something...
I'll send a patch for Documentation.
Do I need to wait this series is officially released (4.9-rc1) or can I
send it now ?
Regards
Alex
It's a simple linear domain. So how can data->parent_data be a valid
irq_data pointer? Answer: It can't!
But it doesn't blow up in your face simply because the alloc/free callbacks
are never invoked for simple non hierarchical domains. So you should have
removed that stuff after copying some other irqchip driver.
Thanks,
tglx
I have a hard time to understand this. The irq domain is not hierarchical.
Actually, I wanted to test ".free" callback function of gpio_irq_domain
in STM32 pinctrl driver. To do that I modified gpio driver: just after
getting virq through gpio_to_irq, I called "irq_dispose_mapping(virq)".
I know it is dirty but I thought it was the only way to test.
Doing that, I see that ".free" callback of gpio domain is called but as
it is hirerchical ".free" callback for parent domain (exti one) is also
called. I observed that virq was well unmapped, but not masked at exti
level. It is for this reason than I added
"irq_gc_mask_clr_bit(data->parent_data);" which mask interrupt at exti
level.
Maybe this use case can never happen ? (and in this case all this stuff
is not needed)
It's a simple linear domain. So how can data->parent_data be a valid
irq_data pointer? Answer: It can't!
This Exti domain is parent of stm32 gpio domain. When ".free" callback
of stm32 gpio domain is called then ".free" callback of Exti domain will
be automatically called. Those both ".free" callbacks are called with
"virq". This virq is created through stm32 gpio domain (in stm32 pinctrl
driver), data and parent->data are then associated to it.
Sorry if it is not clear.
Regards
Alex
But it doesn't blow up in your face simply because the alloc/free callbacks
are never invoked for simple non hierarchical domains. So you should have
removed that stuff after copying some other irqchip driver.
I have a hard time to understand this. The irq domain is not hierarchical.
Actually, I wanted to test ".free" callback function of gpio_irq_domain in
STM32 pinctrl driver. To do that I modified gpio driver: just after getting
virq through gpio_to_irq, I called "irq_dispose_mapping(virq)".
I know it is dirty but I thought it was the only way to test.
Doing that, I see that ".free" callback of gpio domain is called but as it is
hirerchical ".free" callback for parent domain (exti one) is also called. I
observed that virq was well unmapped, but not masked at exti level. It is for
this reason than I added "irq_gc_mask_clr_bit(data->parent_data);" which mask
interrupt at exti level.
Aargh. I really misread the patch, but this is entirely non obvious and you
should do:
struct irq_data *data = irq_domain_get_irq_data(d, virq);
irq_gc_mask_clr_bit(d);
Then it is entirely clear that you mask the interrupt of _this_ (the exti)
domain.
Now what really bugs me is that you do that at all. An interrupt which is
freed must be masked already. Why is it unmasked in the first place?
Thanks,
tglx
I have a hard time to understand this. The irq domain is not hierarchical.
Actually, I wanted to test ".free" callback function of gpio_irq_domain in
STM32 pinctrl driver. To do that I modified gpio driver: just after getting
virq through gpio_to_irq, I called "irq_dispose_mapping(virq)".
I know it is dirty but I thought it was the only way to test.
Doing that, I see that ".free" callback of gpio domain is called but as it is
hirerchical ".free" callback for parent domain (exti one) is also called. I
observed that virq was well unmapped, but not masked at exti level. It is for
this reason than I added "irq_gc_mask_clr_bit(data->parent_data);" which mask
interrupt at exti level.
Aargh. I really misread the patch, but this is entirely non obvious and you
should do:
struct irq_data *data = irq_domain_get_irq_data(d, virq);
irq_gc_mask_clr_bit(d);
Then it is entirely clear that you mask the interrupt of _this_ (the exti)
domain.
Ok, it's easier to understand like that.
Now what really bugs me is that you do that at all. An interrupt which is
freed must be masked already. Why is it unmasked in the first place?
Honestly I don't know. When "devm_free_irq" is called to release virq,
there is no issue and interrupt is well masked. But, when I tried to use
"irq_dispose_mapping(virq)" I observed that .free is called (child and
parent domain) but interrupt is not masked.
Regards
Alex
From: Thomas Gleixner <hidden> Date: 2016-09-14 13:36:54
On Wed, 14 Sep 2016, Alexandre Torgue wrote:
On 09/14/2016 11:19 AM, Thomas Gleixner wrote:
quoted
Now what really bugs me is that you do that at all. An interrupt which is
freed must be masked already. Why is it unmasked in the first place?
Honestly I don't know. When "devm_free_irq" is called to release virq, there
is no issue and interrupt is well masked. But, when I tried to use
"irq_dispose_mapping(virq)" I observed that .free is called (child and parent
domain) but interrupt is not masked.
Well, you just used some function in some context which is not relevant to
the normal operation. So adding that mask() is just paranoia for no value.
Thanks,
tglx
Now what really bugs me is that you do that at all. An interrupt which is
freed must be masked already. Why is it unmasked in the first place?
Honestly I don't know. When "devm_free_irq" is called to release virq, there
is no issue and interrupt is well masked. But, when I tried to use
"irq_dispose_mapping(virq)" I observed that .free is called (child and parent
domain) but interrupt is not masked.
Well, you just used some function in some context which is not relevant to
the normal operation. So adding that mask() is just paranoia for no value.
I agree. I just wanted to "force" a test for .free callback. If it not
relevant I'll remove ".free" callback of exti domain.
As a part of this series has already been taken by Linus (pinctrl part),
I will send a new series only for irqchip part (patches [1] and [2]). Do
you agree ?
Thanks
Alex
Hi Thomas,
On 09/14/2016 03:44 PM, Alexandre Torgue wrote:
On 09/14/2016 03:34 PM, Thomas Gleixner wrote:
quoted
On Wed, 14 Sep 2016, Alexandre Torgue wrote:
quoted
On 09/14/2016 11:19 AM, Thomas Gleixner wrote:
quoted
Now what really bugs me is that you do that at all. An interrupt
which is
freed must be masked already. Why is it unmasked in the first place?
Honestly I don't know. When "devm_free_irq" is called to release
virq, there
is no issue and interrupt is well masked. But, when I tried to use
"irq_dispose_mapping(virq)" I observed that .free is called (child
and parent
domain) but interrupt is not masked.
Well, you just used some function in some context which is not
relevant to
the normal operation. So adding that mask() is just paranoia for no
value.
A gentle reminder ping...
If ".free" callback is not relevant then I 'll remove it from exti domain.
I agree. I just wanted to "force" a test for .free callback. If it not
relevant I'll remove ".free" callback of exti domain.
As a part of this series has already been taken by Linus (pinctrl part),
I will send a new series only for irqchip part (patches [1] and [2]). Do
you agree ?
From: Thomas Gleixner <hidden> Date: 2016-09-20 09:53:58
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
On 09/14/2016 03:34 PM, Thomas Gleixner wrote:
quoted
Well, you just used some function in some context which is not
relevant to
the normal operation. So adding that mask() is just paranoia for no
value.
A gentle reminder ping...
If ".free" callback is not relevant then I 'll remove it from exti domain.
I was not talking about the .free callback in general. I was talking about
the masking. But yes, if the thing is otherwise a NOOP, then you can spare
it completely.
Thanks,
tglx
Thomas,
On 09/20/2016 11:51 AM, Thomas Gleixner wrote:
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
quoted
On 09/14/2016 03:34 PM, Thomas Gleixner wrote:
quoted
Well, you just used some function in some context which is not
relevant to
the normal operation. So adding that mask() is just paranoia for no
value.
A gentle reminder ping...
If ".free" callback is not relevant then I 'll remove it from exti domain.
Sorry for discussing about the same thing again (and again) but I just
want to be sure before sending a new version. As you know I have 2
domains: EXTI domain (parent) and stm32-pinctrl-bank domain (child one).
There does it make sens to have ".free" callbacks defined in both domain
(actually if I define one for the child domain I have to define also
".free" callback for parent domain (EXTI) as it is hierarchical) ?
If ".free" have no chance to be called then I will send a new version by
removing .free callbacks (in both domain).
Regards
Alex
I was not talking about the .free callback in general. I was talking about
the masking. But yes, if the thing is otherwise a NOOP, then you can spare
it completely.
Thanks,
tglx
From: Thomas Gleixner <hidden> Date: 2016-09-20 12:46:48
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
Thomas,
On 09/20/2016 11:51 AM, Thomas Gleixner wrote:
quoted
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
quoted
On 09/14/2016 03:34 PM, Thomas Gleixner wrote:
quoted
Well, you just used some function in some context which is not
relevant to
the normal operation. So adding that mask() is just paranoia for no
value.
A gentle reminder ping...
If ".free" callback is not relevant then I 'll remove it from exti domain.
Sorry for discussing about the same thing again (and again) but I just want to
be sure before sending a new version. As you know I have 2 domains: EXTI
domain (parent) and stm32-pinctrl-bank domain (child one).
There does it make sens to have ".free" callbacks defined in both domain
(actually if I define one for the child domain I have to define also ".free"
callback for parent domain (EXTI) as it is hierarchical) ?
If ".free" have no chance to be called then I will send a new version by
removing .free callbacks (in both domain).
Free will be called when a interrupt in the child domain is torn down,
i.e. when irq_domain_free_irqs() is called. And it will be called for both
domains like the alloc callback is invoked on both domains via
irq_domain_alloc_irqs().
Thanks,
tglx
Thomas,
On 09/20/2016 02:44 PM, Thomas Gleixner wrote:
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
Thomas,
On 09/20/2016 11:51 AM, Thomas Gleixner wrote:
quoted
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
quoted
On 09/14/2016 03:34 PM, Thomas Gleixner wrote:
quoted
Well, you just used some function in some context which is not
relevant to
the normal operation. So adding that mask() is just paranoia for no
value.
A gentle reminder ping...
If ".free" callback is not relevant then I 'll remove it from exti domain.
Sorry for discussing about the same thing again (and again) but I just want to
be sure before sending a new version. As you know I have 2 domains: EXTI
domain (parent) and stm32-pinctrl-bank domain (child one).
There does it make sens to have ".free" callbacks defined in both domain
(actually if I define one for the child domain I have to define also ".free"
callback for parent domain (EXTI) as it is hierarchical) ?
If ".free" have no chance to be called then I will send a new version by
removing .free callbacks (in both domain).
Free will be called when a interrupt in the child domain is torn down,
i.e. when irq_domain_free_irqs() is called. And it will be called for both
domains like the alloc callback is invoked on both domains via
irq_domain_alloc_irqs().
Thanks Thomas for this clarification (I'm sure now that we need .free
callbacks).
irq_domain_free_irqs() is called in 2 scenario:
1- when issue occurs in irq_create_fwspec_mapping()
2- when irq_dispose_mapping() is called
Case 2 is the one I tested some times ago. In this case, I need to mask
interrupts in .free callback of EXTI (parent) domain to avoid spurious
interrupts.
Regards
Alex
From: Thomas Gleixner <hidden> Date: 2016-09-20 14:04:36
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
On 09/20/2016 02:44 PM, Thomas Gleixner wrote:
quoted
Free will be called when a interrupt in the child domain is torn down,
i.e. when irq_domain_free_irqs() is called. And it will be called for both
domains like the alloc callback is invoked on both domains via
irq_domain_alloc_irqs().
Thanks Thomas for this clarification (I'm sure now that we need .free
callbacks).
irq_domain_free_irqs() is called in 2 scenario:
1- when issue occurs in irq_create_fwspec_mapping()
2- when irq_dispose_mapping() is called
Case 2 is the one I tested some times ago. In this case, I need to mask
interrupts in .free callback of EXTI (parent) domain to avoid spurious
interrupts.
And why would irq_dispose_mapping() be called on an unmasked, i.e. active,
interrupt? The masking is just papering over that.
Thanks,
tglx
Thomas,
On 09/20/2016 04:02 PM, Thomas Gleixner wrote:
On Tue, 20 Sep 2016, Alexandre Torgue wrote:
quoted
On 09/20/2016 02:44 PM, Thomas Gleixner wrote:
quoted
Free will be called when a interrupt in the child domain is torn down,
i.e. when irq_domain_free_irqs() is called. And it will be called for both
domains like the alloc callback is invoked on both domains via
irq_domain_alloc_irqs().
Thanks Thomas for this clarification (I'm sure now that we need .free
callbacks).
irq_domain_free_irqs() is called in 2 scenario:
1- when issue occurs in irq_create_fwspec_mapping()
2- when irq_dispose_mapping() is called
Case 2 is the one I tested some times ago. In this case, I need to mask
interrupts in .free callback of EXTI (parent) domain to avoid spurious
interrupts.
And why would irq_dispose_mapping() be called on an unmasked, i.e. active,
interrupt? The masking is just papering over that.
Ok. So my test was wrong and irq_dispose_mapping() has to be called when
irq is masked (for example just after free_irq()). For sure in this
case the mask inside exti free callback has no sens (catch :))
I will change .free callback by:
static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
unsigned int nr_irqs)
{
struct irq_data *data = irq_domain_get_irq_data(d, virq);
irq_domain_reset_irq_data(data);
}
so if you agree I will resend only patches concerning stm32 exti driver
[1],[2],[3],[4]
Thanks for your time.
alex