linux-kernel - Re: [PATCH 1/1] nds32: Power management for nds32

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Oct 2018 08:51:17 +0200
From:   Pavel Machek <pavel@....cz>
To:     Nickhu <nickhu@...estech.com>
Cc:     greentime@...estech.com, linux-kernel@...r.kernel.org,
        arnd@...db.de, deanbo422@...il.com, rjw@...ysocki.net,
        tglx@...utronix.de, jason@...edaemon.net, marc.zyngier@....com,
        zong@...estech.com, linux-pm@...r.kernel.org,
        alankao@...estech.com, green.hu@...il.com
Subject: 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 <nickhu@...estech.com>

Is "Nickhu" complete name?

> diff --git a/arch/nds32/kernel/pm.c b/arch/nds32/kernel/pm.c
> new file mode 100644
> index 000000000000..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<asm/memory.h>

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

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)