linux-kernel - Re: [PATCH] cpuidle: add riscv cpuidle driver

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]

Message-ID: <80bc85f5-fba3-5f08-4d73-372b5f495833@linaro.org>
Date:   Mon, 14 Sep 2020 07:46:50 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     liush <liush@...winnertech.com>, paul.walmsley@...ive.com,
        palmer@...belt.com, aou@...s.berkeley.edu, rjw@...ysocki.net,
        anup.patel@....com, atish.patra@....com, damien.lemoal@....com,
        wangkefeng.wang@...wei.com, kernel@...il.dk, zong.li@...ive.com
Cc:     linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH] cpuidle: add riscv cpuidle driver

On 14/09/2020 03:52, liush wrote:
> This patch adds a cpuidle driver for systems based RISCV architecture.
> This patch supports state WFI. Other states will be supported in the
> future.
> 
> Signed-off-by: liush <liush@...winnertech.com>
> ---

[ ... ]

>  
>  obj-$(CONFIG_RISCV_M_MODE)	+= traps_misaligned.o
> diff --git a/arch/riscv/kernel/cpuidle.c b/arch/riscv/kernel/cpuidle.c
> new file mode 100644
> index 00000000..a3289e7
> --- /dev/null
> +++ b/arch/riscv/kernel/cpuidle.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <asm/cpuidle.h>
> +
> +void cpu_do_idle(void)
> +{
> +	__asm__ __volatile__ ("wfi");
> +

extra line

> +}

As for the next deeper states should end up with the cpu_do_idle
function, isn't there an extra operation with the wfi() like flushing
the l1 cache?

> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index c0aeedd..f6be0fd 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -62,6 +62,11 @@ depends on PPC
>  source "drivers/cpuidle/Kconfig.powerpc"
>  endmenu
>  
> +menu "RISCV CPU Idle Drivers"
> +depends on RISCV
> +source "drivers/cpuidle/Kconfig.riscv"
> +endmenu
> +
>  config HALTPOLL_CPUIDLE
>  	tristate "Halt poll cpuidle driver"
>  	depends on X86 && KVM_GUEST
> diff --git a/drivers/cpuidle/Kconfig.riscv b/drivers/cpuidle/Kconfig.riscv
> new file mode 100644
> index 00000000..e86d36b
> --- /dev/null
> +++ b/drivers/cpuidle/Kconfig.riscv
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# RISCV CPU Idle drivers
> +#
> +config RISCV_CPUIDLE
> +        bool "Generic RISCV CPU idle Driver"
> +        select DT_IDLE_STATES
> +	select CPU_IDLE_MULTIPLE_DRIVERS
> +        help
> +          Select this option to enable generic cpuidle driver for RISCV.
> +	  Now only support C0 State.

Identation

Rest looks ok for me.


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog