* [PATCH 2/4] irqchip: ingenic: Error out if IRQ domain creation failed
2019-07-27 19:17 [PATCH 1/4] irqchip: ingenic: Drop redundant irq_suspend / irq_resume functions Paul Cercueil
@ 2019-07-27 19:17 ` Paul Cercueil
2019-07-27 19:17 ` [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain Paul Cercueil
2019-07-27 19:17 ` [PATCH 4/4] irqchip: ingenic: Alloc generic chips " Paul Cercueil
2 siblings, 0 replies; 7+ messages in thread
From: Paul Cercueil @ 2019-07-27 19:17 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier
Cc: linux-kernel, od, Paul Cercueil
If we cannot create the IRQ domain, the driver should fail to probe
instead of succeeding with just a warning message.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/irqchip/irq-ingenic.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-ingenic.c b/drivers/irqchip/irq-ingenic.c
index 06fa810e89bb..d97a3a500249 100644
--- a/drivers/irqchip/irq-ingenic.c
+++ b/drivers/irqchip/irq-ingenic.c
@@ -87,6 +87,14 @@ static int __init ingenic_intc_of_init(struct device_node *node,
goto out_unmap_irq;
}
+ domain = irq_domain_add_legacy(node, num_chips * 32,
+ JZ4740_IRQ_BASE, 0,
+ &irq_domain_simple_ops, NULL);
+ if (!domain) {
+ err = -ENOMEM;
+ goto out_unmap_base;
+ }
+
for (i = 0; i < num_chips; i++) {
/* Mask all irqs */
writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
@@ -112,14 +120,11 @@ static int __init ingenic_intc_of_init(struct device_node *node,
IRQ_NOPROBE | IRQ_LEVEL);
}
- domain = irq_domain_add_legacy(node, num_chips * 32, JZ4740_IRQ_BASE, 0,
- &irq_domain_simple_ops, NULL);
- if (!domain)
- pr_warn("unable to register IRQ domain\n");
-
setup_irq(parent_irq, &intc_cascade_action);
return 0;
+out_unmap_base:
+ iounmap(intc->base);
out_unmap_irq:
irq_dispose_mapping(parent_irq);
out_free:
--
2.21.0.593.g511ec345e18
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain
2019-07-27 19:17 [PATCH 1/4] irqchip: ingenic: Drop redundant irq_suspend / irq_resume functions Paul Cercueil
2019-07-27 19:17 ` [PATCH 2/4] irqchip: ingenic: Error out if IRQ domain creation failed Paul Cercueil
@ 2019-07-27 19:17 ` Paul Cercueil
2019-07-29 10:38 ` Marc Zyngier
2019-07-27 19:17 ` [PATCH 4/4] irqchip: ingenic: Alloc generic chips " Paul Cercueil
2 siblings, 1 reply; 7+ messages in thread
From: Paul Cercueil @ 2019-07-27 19:17 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier
Cc: linux-kernel, od, Paul Cercueil
Get the virq number from the IRQ domain instead of calculating it from
the hardcoded irq base.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/irqchip/irq-ingenic.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-ingenic.c b/drivers/irqchip/irq-ingenic.c
index d97a3a500249..82a079fa3a3d 100644
--- a/drivers/irqchip/irq-ingenic.c
+++ b/drivers/irqchip/irq-ingenic.c
@@ -21,6 +21,7 @@
struct ingenic_intc_data {
void __iomem *base;
+ struct irq_domain *domain;
unsigned num_chips;
};
@@ -34,6 +35,7 @@ struct ingenic_intc_data {
static irqreturn_t intc_cascade(int irq, void *data)
{
struct ingenic_intc_data *intc = irq_get_handler_data(irq);
+ struct irq_domain *domain = intc->domain;
uint32_t irq_reg;
unsigned i;
@@ -43,7 +45,8 @@ static irqreturn_t intc_cascade(int irq, void *data)
if (!irq_reg)
continue;
- generic_handle_irq(__fls(irq_reg) + (i * 32) + JZ4740_IRQ_BASE);
+ irq = irq_find_mapping(domain, __fls(irq_reg) + (i * 32));
+ generic_handle_irq(irq);
}
return IRQ_HANDLED;
@@ -95,6 +98,8 @@ static int __init ingenic_intc_of_init(struct device_node *node,
goto out_unmap_base;
}
+ intc->domain = domain;
+
for (i = 0; i < num_chips; i++) {
/* Mask all irqs */
writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
--
2.21.0.593.g511ec345e18
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain
2019-07-27 19:17 ` [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain Paul Cercueil
@ 2019-07-29 10:38 ` Marc Zyngier
2019-07-29 16:57 ` Paul Cercueil
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2019-07-29 10:38 UTC (permalink / raw)
To: Paul Cercueil, Thomas Gleixner, Jason Cooper
Cc: linux-kernel, od, Zhou Yanjie
[+ Zhou Yanjie]
Paul,
On 27/07/2019 20:17, Paul Cercueil wrote:
> Get the virq number from the IRQ domain instead of calculating it from
> the hardcoded irq base.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/irqchip/irq-ingenic.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-ingenic.c b/drivers/irqchip/irq-ingenic.c
> index d97a3a500249..82a079fa3a3d 100644
> --- a/drivers/irqchip/irq-ingenic.c
> +++ b/drivers/irqchip/irq-ingenic.c
> @@ -21,6 +21,7 @@
>
> struct ingenic_intc_data {
> void __iomem *base;
> + struct irq_domain *domain;
> unsigned num_chips;
> };
>
> @@ -34,6 +35,7 @@ struct ingenic_intc_data {
> static irqreturn_t intc_cascade(int irq, void *data)
> {
> struct ingenic_intc_data *intc = irq_get_handler_data(irq);
> + struct irq_domain *domain = intc->domain;
> uint32_t irq_reg;
> unsigned i;
>
> @@ -43,7 +45,8 @@ static irqreturn_t intc_cascade(int irq, void *data)
> if (!irq_reg)
> continue;
>
> - generic_handle_irq(__fls(irq_reg) + (i * 32) + JZ4740_IRQ_BASE);
> + irq = irq_find_mapping(domain, __fls(irq_reg) + (i * 32));
> + generic_handle_irq(irq);
> }
>
> return IRQ_HANDLED;
> @@ -95,6 +98,8 @@ static int __init ingenic_intc_of_init(struct device_node *node,
> goto out_unmap_base;
> }
>
> + intc->domain = domain;
> +
> for (i = 0; i < num_chips; i++) {
> /* Mask all irqs */
> writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
>
This is likely to conflict with this[1] series, which turns the
intc_cascade function into a chained handler (which it should have been
from the start). Can you please work with Zhou to post a unified series?
Having two people working independently on the same file is likely to
end badly otherwise.
Thanks,
M.
[1]
https://lore.kernel.org/lkml/1564335273-22931-1-git-send-email-zhouyanjie@zoho.com/
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain
2019-07-29 10:38 ` Marc Zyngier
@ 2019-07-29 16:57 ` Paul Cercueil
2019-07-30 6:01 ` Zhou Yanjie
0 siblings, 1 reply; 7+ messages in thread
From: Paul Cercueil @ 2019-07-29 16:57 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Thomas Gleixner, Jason Cooper, linux-kernel, od, Zhou Yanjie
Hi Marc,
Le lun. 29 juil. 2019 à 6:38, Marc Zyngier <marc.zyngier@arm.com> a
écrit :
> [+ Zhou Yanjie]
>
> Paul,
>
> On 27/07/2019 20:17, Paul Cercueil wrote:
>> Get the virq number from the IRQ domain instead of calculating it
>> from
>> the hardcoded irq base.
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> drivers/irqchip/irq-ingenic.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-ingenic.c
>> b/drivers/irqchip/irq-ingenic.c
>> index d97a3a500249..82a079fa3a3d 100644
>> --- a/drivers/irqchip/irq-ingenic.c
>> +++ b/drivers/irqchip/irq-ingenic.c
>> @@ -21,6 +21,7 @@
>>
>> struct ingenic_intc_data {
>> void __iomem *base;
>> + struct irq_domain *domain;
>> unsigned num_chips;
>> };
>>
>> @@ -34,6 +35,7 @@ struct ingenic_intc_data {
>> static irqreturn_t intc_cascade(int irq, void *data)
>> {
>> struct ingenic_intc_data *intc = irq_get_handler_data(irq);
>> + struct irq_domain *domain = intc->domain;
>> uint32_t irq_reg;
>> unsigned i;
>>
>> @@ -43,7 +45,8 @@ static irqreturn_t intc_cascade(int irq, void
>> *data)
>> if (!irq_reg)
>> continue;
>>
>> - generic_handle_irq(__fls(irq_reg) + (i * 32) + JZ4740_IRQ_BASE);
>> + irq = irq_find_mapping(domain, __fls(irq_reg) + (i * 32));
>> + generic_handle_irq(irq);
>> }
>>
>> return IRQ_HANDLED;
>> @@ -95,6 +98,8 @@ static int __init ingenic_intc_of_init(struct
>> device_node *node,
>> goto out_unmap_base;
>> }
>>
>> + intc->domain = domain;
>> +
>> for (i = 0; i < num_chips; i++) {
>> /* Mask all irqs */
>> writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
>>
>
> This is likely to conflict with this[1] series, which turns the
> intc_cascade function into a chained handler (which it should have
> been
> from the start). Can you please work with Zhou to post a unified
> series?
>
> Having two people working independently on the same file is likely to
> end badly otherwise.
I'm registered as maintainer for Ingenic SoCs (including ingenic-irq.c)
and Zhou didn't Cc me on his patchset... And if he did I'd have a few
comments on his patches that would have to be addressed in a V5.
If you think my patchset is fine, then maybe merge it then Zhou can just
rebase on top?
Cheers,
-Paul
> Thanks,
>
> M.
>
> [1]
> https://lore.kernel.org/lkml/1564335273-22931-1-git-send-email-zhouyanjie@zoho.com/
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain
2019-07-29 16:57 ` Paul Cercueil
@ 2019-07-30 6:01 ` Zhou Yanjie
0 siblings, 0 replies; 7+ messages in thread
From: Zhou Yanjie @ 2019-07-30 6:01 UTC (permalink / raw)
To: Paul Cercueil, Marc Zyngier
Cc: Thomas Gleixner, Jason Cooper, linux-kernel, od
Hi Paul,
These patches was originally sent on January 26th. It was still the old
maintainer
information. When sending v4, I may have some problems with cc setting
so that
no cc is given to you. I am really sorry. The main purpose on this patch
is to change
the cascade irq to chained irq, I think chained irq is more generic way.
Look forward
to your and Marc's comments.
On 2019年07月30日 00:57, Paul Cercueil wrote:
> Hi Marc,
>
>
> Le lun. 29 juil. 2019 à 6:38, Marc Zyngier <marc.zyngier@arm.com> a
> écrit :
>> [+ Zhou Yanjie]
>>
>> Paul,
>>
>> On 27/07/2019 20:17, Paul Cercueil wrote:
>>> Get the virq number from the IRQ domain instead of calculating it from
>>> the hardcoded irq base.
>>>
>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>> ---
>>> drivers/irqchip/irq-ingenic.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/irq-ingenic.c
>>> b/drivers/irqchip/irq-ingenic.c
>>> index d97a3a500249..82a079fa3a3d 100644
>>> --- a/drivers/irqchip/irq-ingenic.c
>>> +++ b/drivers/irqchip/irq-ingenic.c
>>> @@ -21,6 +21,7 @@
>>>
>>> struct ingenic_intc_data {
>>> void __iomem *base;
>>> + struct irq_domain *domain;
>>> unsigned num_chips;
>>> };
>>>
>>> @@ -34,6 +35,7 @@ struct ingenic_intc_data {
>>> static irqreturn_t intc_cascade(int irq, void *data)
>>> {
>>> struct ingenic_intc_data *intc = irq_get_handler_data(irq);
>>> + struct irq_domain *domain = intc->domain;
>>> uint32_t irq_reg;
>>> unsigned i;
>>>
>>> @@ -43,7 +45,8 @@ static irqreturn_t intc_cascade(int irq, void *data)
>>> if (!irq_reg)
>>> continue;
>>>
>>> - generic_handle_irq(__fls(irq_reg) + (i * 32) +
>>> JZ4740_IRQ_BASE);
>>> + irq = irq_find_mapping(domain, __fls(irq_reg) + (i * 32));
>>> + generic_handle_irq(irq);
>>> }
>>>
>>> return IRQ_HANDLED;
>>> @@ -95,6 +98,8 @@ static int __init ingenic_intc_of_init(struct
>>> device_node *node,
>>> goto out_unmap_base;
>>> }
>>>
>>> + intc->domain = domain;
>>> +
>>> for (i = 0; i < num_chips; i++) {
>>> /* Mask all irqs */
>>> writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
>>>
>>
>> This is likely to conflict with this[1] series, which turns the
>> intc_cascade function into a chained handler (which it should have been
>> from the start). Can you please work with Zhou to post a unified series?
>>
>> Having two people working independently on the same file is likely to
>> end badly otherwise.
>
> I'm registered as maintainer for Ingenic SoCs (including ingenic-irq.c)
> and Zhou didn't Cc me on his patchset... And if he did I'd have a few
> comments on his patches that would have to be addressed in a V5.
>
> If you think my patchset is fine, then maybe merge it then Zhou can just
> rebase on top?
>
> Cheers,
> -Paul
>
>> Thanks,
>>
>> M.
>>
>> [1]
>> https://lore.kernel.org/lkml/1564335273-22931-1-git-send-email-zhouyanjie@zoho.com/
>>
>> --
>> Jazz is not dead. It just smells funny...
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/4] irqchip: ingenic: Alloc generic chips from IRQ domain
2019-07-27 19:17 [PATCH 1/4] irqchip: ingenic: Drop redundant irq_suspend / irq_resume functions Paul Cercueil
2019-07-27 19:17 ` [PATCH 2/4] irqchip: ingenic: Error out if IRQ domain creation failed Paul Cercueil
2019-07-27 19:17 ` [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain Paul Cercueil
@ 2019-07-27 19:17 ` Paul Cercueil
2 siblings, 0 replies; 7+ messages in thread
From: Paul Cercueil @ 2019-07-27 19:17 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier
Cc: linux-kernel, od, Paul Cercueil
By creating the generic chips from the IRQ domain, we don't rely on the
JZ4740_IRQ_BASE macro. It also makes the code a bit cleaner.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/irqchip/irq-ingenic.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/irqchip/irq-ingenic.c b/drivers/irqchip/irq-ingenic.c
index 82a079fa3a3d..06ab3ad22ad2 100644
--- a/drivers/irqchip/irq-ingenic.c
+++ b/drivers/irqchip/irq-ingenic.c
@@ -36,12 +36,14 @@ static irqreturn_t intc_cascade(int irq, void *data)
{
struct ingenic_intc_data *intc = irq_get_handler_data(irq);
struct irq_domain *domain = intc->domain;
+ struct irq_chip_generic *gc;
uint32_t irq_reg;
unsigned i;
for (i = 0; i < intc->num_chips; i++) {
- irq_reg = readl(intc->base + (i * CHIP_SIZE) +
- JZ_REG_INTC_PENDING);
+ gc = irq_get_domain_generic_chip(domain, i * 32);
+
+ irq_reg = irq_reg_readl(gc, JZ_REG_INTC_PENDING);
if (!irq_reg)
continue;
@@ -92,7 +94,7 @@ static int __init ingenic_intc_of_init(struct device_node *node,
domain = irq_domain_add_legacy(node, num_chips * 32,
JZ4740_IRQ_BASE, 0,
- &irq_domain_simple_ops, NULL);
+ &irq_generic_chip_ops, NULL);
if (!domain) {
err = -ENOMEM;
goto out_unmap_base;
@@ -100,17 +102,17 @@ static int __init ingenic_intc_of_init(struct device_node *node,
intc->domain = domain;
- for (i = 0; i < num_chips; i++) {
- /* Mask all irqs */
- writel(0xffffffff, intc->base + (i * CHIP_SIZE) +
- JZ_REG_INTC_SET_MASK);
+ err = irq_alloc_domain_generic_chips(domain, 32, 1, "INTC",
+ handle_level_irq, 0,
+ IRQ_NOPROBE | IRQ_LEVEL, 0);
+ if (err)
+ goto out_domain_remove;
- gc = irq_alloc_generic_chip("INTC", 1,
- JZ4740_IRQ_BASE + (i * 32),
- intc->base + (i * CHIP_SIZE),
- handle_level_irq);
+ for (i = 0; i < num_chips; i++) {
+ gc = irq_get_domain_generic_chip(domain, i * 32);
gc->wake_enabled = IRQ_MSK(32);
+ gc->reg_base = intc->base + (i * CHIP_SIZE);
ct = gc->chip_types;
ct->regs.enable = JZ_REG_INTC_CLEAR_MASK;
@@ -121,13 +123,15 @@ static int __init ingenic_intc_of_init(struct device_node *node,
ct->chip.irq_set_wake = irq_gc_set_wake;
ct->chip.flags = IRQCHIP_MASK_ON_SUSPEND;
- irq_setup_generic_chip(gc, IRQ_MSK(32), 0, 0,
- IRQ_NOPROBE | IRQ_LEVEL);
+ /* Mask all irqs */
+ irq_reg_writel(gc, IRQ_MSK(32), JZ_REG_INTC_SET_MASK);
}
setup_irq(parent_irq, &intc_cascade_action);
return 0;
+out_domain_remove:
+ irq_domain_remove(domain);
out_unmap_base:
iounmap(intc->base);
out_unmap_irq:
--
2.21.0.593.g511ec345e18
^ permalink raw reply related [flat|nested] 7+ messages in thread