* [PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-21 13:50 [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy Radoslaw Pietrzyk
@ 2018-02-21 13:50 ` Radoslaw Pietrzyk
2018-02-22 10:55 ` [1/2] " Ludovic BARRE
2018-02-21 13:50 ` [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Radoslaw Pietrzyk @ 2018-02-21 13:50 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Radoslaw Pietrzyk,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
- allocates generic chip with handle_simple_irq
which simplifies irq_domain_ops.alloc callback
- removes ack register from generic chip which is not used for
handle_simple_irq as acking is done in a chained handler
- removes unneeded irq_domain_ops.xlate callback
Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
drivers/irqchip/irq-stm32-exti.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index 36f0fbe..42e74e3 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -176,16 +176,12 @@ static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_chip_generic *gc;
struct irq_fwspec *fwspec = data;
irq_hw_number_t hwirq;
hwirq = fwspec->param[0];
- gc = irq_get_domain_generic_chip(d, hwirq);
irq_map_generic_chip(d, virq, hwirq);
- irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
- handle_simple_irq, NULL, NULL);
return 0;
}
@@ -200,7 +196,6 @@ static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
struct irq_domain_ops irq_exti_domain_ops = {
.map = irq_map_generic_chip,
- .xlate = irq_domain_xlate_onetwocell,
.alloc = stm32_exti_alloc,
.free = stm32_exti_free,
};
@@ -231,7 +226,7 @@ __init stm32_exti_init(const struct stm32_exti_bank **stm32_exti_banks,
}
ret = irq_alloc_domain_generic_chips(domain, IRQS_PER_BANK, 1, "exti",
- handle_edge_irq, clr, 0, 0);
+ handle_simple_irq, clr, 0, 0);
if (ret) {
pr_err("%pOF: Could not allocate generic interrupt chip.\n",
node);
@@ -246,13 +241,10 @@ __init stm32_exti_init(const struct stm32_exti_bank **stm32_exti_banks,
gc->reg_base = base;
gc->chip_types->type = IRQ_TYPE_EDGE_BOTH;
- gc->chip_types->chip.irq_ack = irq_gc_ack_set_bit;
gc->chip_types->chip.irq_mask = irq_gc_mask_clr_bit;
gc->chip_types->chip.irq_unmask = irq_gc_mask_set_bit;
gc->chip_types->chip.irq_set_type = stm32_irq_set_type;
gc->chip_types->chip.irq_set_wake = stm32_irq_set_wake;
- gc->chip_types->regs.ack = stm32_bank->pr_ofst;
- gc->chip_types->regs.mask = stm32_bank->imr_ofst;
gc->private = (void *)stm32_bank;
/* Determine number of irqs supported */
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-21 13:50 ` [PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
@ 2018-02-22 10:55 ` Ludovic BARRE
2018-02-22 11:01 ` Radosław Pietrzyk
0 siblings, 1 reply; 28+ messages in thread
From: Ludovic BARRE @ 2018-02-22 10:55 UTC (permalink / raw)
To: radek, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Alexandre Torgue, Linus Walleij,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
Hi Radek
I've tested your change on stm32h743i-eval.dts board
and gpio-keys tests are not functional.
No interrupt occurs when we push or release button
(cat /proc/interrupt).
Board: stm32h743i-eval.dts
Test description: gpio-keys (gpioc 13 => exti 13), with gpio or
interrupt property.
comment below
On 02/21/2018 02:50 PM, radek wrote:
> - allocates generic chip with handle_simple_irq
> which simplifies irq_domain_ops.alloc callback
> - removes ack register from generic chip which is not used for
> handle_simple_irq as acking is done in a chained handler
> - removes unneeded irq_domain_ops.xlate callback
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/irqchip/irq-stm32-exti.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
> index 36f0fbe..42e74e3 100644
> --- a/drivers/irqchip/irq-stm32-exti.c
> +++ b/drivers/irqchip/irq-stm32-exti.c
> @@ -176,16 +176,12 @@ static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
> unsigned int nr_irqs, void *data)
> {
> - struct irq_chip_generic *gc;
> struct irq_fwspec *fwspec = data;
> irq_hw_number_t hwirq;
>
> hwirq = fwspec->param[0];
> - gc = irq_get_domain_generic_chip(d, hwirq);
>
> irq_map_generic_chip(d, virq, hwirq);
> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
> - handle_simple_irq, NULL, NULL);
>
> return 0;
> }
> @@ -200,7 +196,6 @@ static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
>
> struct irq_domain_ops irq_exti_domain_ops = {
> .map = irq_map_generic_chip,
> - .xlate = irq_domain_xlate_onetwocell,
> .alloc = stm32_exti_alloc,
> .free = stm32_exti_free,
> };
> @@ -231,7 +226,7 @@ __init stm32_exti_init(const struct stm32_exti_bank **stm32_exti_banks,
> }
>
> ret = irq_alloc_domain_generic_chips(domain, IRQS_PER_BANK, 1, "exti",
> - handle_edge_irq, clr, 0, 0);
> + handle_simple_irq, clr, 0, 0);
EXTI hardware block is trigged on pulse event rising or failing edge.
Why do you change to handle_simple_irq ?
> if (ret) {
> pr_err("%pOF: Could not allocate generic interrupt chip.\n",
> node);
> @@ -246,13 +241,10 @@ __init stm32_exti_init(const struct stm32_exti_bank **stm32_exti_banks,
>
> gc->reg_base = base;
> gc->chip_types->type = IRQ_TYPE_EDGE_BOTH;
> - gc->chip_types->chip.irq_ack = irq_gc_ack_set_bit;
> gc->chip_types->chip.irq_mask = irq_gc_mask_clr_bit;
> gc->chip_types->chip.irq_unmask = irq_gc_mask_set_bit;
> gc->chip_types->chip.irq_set_type = stm32_irq_set_type;
> gc->chip_types->chip.irq_set_wake = stm32_irq_set_wake;
> - gc->chip_types->regs.ack = stm32_bank->pr_ofst;
> - gc->chip_types->regs.mask = stm32_bank->imr_ofst;
> gc->private = (void *)stm32_bank;
>
> /* Determine number of irqs supported */
>
BR
Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-22 10:55 ` [1/2] " Ludovic BARRE
@ 2018-02-22 11:01 ` Radosław Pietrzyk
2018-02-23 8:34 ` Radosław Pietrzyk
0 siblings, 1 reply; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-02-22 11:01 UTC (permalink / raw)
To: Ludovic BARRE
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Benjamin Gaignard,
Philipp Zabel, open list, moderated list:ARM/STM32 ARCHITECTURE,
linux-gpio
Hi Ludovic,
I have tested on stm32f429-disco and it works without a problem.
handle_edge_irq is not used in current implementation because it is
substituted by handle_simple_irq in an alloc callback either way which
causes that irq_ack callback is not invoked as well (acking is done in
chained handler).
2018-02-22 11:55 GMT+01:00 Ludovic BARRE <ludovic.barre@st.com>:
> Hi Radek
>
> I've tested your change on stm32h743i-eval.dts board
> and gpio-keys tests are not functional.
> No interrupt occurs when we push or release button
> (cat /proc/interrupt).
>
> Board: stm32h743i-eval.dts
> Test description: gpio-keys (gpioc 13 => exti 13), with gpio or interrupt
> property.
>
> comment below
>
>
> On 02/21/2018 02:50 PM, radek wrote:
>>
>> - allocates generic chip with handle_simple_irq
>> which simplifies irq_domain_ops.alloc callback
>> - removes ack register from generic chip which is not used for
>> handle_simple_irq as acking is done in a chained handler
>> - removes unneeded irq_domain_ops.xlate callback
>>
>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>> ---
>> drivers/irqchip/irq-stm32-exti.c | 10 +---------
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-stm32-exti.c
>> b/drivers/irqchip/irq-stm32-exti.c
>> index 36f0fbe..42e74e3 100644
>> --- a/drivers/irqchip/irq-stm32-exti.c
>> +++ b/drivers/irqchip/irq-stm32-exti.c
>> @@ -176,16 +176,12 @@ static int stm32_irq_set_wake(struct irq_data *data,
>> unsigned int on)
>> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
>> unsigned int nr_irqs, void *data)
>> {
>> - struct irq_chip_generic *gc;
>> struct irq_fwspec *fwspec = data;
>> irq_hw_number_t hwirq;
>> hwirq = fwspec->param[0];
>> - gc = irq_get_domain_generic_chip(d, hwirq);
>> irq_map_generic_chip(d, virq, hwirq);
>> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
>> - handle_simple_irq, NULL, NULL);
>> return 0;
>> }
>> @@ -200,7 +196,6 @@ static void stm32_exti_free(struct irq_domain *d,
>> unsigned int virq,
>> struct irq_domain_ops irq_exti_domain_ops = {
>> .map = irq_map_generic_chip,
>> - .xlate = irq_domain_xlate_onetwocell,
>> .alloc = stm32_exti_alloc,
>> .free = stm32_exti_free,
>> };
>> @@ -231,7 +226,7 @@ __init stm32_exti_init(const struct stm32_exti_bank
>> **stm32_exti_banks,
>> }
>> ret = irq_alloc_domain_generic_chips(domain, IRQS_PER_BANK, 1,
>> "exti",
>> - handle_edge_irq, clr, 0, 0);
>> + handle_simple_irq, clr, 0,
>> 0);
>
>
> EXTI hardware block is trigged on pulse event rising or failing edge.
> Why do you change to handle_simple_irq ?
>
>> if (ret) {
>> pr_err("%pOF: Could not allocate generic interrupt
>> chip.\n",
>> node);
>> @@ -246,13 +241,10 @@ __init stm32_exti_init(const struct stm32_exti_bank
>> **stm32_exti_banks,
>> gc->reg_base = base;
>> gc->chip_types->type = IRQ_TYPE_EDGE_BOTH;
>> - gc->chip_types->chip.irq_ack = irq_gc_ack_set_bit;
>> gc->chip_types->chip.irq_mask = irq_gc_mask_clr_bit;
>> gc->chip_types->chip.irq_unmask = irq_gc_mask_set_bit;
>> gc->chip_types->chip.irq_set_type = stm32_irq_set_type;
>> gc->chip_types->chip.irq_set_wake = stm32_irq_set_wake;
>> - gc->chip_types->regs.ack = stm32_bank->pr_ofst;
>> - gc->chip_types->regs.mask = stm32_bank->imr_ofst;
>> gc->private = (void *)stm32_bank;
>> /* Determine number of irqs supported */
>>
>
> BR
> Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-22 11:01 ` Radosław Pietrzyk
@ 2018-02-23 8:34 ` Radosław Pietrzyk
0 siblings, 0 replies; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-02-23 8:34 UTC (permalink / raw)
To: Ludovic BARRE
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Benjamin Gaignard,
Philipp Zabel, open list, moderated list:ARM/STM32 ARCHITECTURE,
linux-gpio
Hi Ludovic,
Please check latest v2 patches series and let me know if it works on
stm32h7 as it does on stm32f4
2018-02-22 12:01 GMT+01:00 Radosław Pietrzyk <radoslaw.pietrzyk@gmail.com>:
> Hi Ludovic,
> I have tested on stm32f429-disco and it works without a problem.
> handle_edge_irq is not used in current implementation because it is
> substituted by handle_simple_irq in an alloc callback either way which
> causes that irq_ack callback is not invoked as well (acking is done in
> chained handler).
>
> 2018-02-22 11:55 GMT+01:00 Ludovic BARRE <ludovic.barre@st.com>:
>> Hi Radek
>>
>> I've tested your change on stm32h743i-eval.dts board
>> and gpio-keys tests are not functional.
>> No interrupt occurs when we push or release button
>> (cat /proc/interrupt).
>>
>> Board: stm32h743i-eval.dts
>> Test description: gpio-keys (gpioc 13 => exti 13), with gpio or interrupt
>> property.
>>
>> comment below
>>
>>
>> On 02/21/2018 02:50 PM, radek wrote:
>>>
>>> - allocates generic chip with handle_simple_irq
>>> which simplifies irq_domain_ops.alloc callback
>>> - removes ack register from generic chip which is not used for
>>> handle_simple_irq as acking is done in a chained handler
>>> - removes unneeded irq_domain_ops.xlate callback
>>>
>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>> ---
>>> drivers/irqchip/irq-stm32-exti.c | 10 +---------
>>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-stm32-exti.c
>>> b/drivers/irqchip/irq-stm32-exti.c
>>> index 36f0fbe..42e74e3 100644
>>> --- a/drivers/irqchip/irq-stm32-exti.c
>>> +++ b/drivers/irqchip/irq-stm32-exti.c
>>> @@ -176,16 +176,12 @@ static int stm32_irq_set_wake(struct irq_data *data,
>>> unsigned int on)
>>> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
>>> unsigned int nr_irqs, void *data)
>>> {
>>> - struct irq_chip_generic *gc;
>>> struct irq_fwspec *fwspec = data;
>>> irq_hw_number_t hwirq;
>>> hwirq = fwspec->param[0];
>>> - gc = irq_get_domain_generic_chip(d, hwirq);
>>> irq_map_generic_chip(d, virq, hwirq);
>>> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
>>> - handle_simple_irq, NULL, NULL);
>>> return 0;
>>> }
>>> @@ -200,7 +196,6 @@ static void stm32_exti_free(struct irq_domain *d,
>>> unsigned int virq,
>>> struct irq_domain_ops irq_exti_domain_ops = {
>>> .map = irq_map_generic_chip,
>>> - .xlate = irq_domain_xlate_onetwocell,
>>> .alloc = stm32_exti_alloc,
>>> .free = stm32_exti_free,
>>> };
>>> @@ -231,7 +226,7 @@ __init stm32_exti_init(const struct stm32_exti_bank
>>> **stm32_exti_banks,
>>> }
>>> ret = irq_alloc_domain_generic_chips(domain, IRQS_PER_BANK, 1,
>>> "exti",
>>> - handle_edge_irq, clr, 0, 0);
>>> + handle_simple_irq, clr, 0,
>>> 0);
>>
>>
>> EXTI hardware block is trigged on pulse event rising or failing edge.
>> Why do you change to handle_simple_irq ?
>>
>>> if (ret) {
>>> pr_err("%pOF: Could not allocate generic interrupt
>>> chip.\n",
>>> node);
>>> @@ -246,13 +241,10 @@ __init stm32_exti_init(const struct stm32_exti_bank
>>> **stm32_exti_banks,
>>> gc->reg_base = base;
>>> gc->chip_types->type = IRQ_TYPE_EDGE_BOTH;
>>> - gc->chip_types->chip.irq_ack = irq_gc_ack_set_bit;
>>> gc->chip_types->chip.irq_mask = irq_gc_mask_clr_bit;
>>> gc->chip_types->chip.irq_unmask = irq_gc_mask_set_bit;
>>> gc->chip_types->chip.irq_set_type = stm32_irq_set_type;
>>> gc->chip_types->chip.irq_set_wake = stm32_irq_set_wake;
>>> - gc->chip_types->regs.ack = stm32_bank->pr_ofst;
>>> - gc->chip_types->regs.mask = stm32_bank->imr_ofst;
>>> gc->private = (void *)stm32_bank;
>>> /* Determine number of irqs supported */
>>>
>>
>> BR
>> Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-21 13:50 [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy Radoslaw Pietrzyk
2018-02-21 13:50 ` [PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
@ 2018-02-21 13:50 ` Radoslaw Pietrzyk
2018-02-21 15:11 ` Alexandre Torgue
` (2 more replies)
2018-02-23 8:31 ` [PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy Radoslaw Pietrzyk
` (2 subsequent siblings)
4 siblings, 3 replies; 28+ messages in thread
From: Radoslaw Pietrzyk @ 2018-02-21 13:50 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Radoslaw Pietrzyk,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
- removes unneeded irq_chip.irq_eoi callback
- adds irq_chip.irq_set_wake callback for possible
in the future GPIO wakeup
Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
drivers/pinctrl/stm32/pinctrl-stm32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 617df16..5b1740d 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -268,10 +268,10 @@ 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_mask = irq_chip_mask_parent,
.irq_unmask = irq_chip_unmask_parent,
.irq_set_type = irq_chip_set_type_parent,
+ .irq_set_wake = irq_chip_set_wake_parent,
.irq_request_resources = stm32_gpio_irq_request_resources,
.irq_release_resources = stm32_gpio_irq_release_resources,
};
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-21 13:50 ` [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
@ 2018-02-21 15:11 ` Alexandre Torgue
2018-02-21 15:12 ` Alexandre Torgue
2018-03-01 21:04 ` Linus Walleij
2 siblings, 0 replies; 28+ messages in thread
From: Alexandre Torgue @ 2018-02-21 15:11 UTC (permalink / raw)
To: Radoslaw Pietrzyk, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Linus Walleij, Benjamin Gaignard, Philipp Zabel,
linux-kernel, linux-arm-kernel, linux-gpio
Hi Radoslaw,
On 02/21/2018 02:50 PM, Radoslaw Pietrzyk wrote:
> - removes unneeded irq_chip.irq_eoi callback
> - adds irq_chip.irq_set_wake callback for possible
> in the future GPIO wakeup
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 617df16..5b1740d 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -268,10 +268,10 @@ 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_mask = irq_chip_mask_parent,
> .irq_unmask = irq_chip_unmask_parent,
> .irq_set_type = irq_chip_set_type_parent,
> + .irq_set_wake = irq_chip_set_wake_parent,
Thanks, this one was in my backlog.
Acked-by: Alexandre TORGUE <alexandre.torgue@st.com>
> .irq_request_resources = stm32_gpio_irq_request_resources,
> .irq_release_resources = stm32_gpio_irq_release_resources,
> };
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-21 13:50 ` [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
2018-02-21 15:11 ` Alexandre Torgue
@ 2018-02-21 15:12 ` Alexandre Torgue
2018-03-01 21:04 ` Linus Walleij
2 siblings, 0 replies; 28+ messages in thread
From: Alexandre Torgue @ 2018-02-21 15:12 UTC (permalink / raw)
To: Radoslaw Pietrzyk, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Linus Walleij, Benjamin Gaignard, Philipp Zabel,
linux-kernel, linux-arm-kernel, linux-gpio
Maybe you could remove ARM from commit header.
On 02/21/2018 02:50 PM, Radoslaw Pietrzyk wrote:
> - removes unneeded irq_chip.irq_eoi callback
> - adds irq_chip.irq_set_wake callback for possible
> in the future GPIO wakeup
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 617df16..5b1740d 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -268,10 +268,10 @@ 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_mask = irq_chip_mask_parent,
> .irq_unmask = irq_chip_unmask_parent,
> .irq_set_type = irq_chip_set_type_parent,
> + .irq_set_wake = irq_chip_set_wake_parent,
> .irq_request_resources = stm32_gpio_irq_request_resources,
> .irq_release_resources = stm32_gpio_irq_release_resources,
> };
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-21 13:50 ` [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
2018-02-21 15:11 ` Alexandre Torgue
2018-02-21 15:12 ` Alexandre Torgue
@ 2018-03-01 21:04 ` Linus Walleij
2018-03-01 21:24 ` Radosław Pietrzyk
2 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2018-03-01 21:04 UTC (permalink / raw)
To: Radoslaw Pietrzyk
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Benjamin Gaignard, Philipp Zabel, linux-kernel,
Linux ARM, open list:GPIO SUBSYSTEM
On Wed, Feb 21, 2018 at 2:50 PM, Radoslaw Pietrzyk
<radoslaw.pietrzyk@gmail.com> wrote:
> - removes unneeded irq_chip.irq_eoi callback
> - adds irq_chip.irq_set_wake callback for possible
> in the future GPIO wakeup
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
Patch applied with Alexandre's ACK, stripping ARM from the
commit message, thanks!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-03-01 21:04 ` Linus Walleij
@ 2018-03-01 21:24 ` Radosław Pietrzyk
2018-03-02 8:14 ` Alexandre Torgue
0 siblings, 1 reply; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-03-01 21:24 UTC (permalink / raw)
To: Linus Walleij
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Benjamin Gaignard, Philipp Zabel, linux-kernel,
Linux ARM, open list:GPIO SUBSYSTEM
I don't know if Alexandre agrees but It might be better to take v2
where irq_ack is added.
2018-03-01 22:04 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
> On Wed, Feb 21, 2018 at 2:50 PM, Radoslaw Pietrzyk
> <radoslaw.pietrzyk@gmail.com> wrote:
>
>> - removes unneeded irq_chip.irq_eoi callback
>> - adds irq_chip.irq_set_wake callback for possible
>> in the future GPIO wakeup
>>
>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>
> Patch applied with Alexandre's ACK, stripping ARM from the
> commit message, thanks!
>
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-03-01 21:24 ` Radosław Pietrzyk
@ 2018-03-02 8:14 ` Alexandre Torgue
2018-03-02 8:37 ` Radosław Pietrzyk
0 siblings, 1 reply; 28+ messages in thread
From: Alexandre Torgue @ 2018-03-02 8:14 UTC (permalink / raw)
To: Radosław Pietrzyk, Linus Walleij
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Benjamin Gaignard, Philipp Zabel, linux-kernel, Linux ARM,
open list:GPIO SUBSYSTEM
Hi Radoslaw and Linus,
On 03/01/2018 10:24 PM, Radosław Pietrzyk wrote:
> I don't know if Alexandre agrees but It might be better to take v2
> where irq_ack is added.
Sorry Linus, I agree with Rodoslaw, you need to take V2 as stm32-exti
patch is linked to pinctrl v2 patch. I gonna add my acked-by.
Regards
Alex
>
> 2018-03-01 22:04 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
>> On Wed, Feb 21, 2018 at 2:50 PM, Radoslaw Pietrzyk
>> <radoslaw.pietrzyk@gmail.com> wrote:
>>
>>> - removes unneeded irq_chip.irq_eoi callback
>>> - adds irq_chip.irq_set_wake callback for possible
>>> in the future GPIO wakeup
>>>
>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>
>> Patch applied with Alexandre's ACK, stripping ARM from the
>> commit message, thanks!
>>
>> Yours,
>> Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-03-02 8:14 ` Alexandre Torgue
@ 2018-03-02 8:37 ` Radosław Pietrzyk
2018-03-02 8:49 ` Alexandre Torgue
0 siblings, 1 reply; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-03-02 8:37 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Linus Walleij, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Benjamin Gaignard, Philipp Zabel, linux-kernel,
Linux ARM, open list:GPIO SUBSYSTEM
Linus responded in v2 mail thread that he's gonna take it instead of v1.
2018-03-02 9:14 GMT+01:00 Alexandre Torgue <alexandre.torgue@st.com>:
> Hi Radoslaw and Linus,
>
>
> On 03/01/2018 10:24 PM, Radosław Pietrzyk wrote:
>>
>> I don't know if Alexandre agrees but It might be better to take v2
>> where irq_ack is added.
>
>
>
> Sorry Linus, I agree with Rodoslaw, you need to take V2 as stm32-exti patch
> is linked to pinctrl v2 patch. I gonna add my acked-by.
>
> Regards
> Alex
>
>
>>
>> 2018-03-01 22:04 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
>>>
>>> On Wed, Feb 21, 2018 at 2:50 PM, Radoslaw Pietrzyk
>>> <radoslaw.pietrzyk@gmail.com> wrote:
>>>
>>>> - removes unneeded irq_chip.irq_eoi callback
>>>> - adds irq_chip.irq_set_wake callback for possible
>>>> in the future GPIO wakeup
>>>>
>>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>>
>>>
>>> Patch applied with Alexandre's ACK, stripping ARM from the
>>> commit message, thanks!
>>>
>>> Yours,
>>> Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-03-02 8:37 ` Radosław Pietrzyk
@ 2018-03-02 8:49 ` Alexandre Torgue
0 siblings, 0 replies; 28+ messages in thread
From: Alexandre Torgue @ 2018-03-02 8:49 UTC (permalink / raw)
To: Radosław Pietrzyk
Cc: Linus Walleij, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Benjamin Gaignard, Philipp Zabel, linux-kernel,
Linux ARM, open list:GPIO SUBSYSTEM
On 03/02/2018 09:37 AM, Radosław Pietrzyk wrote:
> Linus responded in v2 mail thread that he's gonna take it instead of v1.
I saw that too late :).
Regards
Alex
>
> 2018-03-02 9:14 GMT+01:00 Alexandre Torgue <alexandre.torgue@st.com>:
>> Hi Radoslaw and Linus,
>>
>>
>> On 03/01/2018 10:24 PM, Radosław Pietrzyk wrote:
>>>
>>> I don't know if Alexandre agrees but It might be better to take v2
>>> where irq_ack is added.
>>
>>
>>
>> Sorry Linus, I agree with Rodoslaw, you need to take V2 as stm32-exti patch
>> is linked to pinctrl v2 patch. I gonna add my acked-by.
>>
>> Regards
>> Alex
>>
>>
>>>
>>> 2018-03-01 22:04 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
>>>>
>>>> On Wed, Feb 21, 2018 at 2:50 PM, Radoslaw Pietrzyk
>>>> <radoslaw.pietrzyk@gmail.com> wrote:
>>>>
>>>>> - removes unneeded irq_chip.irq_eoi callback
>>>>> - adds irq_chip.irq_set_wake callback for possible
>>>>> in the future GPIO wakeup
>>>>>
>>>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>>>
>>>>
>>>> Patch applied with Alexandre's ACK, stripping ARM from the
>>>> commit message, thanks!
>>>>
>>>> Yours,
>>>> Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy
2018-02-21 13:50 [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy Radoslaw Pietrzyk
2018-02-21 13:50 ` [PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
2018-02-21 13:50 ` [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
@ 2018-02-23 8:31 ` Radoslaw Pietrzyk
2018-02-23 8:31 ` [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
2018-02-23 8:31 ` [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
4 siblings, 0 replies; 28+ messages in thread
From: Radoslaw Pietrzyk @ 2018-02-23 8:31 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Radoslaw Pietrzyk,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio, Ludovic BARRE
Radoslaw Pietrzyk (2):
irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
drivers/irqchip/irq-stm32-exti.c | 13 -------------
drivers/pinctrl/stm32/pinctrl-stm32.c | 3 ++-
2 files changed, 2 insertions(+), 14 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-21 13:50 [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy Radoslaw Pietrzyk
` (2 preceding siblings ...)
2018-02-23 8:31 ` [PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy Radoslaw Pietrzyk
@ 2018-02-23 8:31 ` Radoslaw Pietrzyk
2018-02-23 8:42 ` Thomas Gleixner
2018-02-23 15:46 ` Ludovic BARRE
2018-02-23 8:31 ` [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
4 siblings, 2 replies; 28+ messages in thread
From: Radoslaw Pietrzyk @ 2018-02-23 8:31 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Radoslaw Pietrzyk,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio, Ludovic BARRE
- discards setting handle_simple_irq handler for hierarchy interrupts
- removes acking in chained irq handler as this is done by
irq_chip itself inside handle_edge_irq
- removes unneeded irq_domain_ops.xlate callback
Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
drivers/irqchip/irq-stm32-exti.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index 36f0fbe..8013a87 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -79,13 +79,6 @@ static unsigned long stm32_exti_pending(struct irq_chip_generic *gc)
return irq_reg_readl(gc, stm32_bank->pr_ofst);
}
-static void stm32_exti_irq_ack(struct irq_chip_generic *gc, u32 mask)
-{
- const struct stm32_exti_bank *stm32_bank = gc->private;
-
- irq_reg_writel(gc, mask, stm32_bank->pr_ofst);
-}
-
static void stm32_irq_handler(struct irq_desc *desc)
{
struct irq_domain *domain = irq_desc_get_handler_data(desc);
@@ -106,7 +99,6 @@ static void stm32_irq_handler(struct irq_desc *desc)
for_each_set_bit(n, &pending, IRQS_PER_BANK) {
virq = irq_find_mapping(domain, irq_base + n);
generic_handle_irq(virq);
- stm32_exti_irq_ack(gc, BIT(n));
}
}
}
@@ -176,16 +168,12 @@ static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_chip_generic *gc;
struct irq_fwspec *fwspec = data;
irq_hw_number_t hwirq;
hwirq = fwspec->param[0];
- gc = irq_get_domain_generic_chip(d, hwirq);
irq_map_generic_chip(d, virq, hwirq);
- irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
- handle_simple_irq, NULL, NULL);
return 0;
}
@@ -200,7 +188,6 @@ static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
struct irq_domain_ops irq_exti_domain_ops = {
.map = irq_map_generic_chip,
- .xlate = irq_domain_xlate_onetwocell,
.alloc = stm32_exti_alloc,
.free = stm32_exti_free,
};
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 8:31 ` [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
@ 2018-02-23 8:42 ` Thomas Gleixner
2018-02-23 9:05 ` Radosław Pietrzyk
2018-03-14 11:09 ` Marc Zyngier
2018-02-23 15:46 ` Ludovic BARRE
1 sibling, 2 replies; 28+ messages in thread
From: Thomas Gleixner @ 2018-02-23 8:42 UTC (permalink / raw)
To: Radoslaw Pietrzyk
Cc: Jason Cooper, Marc Zyngier, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Benjamin Gaignard, Philipp Zabel, linux-kernel,
linux-arm-kernel, linux-gpio, Ludovic BARRE
Radoslaw,
On Fri, 23 Feb 2018, Radoslaw Pietrzyk wrote:
> - discards setting handle_simple_irq handler for hierarchy interrupts
> - removes acking in chained irq handler as this is done by
> irq_chip itself inside handle_edge_irq
> - removes unneeded irq_domain_ops.xlate callback
if that's all functionally correct, then this is a nice cleanup. Though
from the above changelog its hard to tell because it merily tells WHAT the
patch does, but not WHY. The WHY is the important information for a
reviewer who is not familiar with the particular piece of code/hardware.
Can you please amend the changelog with proper explanations why a
particular piece of code is not needed or has to be changed to something
else?
Thanks,
tglx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 8:42 ` Thomas Gleixner
@ 2018-02-23 9:05 ` Radosław Pietrzyk
2018-03-14 11:09 ` Marc Zyngier
1 sibling, 0 replies; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-02-23 9:05 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jason Cooper, Marc Zyngier, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Benjamin Gaignard, Philipp Zabel, open list,
moderated list:ARM/STM32 ARCHITECTURE, linux-gpio, Ludovic BARRE
I think the HW is fairly simple and straightforward comparing to other
irq chips so it's rather a logic that concerned me the most i.e. why
gpio irqs were handled in a "simple" manner whereas the rest of
interrupts in "edge" manner. According to specification all interrupts
are edge driven and that's how are they treated in set_type callback.
First I thought that all can be handled as simple but then I realized
it makes more sense in handling all of them as edge as they should
according to specification. I will prepare a v3 series with more
detailed explanation in it.
2018-02-23 9:42 GMT+01:00 Thomas Gleixner <tglx@linutronix.de>:
> Radoslaw,
>
> On Fri, 23 Feb 2018, Radoslaw Pietrzyk wrote:
>
>> - discards setting handle_simple_irq handler for hierarchy interrupts
>> - removes acking in chained irq handler as this is done by
>> irq_chip itself inside handle_edge_irq
>> - removes unneeded irq_domain_ops.xlate callback
>
> if that's all functionally correct, then this is a nice cleanup. Though
> from the above changelog its hard to tell because it merily tells WHAT the
> patch does, but not WHY. The WHY is the important information for a
> reviewer who is not familiar with the particular piece of code/hardware.
>
> Can you please amend the changelog with proper explanations why a
> particular piece of code is not needed or has to be changed to something
> else?
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 8:42 ` Thomas Gleixner
2018-02-23 9:05 ` Radosław Pietrzyk
@ 2018-03-14 11:09 ` Marc Zyngier
[not found] ` <CAFvLkMSZjgFMAe0VRc0AEpG89BgXkfMb9+ivJ8V+PFighOoDRA@mail.gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: Marc Zyngier @ 2018-03-14 11:09 UTC (permalink / raw)
To: Radoslaw Pietrzyk
Cc: Thomas Gleixner, Jason Cooper, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Benjamin Gaignard, Philipp Zabel, linux-kernel,
linux-arm-kernel, linux-gpio, Ludovic BARRE
Radoslaw,
On 23/02/18 08:42, Thomas Gleixner wrote:
> Radoslaw,
>
> On Fri, 23 Feb 2018, Radoslaw Pietrzyk wrote:
>
>> - discards setting handle_simple_irq handler for hierarchy interrupts
>> - removes acking in chained irq handler as this is done by
>> irq_chip itself inside handle_edge_irq
>> - removes unneeded irq_domain_ops.xlate callback
>
> if that's all functionally correct, then this is a nice cleanup. Though
> from the above changelog its hard to tell because it merily tells WHAT the
> patch does, but not WHY. The WHY is the important information for a
> reviewer who is not familiar with the particular piece of code/hardware.
>
> Can you please amend the changelog with proper explanations why a
> particular piece of code is not needed or has to be changed to something
> else?
Any update on this? I'd like to queue this for 4.17, but Thomas'
comments should be addressed before that happens. Ca you please respin a
version with a better change log and the various review tags?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 8:31 ` [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
2018-02-23 8:42 ` Thomas Gleixner
@ 2018-02-23 15:46 ` Ludovic BARRE
2018-02-23 16:16 ` Ludovic BARRE
2018-02-23 17:37 ` Radosław Pietrzyk
1 sibling, 2 replies; 28+ messages in thread
From: Ludovic BARRE @ 2018-02-23 15:46 UTC (permalink / raw)
To: Radoslaw Pietrzyk, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Alexandre Torgue, Linus Walleij,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
hi Radoslaw
The gpio-keys tests it's now functional on H7, great.
However the gpio-key test only the bank1 (like stm32f429).
Like the H7 introduce the multi-bank management,
we must perform complementary test.
comment below about ack in handler
On 02/23/2018 09:31 AM, Radoslaw Pietrzyk wrote:
> - discards setting handle_simple_irq handler for hierarchy interrupts
> - removes acking in chained irq handler as this is done by
> irq_chip itself inside handle_edge_irq
> - removes unneeded irq_domain_ops.xlate callback
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/irqchip/irq-stm32-exti.c | 13 -------------
> 1 file changed, 13 deletions(-)
>
> diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
> index 36f0fbe..8013a87 100644
> --- a/drivers/irqchip/irq-stm32-exti.c
> +++ b/drivers/irqchip/irq-stm32-exti.c
> @@ -79,13 +79,6 @@ static unsigned long stm32_exti_pending(struct irq_chip_generic *gc)
> return irq_reg_readl(gc, stm32_bank->pr_ofst);
> }
>
> -static void stm32_exti_irq_ack(struct irq_chip_generic *gc, u32 mask)
> -{
> - const struct stm32_exti_bank *stm32_bank = gc->private;
> -
> - irq_reg_writel(gc, mask, stm32_bank->pr_ofst);
> -}
> -
> static void stm32_irq_handler(struct irq_desc *desc)
> {
> struct irq_domain *domain = irq_desc_get_handler_data(desc);
> @@ -106,7 +99,6 @@ static void stm32_irq_handler(struct irq_desc *desc)
> for_each_set_bit(n, &pending, IRQS_PER_BANK) {
> virq = irq_find_mapping(domain, irq_base + n);
> generic_handle_irq(virq);
> - stm32_exti_irq_ack(gc, BIT(n));
the while loop " while ((pending = " has been introduce while original
upstream of this driver in V2 to V3
https://patchwork.ozlabs.org/patch/604190/
https://patchwork.ozlabs.org/patch/665242/
the "ack" is needed to acknowledge the irq which are handled and which
is not the original irq for which we enter.
> }
> }
> }
> @@ -176,16 +168,12 @@ static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
> unsigned int nr_irqs, void *data)
> {
> - struct irq_chip_generic *gc;
> struct irq_fwspec *fwspec = data;
> irq_hw_number_t hwirq;
>
> hwirq = fwspec->param[0];
> - gc = irq_get_domain_generic_chip(d, hwirq);
>
> irq_map_generic_chip(d, virq, hwirq);
> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
> - handle_simple_irq, NULL, NULL);
I see if it is not disturb the multi-bank.
>
> return 0;
> }
> @@ -200,7 +188,6 @@ static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
>
> struct irq_domain_ops irq_exti_domain_ops = {
> .map = irq_map_generic_chip,
> - .xlate = irq_domain_xlate_onetwocell,
> .alloc = stm32_exti_alloc,
> .free = stm32_exti_free,
> };
>
BR
Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 15:46 ` Ludovic BARRE
@ 2018-02-23 16:16 ` Ludovic BARRE
2018-02-23 17:37 ` Radosław Pietrzyk
1 sibling, 0 replies; 28+ messages in thread
From: Ludovic BARRE @ 2018-02-23 16:16 UTC (permalink / raw)
To: Radoslaw Pietrzyk, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Alexandre Torgue, Linus Walleij,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
On 02/23/2018 04:46 PM, Ludovic BARRE wrote:
> hi Radoslaw
>
> The gpio-keys tests it's now functional on H7, great.
> However the gpio-key test only the bank1 (like stm32f429).
> Like the H7 introduce the multi-bank management,
> we must perform complementary test.
>
> comment below about ack in handler
>
> On 02/23/2018 09:31 AM, Radoslaw Pietrzyk wrote:
>> - discards setting handle_simple_irq handler for hierarchy interrupts
>> - removes acking in chained irq handler as this is done by
>> irq_chip itself inside handle_edge_irq
>> - removes unneeded irq_domain_ops.xlate callback
>>
>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>> ---
>> drivers/irqchip/irq-stm32-exti.c | 13 -------------
>> 1 file changed, 13 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-stm32-exti.c
>> b/drivers/irqchip/irq-stm32-exti.c
>> index 36f0fbe..8013a87 100644
>> --- a/drivers/irqchip/irq-stm32-exti.c
>> +++ b/drivers/irqchip/irq-stm32-exti.c
>> @@ -79,13 +79,6 @@ static unsigned long stm32_exti_pending(struct
>> irq_chip_generic *gc)
>> return irq_reg_readl(gc, stm32_bank->pr_ofst);
>> }
>> -static void stm32_exti_irq_ack(struct irq_chip_generic *gc, u32 mask)
>> -{
>> - const struct stm32_exti_bank *stm32_bank = gc->private;
>> -
>> - irq_reg_writel(gc, mask, stm32_bank->pr_ofst);
>> -}
>> -
>> static void stm32_irq_handler(struct irq_desc *desc)
>> {
>> struct irq_domain *domain = irq_desc_get_handler_data(desc);
>> @@ -106,7 +99,6 @@ static void stm32_irq_handler(struct irq_desc *desc)
>> for_each_set_bit(n, &pending, IRQS_PER_BANK) {
>> virq = irq_find_mapping(domain, irq_base + n);
>> generic_handle_irq(virq);
>> - stm32_exti_irq_ack(gc, BIT(n));
>
> the while loop " while ((pending = " has been introduce while original
> upstream of this driver in V2 to V3
> https://patchwork.ozlabs.org/patch/604190/
> https://patchwork.ozlabs.org/patch/665242/
>
> the "ack" is needed to acknowledge the irq which are handled and which
> is not the original irq for which we enter.
>
>> }
>> }
>> }
>> @@ -176,16 +168,12 @@ static int stm32_irq_set_wake(struct irq_data
>> *data, unsigned int on)
>> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
>> unsigned int nr_irqs, void *data)
>> {
>> - struct irq_chip_generic *gc;
>> struct irq_fwspec *fwspec = data;
>> irq_hw_number_t hwirq;
>> hwirq = fwspec->param[0];
>> - gc = irq_get_domain_generic_chip(d, hwirq);
>> irq_map_generic_chip(d, virq, hwirq);
>> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
>> - handle_simple_irq, NULL, NULL);
>
> I see if it is not disturb the multi-bank.
you are right, normally irq_domain_set_info is already set in
irq_map_generic_chip.
>
>> return 0;
>> }
>> @@ -200,7 +188,6 @@ static void stm32_exti_free(struct irq_domain *d,
>> unsigned int virq,
>> struct irq_domain_ops irq_exti_domain_ops = {
>> .map = irq_map_generic_chip,
>> - .xlate = irq_domain_xlate_onetwocell,
>> .alloc = stm32_exti_alloc,
>> .free = stm32_exti_free,
>> };
>>
>
> BR
> Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 15:46 ` Ludovic BARRE
2018-02-23 16:16 ` Ludovic BARRE
@ 2018-02-23 17:37 ` Radosław Pietrzyk
2018-02-26 15:23 ` Ludovic BARRE
1 sibling, 1 reply; 28+ messages in thread
From: Radosław Pietrzyk @ 2018-02-23 17:37 UTC (permalink / raw)
To: Ludovic BARRE
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Benjamin Gaignard,
Philipp Zabel, open list, moderated list:ARM/STM32 ARCHITECTURE,
linux-gpio
I don't fully get it to be honest. If interrupt is pending it must
have been enabled (unmasked) and requires to be handled acked. It will
be acked by irq_chip.irq_ack handler within the edge handler.
Therefore this additional acking is meaningless.
2018-02-23 16:46 GMT+01:00 Ludovic BARRE <ludovic.barre@st.com>:
> hi Radoslaw
>
> The gpio-keys tests it's now functional on H7, great.
> However the gpio-key test only the bank1 (like stm32f429).
> Like the H7 introduce the multi-bank management,
> we must perform complementary test.
>
> comment below about ack in handler
>
>
> On 02/23/2018 09:31 AM, Radoslaw Pietrzyk wrote:
>>
>> - discards setting handle_simple_irq handler for hierarchy interrupts
>> - removes acking in chained irq handler as this is done by
>> irq_chip itself inside handle_edge_irq
>> - removes unneeded irq_domain_ops.xlate callback
>>
>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>> ---
>> drivers/irqchip/irq-stm32-exti.c | 13 -------------
>> 1 file changed, 13 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-stm32-exti.c
>> b/drivers/irqchip/irq-stm32-exti.c
>> index 36f0fbe..8013a87 100644
>> --- a/drivers/irqchip/irq-stm32-exti.c
>> +++ b/drivers/irqchip/irq-stm32-exti.c
>> @@ -79,13 +79,6 @@ static unsigned long stm32_exti_pending(struct
>> irq_chip_generic *gc)
>> return irq_reg_readl(gc, stm32_bank->pr_ofst);
>> }
>> -static void stm32_exti_irq_ack(struct irq_chip_generic *gc, u32 mask)
>> -{
>> - const struct stm32_exti_bank *stm32_bank = gc->private;
>> -
>> - irq_reg_writel(gc, mask, stm32_bank->pr_ofst);
>> -}
>> -
>> static void stm32_irq_handler(struct irq_desc *desc)
>> {
>> struct irq_domain *domain = irq_desc_get_handler_data(desc);
>> @@ -106,7 +99,6 @@ static void stm32_irq_handler(struct irq_desc *desc)
>> for_each_set_bit(n, &pending, IRQS_PER_BANK) {
>> virq = irq_find_mapping(domain, irq_base +
>> n);
>> generic_handle_irq(virq);
>> - stm32_exti_irq_ack(gc, BIT(n));
>
>
> the while loop " while ((pending = " has been introduce while original
> upstream of this driver in V2 to V3
> https://patchwork.ozlabs.org/patch/604190/
> https://patchwork.ozlabs.org/patch/665242/
>
> the "ack" is needed to acknowledge the irq which are handled and which is
> not the original irq for which we enter.
>
>> }
>> }
>> }
>> @@ -176,16 +168,12 @@ static int stm32_irq_set_wake(struct irq_data *data,
>> unsigned int on)
>> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
>> unsigned int nr_irqs, void *data)
>> {
>> - struct irq_chip_generic *gc;
>> struct irq_fwspec *fwspec = data;
>> irq_hw_number_t hwirq;
>> hwirq = fwspec->param[0];
>> - gc = irq_get_domain_generic_chip(d, hwirq);
>> irq_map_generic_chip(d, virq, hwirq);
>> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
>> - handle_simple_irq, NULL, NULL);
>
>
> I see if it is not disturb the multi-bank.
>
>> return 0;
>> }
>> @@ -200,7 +188,6 @@ static void stm32_exti_free(struct irq_domain *d,
>> unsigned int virq,
>> struct irq_domain_ops irq_exti_domain_ops = {
>> .map = irq_map_generic_chip,
>> - .xlate = irq_domain_xlate_onetwocell,
>> .alloc = stm32_exti_alloc,
>> .free = stm32_exti_free,
>> };
>>
>
> BR
> Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
2018-02-23 17:37 ` Radosław Pietrzyk
@ 2018-02-26 15:23 ` Ludovic BARRE
0 siblings, 0 replies; 28+ messages in thread
From: Ludovic BARRE @ 2018-02-26 15:23 UTC (permalink / raw)
To: Radosław Pietrzyk
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Benjamin Gaignard,
Philipp Zabel, open list, moderated list:ARM/STM32 ARCHITECTURE,
linux-gpio
hi Radosław
On 02/23/2018 06:37 PM, Radosław Pietrzyk wrote:
> I don't fully get it to be honest. If interrupt is pending it must
> have been enabled (unmasked) and requires to be handled acked. It will
> be acked by irq_chip.irq_ack handler within the edge handler.
> Therefore this additional acking is meaningless.
>
Sorry, I did not see modification in pinctrl-stm32.c.
So it's ok for me
Acked-by: Ludovic Barre <ludovic.barre@st.com>
Tested-by: Ludovic Barre <ludovic.barre@st.com>
BR
Ludo
> 2018-02-23 16:46 GMT+01:00 Ludovic BARRE <ludovic.barre@st.com>:
>> hi Radoslaw
>>
>> The gpio-keys tests it's now functional on H7, great.
>> However the gpio-key test only the bank1 (like stm32f429).
>> Like the H7 introduce the multi-bank management,
>> we must perform complementary test.
>>
>> comment below about ack in handler
>>
>>
>> On 02/23/2018 09:31 AM, Radoslaw Pietrzyk wrote:
>>>
>>> - discards setting handle_simple_irq handler for hierarchy interrupts
>>> - removes acking in chained irq handler as this is done by
>>> irq_chip itself inside handle_edge_irq
>>> - removes unneeded irq_domain_ops.xlate callback
>>>
>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>> ---
>>> drivers/irqchip/irq-stm32-exti.c | 13 -------------
>>> 1 file changed, 13 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-stm32-exti.c
>>> b/drivers/irqchip/irq-stm32-exti.c
>>> index 36f0fbe..8013a87 100644
>>> --- a/drivers/irqchip/irq-stm32-exti.c
>>> +++ b/drivers/irqchip/irq-stm32-exti.c
>>> @@ -79,13 +79,6 @@ static unsigned long stm32_exti_pending(struct
>>> irq_chip_generic *gc)
>>> return irq_reg_readl(gc, stm32_bank->pr_ofst);
>>> }
>>> -static void stm32_exti_irq_ack(struct irq_chip_generic *gc, u32 mask)
>>> -{
>>> - const struct stm32_exti_bank *stm32_bank = gc->private;
>>> -
>>> - irq_reg_writel(gc, mask, stm32_bank->pr_ofst);
>>> -}
>>> -
>>> static void stm32_irq_handler(struct irq_desc *desc)
>>> {
>>> struct irq_domain *domain = irq_desc_get_handler_data(desc);
>>> @@ -106,7 +99,6 @@ static void stm32_irq_handler(struct irq_desc *desc)
>>> for_each_set_bit(n, &pending, IRQS_PER_BANK) {
>>> virq = irq_find_mapping(domain, irq_base +
>>> n);
>>> generic_handle_irq(virq);
>>> - stm32_exti_irq_ack(gc, BIT(n));
>>
>>
>> the while loop " while ((pending = " has been introduce while original
>> upstream of this driver in V2 to V3
>> https://patchwork.ozlabs.org/patch/604190/
>> https://patchwork.ozlabs.org/patch/665242/
>>
>> the "ack" is needed to acknowledge the irq which are handled and which is
>> not the original irq for which we enter.
>>
>>> }
>>> }
>>> }
>>> @@ -176,16 +168,12 @@ static int stm32_irq_set_wake(struct irq_data *data,
>>> unsigned int on)
>>> static int stm32_exti_alloc(struct irq_domain *d, unsigned int virq,
>>> unsigned int nr_irqs, void *data)
>>> {
>>> - struct irq_chip_generic *gc;
>>> struct irq_fwspec *fwspec = data;
>>> irq_hw_number_t hwirq;
>>> hwirq = fwspec->param[0];
>>> - gc = irq_get_domain_generic_chip(d, hwirq);
>>> irq_map_generic_chip(d, virq, hwirq);
>>> - irq_domain_set_info(d, virq, hwirq, &gc->chip_types->chip, gc,
>>> - handle_simple_irq, NULL, NULL);
>>
>>
>> I see if it is not disturb the multi-bank.
>>
>>> return 0;
>>> }
>>> @@ -200,7 +188,6 @@ static void stm32_exti_free(struct irq_domain *d,
>>> unsigned int virq,
>>> struct irq_domain_ops irq_exti_domain_ops = {
>>> .map = irq_map_generic_chip,
>>> - .xlate = irq_domain_xlate_onetwocell,
>>> .alloc = stm32_exti_alloc,
>>> .free = stm32_exti_free,
>>> };
>>>
>>
>> BR
>> Ludo
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-21 13:50 [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy Radoslaw Pietrzyk
` (3 preceding siblings ...)
2018-02-23 8:31 ` [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
@ 2018-02-23 8:31 ` Radoslaw Pietrzyk
2018-02-26 15:25 ` Ludovic BARRE
2018-03-01 22:41 ` Linus Walleij
4 siblings, 2 replies; 28+ messages in thread
From: Radoslaw Pietrzyk @ 2018-02-23 8:31 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Linus Walleij, Radoslaw Pietrzyk,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio, Ludovic BARRE
- removes unneeded irq_chip.irq_eoi callback
- adds irq_chip.irq_set_wake callback for possible
in the future GPIO wakeup
- adds irq_chip.irq_ack callback
Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
drivers/pinctrl/stm32/pinctrl-stm32.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 617df16..6cbcff4 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -268,10 +268,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_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_wake = irq_chip_set_wake_parent,
.irq_request_resources = stm32_gpio_irq_request_resources,
.irq_release_resources = stm32_gpio_irq_release_resources,
};
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-23 8:31 ` [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
@ 2018-02-26 15:25 ` Ludovic BARRE
2018-03-01 22:41 ` Linus Walleij
1 sibling, 0 replies; 28+ messages in thread
From: Ludovic BARRE @ 2018-02-26 15:25 UTC (permalink / raw)
To: Radoslaw Pietrzyk, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Maxime Coquelin, Alexandre Torgue, Linus Walleij,
Benjamin Gaignard, Philipp Zabel, linux-kernel, linux-arm-kernel,
linux-gpio
hi Radoslaw
Reviewed-by: Ludovic Barre <ludovic.barre@st.com>
Tested-by: Ludovic Barre <ludovic.barre@st.com>
BR
Ludo
On 02/23/2018 09:31 AM, Radoslaw Pietrzyk wrote:
> - removes unneeded irq_chip.irq_eoi callback
> - adds irq_chip.irq_set_wake callback for possible
> in the future GPIO wakeup
> - adds irq_chip.irq_ack callback
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 617df16..6cbcff4 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -268,10 +268,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_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_wake = irq_chip_set_wake_parent,
> .irq_request_resources = stm32_gpio_irq_request_resources,
> .irq_release_resources = stm32_gpio_irq_release_resources,
> };
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip
2018-02-23 8:31 ` [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip Radoslaw Pietrzyk
2018-02-26 15:25 ` Ludovic BARRE
@ 2018-03-01 22:41 ` Linus Walleij
1 sibling, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2018-03-01 22:41 UTC (permalink / raw)
To: Radoslaw Pietrzyk
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Maxime Coquelin,
Alexandre Torgue, Benjamin Gaignard, Philipp Zabel, linux-kernel,
Linux ARM, open list:GPIO SUBSYSTEM, Ludovic BARRE
On Fri, Feb 23, 2018 at 9:31 AM, Radoslaw Pietrzyk
<radoslaw.pietrzyk@gmail.com> wrote:
> - removes unneeded irq_chip.irq_eoi callback
> - adds irq_chip.irq_set_wake callback for possible
> in the future GPIO wakeup
> - adds irq_chip.irq_ack callback
>
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
This v2 patch applied with Ludovic's tags.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread