During suspend and resume, other CPUs are hot-unpluged and IRQs are
migrated to CPU0. So it is not necessary to restore irq affinity for
eiointc irq controller.
Also there is some optimization for the interrupt dispatch function
eiointc_irq_dispatch. There are 256 IRQs supported for eiointc, eiointc
irq handler reads the bitmap and find pending irqs when irq happens.
So there are four times of consecutive iocsr_read64 operations for the
total 256 bits to find all pending irqs. If the pending bitmap is zero,
it means that there is no pending irq for the this irq bitmap range,
we can skip handling to avoid some useless operations such as clearing
hw ISR.
---
Changes in v3:
Split the patch into three small patches
Changes in v2:
Modify changelog and comments
---
Bibo Mao (3):
irqchip/loongson-eiointc: Skip handling if there is no pending irq
irqchip/loongson-eiointc: Refine irq affinity setting during resume
irqchip/loongson-eiointc: Typo fix in function eiointc_domain_alloc
drivers/irqchip/irq-loongson-eiointc.c | 29 +++++++++++---------------
1 file changed, 12 insertions(+), 17 deletions(-)
base-commit: 052d534373b7ed33712a63d5e17b2b6cdbce84fd
--
2.39.3
There is one simple optimization in the interrupt dispatch function
eiointc_irq_dispatch. There are 256 IRQs supported for eiointc, eiointc
irq handler reads the bitmap and find pending irqs when irq happens.
So there are four times of consecutive iocsr_read64 operations for the
total 256 bits to find all pending irqs. If the pending bitmap is zero,
it means that there is no pending irq for the this irq bitmap range,
we can skip handling to avoid some useless operations such as clearing
hw ISR.
Signed-off-by: Bibo Mao <[email protected]>
---
drivers/irqchip/irq-loongson-eiointc.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 1623cd779175..6143adb1b73b 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -198,6 +198,17 @@ static void eiointc_irq_dispatch(struct irq_desc *desc)
for (i = 0; i < eiointc_priv[0]->vec_count / VEC_COUNT_PER_REG; i++) {
pending = iocsr_read64(EIOINTC_REG_ISR + (i << 3));
+
+ /*
+ * Get pending eiointc irq from bitmap status, there are 4 times
+ * consecutive iocsr_read64 operations for 256 IRQs.
+ *
+ * Skip handling if pending bitmap is zero
+ */
+ if (!pending)
+ continue;
+
+ /* Clear the IRQs */
iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3));
while (pending) {
int bit = __ffs(pending);
--
2.39.3
During suspend and resume, CPUs except CPU0 are hot-unpluged and IRQs
are migrated to CPU0. So it is not necessary to restore irq affinity for
eiointc irq controller when system resumes. This patch removes the piece
of code about irq affinity restoring in function eiointc_resume.
Signed-off-by: Bibo Mao <[email protected]>
---
drivers/irqchip/irq-loongson-eiointc.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 6143adb1b73b..86f4faad0695 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -315,23 +315,7 @@ static int eiointc_suspend(void)
static void eiointc_resume(void)
{
- int i, j;
- struct irq_desc *desc;
- struct irq_data *irq_data;
-
eiointc_router_init(0);
-
- for (i = 0; i < nr_pics; i++) {
- for (j = 0; j < eiointc_priv[0]->vec_count; j++) {
- desc = irq_resolve_mapping(eiointc_priv[i]->eiointc_domain, j);
- if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) {
- raw_spin_lock(&desc->lock);
- irq_data = irq_domain_get_irq_data(eiointc_priv[i]->eiointc_domain, irq_desc_get_irq(desc));
- eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0);
- raw_spin_unlock(&desc->lock);
- }
- }
- }
}
static struct syscore_ops eiointc_syscore_ops = {
--
2.39.3
There is small typo in function eiointc_domain_alloc, and there is no
definition about struct eiointc, instead it should be struct eiointc_priv.
It is strange that there is no warning with gcc compiler. This patch
fixes the typo issue.
Signed-off-by: Bibo Mao <[email protected]>
---
drivers/irqchip/irq-loongson-eiointc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 86f4faad0695..1a25e0613d50 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -252,7 +252,7 @@ static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
int ret;
unsigned int i, type;
unsigned long hwirq = 0;
- struct eiointc *priv = domain->host_data;
+ struct eiointc_priv *priv = domain->host_data;
ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
if (ret)
--
2.39.3
On 2024/1/24 下午5:51, Huacai Chen wrote:
> Hi, Bibo,
>
> On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>>
>> There is one simple optimization in the interrupt dispatch function
>> eiointc_irq_dispatch. There are 256 IRQs supported for eiointc, eiointc
>> irq handler reads the bitmap and find pending irqs when irq happens.
>> So there are four times of consecutive iocsr_read64 operations for the
>> total 256 bits to find all pending irqs. If the pending bitmap is zero,
>> it means that there is no pending irq for the this irq bitmap range,
>> we can skip handling to avoid some useless operations such as clearing
>> hw ISR.
>>
>> Signed-off-by: Bibo Mao <[email protected]>
>> ---
>> drivers/irqchip/irq-loongson-eiointc.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>> index 1623cd779175..6143adb1b73b 100644
>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>> @@ -198,6 +198,17 @@ static void eiointc_irq_dispatch(struct irq_desc *desc)
>>
>> for (i = 0; i < eiointc_priv[0]->vec_count / VEC_COUNT_PER_REG; i++) {
>> pending = iocsr_read64(EIOINTC_REG_ISR + (i << 3));
>> +
>> + /*
>> + * Get pending eiointc irq from bitmap status, there are 4 times
>> + * consecutive iocsr_read64 operations for 256 IRQs.
>> + *
>> + * Skip handling if pending bitmap is zero
> This driver is shared by Loongson-2 and Loongson-3 series, for
> Loongson-2K0500 there is only 128 IRQs, so I suggest only keep the
> last line "Skip handling if current pending bitmap is zero" is enough.
Sure, will refine the patch in this way.
Regards
Bibo Mao
>
> Huacai
>
>> + */
>> + if (!pending)
>> + continue;
>> +
>> + /* Clear the IRQs */
>> iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3));
>> while (pending) {
>> int bit = __ffs(pending);
>> --
>> 2.39.3
>>
On 2024/1/24 下午5:52, Huacai Chen wrote:
> Acked-by: Huacai Chen <[email protected]>
>
> But I think moving this simplest patch to the first one is better.
Huacai,
Thanks for reviewing the patch, will do in next version.
Regards
Bibo Mao
>
> Huacai
>
> On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>>
>> There is small typo in function eiointc_domain_alloc, and there is no
>> definition about struct eiointc, instead it should be struct eiointc_priv.
>> It is strange that there is no warning with gcc compiler. This patch
>> fixes the typo issue.
>>
>> Signed-off-by: Bibo Mao <[email protected]>
>> ---
>> drivers/irqchip/irq-loongson-eiointc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>> index 86f4faad0695..1a25e0613d50 100644
>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>> @@ -252,7 +252,7 @@ static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> int ret;
>> unsigned int i, type;
>> unsigned long hwirq = 0;
>> - struct eiointc *priv = domain->host_data;
>> + struct eiointc_priv *priv = domain->host_data;
>>
>> ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
>> if (ret)
>> --
>> 2.39.3
>>
Hi, Bibo,
On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>
> There is one simple optimization in the interrupt dispatch function
> eiointc_irq_dispatch. There are 256 IRQs supported for eiointc, eiointc
> irq handler reads the bitmap and find pending irqs when irq happens.
> So there are four times of consecutive iocsr_read64 operations for the
> total 256 bits to find all pending irqs. If the pending bitmap is zero,
> it means that there is no pending irq for the this irq bitmap range,
> we can skip handling to avoid some useless operations such as clearing
> hw ISR.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> drivers/irqchip/irq-loongson-eiointc.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index 1623cd779175..6143adb1b73b 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -198,6 +198,17 @@ static void eiointc_irq_dispatch(struct irq_desc *desc)
>
> for (i = 0; i < eiointc_priv[0]->vec_count / VEC_COUNT_PER_REG; i++) {
> pending = iocsr_read64(EIOINTC_REG_ISR + (i << 3));
> +
> + /*
> + * Get pending eiointc irq from bitmap status, there are 4 times
> + * consecutive iocsr_read64 operations for 256 IRQs.
> + *
> + * Skip handling if pending bitmap is zero
This driver is shared by Loongson-2 and Loongson-3 series, for
Loongson-2K0500 there is only 128 IRQs, so I suggest only keep the
last line "Skip handling if current pending bitmap is zero" is enough.
Huacai
> + */
> + if (!pending)
> + continue;
> +
> + /* Clear the IRQs */
> iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3));
> while (pending) {
> int bit = __ffs(pending);
> --
> 2.39.3
>
Acked-by: Huacai Chen <[email protected]>
But I think moving this simplest patch to the first one is better.
Huacai
On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>
> There is small typo in function eiointc_domain_alloc, and there is no
> definition about struct eiointc, instead it should be struct eiointc_priv.
> It is strange that there is no warning with gcc compiler. This patch
> fixes the typo issue.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> drivers/irqchip/irq-loongson-eiointc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index 86f4faad0695..1a25e0613d50 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -252,7 +252,7 @@ static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
> int ret;
> unsigned int i, type;
> unsigned long hwirq = 0;
> - struct eiointc *priv = domain->host_data;
> + struct eiointc_priv *priv = domain->host_data;
>
> ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
> if (ret)
> --
> 2.39.3
>
I can give an Acked-by if these lines are also removed in our internal repo.
Huacai
On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>
> During suspend and resume, CPUs except CPU0 are hot-unpluged and IRQs
> are migrated to CPU0. So it is not necessary to restore irq affinity for
> eiointc irq controller when system resumes. This patch removes the piece
> of code about irq affinity restoring in function eiointc_resume.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> drivers/irqchip/irq-loongson-eiointc.c | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index 6143adb1b73b..86f4faad0695 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -315,23 +315,7 @@ static int eiointc_suspend(void)
>
> static void eiointc_resume(void)
> {
> - int i, j;
> - struct irq_desc *desc;
> - struct irq_data *irq_data;
> -
> eiointc_router_init(0);
> -
> - for (i = 0; i < nr_pics; i++) {
> - for (j = 0; j < eiointc_priv[0]->vec_count; j++) {
> - desc = irq_resolve_mapping(eiointc_priv[i]->eiointc_domain, j);
> - if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) {
> - raw_spin_lock(&desc->lock);
> - irq_data = irq_domain_get_irq_data(eiointc_priv[i]->eiointc_domain, irq_desc_get_irq(desc));
> - eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0);
> - raw_spin_unlock(&desc->lock);
> - }
> - }
> - }
> }
>
> static struct syscore_ops eiointc_syscore_ops = {
> --
> 2.39.3
>
My work area is hypervisor and I find gaps between other architectures
and hypervisors, try to keep up. And We prepare to provide eiointc
virtualization extension patch, so there is such patch.
For internal repo, you can give suggestion in internal forum to driver
owner. This is linux kernel mainline forum, I suggest we had better
discuss patch based on mainline base code.
Regards
Bibo Mao
On 2024/1/24 下午5:56, Huacai Chen wrote:
> I can give an Acked-by if these lines are also removed in our internal repo.
>
> Huacai
>
> On Thu, Jan 18, 2024 at 8:15 PM Bibo Mao <[email protected]> wrote:
>>
>> During suspend and resume, CPUs except CPU0 are hot-unpluged and IRQs
>> are migrated to CPU0. So it is not necessary to restore irq affinity for
>> eiointc irq controller when system resumes. This patch removes the piece
>> of code about irq affinity restoring in function eiointc_resume.
>>
>> Signed-off-by: Bibo Mao <[email protected]>
>> ---
>> drivers/irqchip/irq-loongson-eiointc.c | 16 ----------------
>> 1 file changed, 16 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>> index 6143adb1b73b..86f4faad0695 100644
>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>> @@ -315,23 +315,7 @@ static int eiointc_suspend(void)
>>
>> static void eiointc_resume(void)
>> {
>> - int i, j;
>> - struct irq_desc *desc;
>> - struct irq_data *irq_data;
>> -
>> eiointc_router_init(0);
>> -
>> - for (i = 0; i < nr_pics; i++) {
>> - for (j = 0; j < eiointc_priv[0]->vec_count; j++) {
>> - desc = irq_resolve_mapping(eiointc_priv[i]->eiointc_domain, j);
>> - if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) {
>> - raw_spin_lock(&desc->lock);
>> - irq_data = irq_domain_get_irq_data(eiointc_priv[i]->eiointc_domain, irq_desc_get_irq(desc));
>> - eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0);
>> - raw_spin_unlock(&desc->lock);
>> - }
>> - }
>> - }
>> }
>>
>> static struct syscore_ops eiointc_syscore_ops = {
>> --
>> 2.39.3
>>