* [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
* [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 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 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 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
* 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