Re: [1/2] ARM: irqchip: stm32: Optimizes and cleans up stm32-exti irq_domain
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: > 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 : >> 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 >>> --- >>> 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, >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
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 : > 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 : >> 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 >>> --- >>> 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, >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
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: > 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 >> --- >> 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, >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
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 : > 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 >> --- >> 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, >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
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--- 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, >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
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 --- 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, >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