| Message ID | 20231219095158.285408-1-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | irqchip/loongson-eiointc: Refine irq affinity setting during resume | expand |
+ irqchip maintainer Thomas. Jianmin, Could you give some feedback about this patch since you are author about this patch? By searching code of all hw irqchip drivers, there is no affinity restoring during s3/s4. Regards Bibo Mao On 2023/12/19 下午5:51, Bibo Mao wrote: > During suspend and resume, other CPUs are removed and IRQs are migrated > to CPU0. So it is not necessary to restore irq affinity for eiointc. > > Also there is some optimization for function eiointc_irq_dispatch, > in genral there are 256 IRQs supported for eiointc. When irq happens, > eiointc irq handler reads the bitmap and find pending irqs. There are > 4 times of consecutive iocsr_read64 operations for the total 256 bits, > indeed in most scenario pending value is zero in 3 times, and not zero > in one time. Here zero checking is added to avoid some useless > operations sush as clearing hw ISR. > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > drivers/irqchip/irq-loongson-eiointc.c | 29 +++++++++++--------------- > 1 file changed, 12 insertions(+), 17 deletions(-) > > diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c > index 1623cd779175..b01be85b8ebc 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. > + * > + * In most scenario value of pending is 0 if no multiple IRQs > + * happen at the same time > + */ > + if (!pending) > + continue; > + > iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3)); > while (pending) { > int bit = __ffs(pending); > @@ -241,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) > @@ -304,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 = { >
> -----Original Messages----- > From: maobibo <maobibo@loongson.cn> > Send time:Thursday, 01/04/2024 15:53:51 > To: "Huacai Chen" <chenhuacai@kernel.org>, "Jiaxun Yang" <jiaxun.yang@flygoat.com>, "Thomas Gleixner" <tglx@linutronix.de> > Cc: linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, "Jianmin Lv" <lvjianmin@loongson.cn> > Subject: Re: [PATCH] irqchip/loongson-eiointc: Refine irq affinity setting during resume > > + irqchip maintainer Thomas. > > Jianmin, > > Could you give some feedback about this patch since you are author about > this patch? By searching code of all hw irqchip drivers, there is no > affinity restoring during s3/s4. > > Regards > Bibo Mao > Hi, bibo I looked into the patch, IMO, the change here in your patch is reasonable. The original restoring code here is aimed to restore irq routing registers from its' affinity, but it seems that the affinity may be changed during cpu offline, so we should not use it this way. Moreover, the affinity of each irq will be set to boot cpu after all of non-boot cpus are offline before suspend callback, so it seems that affinity restoring is not needed. > On 2023/12/19 下午5:51, Bibo Mao wrote: > > During suspend and resume, other CPUs are removed and IRQs are migrated > > to CPU0. So it is not necessary to restore irq affinity for eiointc. > > > > Also there is some optimization for function eiointc_irq_dispatch, > > in genral there are 256 IRQs supported for eiointc. When irq happens, > > eiointc irq handler reads the bitmap and find pending irqs. There are > > 4 times of consecutive iocsr_read64 operations for the total 256 bits, > > indeed in most scenario pending value is zero in 3 times, and not zero > > in one time. Here zero checking is added to avoid some useless > > operations sush as clearing hw ISR. > > > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > > --- > > drivers/irqchip/irq-loongson-eiointc.c | 29 +++++++++++--------------- > > 1 file changed, 12 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c > > index 1623cd779175..b01be85b8ebc 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. > > + * > > + * In most scenario value of pending is 0 if no multiple IRQs > > + * happen at the same time > > + */ > > + if (!pending) > > + continue; > > + > > iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3)); > > while (pending) { > > int bit = __ffs(pending); > > @@ -241,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) > > @@ -304,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 = { > > 本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c index 1623cd779175..b01be85b8ebc 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. + * + * In most scenario value of pending is 0 if no multiple IRQs + * happen at the same time + */ + if (!pending) + continue; + iocsr_write64(pending, EIOINTC_REG_ISR + (i << 3)); while (pending) { int bit = __ffs(pending); @@ -241,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) @@ -304,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 = {
During suspend and resume, other CPUs are removed and IRQs are migrated to CPU0. So it is not necessary to restore irq affinity for eiointc. Also there is some optimization for function eiointc_irq_dispatch, in genral there are 256 IRQs supported for eiointc. When irq happens, eiointc irq handler reads the bitmap and find pending irqs. There are 4 times of consecutive iocsr_read64 operations for the total 256 bits, indeed in most scenario pending value is zero in 3 times, and not zero in one time. Here zero checking is added to avoid some useless operations sush as clearing hw ISR. Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- drivers/irqchip/irq-loongson-eiointc.c | 29 +++++++++++--------------- 1 file changed, 12 insertions(+), 17 deletions(-)