Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1700081

Re: [PATCH] psci: add CPU_IDLE dependency

From Jean Delvare <jdelvare@suse.de>
Newsgroups linux.kernel
Subject Re: [PATCH] psci: add CPU_IDLE dependency
Date 2017-07-31 16:20 +0200
Message-ID <u9jI6-7iZ-11@gated-at.bofh.it> (permalink)
References <u9eIq-44A-23@gated-at.bofh.it> <u9fXQ-513-7@gated-at.bofh.it>
Organization SUSE Linux

Show all headers | View raw


Hi Kevin,

On Mon, 31 Jul 2017 11:19:39 +0100, Kevin Brodsky wrote:
> On 31/07/17 09:55, Arnd Bergmann wrote:
> > I ran into a build error for the psci_checker:
> >
> > drivers/firmware/psci_checker.o: In function `psci_checker':
> > psci_checker.c:(.init.text+0x528): undefined reference to `cpuidle_devices'
> >
> > As far as I can tell, this is simply a very rare combination of options,
> > but the problem has existed since the code was initially added.
> > Adding a Kconfig dependency makes it build properly.  
> 
> Good catch! For some reason I missed this config option when figuring out the 
> dependencies... I wonder though, shouldn't cpuidle.h declare cpuidle_devices 
> conditionally on CONFIG_CPU_IDLE?

Such conditional declarations only make sense if there is a legitimate
use of the disabled case and if they make the disabled case fully
transparent to the users. This is typically done by replacing function
declarations by inline stubs doing nothing in the right way when the
feature is disabled. It avoids having to put the condition checks on the
side of all users.

In this case however, you can't stub out cpuidle_devices alone. If you
omit the declaration when CONFIG_CPU_IDLE isn't set, all you'll get is a
failure at compilation time, instead of at linkage time. This barely
helps. For it to be useful, you would additionally have to provide
wrappers around
	this_cpu_read(cpuidle_devices)
and
	per_cpu(cpuidle_devices, cpu)
and stub out these wrappers when CONFIG_CPU_IDLE is disabled (so you
don't refer to cpuidle_devices at all when it isn't available.)

But then again this would only make sense if the psci_checker still
serves a purpose when CONFIG_CPU_IDLE isn't set. Not my area, but after
a quick look at the code I strongly suspect this is not the case.

> > Fixes: ea8b1c4a6019 ("drivers: psci: PSCI checker module")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>  
> 
> Acked-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] psci: add CPU_IDLE dependency Arnd Bergmann <arnd@arndb.de> - 2017-07-31 11:00 +0200
  Re: [PATCH] psci: add CPU_IDLE dependency Kevin Brodsky <kevin.brodsky@arm.com> - 2017-07-31 12:20 +0200
    Re: [PATCH] psci: add CPU_IDLE dependency Jean Delvare <jdelvare@suse.de> - 2017-07-31 16:20 +0200
      Re: [PATCH] psci: add CPU_IDLE dependency Kevin Brodsky <kevin.brodsky@arm.com> - 2017-07-31 18:30 +0200
  Re: [PATCH] psci: add CPU_IDLE dependency Nishanth Menon <nm@ti.com> - 2017-07-31 14:40 +0200

csiph-web