[PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Alexandre Torgue]

This series adds the possibility to handle gpio interrupts on level.

GPIO hardware block is directly linked to EXTI block but EXTI handles
external interrupts only on edge. To be able to handle GPIO interrupt on
level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
is retriggered following interrupt type and gpio line value.

In exti irq chip, retrigger ops function is added.

Changes since v1:
 - Fix issue reported by Marek: return statment in "stm32_gpio_set_type"
   function.

Regards
Alex

Alexandre Torgue (2):
  irqchip/stm32: Add irq retrigger support
  pinctrl: stm32: Add level interrupt support to gpio irq chip

 drivers/irqchip/irq-stm32-exti.c      | 14 ++++++++-
 drivers/pinctrl/stm32/pinctrl-stm32.c | 45 +++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Alexandre Torgue]

This commit introduces retrigger support for stm32_ext_h chip.
It consists to rise the GIC interrupt mapped to an EXTI line.

Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index e00f2fa27f00..c971d115edb4 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
 	unregister_syscore_ops(&stm32_exti_h_syscore_ops);
 }
 
+static int stm32_exti_h_retrigger(struct irq_data *d)
+{
+	struct stm32_exti_chip_data *chip_data = irq_data_get_irq_chip_data(d);
+	const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
+	void __iomem *base = chip_data->host_data->base;
+	u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
+
+	writel_relaxed(mask, base + stm32_bank->swier_ofst);
+
+	return irq_chip_retrigger_hierarchy(d);
+}
+
 static struct irq_chip stm32_exti_h_chip = {
 	.name			= "stm32-exti-h",
 	.irq_eoi		= stm32_exti_h_eoi,
 	.irq_mask		= stm32_exti_h_mask,
 	.irq_unmask		= stm32_exti_h_unmask,
-	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_retrigger		= stm32_exti_h_retrigger,
 	.irq_set_type		= stm32_exti_h_set_type,
 	.irq_set_wake		= stm32_exti_h_set_wake,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND,
-- 
2.17.1

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Marek Vasut]

On 2/18/20 2:12 PM, Alexandre Torgue wrote:
> This commit introduces retrigger support for stm32_ext_h chip.
> It consists to rise the GIC interrupt mapped to an EXTI line.
> 
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

Reviewed-by: Marek Vasut <marex@denx.de>
(and I tested the previous version on STM32MP1)

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Alexandre Torgue]

Fix Marc email address

On 2/18/20 2:12 PM, Alexandre Torgue wrote:
> This commit introduces retrigger support for stm32_ext_h chip.
> It consists to rise the GIC interrupt mapped to an EXTI line.
> 
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> 
> diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
> index e00f2fa27f00..c971d115edb4 100644
> --- a/drivers/irqchip/irq-stm32-exti.c
> +++ b/drivers/irqchip/irq-stm32-exti.c
> @@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
>   	unregister_syscore_ops(&stm32_exti_h_syscore_ops);
>   }
>   
> +static int stm32_exti_h_retrigger(struct irq_data *d)
> +{
> +	struct stm32_exti_chip_data *chip_data = irq_data_get_irq_chip_data(d);
> +	const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
> +	void __iomem *base = chip_data->host_data->base;
> +	u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
> +
> +	writel_relaxed(mask, base + stm32_bank->swier_ofst);
> +
> +	return irq_chip_retrigger_hierarchy(d);
> +}
> +
>   static struct irq_chip stm32_exti_h_chip = {
>   	.name			= "stm32-exti-h",
>   	.irq_eoi		= stm32_exti_h_eoi,
>   	.irq_mask		= stm32_exti_h_mask,
>   	.irq_unmask		= stm32_exti_h_unmask,
> -	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_retrigger		= stm32_exti_h_retrigger,
>   	.irq_set_type		= stm32_exti_h_set_type,
>   	.irq_set_wake		= stm32_exti_h_set_wake,
>   	.flags			= IRQCHIP_MASK_ON_SUSPEND,
> 

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Marc Zyngier]

On 2020-02-19 11:33, Alexandre Torgue wrote:
> Fix Marc email address
> 
> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>> This commit introduces retrigger support for stm32_ext_h chip.
>> It consists to rise the GIC interrupt mapped to an EXTI line.
>> 
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>> 
>> diff --git a/drivers/irqchip/irq-stm32-exti.c 
>> b/drivers/irqchip/irq-stm32-exti.c
>> index e00f2fa27f00..c971d115edb4 100644
>> --- a/drivers/irqchip/irq-stm32-exti.c
>> +++ b/drivers/irqchip/irq-stm32-exti.c
>> @@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
>>   	unregister_syscore_ops(&stm32_exti_h_syscore_ops);
>>   }
>>   +static int stm32_exti_h_retrigger(struct irq_data *d)
>> +{
>> +	struct stm32_exti_chip_data *chip_data = 
>> irq_data_get_irq_chip_data(d);
>> +	const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
>> +	void __iomem *base = chip_data->host_data->base;
>> +	u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
>> +
>> +	writel_relaxed(mask, base + stm32_bank->swier_ofst);
>> +
>> +	return irq_chip_retrigger_hierarchy(d);

Calling irq_chip_retrigger_hierarchy here is really odd. If the write
above has the effect of making the interrupt pending again, why do you
need to force the retrigger any further?

             M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Alexandre Torgue]

On 2/19/20 12:43 PM, Marc Zyngier wrote:
> On 2020-02-19 11:33, Alexandre Torgue wrote:
>> Fix Marc email address
>>
>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>> This commit introduces retrigger support for stm32_ext_h chip.
>>> It consists to rise the GIC interrupt mapped to an EXTI line.
>>>
>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>
>>> diff --git a/drivers/irqchip/irq-stm32-exti.c 
>>> b/drivers/irqchip/irq-stm32-exti.c
>>> index e00f2fa27f00..c971d115edb4 100644
>>> --- a/drivers/irqchip/irq-stm32-exti.c
>>> +++ b/drivers/irqchip/irq-stm32-exti.c
>>> @@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
>>>       unregister_syscore_ops(&stm32_exti_h_syscore_ops);
>>>   }
>>>   +static int stm32_exti_h_retrigger(struct irq_data *d)
>>> +{
>>> +    struct stm32_exti_chip_data *chip_data = 
>>> irq_data_get_irq_chip_data(d);
>>> +    const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
>>> +    void __iomem *base = chip_data->host_data->base;
>>> +    u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
>>> +
>>> +    writel_relaxed(mask, base + stm32_bank->swier_ofst);
>>> +
>>> +    return irq_chip_retrigger_hierarchy(d);
> 
> Calling irq_chip_retrigger_hierarchy here is really odd. If the write
> above has the effect of making the interrupt pending again, why do you
> need to force the retrigger any further?

To be honest, as we use hierarchical irq_chip, I thought it was the way 
to follow (to retrigger parent irq_chip). It makes maybe no sens here.
The most important to regenerate gic interrupt (associate to the exti 
line) is to write in SWIER register.

Alex

> 
>              M.

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Marc Zyngier]

On 2020-02-19 13:07, Alexandre Torgue wrote:
> On 2/19/20 12:43 PM, Marc Zyngier wrote:
>> On 2020-02-19 11:33, Alexandre Torgue wrote:
>>> Fix Marc email address
>>> 
>>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>>> This commit introduces retrigger support for stm32_ext_h chip.
>>>> It consists to rise the GIC interrupt mapped to an EXTI line.
>>>> 
>>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>> 
>>>> diff --git a/drivers/irqchip/irq-stm32-exti.c 
>>>> b/drivers/irqchip/irq-stm32-exti.c
>>>> index e00f2fa27f00..c971d115edb4 100644
>>>> --- a/drivers/irqchip/irq-stm32-exti.c
>>>> +++ b/drivers/irqchip/irq-stm32-exti.c
>>>> @@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
>>>>       unregister_syscore_ops(&stm32_exti_h_syscore_ops);
>>>>   }
>>>>   +static int stm32_exti_h_retrigger(struct irq_data *d)
>>>> +{
>>>> +    struct stm32_exti_chip_data *chip_data = 
>>>> irq_data_get_irq_chip_data(d);
>>>> +    const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
>>>> +    void __iomem *base = chip_data->host_data->base;
>>>> +    u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
>>>> +
>>>> +    writel_relaxed(mask, base + stm32_bank->swier_ofst);
>>>> +
>>>> +    return irq_chip_retrigger_hierarchy(d);
>> 
>> Calling irq_chip_retrigger_hierarchy here is really odd. If the write
>> above has the effect of making the interrupt pending again, why do you
>> need to force the retrigger any further?
> 
> To be honest, as we use hierarchical irq_chip, I thought it was the
> way to follow (to retrigger parent irq_chip). It makes maybe no sens
> here.

Indeed, it looks perfectly pointless. What 
irq_chip_retrigger_hierarchy()
does is to look for the first parent irqchip that is able to retrigger
the interrupt. Guess what, you've just done that already. And once 
you've
generated the interrupt, you don't need to ask the other irqchips in the
chain to do the same thing.

> The most important to regenerate gic interrupt (associate to the exti
> line) is to write in SWIER register.

Quite. Hence my question.

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 1/2] irqchip/stm32: Add irq retrigger support

[Alexandre Torgue]

On 2/19/20 2:13 PM, Marc Zyngier wrote:
> On 2020-02-19 13:07, Alexandre Torgue wrote:
>> On 2/19/20 12:43 PM, Marc Zyngier wrote:
>>> On 2020-02-19 11:33, Alexandre Torgue wrote:
>>>> Fix Marc email address
>>>>
>>>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>>>> This commit introduces retrigger support for stm32_ext_h chip.
>>>>> It consists to rise the GIC interrupt mapped to an EXTI line.
>>>>>
>>>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-stm32-exti.c 
>>>>> b/drivers/irqchip/irq-stm32-exti.c
>>>>> index e00f2fa27f00..c971d115edb4 100644
>>>>> --- a/drivers/irqchip/irq-stm32-exti.c
>>>>> +++ b/drivers/irqchip/irq-stm32-exti.c
>>>>> @@ -604,12 +604,24 @@ static void stm32_exti_h_syscore_deinit(void)
>>>>>       unregister_syscore_ops(&stm32_exti_h_syscore_ops);
>>>>>   }
>>>>>   +static int stm32_exti_h_retrigger(struct irq_data *d)
>>>>> +{
>>>>> +    struct stm32_exti_chip_data *chip_data = 
>>>>> irq_data_get_irq_chip_data(d);
>>>>> +    const struct stm32_exti_bank *stm32_bank = chip_data->reg_bank;
>>>>> +    void __iomem *base = chip_data->host_data->base;
>>>>> +    u32 mask = BIT(d->hwirq % IRQS_PER_BANK);
>>>>> +
>>>>> +    writel_relaxed(mask, base + stm32_bank->swier_ofst);
>>>>> +
>>>>> +    return irq_chip_retrigger_hierarchy(d);
>>>
>>> Calling irq_chip_retrigger_hierarchy here is really odd. If the write
>>> above has the effect of making the interrupt pending again, why do you
>>> need to force the retrigger any further?
>>
>> To be honest, as we use hierarchical irq_chip, I thought it was the
>> way to follow (to retrigger parent irq_chip). It makes maybe no sens
>> here.
> 
> Indeed, it looks perfectly pointless. What irq_chip_retrigger_hierarchy()
> does is to look for the first parent irqchip that is able to retrigger
> the interrupt. Guess what, you've just done that already. And once you've
> generated the interrupt, you don't need to ask the other irqchips in the
> chain to do the same thing.

I agree. I gonna remove it v3.

Thanks for the feeback.
Alex

>> The most important to regenerate gic interrupt (associate to the exti
>> line) is to write in SWIER register.
> 
> Quite. Hence my question.
> 
>          M.

[PATCH v2 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

[Alexandre Torgue]

This patch adds level interrupt support to gpio irq chip.

GPIO hardware block is directly linked to EXTI block but EXTI handles
external interrupts only on edge. To be able to handle GPIO interrupt on
level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
is retriggered following interrupt type and gpio line value.

Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
Tested-by: Marek Vasut <marex@denx.de>

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 2d5e0435af0a..dae236562543 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -92,6 +92,7 @@ struct stm32_gpio_bank {
 	u32 bank_nr;
 	u32 bank_ioport_nr;
 	u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
+	u32 irq_type[STM32_GPIO_PINS_PER_BANK];
 };
 
 struct stm32_pinctrl {
@@ -303,6 +304,46 @@ static const struct gpio_chip stm32_gpio_template = {
 	.get_direction		= stm32_gpio_get_direction,
 };
 
+void stm32_gpio_irq_eoi(struct irq_data *d)
+{
+	struct stm32_gpio_bank *bank = d->domain->host_data;
+	int line;
+
+	irq_chip_eoi_parent(d);
+
+	/* If level interrupt type then retrig */
+	line = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
+	if ((line == 0 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_LOW) ||
+	    (line == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
+		irq_chip_retrigger_hierarchy(d);
+};
+
+static int stm32_gpio_set_type(struct irq_data *d, unsigned int type)
+{
+	struct stm32_gpio_bank *bank = d->domain->host_data;
+	u32 parent_type;
+
+	bank->irq_type[d->hwirq] = type;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_EDGE_FALLING:
+	case IRQ_TYPE_EDGE_BOTH:
+		parent_type = type;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		parent_type = IRQ_TYPE_EDGE_RISING;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		parent_type = IRQ_TYPE_EDGE_FALLING;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return irq_chip_set_type_parent(d, parent_type);
+};
+
 static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
 {
 	struct stm32_gpio_bank *bank = irq_data->domain->host_data;
@@ -332,11 +373,11 @@ static void stm32_gpio_irq_release_resources(struct irq_data *irq_data)
 
 static struct irq_chip stm32_gpio_irq_chip = {
 	.name		= "stm32gpio",
-	.irq_eoi	= irq_chip_eoi_parent,
+	.irq_eoi	= stm32_gpio_irq_eoi,
 	.irq_ack	= irq_chip_ack_parent,
 	.irq_mask	= irq_chip_mask_parent,
 	.irq_unmask	= irq_chip_unmask_parent,
-	.irq_set_type	= irq_chip_set_type_parent,
+	.irq_set_type	= stm32_gpio_set_type,
 	.irq_set_wake	= irq_chip_set_wake_parent,
 	.irq_request_resources = stm32_gpio_irq_request_resources,
 	.irq_release_resources = stm32_gpio_irq_release_resources,
-- 
2.17.1

Re: [PATCH v2 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

[Marek Vasut]

On 2/18/20 2:12 PM, Alexandre Torgue wrote:
> This patch adds level interrupt support to gpio irq chip.
> 
> GPIO hardware block is directly linked to EXTI block but EXTI handles
> external interrupts only on edge. To be able to handle GPIO interrupt on
> level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
> is retriggered following interrupt type and gpio line value.
> 
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> Tested-by: Marek Vasut <marex@denx.de>

Reviewed-by: Marek Vasut <marex@denx.de>
(and I tested the previous version on STM32MP1)

Re: [PATCH v2 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

[Alexandre Torgue]

Fix Marc email address.

On 2/18/20 2:12 PM, Alexandre Torgue wrote:
> This patch adds level interrupt support to gpio irq chip.
> 
> GPIO hardware block is directly linked to EXTI block but EXTI handles
> external interrupts only on edge. To be able to handle GPIO interrupt on
> level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
> is retriggered following interrupt type and gpio line value.
> 
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> Tested-by: Marek Vasut <marex@denx.de>
> 
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 2d5e0435af0a..dae236562543 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -92,6 +92,7 @@ struct stm32_gpio_bank {
>   	u32 bank_nr;
>   	u32 bank_ioport_nr;
>   	u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
> +	u32 irq_type[STM32_GPIO_PINS_PER_BANK];
>   };
>   
>   struct stm32_pinctrl {
> @@ -303,6 +304,46 @@ static const struct gpio_chip stm32_gpio_template = {
>   	.get_direction		= stm32_gpio_get_direction,
>   };
>   
> +void stm32_gpio_irq_eoi(struct irq_data *d)
> +{
> +	struct stm32_gpio_bank *bank = d->domain->host_data;
> +	int line;
> +
> +	irq_chip_eoi_parent(d);
> +
> +	/* If level interrupt type then retrig */
> +	line = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
> +	if ((line == 0 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_LOW) ||
> +	    (line == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
> +		irq_chip_retrigger_hierarchy(d);
> +};
> +
> +static int stm32_gpio_set_type(struct irq_data *d, unsigned int type)
> +{
> +	struct stm32_gpio_bank *bank = d->domain->host_data;
> +	u32 parent_type;
> +
> +	bank->irq_type[d->hwirq] = type;
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_EDGE_FALLING:
> +	case IRQ_TYPE_EDGE_BOTH:
> +		parent_type = type;
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		parent_type = IRQ_TYPE_EDGE_RISING;
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		parent_type = IRQ_TYPE_EDGE_FALLING;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return irq_chip_set_type_parent(d, parent_type);
> +};
> +
>   static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
>   {
>   	struct stm32_gpio_bank *bank = irq_data->domain->host_data;
> @@ -332,11 +373,11 @@ static void stm32_gpio_irq_release_resources(struct irq_data *irq_data)
>   
>   static struct irq_chip stm32_gpio_irq_chip = {
>   	.name		= "stm32gpio",
> -	.irq_eoi	= irq_chip_eoi_parent,
> +	.irq_eoi	= stm32_gpio_irq_eoi,
>   	.irq_ack	= irq_chip_ack_parent,
>   	.irq_mask	= irq_chip_mask_parent,
>   	.irq_unmask	= irq_chip_unmask_parent,
> -	.irq_set_type	= irq_chip_set_type_parent,
> +	.irq_set_type	= stm32_gpio_set_type,
>   	.irq_set_wake	= irq_chip_set_wake_parent,
>   	.irq_request_resources = stm32_gpio_irq_request_resources,
>   	.irq_release_resources = stm32_gpio_irq_release_resources,
> 

Re: [PATCH v2 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

[Marc Zyngier]

On 2020-02-19 11:34, Alexandre Torgue wrote:
> Fix Marc email address.
> 
> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>> This patch adds level interrupt support to gpio irq chip.

A commit message should not contain "this patch".

>> 
>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>> external interrupts only on edge. To be able to handle GPIO interrupt 
>> on
>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq 
>> chip)
>> is retriggered following interrupt type and gpio line value.
>> 
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>> Tested-by: Marek Vasut <marex@denx.de>
>> 
>> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c 
>> b/drivers/pinctrl/stm32/pinctrl-stm32.c
>> index 2d5e0435af0a..dae236562543 100644
>> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
>> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
>> @@ -92,6 +92,7 @@ struct stm32_gpio_bank {
>>   	u32 bank_nr;
>>   	u32 bank_ioport_nr;
>>   	u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
>> +	u32 irq_type[STM32_GPIO_PINS_PER_BANK];

Do you really need a u32 here? an array of u8 seems enough. After all,
you only need two bits of information per interrupts (level or not,
low or high).

>>   };
>>     struct stm32_pinctrl {
>> @@ -303,6 +304,46 @@ static const struct gpio_chip stm32_gpio_template 
>> = {
>>   	.get_direction		= stm32_gpio_get_direction,
>>   };
>>   +void stm32_gpio_irq_eoi(struct irq_data *d)
>> +{
>> +	struct stm32_gpio_bank *bank = d->domain->host_data;
>> +	int line;
>> +
>> +	irq_chip_eoi_parent(d);
>> +
>> +	/* If level interrupt type then retrig */
>> +	line = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
>> +	if ((line == 0 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_LOW) ||
>> +	    (line == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
>> +		irq_chip_retrigger_hierarchy(d);

s/line/level/

>> +};
>> +
>> +static int stm32_gpio_set_type(struct irq_data *d, unsigned int type)
>> +{
>> +	struct stm32_gpio_bank *bank = d->domain->host_data;
>> +	u32 parent_type;
>> +
>> +	bank->irq_type[d->hwirq] = type;

It would make more sense if this this assignment was done *after*
sanitizing the type value.

>> +
>> +	switch (type) {
>> +	case IRQ_TYPE_EDGE_RISING:
>> +	case IRQ_TYPE_EDGE_FALLING:
>> +	case IRQ_TYPE_EDGE_BOTH:
>> +		parent_type = type;
>> +		break;
>> +	case IRQ_TYPE_LEVEL_HIGH:
>> +		parent_type = IRQ_TYPE_EDGE_RISING;
>> +		break;
>> +	case IRQ_TYPE_LEVEL_LOW:
>> +		parent_type = IRQ_TYPE_EDGE_FALLING;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return irq_chip_set_type_parent(d, parent_type);
>> +};
>> +
>>   static int stm32_gpio_irq_request_resources(struct irq_data 
>> *irq_data)
>>   {
>>   	struct stm32_gpio_bank *bank = irq_data->domain->host_data;
>> @@ -332,11 +373,11 @@ static void 
>> stm32_gpio_irq_release_resources(struct irq_data *irq_data)
>>     static struct irq_chip stm32_gpio_irq_chip = {
>>   	.name		= "stm32gpio",
>> -	.irq_eoi	= irq_chip_eoi_parent,
>> +	.irq_eoi	= stm32_gpio_irq_eoi,
>>   	.irq_ack	= irq_chip_ack_parent,
>>   	.irq_mask	= irq_chip_mask_parent,
>>   	.irq_unmask	= irq_chip_unmask_parent,
>> -	.irq_set_type	= irq_chip_set_type_parent,
>> +	.irq_set_type	= stm32_gpio_set_type,
>>   	.irq_set_wake	= irq_chip_set_wake_parent,
>>   	.irq_request_resources = stm32_gpio_irq_request_resources,
>>   	.irq_release_resources = stm32_gpio_irq_release_resources,
>> 

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

[Alexandre Torgue]

Hi Marc

On 2/19/20 1:07 PM, Marc Zyngier wrote:
> On 2020-02-19 11:34, Alexandre Torgue wrote:
>> Fix Marc email address.
>>
>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>> This patch adds level interrupt support to gpio irq chip.
> 
> A commit message should not contain "this patch".

Ok I'll change in v3

> 
>>>
>>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>>> external interrupts only on edge. To be able to handle GPIO interrupt on
>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq 
>>> chip)
>>> is retriggered following interrupt type and gpio line value.
>>>
>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>> Tested-by: Marek Vasut <marex@denx.de>
>>>
>>> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c 
>>> b/drivers/pinctrl/stm32/pinctrl-stm32.c
>>> index 2d5e0435af0a..dae236562543 100644
>>> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
>>> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
>>> @@ -92,6 +92,7 @@ struct stm32_gpio_bank {
>>>       u32 bank_nr;
>>>       u32 bank_ioport_nr;
>>>       u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
>>> +    u32 irq_type[STM32_GPIO_PINS_PER_BANK];
> 
> Do you really need a u32 here? an array of u8 seems enough. After all,
> you only need two bits of information per interrupts (level or not,
> low or high).

I agree. No need to have u32.

> 
>>>   };
>>>     struct stm32_pinctrl {
>>> @@ -303,6 +304,46 @@ static const struct gpio_chip 
>>> stm32_gpio_template = {
>>>       .get_direction        = stm32_gpio_get_direction,
>>>   };
>>>   +void stm32_gpio_irq_eoi(struct irq_data *d)
>>> +{
>>> +    struct stm32_gpio_bank *bank = d->domain->host_data;
>>> +    int line;
>>> +
>>> +    irq_chip_eoi_parent(d);
>>> +
>>> +    /* If level interrupt type then retrig */
>>> +    line = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
>>> +    if ((line == 0 && bank->irq_type[d->hwirq] == 
>>> IRQ_TYPE_LEVEL_LOW) ||
>>> +        (line == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
>>> +        irq_chip_retrigger_hierarchy(d);
> 
> s/line/level/
> 

correct

>>> +};
>>> +
>>> +static int stm32_gpio_set_type(struct irq_data *d, unsigned int type)
>>> +{
>>> +    struct stm32_gpio_bank *bank = d->domain->host_data;
>>> +    u32 parent_type;
>>> +
>>> +    bank->irq_type[d->hwirq] = type;
> 
> It would make more sense if this this assignment was done *after*
> sanitizing the type value.

Ok.

> 
>>> +
>>> +    switch (type) {
>>> +    case IRQ_TYPE_EDGE_RISING:
>>> +    case IRQ_TYPE_EDGE_FALLING:
>>> +    case IRQ_TYPE_EDGE_BOTH:
>>> +        parent_type = type;
>>> +        break;
>>> +    case IRQ_TYPE_LEVEL_HIGH:
>>> +        parent_type = IRQ_TYPE_EDGE_RISING;
>>> +        break;
>>> +    case IRQ_TYPE_LEVEL_LOW:
>>> +        parent_type = IRQ_TYPE_EDGE_FALLING;
>>> +        break;
>>> +    default:
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    return irq_chip_set_type_parent(d, parent_type);
>>> +};
>>> +
>>>   static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
>>>   {
>>>       struct stm32_gpio_bank *bank = irq_data->domain->host_data;
>>> @@ -332,11 +373,11 @@ static void 
>>> stm32_gpio_irq_release_resources(struct irq_data *irq_data)
>>>     static struct irq_chip stm32_gpio_irq_chip = {
>>>       .name        = "stm32gpio",
>>> -    .irq_eoi    = irq_chip_eoi_parent,
>>> +    .irq_eoi    = stm32_gpio_irq_eoi,
>>>       .irq_ack    = irq_chip_ack_parent,
>>>       .irq_mask    = irq_chip_mask_parent,
>>>       .irq_unmask    = irq_chip_unmask_parent,
>>> -    .irq_set_type    = irq_chip_set_type_parent,
>>> +    .irq_set_type    = stm32_gpio_set_type,
>>>       .irq_set_wake    = irq_chip_set_wake_parent,
>>>       .irq_request_resources = stm32_gpio_irq_request_resources,
>>>       .irq_release_resources = stm32_gpio_irq_release_resources,
>>>
> 
> Thanks,
> 
>          M.

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Marek Vasut]

On 2/18/20 2:12 PM, Alexandre Torgue wrote:
> This series adds the possibility to handle gpio interrupts on level.
> 
> GPIO hardware block is directly linked to EXTI block but EXTI handles
> external interrupts only on edge. To be able to handle GPIO interrupt on
> level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
> is retriggered following interrupt type and gpio line value.
> 
> In exti irq chip, retrigger ops function is added.

btw. this might be unrelated, but is it possible to have e.g. gpioC2 set
as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux selection")
prevents that.

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Alexandre Torgue]

Hi Marek

On 2/18/20 5:25 PM, Marek Vasut wrote:
> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>> This series adds the possibility to handle gpio interrupts on level.
>>
>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>> external interrupts only on edge. To be able to handle GPIO interrupt on
>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
>> is retriggered following interrupt type and gpio line value.
>>
>> In exti irq chip, retrigger ops function is added.
> 
> btw. this might be unrelated, but is it possible to have e.g. gpioC2 set
> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux selection")
> prevents that.
> 

No it's not possible. Each gpio line doesn't have a dedicated Exti line 
Each Exti line is muxing between gpio banks.

Mapping is done as following:

EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
...

regards
Alexandre

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Marek Vasut]

On 2/18/20 7:10 PM, Alexandre Torgue wrote:
> Hi Marek

Hi,

> On 2/18/20 5:25 PM, Marek Vasut wrote:
>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>> This series adds the possibility to handle gpio interrupts on level.
>>>
>>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>>> external interrupts only on edge. To be able to handle GPIO interrupt on
>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq
>>> chip)
>>> is retriggered following interrupt type and gpio line value.
>>>
>>> In exti irq chip, retrigger ops function is added.
>>
>> btw. this might be unrelated, but is it possible to have e.g. gpioC2 set
>> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
>> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux selection")
>> prevents that.
>>
> 
> No it's not possible. Each gpio line doesn't have a dedicated Exti line
> Each Exti line is muxing between gpio banks.

OK, that confirms my assumption.

> Mapping is done as following:
> 
> EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
> EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
> EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
> ...

Is it at least possible to have IRQs of the same type on the same exti
line? E.g. gpioA2 of trigger-edge-falling and gpioB2 trigger-edge-falling ?

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Alexandre Torgue]

On 2/18/20 7:13 PM, Marek Vasut wrote:
> On 2/18/20 7:10 PM, Alexandre Torgue wrote:
>> Hi Marek
> 
> Hi,
> 
>> On 2/18/20 5:25 PM, Marek Vasut wrote:
>>> On 2/18/20 2:12 PM, Alexandre Torgue wrote:
>>>> This series adds the possibility to handle gpio interrupts on level.
>>>>
>>>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>>>> external interrupts only on edge. To be able to handle GPIO interrupt on
>>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq
>>>> chip)
>>>> is retriggered following interrupt type and gpio line value.
>>>>
>>>> In exti irq chip, retrigger ops function is added.
>>>
>>> btw. this might be unrelated, but is it possible to have e.g. gpioC2 set
>>> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
>>> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux selection")
>>> prevents that.
>>>
>>
>> No it's not possible. Each gpio line doesn't have a dedicated Exti line
>> Each Exti line is muxing between gpio banks.
> 
> OK, that confirms my assumption.
> 
>> Mapping is done as following:
>>
>> EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
>> EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
>> EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
>> ...
> 
> Is it at least possible to have IRQs of the same type on the same exti
> line? E.g. gpioA2 of trigger-edge-falling and gpioB2 trigger-edge-falling ?
> 

Sorry I don't catch your point. If you already succeed to get gpioA2, 
then you will failed to get gpioB2 but looking at function call stack we 
could get an other issue.

Lets take example where you succeed to get gpioa2 as interrupt (using 
interrupt bindings) and now you try to do the same for gpiob2, you will 
have (roughly):

stm32_gpio_irq_request_resources (for gpiob2) --> succeed

stm32_gpio_set_type
  |
  |--> stm32_exti_set_type type -> change exti line 2 trigger registers
                                   with gpiob2 binding.

stm32_gpio_domain_activate --> failed as exti line2 is already used
			       by gpioa2.

So as stm32_gpio_set_type is called before checking than exti line is 
available, type could be changed and behavior of gpioa2 interrupt broken.

Solution would be to move the exti line mux check from 
stm32_gpio_domain_activate to  stm32_gpio_irq_request_resources callback.

Regards
Alex

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Marek Vasut]

On 2/19/20 10:20 AM, Alexandre Torgue wrote:
Hi,
[...]
>>>>> This series adds the possibility to handle gpio interrupts on level.
>>>>>
>>>>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>>>>> external interrupts only on edge. To be able to handle GPIO
>>>>> interrupt on
>>>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq
>>>>> chip)
>>>>> is retriggered following interrupt type and gpio line value.
>>>>>
>>>>> In exti irq chip, retrigger ops function is added.
>>>>
>>>> btw. this might be unrelated, but is it possible to have e.g. gpioC2
>>>> set
>>>> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
>>>> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux
>>>> selection")
>>>> prevents that.
>>>>
>>>
>>> No it's not possible. Each gpio line doesn't have a dedicated Exti line
>>> Each Exti line is muxing between gpio banks.
>>
>> OK, that confirms my assumption.
>>
>>> Mapping is done as following:
>>>
>>> EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
>>> EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
>>> EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
>>> ...
>>
>> Is it at least possible to have IRQs of the same type on the same exti
>> line? E.g. gpioA2 of trigger-edge-falling and gpioB2
>> trigger-edge-falling ?
>>
> 
> Sorry I don't catch your point. If you already succeed to get gpioA2,
> then you will failed to get gpioB2 but looking at function call stack we
> could get an other issue.

Considering the EXTI line limitations, I'd like to know what kind of IRQ
input configuration is allowed/valid and what kind of configuration is
not valid.

> Lets take example where you succeed to get gpioa2 as interrupt (using
> interrupt bindings) and now you try to do the same for gpiob2, you will
> have (roughly):
> 
> stm32_gpio_irq_request_resources (for gpiob2) --> succeed
> 
> stm32_gpio_set_type
>  |
>  |--> stm32_exti_set_type type -> change exti line 2 trigger registers
>                                   with gpiob2 binding.
> 
> stm32_gpio_domain_activate --> failed as exti line2 is already used
>                    by gpioa2.
> 
> So as stm32_gpio_set_type is called before checking than exti line is
> available, type could be changed and behavior of gpioa2 interrupt broken.
> 
> Solution would be to move the exti line mux check from
> stm32_gpio_domain_activate to  stm32_gpio_irq_request_resources callback.

So the hardware does support using both gpioA2 and gpioB2 as an
interrupt source, for different drivers, if they are of the same
interrupt type. Except the current implementation does not permit that.

If the interrupt types are different, that is not supported by the hardware.

Correct ?

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Alexandre Torgue]

Hi Marek

On 2/19/20 6:24 PM, Marek Vasut wrote:
> On 2/19/20 10:20 AM, Alexandre Torgue wrote:
> Hi,
> [...]
>>>>>> This series adds the possibility to handle gpio interrupts on level.
>>>>>>
>>>>>> GPIO hardware block is directly linked to EXTI block but EXTI handles
>>>>>> external interrupts only on edge. To be able to handle GPIO
>>>>>> interrupt on
>>>>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq
>>>>>> chip)
>>>>>> is retriggered following interrupt type and gpio line value.
>>>>>>
>>>>>> In exti irq chip, retrigger ops function is added.
>>>>>
>>>>> btw. this might be unrelated, but is it possible to have e.g. gpioC2
>>>>> set
>>>>> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It seems
>>>>> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux
>>>>> selection")
>>>>> prevents that.
>>>>>
>>>>
>>>> No it's not possible. Each gpio line doesn't have a dedicated Exti line
>>>> Each Exti line is muxing between gpio banks.
>>>
>>> OK, that confirms my assumption.
>>>
>>>> Mapping is done as following:
>>>>
>>>> EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
>>>> EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
>>>> EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
>>>> ...
>>>
>>> Is it at least possible to have IRQs of the same type on the same exti
>>> line? E.g. gpioA2 of trigger-edge-falling and gpioB2
>>> trigger-edge-falling ?
>>>
>>
>> Sorry I don't catch your point. If you already succeed to get gpioA2,
>> then you will failed to get gpioB2 but looking at function call stack we
>> could get an other issue.
> 
> Considering the EXTI line limitations, I'd like to know what kind of IRQ
> input configuration is allowed/valid and what kind of configuration is
> not valid.

As a mux is used to select which GPIO[A..Z]_X has to be mapped on exti_X 
line, only one GPIO can be used on the EXTI line.

For example, on EXTI2 you could map either gpioa2 or gpiob2 or 
....gpioz2 but not gpioa2 and gpiob2 in the same time.


> 
>> Lets take example where you succeed to get gpioa2 as interrupt (using
>> interrupt bindings) and now you try to do the same for gpiob2, you will
>> have (roughly):
>>
>> stm32_gpio_irq_request_resources (for gpiob2) --> succeed
>>
>> stm32_gpio_set_type
>>   |
>>   |--> stm32_exti_set_type type -> change exti line 2 trigger registers
>>                                    with gpiob2 binding.
>>
>> stm32_gpio_domain_activate --> failed as exti line2 is already used
>>                     by gpioa2.
>>
>> So as stm32_gpio_set_type is called before checking than exti line is
>> available, type could be changed and behavior of gpioa2 interrupt broken.
>>
>> Solution would be to move the exti line mux check from
>> stm32_gpio_domain_activate to  stm32_gpio_irq_request_resources callback.
> 
> So the hardware does support using both gpioA2 and gpioB2 as an
> interrupt source, for different drivers, if they are of the same
> interrupt type. Except the current implementation does not permit that.
> 

No hardware doesn't allow it for reason explain above.

> If the interrupt types are different, that is not supported by the hardware.
> 
> Correct ?
> 

Re: [PATCH v2 0/2] Add GPIO level-sensitive interrupt support

[Marek Vasut]

On 2/20/20 2:09 PM, Alexandre Torgue wrote:
> Hi Marek

Hi,

> On 2/19/20 6:24 PM, Marek Vasut wrote:
>> On 2/19/20 10:20 AM, Alexandre Torgue wrote:
>> Hi,
>> [...]
>>>>>>> This series adds the possibility to handle gpio interrupts on level.
>>>>>>>
>>>>>>> GPIO hardware block is directly linked to EXTI block but EXTI
>>>>>>> handles
>>>>>>> external interrupts only on edge. To be able to handle GPIO
>>>>>>> interrupt on
>>>>>>> level a "hack" is done in gpio irq chip: parent interrupt (exti irq
>>>>>>> chip)
>>>>>>> is retriggered following interrupt type and gpio line value.
>>>>>>>
>>>>>>> In exti irq chip, retrigger ops function is added.
>>>>>>
>>>>>> btw. this might be unrelated, but is it possible to have e.g. gpioC2
>>>>>> set
>>>>>> as trigger-level-low and gpioD2 set as trigger-edge-falling ? It
>>>>>> seems
>>>>>> 8eb2dfee9fb1 ("pinctrl: stm32: add lock mechanism for irqmux
>>>>>> selection")
>>>>>> prevents that.
>>>>>>
>>>>>
>>>>> No it's not possible. Each gpio line doesn't have a dedicated Exti
>>>>> line
>>>>> Each Exti line is muxing between gpio banks.
>>>>
>>>> OK, that confirms my assumption.
>>>>
>>>>> Mapping is done as following:
>>>>>
>>>>> EXTI0 = A0 or B0 or C0 .... or Z0 : selected by Mux
>>>>> EXTI1 = A1 or B1 or C1 ....or Z1 : selected by Mux
>>>>> EXTI2 = A2 or B2 or C2 ....or Z2 : selected by Mux
>>>>> ...
>>>>
>>>> Is it at least possible to have IRQs of the same type on the same exti
>>>> line? E.g. gpioA2 of trigger-edge-falling and gpioB2
>>>> trigger-edge-falling ?
>>>>
>>>
>>> Sorry I don't catch your point. If you already succeed to get gpioA2,
>>> then you will failed to get gpioB2 but looking at function call stack we
>>> could get an other issue.
>>
>> Considering the EXTI line limitations, I'd like to know what kind of IRQ
>> input configuration is allowed/valid and what kind of configuration is
>> not valid.
> 
> As a mux is used to select which GPIO[A..Z]_X has to be mapped on exti_X
> line, only one GPIO can be used on the EXTI line.
> 
> For example, on EXTI2 you could map either gpioa2 or gpiob2 or
> ....gpioz2 but not gpioa2 and gpiob2 in the same time.

Got it, thank you for the clarification.

Maybe that could be something to improve for MP2 :-)