LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy
@ 2018-02-21 13:50 Radoslaw Pietrzyk
  2018-02-21 13:50 ` [PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain Radoslaw Pietrzyk
                   ` (4 more replies)
  0 siblings, 5 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

- optimizes and cleans up stm32-exti irq domain
- optimizes and enhances stm32gpio irq chip

Radoslaw Pietrzyk (2):
  ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
  ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

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

-- 
1.9.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [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	[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	[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: [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

* [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	[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	[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

* 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: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

* 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 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 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

* 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

* 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
       [not found]       ` <CAFvLkMSZjgFMAe0VRc0AEpG89BgXkfMb9+ivJ8V+PFighOoDRA@mail.gmail.com>
@ 2018-03-14 12:04         ` Marc Zyngier
  2018-04-19 13:24           ` Ludovic BARRE
  0 siblings, 1 reply; 28+ messages in thread
From: Marc Zyngier @ 2018-03-14 12:04 UTC (permalink / raw)
  To: Radosław Pietrzyk
  Cc: Thomas Gleixner, Jason Cooper, Maxime Coquelin, Alexandre Torgue,
	Linus Walleij, Benjamin Gaignard, Philipp Zabel, open list,
	moderated list:ARM/STM32 ARCHITECTURE, open list:GPIO SUBSYSTEM,
	Ludovic BARRE

On 14/03/18 11:46, Radosław Pietrzyk wrote:
> Hi Marc,
> We had a quite fruitful discussion in this mail thread regarding this
> topic and Ludovic acked it so recently I have asked Thomas if he still
> needs this v3 patch with detailed explanation especially as v2 version
> of stm32-gpio patch has been already taken by Linus. However if you
> require I can resend v3 of this patch only with this detailed explanation.

That'd be useful. The changelog is the only thing that will be left from
this discussion, so it'd better be complete and accurate. If you quickly
send a v3 for this single patch, I'll queue it right away.

Thanks,

	M.

> 
> 2018-03-14 12:09 GMT+01:00 Marc Zyngier <marc.zyngier@arm.com
> <mailto:marc.zyngier@arm.com>>:
> 
>     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...
> 
> 


-- 
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-03-14 12:04         ` Marc Zyngier
@ 2018-04-19 13:24           ` Ludovic BARRE
       [not found]             ` <CAFvLkMQv3AajhFaRxsBt+UzdGFeix_FA-0mhtYqwMo2x0zgrnA@mail.gmail.com>
  0 siblings, 1 reply; 28+ messages in thread
From: Ludovic BARRE @ 2018-04-19 13:24 UTC (permalink / raw)
  To: Marc Zyngier, Radosław Pietrzyk
  Cc: Thomas Gleixner, Jason Cooper, Maxime Coquelin, Alexandre Torgue,
	Linus Walleij, Benjamin Gaignard, Philipp Zabel, open list,
	moderated list:ARM/STM32 ARCHITECTURE, open list:GPIO SUBSYSTEM

Hi Radoslaw

I preparing a patch serie which add support of stm32mp1.
Would you like, I add your patch (with commit message updated)
in my serie?
patch:
-irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

BR
Ludo

On 03/14/2018 01:04 PM, Marc Zyngier wrote:
> On 14/03/18 11:46, Radosław Pietrzyk wrote:
>> Hi Marc,
>> We had a quite fruitful discussion in this mail thread regarding this
>> topic and Ludovic acked it so recently I have asked Thomas if he still
>> needs this v3 patch with detailed explanation especially as v2 version
>> of stm32-gpio patch has been already taken by Linus. However if you
>> require I can resend v3 of this patch only with this detailed explanation.
> 
> That'd be useful. The changelog is the only thing that will be left from
> this discussion, so it'd better be complete and accurate. If you quickly
> send a v3 for this single patch, I'll queue it right away.
> 
> Thanks,
> 
> 	M.
> 
>>
>> 2018-03-14 12:09 GMT+01:00 Marc Zyngier <marc.zyngier@arm.com
>> <mailto:marc.zyngier@arm.com>>:
>>
>>      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
       [not found]             ` <CAFvLkMQv3AajhFaRxsBt+UzdGFeix_FA-0mhtYqwMo2x0zgrnA@mail.gmail.com>
@ 2018-04-20  7:18               ` Ludovic BARRE
  0 siblings, 0 replies; 28+ messages in thread
From: Ludovic BARRE @ 2018-04-20  7:18 UTC (permalink / raw)
  To: Radosław Pietrzyk
  Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Maxime Coquelin,
	Alexandre Torgue, Linus Walleij, Benjamin Gaignard,
	Philipp Zabel, open list, moderated list:ARM/STM32 ARCHITECTURE,
	open list:GPIO SUBSYSTEM

ok, So I include your patch in my serie
- irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

BR
Ludo

On 04/19/2018 10:03 PM, Radosław Pietrzyk wrote:
> Sure, I don't mind. I didn't have time to resend v3 with more verbose 
> description.
> 
> 2018-04-19 15:24 GMT+02:00 Ludovic BARRE <ludovic.barre@st.com 
> <mailto:ludovic.barre@st.com>>:
> 
>     Hi Radoslaw
> 
>     I preparing a patch serie which add support of stm32mp1.
>     Would you like, I add your patch (with commit message updated)
>     in my serie?
>     patch:
>     -irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
> 
>     BR
>     Ludo
> 
> 
>     On 03/14/2018 01:04 PM, Marc Zyngier wrote:
> 
>         On 14/03/18 11:46, Radosław Pietrzyk wrote:
> 
>             Hi Marc,
>             We had a quite fruitful discussion in this mail thread
>             regarding this
>             topic and Ludovic acked it so recently I have asked Thomas
>             if he still
>             needs this v3 patch with detailed explanation especially as
>             v2 version
>             of stm32-gpio patch has been already taken by Linus. However
>             if you
>             require I can resend v3 of this patch only with this
>             detailed explanation.
> 
> 
>         That'd be useful. The changelog is the only thing that will be
>         left from
>         this discussion, so it'd better be complete and accurate. If you
>         quickly
>         send a v3 for this single patch, I'll queue it right away.
> 
>         Thanks,
> 
>                  M.
> 
> 
>             2018-03-14 12:09 GMT+01:00 Marc Zyngier
>             <marc.zyngier@arm.com <mailto:marc.zyngier@arm.com>
>             <mailto:marc.zyngier@arm.com <mailto:marc.zyngier@arm.com>>>:
> 
>                   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

end of thread, other threads:[~2018-04-20  7:19 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-22 10:55   ` [1/2] " Ludovic BARRE
2018-02-22 11:01     ` Radosław Pietrzyk
2018-02-23  8:34       ` Radosław Pietrzyk
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
2018-03-02  8:14       ` Alexandre Torgue
2018-03-02  8:37         ` Radosław Pietrzyk
2018-03-02  8:49           ` Alexandre Torgue
2018-02-23  8:31 ` [PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy 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: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>
2018-03-14 12:04         ` Marc Zyngier
2018-04-19 13:24           ` Ludovic BARRE
     [not found]             ` <CAFvLkMQv3AajhFaRxsBt+UzdGFeix_FA-0mhtYqwMo2x0zgrnA@mail.gmail.com>
2018-04-20  7:18               ` Ludovic BARRE
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).