cpuidle: change #ifdef for the declaration of cpuidle_enter_s2idle()

PATCH zhuguangqing83 2 participants 2 messages Sep 26, 2020 – Sep 28, 2020

Share & tools

zhuguangqing83

author

2020-09-26 01:30 UTC #0 lore raw

Quoted 13 lines · On Thu, Sep 24, 2020 at 10:01 AM <[email protected]> wrote:

Quoted 11 lines
From: zhuguangqing <[email protected]>
Currently, if CONFIG_SUSPEND=n and CONFIG_CPU_IDLE=y, the function cpuidle_enter_s2idle() is declared but not defined, it may cause error when cpuidle_enter_s2idle() is called.
If CONFIG_SUSPEND=y and CONFIG_CPU_IDLE=n, the function cpuidle_enter_s2idle() is defined as "return -ENODEV;" which is not supposed to be.

Quoted 7 lines

Quoted 3 lines
Change #ifdef CONFIG_CPU_IDLE to #ifdef CONFIG_SUSPEND for cpuidle_enter_s2idle() in cpuidle.h, which is consistent with its defination in cpuidle.c.
Well, what about the case when CONFIG_SUSPEND is set, but CONFIG_CPU_IDLE isn't?

When CONFIG_SUSPEND is set, but CONFIG_CPU_IDLE isn't, the function cpuidle_enter_s2idle() is defined in cpuidle.c, but the defination in cpuidle.c is not used actually because CONFIG_CPU_IDLE isn't set, we only use its defination as "return -ENODEV;" in cpuidle.h.

Quoted 46 lines

include/linux/cpuidle.h static inline struct cpuidle_device

Quoted 11 lines

Signed-off-by zhuguangqing <[email protected]>

---
 include/linux/cpuidle.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

include/linux/cpuidle.h +0 -0

index 6175c77bf25e..2aa8cead1727 100644
@@ -216,22 +216,26 @@ static inline struct cpuidle_device

*cpuidle_get_device(void) {return NULL; }

Quoted 34 lines

include/linux/cpuidle.h +9 -6

 extern int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
                                      struct cpuidle_device *dev,
                                      u64 latency_limit_ns);
-extern int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
-                               struct cpuidle_device *dev);
 extern void cpuidle_use_deepest_state(u64 latency_limit_ns);
 #else
 static inline int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
                                             struct cpuidle_device *dev,
                                             u64 latency_limit_ns)
 {return -ENODEV; }
-static inline int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
-                                      struct cpuidle_device *dev)
-{return -ENODEV; }
 static inline void cpuidle_use_deepest_state(u64 latency_limit_ns)
 {
 }
 #endif
 
+#ifdef CONFIG_SUSPEND
+extern int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
+                               struct cpuidle_device *dev);
+#else
+static inline int cpuidle_enter_s2idle(struct cpuidle_driver *drv,
+                                      struct cpuidle_device *dev)
+{return -ENODEV; }
+#endif
+
 /* kernel/sched/idle.c */
 extern void sched_idle_set_state(struct cpuidle_state *idle_state);
 extern void default_idle_call(void);
--

2.17.1

Rafael J. Wysocki

2020-09-28 14:10 UTC #1 lore raw

Quoted 18 lines · On Sat, Sep 26, 2020 at 3:30 AM zhuguangqing83 <[email protected]> wrote:

Quoted 18 lines · On Thu, Sep 24, 2020 at 10:01 AM <[email protected]> wrote:
Quoted 14 lines
From: zhuguangqing <[email protected]>
Currently, if CONFIG_SUSPEND=n and CONFIG_CPU_IDLE=y, the function cpuidle_enter_s2idle() is declared but not defined, it may cause error when cpuidle_enter_s2idle() is called.
If CONFIG_SUSPEND=y and CONFIG_CPU_IDLE=n, the function cpuidle_enter_s2idle() is defined as "return -ENODEV;" which is not supposed to be.
Change #ifdef CONFIG_CPU_IDLE to #ifdef CONFIG_SUSPEND for cpuidle_enter_s2idle() in cpuidle.h, which is consistent with its defination in cpuidle.c.

Quoted 8 lines

Quoted 3 lines
Well, what about the case when CONFIG_SUSPEND is set, but CONFIG_CPU_IDLE isn't?
When CONFIG_SUSPEND is set, but CONFIG_CPU_IDLE isn't, the function cpuidle_enter_s2idle() is defined in cpuidle.c, but the defination in cpuidle.c is not used actually because CONFIG_CPU_IDLE isn't set, we only use its defination as "return -ENODEV;" in cpuidle.h.

Actually, if CONFIG_CPU_IDLE is not set, cpuidle.c is not compiled at all AFAICS, but after the $subject patch the compiler will be looking for cpuidle_enter_s2idle() in that case, won't it? [The static inline stub is only present for CONFIG_SUSPEND unset.]