2019-07-27 19:20:22

by Paul Cercueil

Subject: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain

Get the virq number from the IRQ domain instead of calculating it from
the hardcoded irq base.

Signed-off-by: Paul Cercueil <[email protected]>
---
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

2019-07-29 13:19:36

by Marc Zyngier

Subject: Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain

[+ 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 <[email protected]>
> ---
> 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/[email protected]/
--
Jazz is not dead. It just smells funny...

2019-07-29 17:07:00

by Paul Cercueil

Subject: Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain

Hi Marc,


Le lun. 29 juil. 2019 ? 6:38, Marc Zyngier <[email protected]> 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 <[email protected]>
>> ---
>> 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/[email protected]/
> --
> Jazz is not dead. It just smells funny...

2019-07-30 11:11:23

by Zhou Yanjie

Subject: Re: [PATCH 3/4] irqchip: ingenic: Get virq number from IRQ domain

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 <[email protected]> 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 <[email protected]>
>>> ---
>>> 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/[email protected]/
>>
>> --
>> Jazz is not dead. It just smells funny...
>
>