Re: [PATCH 1/1] nds32: Power management for nds32
Hi!
> > Can we get rid of "is_bit_1" array here, and use normal bit operations
> > on another variable here?
> >
> Do you mean like this:
>
> static int nointc_set_wake(struct irq_data *data, unsigned int on) {
> unsigned long int_mask = __nds32__mfsr(NDS32_SR_INT_MASK);
> static unsigned long irq_orig_bit = 0;
> u32 bit = 1 << data->hwirq;
>
> if (on) {
> if (int_mask & bit)
> __assign_bit(data->hwirq, &irq_orig_bit, true);
> else
> __assign_bit(data->hwirq, &irq_orig_bit, false);
>
> __assign_bit(data->hwirq, &int_mask, true);
> __assign_bit(data->hwirq, &wake_mask, true);
>
> } else {
> if (!(irq_orig_bit & bit))
> __assign_bit(data->hwirq, &int_mask, false);
>
> __assign_bit(data->hwirq, &wake_mask, false);
> __assign_bit(data->hwirq, &irq_orig_bit, false);
> }
>
> __nds32__mtsr_dsb(int_mask, NDS32_SR_INT_MASK);
>
> return 0;
Yes, that is better. You don't need = 0 on static variable
afaict. (And may want to put it out of a function so it stands out).
You can add my Acked-by on resulting patch.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
Re: [PATCH 1/1] nds32: Power management for nds32
Hi Pavel,
On Wed, Oct 24, 2018 at 02:51:17PM +0800, Pavel Machek wrote:
> On Wed 2018-10-24 11:40:07, Nickhu wrote:
> > There are three sleep states in nds32:
> > suspend to idle,
> > suspend to standby,
> > suspend to ram
> >
> > In suspend to ram, we use the 'standby' instruction to emulate
> > power management device to hang the system util wakeup source
> > send wakeup events to break the loop.
> >
> > First, we push the general purpose registers and system registers
> > to stack. Second, we translate stack pointer to physical address
> > and store to memory to save the stack pointer. Third, after write
> > back and invalid the cache we hang in 'standby' intruction.
> > When wakeup source trigger wake up events, the loop will be break
> > and resume the system.
> >
> > Signed-off-by: Nickhu
>
> Is "Nickhu" complete name?
>
Oh, Nick Hu is my complete name.
I will fix it.
Thanks!
> > diff --git a/arch/nds32/kernel/pm.c b/arch/nds32/kernel/pm.c
> > new file mode 100644
> > index ..e1eaf3bac709
> > --- /dev/null
> > +++ b/arch/nds32/kernel/pm.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2008-2017 Andes Technology Corporation
> > +
> > +/*
> > + * nds32 Power Management Routines
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License.
> > + *
> > + * Abstract:
> > + *
> > + *This program is for nds32 power management routines.
> > + *
> > + */
>
> I'd get rid of "abstract" here, repeating GPL twice and "nds32 power
> management routines" twice does not make much sense either.
>
Ok, I will get rid of it.
> > @@ -0,0 +1,129 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (C) 2017 Andes Technology Corporation */
> > +
> > +#include
>
> Missing space.
>
> > +static int nointc_set_wake(struct irq_data *data, unsigned int on)
> > +{
> > + unsigned long int_mask = __nds32__mfsr(NDS32_SR_INT_MASK);
> > + static bool is_bit_1[NR_IRQS] = {false};
> > + u32 bit = 1 << data->hwirq;
> > +
> > + if (on) {
> > + if (int_mask & bit)
> > + is_bit_1[data->hwirq] = true;
> > + else
> > + is_bit_1[data->hwirq] = false;
> > +
> > + int_mask |= bit;
> > + wake_mask |= bit;
> > + } else {
> > + if (!is_bit_1[data->hwirq])
> > + int_mask &= ~bit;
> > +
> > + wake_mask &= ~bit;
> > + is_bit_1[data->hwirq] = false;
> > + }
>
> Can we get rid of "is_bit_1" array here, and use normal bit operations
> on another variable here?
>
Do you mean like this:
static int nointc_set_wake(struct irq_data *data, unsigned int on) {
unsigned long int_mask = __nds32__mfsr(NDS32_SR_INT_MASK);
static unsigned long irq_orig_bit = 0;
u32 bit = 1 << data->hwirq;
if (on) {
if (int_mask & bit)
__assign_bit(data->hwirq, &irq_orig_bit, true);
else
__assign_bit(data->hwirq, &irq_orig_bit, false);
__assign_bit(data->hwirq, &int_mask, true);
__assign_bit(data->hwirq, &wake_mask, true);
} else {
if (!(irq_orig_bit & bit))
__assign_bit(data->hwirq, &int_mask, false);
__assign_bit(data->hwirq, &wake_mask, false);
__assign_bit(data->hwirq, &irq_orig_bit, false);
}
__nds32__mtsr_dsb(int_mask, NDS32_SR_INT_MASK);
return 0;
}
> Thanks,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Thank your for quick reply,
Nick Hu.
Re: [PATCH 1/1] nds32: Power management for nds32
On Wed 2018-10-24 11:40:07, Nickhu wrote:
> There are three sleep states in nds32:
> suspend to idle,
> suspend to standby,
> suspend to ram
>
> In suspend to ram, we use the 'standby' instruction to emulate
> power management device to hang the system util wakeup source
> send wakeup events to break the loop.
>
> First, we push the general purpose registers and system registers
> to stack. Second, we translate stack pointer to physical address
> and store to memory to save the stack pointer. Third, after write
> back and invalid the cache we hang in 'standby' intruction.
> When wakeup source trigger wake up events, the loop will be break
> and resume the system.
>
> Signed-off-by: Nickhu
Is "Nickhu" complete name?
> diff --git a/arch/nds32/kernel/pm.c b/arch/nds32/kernel/pm.c
> new file mode 100644
> index ..e1eaf3bac709
> --- /dev/null
> +++ b/arch/nds32/kernel/pm.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2008-2017 Andes Technology Corporation
> +
> +/*
> + * nds32 Power Management Routines
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License.
> + *
> + * Abstract:
> + *
> + *This program is for nds32 power management routines.
> + *
> + */
I'd get rid of "abstract" here, repeating GPL twice and "nds32 power
management routines" twice does not make much sense either.
> @@ -0,0 +1,129 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2017 Andes Technology Corporation */
> +
> +#include
Missing space.
> +static int nointc_set_wake(struct irq_data *data, unsigned int on)
> +{
> + unsigned long int_mask = __nds32__mfsr(NDS32_SR_INT_MASK);
> + static bool is_bit_1[NR_IRQS] = {false};
> + u32 bit = 1 << data->hwirq;
> +
> + if (on) {
> + if (int_mask & bit)
> + is_bit_1[data->hwirq] = true;
> + else
> + is_bit_1[data->hwirq] = false;
> +
> + int_mask |= bit;
> + wake_mask |= bit;
> + } else {
> + if (!is_bit_1[data->hwirq])
> + int_mask &= ~bit;
> +
> + wake_mask &= ~bit;
> + is_bit_1[data->hwirq] = false;
> + }
Can we get rid of "is_bit_1" array here, and use normal bit operations
on another variable here?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
[PATCH 1/1] nds32: Power management for nds32
There are three sleep states in nds32:
suspend to idle,
suspend to standby,
suspend to ram
In suspend to ram, we use the 'standby' instruction to emulate
power management device to hang the system util wakeup source
send wakeup events to break the loop.
First, we push the general purpose registers and system registers
to stack. Second, we translate stack pointer to physical address
and store to memory to save the stack pointer. Third, after write
back and invalid the cache we hang in 'standby' intruction.
When wakeup source trigger wake up events, the loop will be break
and resume the system.
Signed-off-by: Nickhu
---
arch/nds32/Kconfig | 10 +++
arch/nds32/include/asm/suspend.h | 11 +++
arch/nds32/kernel/Makefile | 2 +-
arch/nds32/kernel/pm.c | 91 ++
arch/nds32/kernel/sleep.S| 129 +++
drivers/irqchip/irq-ativic32.c | 29 +++
6 files changed, 271 insertions(+), 1 deletion(-)
create mode 100644 arch/nds32/include/asm/suspend.h
create mode 100644 arch/nds32/kernel/pm.c
create mode 100644 arch/nds32/kernel/sleep.S
diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index dd448d431f5a..8e2c5ac6acd1 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -95,3 +95,13 @@ endmenu
menu "Kernel Features"
source "kernel/Kconfig.hz"
endmenu
+
+menu "Power management options"
+config SYS_SUPPORTS_APM_EMULATION
+ bool
+
+config ARCH_SUSPEND_POSSIBLE
+ def_bool y
+
+source "kernel/power/Kconfig"
+endmenu
diff --git a/arch/nds32/include/asm/suspend.h b/arch/nds32/include/asm/suspend.h
new file mode 100644
index ..6ed2418af1ac
--- /dev/null
+++ b/arch/nds32/include/asm/suspend.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (C) 2008-2017 Andes Technology Corporation
+
+#ifndef __ASM_NDS32_SUSPEND_H
+#define __ASM_NDS32_SUSPEND_H
+
+extern void suspend2ram(void);
+extern void cpu_resume(void);
+extern unsigned long wake_mask;
+
+#endif
diff --git a/arch/nds32/kernel/Makefile b/arch/nds32/kernel/Makefile
index f52bd2744f50..8d62f2ecb1ab 100644
--- a/arch/nds32/kernel/Makefile
+++ b/arch/nds32/kernel/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_OF) += devtree.o
obj-$(CONFIG_CACHE_L2) += atl2c.o
obj-$(CONFIG_PERF_EVENTS) += perf_event_cpu.o
-
+obj-$(CONFIG_PM) += pm.o sleep.o
extra-y := head.o vmlinux.lds
obj-y += vdso/
diff --git a/arch/nds32/kernel/pm.c b/arch/nds32/kernel/pm.c
new file mode 100644
index ..e1eaf3bac709
--- /dev/null
+++ b/arch/nds32/kernel/pm.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2008-2017 Andes Technology Corporation
+
+/*
+ * nds32 Power Management Routines
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License.
+ *
+ * Abstract:
+ *
+ *This program is for nds32 power management routines.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+unsigned int resume_addr;
+unsigned int *phy_addr_sp_tmp;
+
+static void nds32_suspend2ram(void)
+{
+ pgd_t *pgdv;
+ pud_t *pudv;
+ pmd_t *pmdv;
+ pte_t *ptev;
+
+ pgdv = (pgd_t *)__va((__nds32__mfsr(NDS32_SR_L1_PPTB) &
+ L1_PPTB_mskBASE)) + pgd_index((unsigned int)cpu_resume);
+
+ pudv = pud_offset(pgdv, (unsigned int)cpu_resume);
+ pmdv = pmd_offset(pudv, (unsigned int)cpu_resume);
+ ptev = pte_offset_map(pmdv, (unsigned int)cpu_resume);
+
+ resume_addr = ((*ptev) & TLB_DATA_mskPPN)
+ | ((unsigned int)cpu_resume & 0x0fff);
+
+ suspend2ram();
+}
+
+static void nds32_suspend_cpu(void)
+{
+ while (!(__nds32__mfsr(NDS32_SR_INT_PEND) & wake_mask))
+ __asm__ volatile ("standby no_wake_grant\n\t");
+}
+
+static int nds32_pm_valid(suspend_state_t state)
+{
+ switch (state) {
+ case PM_SUSPEND_ON:
+ case PM_SUSPEND_STANDBY:
+ case PM_SUSPEND_MEM:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+static int nds32_pm_enter(suspend_state_t state)
+{
+ pr_debug("%s:state:%d\n", __func__, state);
+ switch (state) {
+ case PM_SUSPEND_STANDBY:
+ nds32_suspend_cpu();
+ return 0;
+ case PM_SUSPEND_MEM:
+ nds32_suspend2ram();
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct platform_suspend_ops nds32_pm_ops = {
+ .valid = nds32_pm_valid,
+ .enter = nds32_pm_enter,
+};
+
+static int __init nds32_pm_init(void)
+{
+ pr_debug("Enter %s\n", __func__);
+ suspend_set_ops(&nds32_pm_ops);
+ return 0;
+}
+late_initcall(nds32_pm_init);
diff --git a/arch/nds32/kernel/sleep.S b/arch/nds32/kernel/sleep.S
ne
