回复：[PATCH] cpuidle: add riscv cpuidle driver

[刘邵华BTD]

Hi Daniel,
> > 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 at allwinnertech.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?

Data cache consistency is mainly ensured by hardware in riscv, and there is no 
implementation of flushing data cache in kernel. Before wfi(),add an memory 
barrier operation - mb(). Is this feasible? 

  //arch/riscv/include/asm/barrier.h

  17 #define RISCV_FENCE(p, s) \
  18         __asm__ __volatile__ ("fence " #p "," #s : : : "memory")
  19 
  20 /* These barriers need to enforce ordering on both devices or memory. */                                                                                                            
  21 #define mb()            RISCV_FENCE(iorw,iorw) 


After modification, the codes is as follows.

81 @@ -0,0 +1,8 @@
82 +// SPDX-License-Identifier: GPL-2.0
83 +#include <asm/cpuidle.h>
84 +
85 +void cpu_do_idle(void)
86 +{
87 +       mb();
88 +       __asm__ __volatile__ ("wfi");
89 +
90 +}

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

I'll fix it. Thank you!