[PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy

[PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy

[Radoslaw Pietrzyk]

- 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

[PATCH 0/2] Optimizes, cleans up and enhances stm32-exti irq hierarchy

[Radoslaw Pietrzyk]

- 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

[PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radoslaw Pietrzyk]

	- 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

[PATCH 1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radoslaw Pietrzyk]

	- 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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radoslaw Pietrzyk]

	- 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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radoslaw Pietrzyk]

	- 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

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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,
>   };
> 

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

[1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

[PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy

[Radoslaw Pietrzyk]

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

[PATCH v2 0/2] v2 patches for stm32-exti irq hierarchy

[Radoslaw Pietrzyk]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radoslaw Pietrzyk]

- 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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radoslaw Pietrzyk]

- 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

[PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radoslaw Pietrzyk]

- 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

[PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radoslaw Pietrzyk]

- 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

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

[1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Thomas Gleixner]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Thomas Gleixner]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Thomas Gleixner]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Radosław Pietrzyk]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Ludovic BARRE]

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,
>   };
> 

Re: [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Ludovic BARRE]

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,
>   };
> 

[PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Ludovic BARRE]

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,
>   };
> 

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Linus Walleij]

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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Linus Walleij]

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

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radosław Pietrzyk]

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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radosław Pietrzyk]

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

Re: [PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Linus Walleij]

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

[PATCH v2 2/2] pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Linus Walleij]

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

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radosław Pietrzyk]

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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Radosław Pietrzyk]

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

Re: [PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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

[PATCH 2/2] ARM: pinctrl: stm32: Optimizes and enhances stm32gpio irqchip

[Alexandre Torgue]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Marc Zyngier]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Marc Zyngier]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Marc Zyngier]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Marc Zyngier]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

Re: [PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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

[PATCH v2 1/2] irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain

[Ludovic BARRE]

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 at arm.com <mailto:marc.zyngier@arm.com>
>             <mailto:marc.zyngier at 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...
> 
> 
> 
> 
>