* [PATCH 0/2] riscv-imsic/aplic: Save and restore the registers @ 2025-07-09 2:55 Nick Hu 2025-07-09 2:55 ` [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers Nick Hu 2025-07-09 2:55 ` [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers Nick Hu 0 siblings, 2 replies; 8+ messages in thread From: Nick Hu @ 2025-07-09 2:55 UTC (permalink / raw) To: Alexandre Ghiti, linux-riscv Cc: Nick Hu, Paul Walmsley, Palmer Dabbelt, Albert Ou Since the APLIC and IMSIC may be powered down when the CPUs enter a deep sleep state, it is necessary to implement save and restore functions to ensure their states are correctly preserved and restored. Nick Hu (2): irqchip/riscv-imsic: Restore the IMSIC register irqchip/riscv-aplic: Add save and restore functions drivers/irqchip/irq-riscv-aplic-direct.c | 14 ++- drivers/irqchip/irq-riscv-aplic-main.c | 143 +++++++++++++++++++++++ drivers/irqchip/irq-riscv-aplic-main.h | 12 ++ drivers/irqchip/irq-riscv-aplic-msi.c | 3 +- drivers/irqchip/irq-riscv-imsic-early.c | 40 +++++-- 5 files changed, 203 insertions(+), 9 deletions(-) -- 2.17.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers 2025-07-09 2:55 [PATCH 0/2] riscv-imsic/aplic: Save and restore the registers Nick Hu @ 2025-07-09 2:55 ` Nick Hu 2025-07-17 5:15 ` Anup Patel 2025-07-09 2:55 ` [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers Nick Hu 1 sibling, 1 reply; 8+ messages in thread From: Nick Hu @ 2025-07-09 2:55 UTC (permalink / raw) To: Alexandre Ghiti, linux-riscv, linux-kernel Cc: Nick Hu, Anup Patel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou When the system woken up from the low power state, the IMSIC might be in the reset state. Therefore adding the CPU PM callbacks to restore the IMSIC register when the cpu resume from the low power state. Signed-off-by: Nick Hu <nick.hu@sifive.com> Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> Reviewed-by: Cyan Yang <cyan.yang@sifive.com> --- drivers/irqchip/irq-riscv-imsic-early.c | 41 ++++++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c index d9ae87808651..f64d9a0642bb 100644 --- a/drivers/irqchip/irq-riscv-imsic-early.c +++ b/drivers/irqchip/irq-riscv-imsic-early.c @@ -7,6 +7,7 @@ #define pr_fmt(fmt) "riscv-imsic: " fmt #include <linux/acpi.h> #include <linux/cpu.h> +#include <linux/cpu_pm.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/irq.h> @@ -109,14 +110,8 @@ static void imsic_handle_irq(struct irq_desc *desc) chained_irq_exit(chip, desc); } -static int imsic_starting_cpu(unsigned int cpu) +static void imsic_restore(void) { - /* Mark per-CPU IMSIC state as online */ - imsic_state_online(); - - /* Enable per-CPU parent interrupt */ - enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq)); - /* Setup IPIs */ imsic_ipi_starting_cpu(); @@ -128,6 +123,19 @@ static int imsic_starting_cpu(unsigned int cpu) /* Enable local interrupt delivery */ imsic_local_delivery(true); +} + +static int imsic_starting_cpu(unsigned int cpu) +{ + /* Mark per-CPU IMSIC state as online */ + imsic_state_online(); + + /* Enable per-CPU parent interrupt */ + enable_percpu_irq(imsic_parent_irq, + irq_get_trigger_type(imsic_parent_irq)); + + /* Restore the imsic reg */ + imsic_restore(); return 0; } @@ -143,6 +151,23 @@ static int imsic_dying_cpu(unsigned int cpu) return 0; } +static int imsic_notifier(struct notifier_block *self, unsigned long cmd, + void *v) +{ + switch (cmd) { + case CPU_PM_EXIT: + /* Restore the imsic reg */ + imsic_restore(); + break; + } + + return NOTIFY_OK; +} + +static struct notifier_block imsic_notifier_block = { + .notifier_call = imsic_notifier, +}; + static int __init imsic_early_probe(struct fwnode_handle *fwnode) { struct irq_domain *domain; @@ -180,7 +205,7 @@ static int __init imsic_early_probe(struct fwnode_handle *fwnode) cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_IMSIC_STARTING, "irqchip/riscv/imsic:starting", imsic_starting_cpu, imsic_dying_cpu); - return 0; + return cpu_pm_register_notifier(&imsic_notifier_block); } static int __init imsic_early_dt_init(struct device_node *node, struct device_node *parent) -- 2.17.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers 2025-07-09 2:55 ` [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers Nick Hu @ 2025-07-17 5:15 ` Anup Patel 2025-07-17 7:24 ` Nick Hu 0 siblings, 1 reply; 8+ messages in thread From: Anup Patel @ 2025-07-17 5:15 UTC (permalink / raw) To: Nick Hu Cc: Alexandre Ghiti, linux-riscv, linux-kernel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou On Wed, Jul 9, 2025 at 8:26 AM Nick Hu <nick.hu@sifive.com> wrote: > > When the system woken up from the low power state, the IMSIC might be in > the reset state. Therefore adding the CPU PM callbacks to restore the > IMSIC register when the cpu resume from the low power state. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > Reviewed-by: Cyan Yang <cyan.yang@sifive.com> > --- > drivers/irqchip/irq-riscv-imsic-early.c | 41 ++++++++++++++++++++----- > 1 file changed, 33 insertions(+), 8 deletions(-) > > diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c > index d9ae87808651..f64d9a0642bb 100644 > --- a/drivers/irqchip/irq-riscv-imsic-early.c > +++ b/drivers/irqchip/irq-riscv-imsic-early.c > @@ -7,6 +7,7 @@ > #define pr_fmt(fmt) "riscv-imsic: " fmt > #include <linux/acpi.h> > #include <linux/cpu.h> > +#include <linux/cpu_pm.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/irq.h> > @@ -109,14 +110,8 @@ static void imsic_handle_irq(struct irq_desc *desc) > chained_irq_exit(chip, desc); > } > > -static int imsic_starting_cpu(unsigned int cpu) > +static void imsic_restore(void) > { > - /* Mark per-CPU IMSIC state as online */ > - imsic_state_online(); > - > - /* Enable per-CPU parent interrupt */ > - enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq)); > - > /* Setup IPIs */ > imsic_ipi_starting_cpu(); > > @@ -128,6 +123,19 @@ static int imsic_starting_cpu(unsigned int cpu) > > /* Enable local interrupt delivery */ > imsic_local_delivery(true); > +} > + > +static int imsic_starting_cpu(unsigned int cpu) > +{ > + /* Mark per-CPU IMSIC state as online */ > + imsic_state_online(); > + > + /* Enable per-CPU parent interrupt */ > + enable_percpu_irq(imsic_parent_irq, > + irq_get_trigger_type(imsic_parent_irq)); > + > + /* Restore the imsic reg */ > + imsic_restore(); > > return 0; > } > @@ -143,6 +151,23 @@ static int imsic_dying_cpu(unsigned int cpu) > return 0; > } > > +static int imsic_notifier(struct notifier_block *self, unsigned long cmd, > + void *v) s/imsic_notifier/imsic_pm_notifier/ The "void *v" parameter can be on the same line as function declaration. > +{ > + switch (cmd) { > + case CPU_PM_EXIT: > + /* Restore the imsic reg */ > + imsic_restore(); > + break; > + } > + > + return NOTIFY_OK; > +} > + > +static struct notifier_block imsic_notifier_block = { s/imsic_notifier_block/imsic_pm_notifier_block/ > + .notifier_call = imsic_notifier, > +}; > + > static int __init imsic_early_probe(struct fwnode_handle *fwnode) > { > struct irq_domain *domain; > @@ -180,7 +205,7 @@ static int __init imsic_early_probe(struct fwnode_handle *fwnode) > cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_IMSIC_STARTING, "irqchip/riscv/imsic:starting", > imsic_starting_cpu, imsic_dying_cpu); > > - return 0; > + return cpu_pm_register_notifier(&imsic_notifier_block); > } > > static int __init imsic_early_dt_init(struct device_node *node, struct device_node *parent) > -- > 2.17.1 > Otherwise, this looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers 2025-07-17 5:15 ` Anup Patel @ 2025-07-17 7:24 ` Nick Hu 0 siblings, 0 replies; 8+ messages in thread From: Nick Hu @ 2025-07-17 7:24 UTC (permalink / raw) To: Anup Patel Cc: Alexandre Ghiti, linux-riscv, linux-kernel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou On Thu, Jul 17, 2025 at 1:15 PM Anup Patel <anup@brainfault.org> wrote: > > On Wed, Jul 9, 2025 at 8:26 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > When the system woken up from the low power state, the IMSIC might be in > > the reset state. Therefore adding the CPU PM callbacks to restore the > > IMSIC register when the cpu resume from the low power state. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > > Reviewed-by: Cyan Yang <cyan.yang@sifive.com> > > --- > > drivers/irqchip/irq-riscv-imsic-early.c | 41 ++++++++++++++++++++----- > > 1 file changed, 33 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c > > index d9ae87808651..f64d9a0642bb 100644 > > --- a/drivers/irqchip/irq-riscv-imsic-early.c > > +++ b/drivers/irqchip/irq-riscv-imsic-early.c > > @@ -7,6 +7,7 @@ > > #define pr_fmt(fmt) "riscv-imsic: " fmt > > #include <linux/acpi.h> > > #include <linux/cpu.h> > > +#include <linux/cpu_pm.h> > > #include <linux/interrupt.h> > > #include <linux/io.h> > > #include <linux/irq.h> > > @@ -109,14 +110,8 @@ static void imsic_handle_irq(struct irq_desc *desc) > > chained_irq_exit(chip, desc); > > } > > > > -static int imsic_starting_cpu(unsigned int cpu) > > +static void imsic_restore(void) > > { > > - /* Mark per-CPU IMSIC state as online */ > > - imsic_state_online(); > > - > > - /* Enable per-CPU parent interrupt */ > > - enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq)); > > - > > /* Setup IPIs */ > > imsic_ipi_starting_cpu(); > > > > @@ -128,6 +123,19 @@ static int imsic_starting_cpu(unsigned int cpu) > > > > /* Enable local interrupt delivery */ > > imsic_local_delivery(true); > > +} > > + > > +static int imsic_starting_cpu(unsigned int cpu) > > +{ > > + /* Mark per-CPU IMSIC state as online */ > > + imsic_state_online(); > > + > > + /* Enable per-CPU parent interrupt */ > > + enable_percpu_irq(imsic_parent_irq, > > + irq_get_trigger_type(imsic_parent_irq)); > > + > > + /* Restore the imsic reg */ > > + imsic_restore(); > > > > return 0; > > } > > @@ -143,6 +151,23 @@ static int imsic_dying_cpu(unsigned int cpu) > > return 0; > > } > > > > +static int imsic_notifier(struct notifier_block *self, unsigned long cmd, > > + void *v) > > s/imsic_notifier/imsic_pm_notifier/ > Will update it in the next version. Thanks Regards, Nick > The "void *v" parameter can be on the same line as function declaration. > > > +{ > > + switch (cmd) { > > + case CPU_PM_EXIT: > > + /* Restore the imsic reg */ > > + imsic_restore(); > > + break; > > + } > > + > > + return NOTIFY_OK; > > +} > > + > > +static struct notifier_block imsic_notifier_block = { > > s/imsic_notifier_block/imsic_pm_notifier_block/ > > > + .notifier_call = imsic_notifier, > > +}; > > + > > static int __init imsic_early_probe(struct fwnode_handle *fwnode) > > { > > struct irq_domain *domain; > > @@ -180,7 +205,7 @@ static int __init imsic_early_probe(struct fwnode_handle *fwnode) > > cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_IMSIC_STARTING, "irqchip/riscv/imsic:starting", > > imsic_starting_cpu, imsic_dying_cpu); > > > > - return 0; > > + return cpu_pm_register_notifier(&imsic_notifier_block); > > } > > > > static int __init imsic_early_dt_init(struct device_node *node, struct device_node *parent) > > -- > > 2.17.1 > > > > Otherwise, this looks good to me. > > Reviewed-by: Anup Patel <anup@brainfault.org> > > Regards, > Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers 2025-07-09 2:55 [PATCH 0/2] riscv-imsic/aplic: Save and restore the registers Nick Hu 2025-07-09 2:55 ` [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers Nick Hu @ 2025-07-09 2:55 ` Nick Hu 2025-07-10 17:47 ` kernel test robot 2025-07-17 5:15 ` Anup Patel 1 sibling, 2 replies; 8+ messages in thread From: Nick Hu @ 2025-07-09 2:55 UTC (permalink / raw) To: Alexandre Ghiti, linux-riscv, linux-kernel Cc: Nick Hu, Anup Patel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou The APLIC may be powered down when the CPUs enter a deep sleep state. Therefore adding the APLIC save and restore functions to save and restore the states of APLIC. Signed-off-by: Nick Hu <nick.hu@sifive.com> Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> Reviewed-by: Cyan Yang <cyan.yang@sifive.com> --- drivers/irqchip/irq-riscv-aplic-direct.c | 14 ++- drivers/irqchip/irq-riscv-aplic-main.c | 143 +++++++++++++++++++++++ drivers/irqchip/irq-riscv-aplic-main.h | 12 ++ drivers/irqchip/irq-riscv-aplic-msi.c | 3 +- 4 files changed, 170 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c index 205ad61d15e4..df42f979d02c 100644 --- a/drivers/irqchip/irq-riscv-aplic-direct.c +++ b/drivers/irqchip/irq-riscv-aplic-direct.c @@ -8,6 +8,7 @@ #include <linux/bitfield.h> #include <linux/bitops.h> #include <linux/cpu.h> +#include <linux/cpumask.h> #include <linux/interrupt.h> #include <linux/irqchip.h> #include <linux/irqchip/chained_irq.h> @@ -171,6 +172,16 @@ static void aplic_idc_set_delivery(struct aplic_idc *idc, bool en) writel(de, idc->regs + APLIC_IDC_IDELIVERY); } +void aplic_direct_restore(struct aplic_priv *priv) +{ + struct aplic_direct *direct = + container_of(priv, struct aplic_direct, priv); + int cpu; + + for_each_cpu(cpu, &direct->lmask) + aplic_idc_set_delivery(per_cpu_ptr(&aplic_idcs, cpu), true); +} + static int aplic_direct_dying_cpu(unsigned int cpu) { if (aplic_direct_parent_irq) @@ -343,5 +354,6 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs) dev_info(dev, "%d interrupts directly connected to %d CPUs\n", priv->nr_irqs, priv->nr_idcs); - return 0; + /* Add the aplic_priv to the list */ + return aplic_add(dev, priv); } diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c index 93e7c51f944a..9054ce7a7256 100644 --- a/drivers/irqchip/irq-riscv-aplic-main.c +++ b/drivers/irqchip/irq-riscv-aplic-main.c @@ -12,10 +12,130 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/pm_runtime.h> #include <linux/printk.h> +#include <linux/syscore_ops.h> #include "irq-riscv-aplic-main.h" +static LIST_HEAD(aplics); + +static void aplic_restore(struct aplic_priv *priv) +{ + int i; + u32 clrip; + + writel(priv->saved_domaincfg, priv->regs + APLIC_DOMAINCFG); +#ifdef CONFIG_RISCV_M_MODE + writel(priv->saved_msiaddr, priv->regs + APLIC_xMSICFGADDR); + writel(priv->saved_msiaddrh, priv->regs + APLIC_xMSICFGADDRH); +#endif + for (i = 1; i <= priv->nr_irqs; i++) { + writel(priv->saved_sourcecfg[i - 1], + priv->regs + APLIC_SOURCECFG_BASE + + (i - 1) * sizeof(u32)); + writel(priv->saved_target[i - 1], + priv->regs + APLIC_TARGET_BASE + + (i - 1) * sizeof(u32)); + } + + for (i = 0; i <= priv->nr_irqs; i += 32) { + writel(-1U, priv->regs + APLIC_CLRIE_BASE + + (i / 32) * sizeof(u32)); + writel(priv->saved_ie[i / 32], + priv->regs + APLIC_SETIE_BASE + + (i / 32) * sizeof(u32)); + } + + if (priv->nr_idcs) { + aplic_direct_restore(priv); + } else { + /* Re-trigger the interrupts */ + for (i = 0; i <= priv->nr_irqs; i += 32) { + clrip = readl(priv->regs + APLIC_CLRIP_BASE + + (i / 32) * sizeof(u32)); + writel(clrip, priv->regs + APLIC_SETIP_BASE + + (i / 32) * sizeof(u32)); + } + } +} + +static void aplic_save(struct aplic_priv *priv) +{ + int i; + + for (i = 1; i <= priv->nr_irqs; i++) { + priv->saved_target[i - 1] = readl(priv->regs + + APLIC_TARGET_BASE + + (i - 1) * sizeof(u32)); + } + + for (i = 0; i <= priv->nr_irqs; i += 32) { + priv->saved_ie[i / 32] = readl(priv->regs + + APLIC_SETIE_BASE + + (i / 32) * sizeof(u32)); + } +} + +static int aplic_syscore_suspend(void) +{ + struct aplic_priv *priv; + + list_for_each_entry(priv, &aplics, list) { + aplic_save(priv); + } + + return 0; +} + +static void aplic_syscore_resume(void) +{ + struct aplic_priv *priv; + + list_for_each_entry(priv, &aplics, list) { + aplic_restore(priv); + } +} + +static struct syscore_ops aplic_syscore_ops = { + .suspend = aplic_syscore_suspend, + .resume = aplic_syscore_resume, +}; + +static int aplic_notifier(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct aplic_priv *priv = container_of(nb, struct aplic_priv, genpd_nb); + + switch (action) { + case GENPD_NOTIFY_PRE_OFF: + aplic_save(priv); + break; + case GENPD_NOTIFY_ON: + aplic_restore(priv); + break; + default: + break; + } + + return 0; +} + +int aplic_add(struct device *dev, struct aplic_priv *priv) +{ + int ret; + + list_add(&priv->list, &aplics); + /* Add genpd notifier */ + priv->genpd_nb.notifier_call = aplic_notifier; + ret = dev_pm_genpd_add_notifier(dev, &priv->genpd_nb); + if (!ret) + return devm_pm_runtime_enable(dev); + + return ret == -ENODEV || ret == -EOPNOTSUPP ? 0 : ret; +} + void aplic_irq_unmask(struct irq_data *d) { struct aplic_priv *priv = irq_data_get_irq_chip_data(d); @@ -59,6 +179,7 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type) sourcecfg = priv->regs + APLIC_SOURCECFG_BASE; sourcecfg += (d->hwirq - 1) * sizeof(u32); writel(val, sourcecfg); + priv->saved_sourcecfg[d->hwirq - 1] = val; return 0; } @@ -95,6 +216,8 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) valh |= FIELD_PREP(APLIC_xMSICFGADDRH_HHXS, priv->msicfg.hhxs); writel(val, priv->regs + APLIC_xMSICFGADDR); writel(valh, priv->regs + APLIC_xMSICFGADDRH); + priv->saved_msiaddr = val; + priv->saved_msiaddrh = valH; } #endif @@ -106,6 +229,7 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) writel(val, priv->regs + APLIC_DOMAINCFG); if (readl(priv->regs + APLIC_DOMAINCFG) != val) dev_warn(priv->dev, "unable to write 0x%x in domaincfg\n", val); + priv->saved_domaincfg = val; } static void aplic_init_hw_irqs(struct aplic_priv *priv) @@ -176,6 +300,23 @@ int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem * /* Setup initial state APLIC interrupts */ aplic_init_hw_irqs(priv); + /* For power management */ + priv->saved_target = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), + GFP_KERNEL); + if (!priv->saved_target) + return -ENOMEM; + + priv->saved_sourcecfg = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), + GFP_KERNEL); + if (!priv->saved_sourcecfg) + return -ENOMEM; + + priv->saved_ie = devm_kzalloc(dev, + DIV_ROUND_UP(priv->nr_irqs, 32) * sizeof(u32), + GFP_KERNEL); + if (!priv->saved_ie) + return -ENOMEM; + return 0; } @@ -209,6 +350,8 @@ static int aplic_probe(struct platform_device *pdev) if (rc) dev_err_probe(dev, rc, "failed to setup APLIC in %s mode\n", msi_mode ? "MSI" : "direct"); + else + register_syscore_ops(&aplic_syscore_ops); #ifdef CONFIG_ACPI if (!acpi_disabled) diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h index b0ad8cde69b1..969319242dbc 100644 --- a/drivers/irqchip/irq-riscv-aplic-main.h +++ b/drivers/irqchip/irq-riscv-aplic-main.h @@ -31,6 +31,16 @@ struct aplic_priv { u32 acpi_aplic_id; void __iomem *regs; struct aplic_msicfg msicfg; + struct notifier_block genpd_nb; + struct list_head list; + u32 *saved_target; + u32 *saved_sourcecfg; + u32 *saved_ie; + u32 saved_domaincfg; +#ifdef CONFIG_RISCV_M_MODE + u32 saved_msiaddr; + u32 saved_msiaddrh; +#endif }; void aplic_irq_unmask(struct irq_data *d); @@ -39,7 +49,9 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type); int aplic_irqdomain_translate(struct irq_fwspec *fwspec, u32 gsi_base, unsigned long *hwirq, unsigned int *type); void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode); +void aplic_direct_restore(struct aplic_priv *priv); int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem *regs); +int aplic_add(struct device *dev, struct aplic_priv *priv); int aplic_direct_setup(struct device *dev, void __iomem *regs); #ifdef CONFIG_RISCV_APLIC_MSI int aplic_msi_setup(struct device *dev, void __iomem *regs); diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c index fb8d1838609f..c9b4f7d5e6ea 100644 --- a/drivers/irqchip/irq-riscv-aplic-msi.c +++ b/drivers/irqchip/irq-riscv-aplic-msi.c @@ -281,5 +281,6 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs) pa = priv->msicfg.base_ppn << APLIC_xMSICFGADDR_PPN_SHIFT; dev_info(dev, "%d interrupts forwarded to MSI base %pa\n", priv->nr_irqs, &pa); - return 0; + /* Add the aplic_priv to the list */ + return aplic_add(dev, priv); } -- 2.17.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers 2025-07-09 2:55 ` [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers Nick Hu @ 2025-07-10 17:47 ` kernel test robot 2025-07-17 5:15 ` Anup Patel 1 sibling, 0 replies; 8+ messages in thread From: kernel test robot @ 2025-07-10 17:47 UTC (permalink / raw) To: Nick Hu, Alexandre Ghiti, linux-riscv, linux-kernel Cc: oe-kbuild-all, Nick Hu, Anup Patel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou Hi Nick, kernel test robot noticed the following build errors: [auto build test ERROR on tip/irq/core] [also build test ERROR on linus/master v6.16-rc5 next-20250710] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Nick-Hu/irqchip-riscv-aplic-Save-and-restore-APLIC-registers/20250709-115608 base: tip/irq/core patch link: https://lore.kernel.org/r/20250709025516.28051-3-nick.hu%40sifive.com patch subject: [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers config: riscv-randconfig-r132-20250710 (https://download.01.org/0day-ci/archive/20250711/202507110131.OzZXquiC-lkp@intel.com/config) compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 01c97b4953e87ae455bd4c41e3de3f0f0f29c61c) reproduce: (https://download.01.org/0day-ci/archive/20250711/202507110131.OzZXquiC-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507110131.OzZXquiC-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:804:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 804 | insb(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:104:53: note: expanded from macro 'insb' 104 | #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:812:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 812 | insw(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' 105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:820:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 820 | insl(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl' 106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:829:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 829 | outsb(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb' 118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:838:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 838 | outsw(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw' 119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:847:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 847 | outsl(addr, buffer, count); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl' 120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count) | ~~~~~~~~~~ ^ In file included from drivers/irqchip/irq-riscv-aplic-main.c:13: In file included from include/linux/of_irq.h:7: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:12: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 1175 | return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port; | ~~~~~~~~~~ ^ >> drivers/irqchip/irq-riscv-aplic-main.c:220:26: error: use of undeclared identifier 'valH' 220 | priv->saved_msiaddrh = valH; | ^~~~ 7 warnings and 1 error generated. vim +/valH +220 drivers/irqchip/irq-riscv-aplic-main.c 203 204 void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) 205 { 206 u32 val; 207 #ifdef CONFIG_RISCV_M_MODE 208 u32 valh; 209 210 if (msi_mode) { 211 val = lower_32_bits(priv->msicfg.base_ppn); 212 valh = FIELD_PREP(APLIC_xMSICFGADDRH_BAPPN, upper_32_bits(priv->msicfg.base_ppn)); 213 valh |= FIELD_PREP(APLIC_xMSICFGADDRH_LHXW, priv->msicfg.lhxw); 214 valh |= FIELD_PREP(APLIC_xMSICFGADDRH_HHXW, priv->msicfg.hhxw); 215 valh |= FIELD_PREP(APLIC_xMSICFGADDRH_LHXS, priv->msicfg.lhxs); 216 valh |= FIELD_PREP(APLIC_xMSICFGADDRH_HHXS, priv->msicfg.hhxs); 217 writel(val, priv->regs + APLIC_xMSICFGADDR); 218 writel(valh, priv->regs + APLIC_xMSICFGADDRH); 219 priv->saved_msiaddr = val; > 220 priv->saved_msiaddrh = valH; 221 } 222 #endif 223 224 /* Setup APLIC domaincfg register */ 225 val = readl(priv->regs + APLIC_DOMAINCFG); 226 val |= APLIC_DOMAINCFG_IE; 227 if (msi_mode) 228 val |= APLIC_DOMAINCFG_DM; 229 writel(val, priv->regs + APLIC_DOMAINCFG); 230 if (readl(priv->regs + APLIC_DOMAINCFG) != val) 231 dev_warn(priv->dev, "unable to write 0x%x in domaincfg\n", val); 232 priv->saved_domaincfg = val; 233 } 234 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers 2025-07-09 2:55 ` [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers Nick Hu 2025-07-10 17:47 ` kernel test robot @ 2025-07-17 5:15 ` Anup Patel 2025-07-17 7:26 ` Nick Hu 1 sibling, 1 reply; 8+ messages in thread From: Anup Patel @ 2025-07-17 5:15 UTC (permalink / raw) To: Nick Hu Cc: Alexandre Ghiti, linux-riscv, linux-kernel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou On Wed, Jul 9, 2025 at 8:26 AM Nick Hu <nick.hu@sifive.com> wrote: > > The APLIC may be powered down when the CPUs enter a deep sleep state. > Therefore adding the APLIC save and restore functions to save and > restore the states of APLIC. > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > Reviewed-by: Cyan Yang <cyan.yang@sifive.com> > --- > drivers/irqchip/irq-riscv-aplic-direct.c | 14 ++- > drivers/irqchip/irq-riscv-aplic-main.c | 143 +++++++++++++++++++++++ > drivers/irqchip/irq-riscv-aplic-main.h | 12 ++ > drivers/irqchip/irq-riscv-aplic-msi.c | 3 +- > 4 files changed, 170 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c > index 205ad61d15e4..df42f979d02c 100644 > --- a/drivers/irqchip/irq-riscv-aplic-direct.c > +++ b/drivers/irqchip/irq-riscv-aplic-direct.c > @@ -8,6 +8,7 @@ > #include <linux/bitfield.h> > #include <linux/bitops.h> > #include <linux/cpu.h> > +#include <linux/cpumask.h> > #include <linux/interrupt.h> > #include <linux/irqchip.h> > #include <linux/irqchip/chained_irq.h> > @@ -171,6 +172,16 @@ static void aplic_idc_set_delivery(struct aplic_idc *idc, bool en) > writel(de, idc->regs + APLIC_IDC_IDELIVERY); > } > > +void aplic_direct_restore(struct aplic_priv *priv) > +{ > + struct aplic_direct *direct = > + container_of(priv, struct aplic_direct, priv); > + int cpu; > + > + for_each_cpu(cpu, &direct->lmask) > + aplic_idc_set_delivery(per_cpu_ptr(&aplic_idcs, cpu), true); > +} > + > static int aplic_direct_dying_cpu(unsigned int cpu) > { > if (aplic_direct_parent_irq) > @@ -343,5 +354,6 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs) > dev_info(dev, "%d interrupts directly connected to %d CPUs\n", > priv->nr_irqs, priv->nr_idcs); > > - return 0; > + /* Add the aplic_priv to the list */ > + return aplic_add(dev, priv); > } > diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c > index 93e7c51f944a..9054ce7a7256 100644 > --- a/drivers/irqchip/irq-riscv-aplic-main.c > +++ b/drivers/irqchip/irq-riscv-aplic-main.c > @@ -12,10 +12,130 @@ > #include <linux/of.h> > #include <linux/of_irq.h> > #include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > #include <linux/printk.h> > +#include <linux/syscore_ops.h> > > #include "irq-riscv-aplic-main.h" > > +static LIST_HEAD(aplics); > + > +static void aplic_restore(struct aplic_priv *priv) > +{ > + int i; > + u32 clrip; > + > + writel(priv->saved_domaincfg, priv->regs + APLIC_DOMAINCFG); > +#ifdef CONFIG_RISCV_M_MODE > + writel(priv->saved_msiaddr, priv->regs + APLIC_xMSICFGADDR); > + writel(priv->saved_msiaddrh, priv->regs + APLIC_xMSICFGADDRH); > +#endif > + for (i = 1; i <= priv->nr_irqs; i++) { > + writel(priv->saved_sourcecfg[i - 1], > + priv->regs + APLIC_SOURCECFG_BASE + > + (i - 1) * sizeof(u32)); > + writel(priv->saved_target[i - 1], > + priv->regs + APLIC_TARGET_BASE + > + (i - 1) * sizeof(u32)); > + } > + > + for (i = 0; i <= priv->nr_irqs; i += 32) { > + writel(-1U, priv->regs + APLIC_CLRIE_BASE + > + (i / 32) * sizeof(u32)); > + writel(priv->saved_ie[i / 32], > + priv->regs + APLIC_SETIE_BASE + > + (i / 32) * sizeof(u32)); > + } > + > + if (priv->nr_idcs) { > + aplic_direct_restore(priv); > + } else { > + /* Re-trigger the interrupts */ > + for (i = 0; i <= priv->nr_irqs; i += 32) { > + clrip = readl(priv->regs + APLIC_CLRIP_BASE + > + (i / 32) * sizeof(u32)); > + writel(clrip, priv->regs + APLIC_SETIP_BASE + > + (i / 32) * sizeof(u32)); > + } > + } > +} > + > +static void aplic_save(struct aplic_priv *priv) > +{ > + int i; > + > + for (i = 1; i <= priv->nr_irqs; i++) { > + priv->saved_target[i - 1] = readl(priv->regs + > + APLIC_TARGET_BASE + > + (i - 1) * sizeof(u32)); > + } > + > + for (i = 0; i <= priv->nr_irqs; i += 32) { > + priv->saved_ie[i / 32] = readl(priv->regs + > + APLIC_SETIE_BASE + > + (i / 32) * sizeof(u32)); > + } > +} > + > +static int aplic_syscore_suspend(void) > +{ > + struct aplic_priv *priv; > + > + list_for_each_entry(priv, &aplics, list) { > + aplic_save(priv); > + } > + > + return 0; > +} > + > +static void aplic_syscore_resume(void) > +{ > + struct aplic_priv *priv; > + > + list_for_each_entry(priv, &aplics, list) { > + aplic_restore(priv); > + } > +} > + > +static struct syscore_ops aplic_syscore_ops = { > + .suspend = aplic_syscore_suspend, > + .resume = aplic_syscore_resume, > +}; > + > +static int aplic_notifier(struct notifier_block *nb, unsigned long action, > + void *data) s/aplic_notifier/aplic_pm_notifier/ The "void *data" parameter can be on the same line as function declaration. > +{ > + struct aplic_priv *priv = container_of(nb, struct aplic_priv, genpd_nb); > + > + switch (action) { > + case GENPD_NOTIFY_PRE_OFF: > + aplic_save(priv); > + break; > + case GENPD_NOTIFY_ON: > + aplic_restore(priv); > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +int aplic_add(struct device *dev, struct aplic_priv *priv) > +{ > + int ret; > + > + list_add(&priv->list, &aplics); > + /* Add genpd notifier */ > + priv->genpd_nb.notifier_call = aplic_notifier; > + ret = dev_pm_genpd_add_notifier(dev, &priv->genpd_nb); > + if (!ret) > + return devm_pm_runtime_enable(dev); > + > + return ret == -ENODEV || ret == -EOPNOTSUPP ? 0 : ret; > +} > + Make aplic_add() as static and directly call it from aplic_setup_priv(). To cleanup upon device removal, use devm_add_action_or_reset(). > void aplic_irq_unmask(struct irq_data *d) > { > struct aplic_priv *priv = irq_data_get_irq_chip_data(d); > @@ -59,6 +179,7 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type) > sourcecfg = priv->regs + APLIC_SOURCECFG_BASE; > sourcecfg += (d->hwirq - 1) * sizeof(u32); > writel(val, sourcecfg); > + priv->saved_sourcecfg[d->hwirq - 1] = val; > > return 0; > } > @@ -95,6 +216,8 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) > valh |= FIELD_PREP(APLIC_xMSICFGADDRH_HHXS, priv->msicfg.hhxs); > writel(val, priv->regs + APLIC_xMSICFGADDR); > writel(valh, priv->regs + APLIC_xMSICFGADDRH); > + priv->saved_msiaddr = val; > + priv->saved_msiaddrh = valH; > } > #endif > > @@ -106,6 +229,7 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) > writel(val, priv->regs + APLIC_DOMAINCFG); > if (readl(priv->regs + APLIC_DOMAINCFG) != val) > dev_warn(priv->dev, "unable to write 0x%x in domaincfg\n", val); > + priv->saved_domaincfg = val; > } > > static void aplic_init_hw_irqs(struct aplic_priv *priv) > @@ -176,6 +300,23 @@ int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem * > /* Setup initial state APLIC interrupts */ > aplic_init_hw_irqs(priv); > > + /* For power management */ > + priv->saved_target = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), > + GFP_KERNEL); > + if (!priv->saved_target) > + return -ENOMEM; > + > + priv->saved_sourcecfg = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), > + GFP_KERNEL); > + if (!priv->saved_sourcecfg) > + return -ENOMEM; > + > + priv->saved_ie = devm_kzalloc(dev, > + DIV_ROUND_UP(priv->nr_irqs, 32) * sizeof(u32), > + GFP_KERNEL); > + if (!priv->saved_ie) > + return -ENOMEM; > + > return 0; > } > > @@ -209,6 +350,8 @@ static int aplic_probe(struct platform_device *pdev) > if (rc) > dev_err_probe(dev, rc, "failed to setup APLIC in %s mode\n", > msi_mode ? "MSI" : "direct"); > + else > + register_syscore_ops(&aplic_syscore_ops); > > #ifdef CONFIG_ACPI > if (!acpi_disabled) > diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h > index b0ad8cde69b1..969319242dbc 100644 > --- a/drivers/irqchip/irq-riscv-aplic-main.h > +++ b/drivers/irqchip/irq-riscv-aplic-main.h > @@ -31,6 +31,16 @@ struct aplic_priv { > u32 acpi_aplic_id; > void __iomem *regs; > struct aplic_msicfg msicfg; > + struct notifier_block genpd_nb; > + struct list_head list; Rename "list" as "head" and make it first variable in "struct aplic_priv" > + u32 *saved_target; > + u32 *saved_sourcecfg; > + u32 *saved_ie; > + u32 saved_domaincfg; > +#ifdef CONFIG_RISCV_M_MODE > + u32 saved_msiaddr; > + u32 saved_msiaddrh; > +#endif > }; > > void aplic_irq_unmask(struct irq_data *d); > @@ -39,7 +49,9 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type); > int aplic_irqdomain_translate(struct irq_fwspec *fwspec, u32 gsi_base, > unsigned long *hwirq, unsigned int *type); > void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode); > +void aplic_direct_restore(struct aplic_priv *priv); > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem *regs); > +int aplic_add(struct device *dev, struct aplic_priv *priv); > int aplic_direct_setup(struct device *dev, void __iomem *regs); > #ifdef CONFIG_RISCV_APLIC_MSI > int aplic_msi_setup(struct device *dev, void __iomem *regs); > diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c > index fb8d1838609f..c9b4f7d5e6ea 100644 > --- a/drivers/irqchip/irq-riscv-aplic-msi.c > +++ b/drivers/irqchip/irq-riscv-aplic-msi.c > @@ -281,5 +281,6 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs) > pa = priv->msicfg.base_ppn << APLIC_xMSICFGADDR_PPN_SHIFT; > dev_info(dev, "%d interrupts forwarded to MSI base %pa\n", priv->nr_irqs, &pa); > > - return 0; > + /* Add the aplic_priv to the list */ > + return aplic_add(dev, priv); > } > -- > 2.17.1 > Also, please address the issue reported by the kernel test robot. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers 2025-07-17 5:15 ` Anup Patel @ 2025-07-17 7:26 ` Nick Hu 0 siblings, 0 replies; 8+ messages in thread From: Nick Hu @ 2025-07-17 7:26 UTC (permalink / raw) To: Anup Patel Cc: Alexandre Ghiti, linux-riscv, linux-kernel, Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou On Thu, Jul 17, 2025 at 1:15 PM Anup Patel <anup@brainfault.org> wrote: > > On Wed, Jul 9, 2025 at 8:26 AM Nick Hu <nick.hu@sifive.com> wrote: > > > > The APLIC may be powered down when the CPUs enter a deep sleep state. > > Therefore adding the APLIC save and restore functions to save and > > restore the states of APLIC. > > > > Signed-off-by: Nick Hu <nick.hu@sifive.com> > > Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > > Reviewed-by: Cyan Yang <cyan.yang@sifive.com> > > --- > > drivers/irqchip/irq-riscv-aplic-direct.c | 14 ++- > > drivers/irqchip/irq-riscv-aplic-main.c | 143 +++++++++++++++++++++++ > > drivers/irqchip/irq-riscv-aplic-main.h | 12 ++ > > drivers/irqchip/irq-riscv-aplic-msi.c | 3 +- > > 4 files changed, 170 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c > > index 205ad61d15e4..df42f979d02c 100644 > > --- a/drivers/irqchip/irq-riscv-aplic-direct.c > > +++ b/drivers/irqchip/irq-riscv-aplic-direct.c > > @@ -8,6 +8,7 @@ > > #include <linux/bitfield.h> > > #include <linux/bitops.h> > > #include <linux/cpu.h> > > +#include <linux/cpumask.h> > > #include <linux/interrupt.h> > > #include <linux/irqchip.h> > > #include <linux/irqchip/chained_irq.h> > > @@ -171,6 +172,16 @@ static void aplic_idc_set_delivery(struct aplic_idc *idc, bool en) > > writel(de, idc->regs + APLIC_IDC_IDELIVERY); > > } > > > > +void aplic_direct_restore(struct aplic_priv *priv) > > +{ > > + struct aplic_direct *direct = > > + container_of(priv, struct aplic_direct, priv); > > + int cpu; > > + > > + for_each_cpu(cpu, &direct->lmask) > > + aplic_idc_set_delivery(per_cpu_ptr(&aplic_idcs, cpu), true); > > +} > > + > > static int aplic_direct_dying_cpu(unsigned int cpu) > > { > > if (aplic_direct_parent_irq) > > @@ -343,5 +354,6 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs) > > dev_info(dev, "%d interrupts directly connected to %d CPUs\n", > > priv->nr_irqs, priv->nr_idcs); > > > > - return 0; > > + /* Add the aplic_priv to the list */ > > + return aplic_add(dev, priv); > > } > > diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c > > index 93e7c51f944a..9054ce7a7256 100644 > > --- a/drivers/irqchip/irq-riscv-aplic-main.c > > +++ b/drivers/irqchip/irq-riscv-aplic-main.c > > @@ -12,10 +12,130 @@ > > #include <linux/of.h> > > #include <linux/of_irq.h> > > #include <linux/platform_device.h> > > +#include <linux/pm_domain.h> > > +#include <linux/pm_runtime.h> > > #include <linux/printk.h> > > +#include <linux/syscore_ops.h> > > > > #include "irq-riscv-aplic-main.h" > > > > +static LIST_HEAD(aplics); > > + > > +static void aplic_restore(struct aplic_priv *priv) > > +{ > > + int i; > > + u32 clrip; > > + > > + writel(priv->saved_domaincfg, priv->regs + APLIC_DOMAINCFG); > > +#ifdef CONFIG_RISCV_M_MODE > > + writel(priv->saved_msiaddr, priv->regs + APLIC_xMSICFGADDR); > > + writel(priv->saved_msiaddrh, priv->regs + APLIC_xMSICFGADDRH); > > +#endif > > + for (i = 1; i <= priv->nr_irqs; i++) { > > + writel(priv->saved_sourcecfg[i - 1], > > + priv->regs + APLIC_SOURCECFG_BASE + > > + (i - 1) * sizeof(u32)); > > + writel(priv->saved_target[i - 1], > > + priv->regs + APLIC_TARGET_BASE + > > + (i - 1) * sizeof(u32)); > > + } > > + > > + for (i = 0; i <= priv->nr_irqs; i += 32) { > > + writel(-1U, priv->regs + APLIC_CLRIE_BASE + > > + (i / 32) * sizeof(u32)); > > + writel(priv->saved_ie[i / 32], > > + priv->regs + APLIC_SETIE_BASE + > > + (i / 32) * sizeof(u32)); > > + } > > + > > + if (priv->nr_idcs) { > > + aplic_direct_restore(priv); > > + } else { > > + /* Re-trigger the interrupts */ > > + for (i = 0; i <= priv->nr_irqs; i += 32) { > > + clrip = readl(priv->regs + APLIC_CLRIP_BASE + > > + (i / 32) * sizeof(u32)); > > + writel(clrip, priv->regs + APLIC_SETIP_BASE + > > + (i / 32) * sizeof(u32)); > > + } > > + } > > +} > > + > > +static void aplic_save(struct aplic_priv *priv) > > +{ > > + int i; > > + > > + for (i = 1; i <= priv->nr_irqs; i++) { > > + priv->saved_target[i - 1] = readl(priv->regs + > > + APLIC_TARGET_BASE + > > + (i - 1) * sizeof(u32)); > > + } > > + > > + for (i = 0; i <= priv->nr_irqs; i += 32) { > > + priv->saved_ie[i / 32] = readl(priv->regs + > > + APLIC_SETIE_BASE + > > + (i / 32) * sizeof(u32)); > > + } > > +} > > + > > +static int aplic_syscore_suspend(void) > > +{ > > + struct aplic_priv *priv; > > + > > + list_for_each_entry(priv, &aplics, list) { > > + aplic_save(priv); > > + } > > + > > + return 0; > > +} > > + > > +static void aplic_syscore_resume(void) > > +{ > > + struct aplic_priv *priv; > > + > > + list_for_each_entry(priv, &aplics, list) { > > + aplic_restore(priv); > > + } > > +} > > + > > +static struct syscore_ops aplic_syscore_ops = { > > + .suspend = aplic_syscore_suspend, > > + .resume = aplic_syscore_resume, > > +}; > > + > > +static int aplic_notifier(struct notifier_block *nb, unsigned long action, > > + void *data) > > s/aplic_notifier/aplic_pm_notifier/ > > The "void *data" parameter can be on the same line as function declaration. > > > +{ > > + struct aplic_priv *priv = container_of(nb, struct aplic_priv, genpd_nb); > > + > > + switch (action) { > > + case GENPD_NOTIFY_PRE_OFF: > > + aplic_save(priv); > > + break; > > + case GENPD_NOTIFY_ON: > > + aplic_restore(priv); > > + break; > > + default: > > + break; > > + } > > + > > + return 0; > > +} > > + > > +int aplic_add(struct device *dev, struct aplic_priv *priv) > > +{ > > + int ret; > > + > > + list_add(&priv->list, &aplics); > > + /* Add genpd notifier */ > > + priv->genpd_nb.notifier_call = aplic_notifier; > > + ret = dev_pm_genpd_add_notifier(dev, &priv->genpd_nb); > > + if (!ret) > > + return devm_pm_runtime_enable(dev); > > + > > + return ret == -ENODEV || ret == -EOPNOTSUPP ? 0 : ret; > > +} > > + > > Make aplic_add() as static and directly call it from aplic_setup_priv(). > > To cleanup upon device removal, use devm_add_action_or_reset(). > > > void aplic_irq_unmask(struct irq_data *d) > > { > > struct aplic_priv *priv = irq_data_get_irq_chip_data(d); > > @@ -59,6 +179,7 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type) > > sourcecfg = priv->regs + APLIC_SOURCECFG_BASE; > > sourcecfg += (d->hwirq - 1) * sizeof(u32); > > writel(val, sourcecfg); > > + priv->saved_sourcecfg[d->hwirq - 1] = val; > > > > return 0; > > } > > @@ -95,6 +216,8 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) > > valh |= FIELD_PREP(APLIC_xMSICFGADDRH_HHXS, priv->msicfg.hhxs); > > writel(val, priv->regs + APLIC_xMSICFGADDR); > > writel(valh, priv->regs + APLIC_xMSICFGADDRH); > > + priv->saved_msiaddr = val; > > + priv->saved_msiaddrh = valH; > > } > > #endif > > > > @@ -106,6 +229,7 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode) > > writel(val, priv->regs + APLIC_DOMAINCFG); > > if (readl(priv->regs + APLIC_DOMAINCFG) != val) > > dev_warn(priv->dev, "unable to write 0x%x in domaincfg\n", val); > > + priv->saved_domaincfg = val; > > } > > > > static void aplic_init_hw_irqs(struct aplic_priv *priv) > > @@ -176,6 +300,23 @@ int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem * > > /* Setup initial state APLIC interrupts */ > > aplic_init_hw_irqs(priv); > > > > + /* For power management */ > > + priv->saved_target = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), > > + GFP_KERNEL); > > + if (!priv->saved_target) > > + return -ENOMEM; > > + > > + priv->saved_sourcecfg = devm_kzalloc(dev, priv->nr_irqs * sizeof(u32), > > + GFP_KERNEL); > > + if (!priv->saved_sourcecfg) > > + return -ENOMEM; > > + > > + priv->saved_ie = devm_kzalloc(dev, > > + DIV_ROUND_UP(priv->nr_irqs, 32) * sizeof(u32), > > + GFP_KERNEL); > > + if (!priv->saved_ie) > > + return -ENOMEM; > > + > > return 0; > > } > > > > @@ -209,6 +350,8 @@ static int aplic_probe(struct platform_device *pdev) > > if (rc) > > dev_err_probe(dev, rc, "failed to setup APLIC in %s mode\n", > > msi_mode ? "MSI" : "direct"); > > + else > > + register_syscore_ops(&aplic_syscore_ops); > > > > #ifdef CONFIG_ACPI > > if (!acpi_disabled) > > diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h > > index b0ad8cde69b1..969319242dbc 100644 > > --- a/drivers/irqchip/irq-riscv-aplic-main.h > > +++ b/drivers/irqchip/irq-riscv-aplic-main.h > > @@ -31,6 +31,16 @@ struct aplic_priv { > > u32 acpi_aplic_id; > > void __iomem *regs; > > struct aplic_msicfg msicfg; > > + struct notifier_block genpd_nb; > > + struct list_head list; > > Rename "list" as "head" and make it first variable > in "struct aplic_priv" > > > + u32 *saved_target; > > + u32 *saved_sourcecfg; > > + u32 *saved_ie; > > + u32 saved_domaincfg; > > +#ifdef CONFIG_RISCV_M_MODE > > + u32 saved_msiaddr; > > + u32 saved_msiaddrh; > > +#endif > > }; > > > > void aplic_irq_unmask(struct irq_data *d); > > @@ -39,7 +49,9 @@ int aplic_irq_set_type(struct irq_data *d, unsigned int type); > > int aplic_irqdomain_translate(struct irq_fwspec *fwspec, u32 gsi_base, > > unsigned long *hwirq, unsigned int *type); > > void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode); > > +void aplic_direct_restore(struct aplic_priv *priv); > > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem *regs); > > +int aplic_add(struct device *dev, struct aplic_priv *priv); > > int aplic_direct_setup(struct device *dev, void __iomem *regs); > > #ifdef CONFIG_RISCV_APLIC_MSI > > int aplic_msi_setup(struct device *dev, void __iomem *regs); > > diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c > > index fb8d1838609f..c9b4f7d5e6ea 100644 > > --- a/drivers/irqchip/irq-riscv-aplic-msi.c > > +++ b/drivers/irqchip/irq-riscv-aplic-msi.c > > @@ -281,5 +281,6 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs) > > pa = priv->msicfg.base_ppn << APLIC_xMSICFGADDR_PPN_SHIFT; > > dev_info(dev, "%d interrupts forwarded to MSI base %pa\n", priv->nr_irqs, &pa); > > > > - return 0; > > + /* Add the aplic_priv to the list */ > > + return aplic_add(dev, priv); > > } > > -- > > 2.17.1 > > > > Also, please address the issue reported by the kernel test robot. > Will apply the above feedback in the next version. Thanks! Regards, Nick > Regards, > Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-17 8:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-09 2:55 [PATCH 0/2] riscv-imsic/aplic: Save and restore the registers Nick Hu 2025-07-09 2:55 ` [PATCH 1/2] irqchip/riscv-imsic: Restore the IMSIC registers Nick Hu 2025-07-17 5:15 ` Anup Patel 2025-07-17 7:24 ` Nick Hu 2025-07-09 2:55 ` [PATCH 2/2] irqchip/riscv-aplic: Save and restore APLIC registers Nick Hu 2025-07-10 17:47 ` kernel test robot 2025-07-17 5:15 ` Anup Patel 2025-07-17 7:26 ` Nick Hu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox