Re: [PATCH 7/12] acpi: fix another compile warning

Previous thread: [PATCH 11/12] pcmcia/net_pcmcia: all net_pcmcia modules depend on PCMCIA by Andreas Herrmann on Tuesday, June 19, 2007 - 3:52 pm. (5 messages)

Next thread: [PATCH 5/12] acpi: fix compile error with ACPI && !ACPI_SYSTEM by Andreas Herrmann on Tuesday, June 19, 2007 - 3:49 pm. (2 messages)
From: Andreas Herrmann
Date: Tuesday, June 19, 2007 - 3:50 pm

Avoid compile warning if !ACPI_BLACKLIST_YEAR

  CC      drivers/acpi/blacklist.o
  drivers/acpi/blacklist.c:76:5: warning: "CONFIG_ACPI_BLACKLIST_YEAR" is not defined

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 drivers/acpi/blacklist.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index 3ec110c..ac96c47 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -73,7 +73,7 @@ static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
 	{""}
 };
 
-#if	CONFIG_ACPI_BLACKLIST_YEAR
+#if	defined(CONFIG_ACPI_BLACKLIST_YEAR) && CONFIG_ACPI_BLACKLIST_YEAR
 
 static int __init blacklist_by_year(void)
 {
-- 
1.5.0.7




-

From: Len Brown
Date: Tuesday, June 19, 2007 - 8:38 pm

How were you able to produce a .config with CONFIG_ACPI_BLACKLIST_YEAR not defined?
Can you send it to me?

thanks,
-

From: Randy Dunlap
Date: Tuesday, June 19, 2007 - 8:49 pm

'make randconfig' does that kind of thing.  It doesn't enforce/follow

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

From: Randy Dunlap
Date: Tuesday, June 19, 2007 - 8:51 pm

I should have also said:  randconfig is good for detecting some
missing conditions/configs or missing header files, but if you find one
that is just plain Invalid (like some of these), just say so

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

From: Len Brown
Date: Tuesday, June 19, 2007 - 9:41 pm

If randconfig ends up with impossible-for-a-user-to-generate configs,
then it seems seriously broken.  Perhaps it would make sense to run
"make oldconfig" after "make randconfig" -- or better yet, have that
built in?

-Len
-

From: Andreas Herrmann
Date: Wednesday, June 20, 2007 - 2:31 am

I think of randconfig as

  "make config with random answers to all questions"

(Or did I miss something?)
This means that randconfig does not give totally random
configurations. The same restrictions apply for config,
randconfig and menuconfig. If not we might consider fixing
scripts/kconfig/conf.c and friends.

So in general a "randconfig" configurations can be generated by a user
as well. He just has to give the same answers during "make config"
as randconfig did.

I started this randconfig thing yesterday and up to now I have looked
at 16 (out of >66) configs which lead to kernel build errors with
the current git tree.
Of course I have seen duplicates of the problems reported.
But there were just 3 (all equal) bogus configurations. There randconfig
did not provide a proper file name for CONFIG_ACPI_CUSTOM_DSDT_FILE.
This is the only case for which I would use the term "user error" or
"bogus randconfig".

IMHO if certain configurations are invalid we should use kconfig-language
to prevent them.


Ah and wrt to the above compile warning. Normally I would have ignored
it - but I looked at acpi code and its Kconfig anyway.
And It's easy to avoid such a warning to keep the terminal clear
for more interesting compile messages ;-)



Regards,

Andreas



-

From: Randy Dunlap
Date: Wednesday, June 20, 2007 - 7:28 am

The last time that I looked at the config code, randconfig generates
a random value for config symbols, taking "depends on" into account,
but it does not take "select" into account.  Or put another way,
it does not follow "select" chains.

and IIRC, running "make oldconfig" after that still won't enforce
the "select"s.  This is a long-running problem and one of the problems

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

From: Andreas Herrmann
Date: Wednesday, June 20, 2007 - 1:49 am

Initially I used randconfig. But you can easily create it using
"make menuconfig", too.

Just do "make mrproper && make menuconfig" and now
deselect CONFIG_PM. On x86_64 ACPI keeps enabled by default.

Now additionally activate an assorted choice of laptop misc devices,
like MSI_LAPTOP, THINKPAD_ACPI etc, and you get:

...
drivers/misc/sony-laptop.c:1458: undefined reference to `ec_read'
drivers/built-in.o: In function `acpi_ec_read':
drivers/misc/thinkpad_acpi.c:247: undefined reference to `ec_read'
drivers/built-in.o: In function `acpi_ec_write':
drivers/misc/thinkpad_acpi.c:260: undefined reference to `ec_write'
drivers/built-in.o: In function `led_write':
drivers/misc/thinkpad_acpi.c:2209: undefined reference to `ec_write'
...

drivers/acpi/blacklist.c:76:5: warning: "CONFIG_ACPI_BLACKLIST_YEAR" is not defi
ned
...

drivers/acpi/bus.c: In function 'acpi_bus_get_power':
drivers/acpi/bus.c:162: warning: implicit declaration of function 'acpi_power_get_inferred_state'
...


In short, it is possible for a user to create such a configuration.
The current kconfig for acpi and those misc devices does not prevent

Attached is a configuration that I've just created with menuconfig and
not randconfig.



Regards,

Andreas
From: Len Brown
Date: Wednesday, June 20, 2007 - 6:36 am

As CONFIG_ACPI depends on CONFIG_PM, then if you are able to end up
with a config having ACPI && !PM, then the Kconfig system is broken.

The brokeness is x86_64 specific, and results from the "select ACPI" below.

Please test if this single patch fixes all your multiple ACPI related build errors.

I fear, however, that this patch defeats the purpose of 
b0bd35e622ffbda2c01dc67a0381c6a18817a29a -- which was to make selecting
NUMA more user-friendly.  So it might make more sense to simply revert that
patch entirely.

The underlying problem is that Kconfig doesn't support using select
on targets which themselves have dependencies.

Signed-off-by: Len Brown <len.brown@intel.com>


diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index 5ce9443..e9d7767 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -364,9 +364,9 @@ config NODES_SHIFT
 config X86_64_ACPI_NUMA
        bool "ACPI NUMA detection"
        depends on NUMA
-       select ACPI 
+       depends on ACPI 
 	select PCI
-       select ACPI_NUMA
+       depends on ACPI_NUMA
        default y
        help
 	 Enable ACPI SRAT based node topology detection.
-

From: Andi Kleen
Date: Wednesday, June 20, 2007 - 6:46 am

The depends on ACPI is fine by me, but I would prefer if the 
ACPI_NUMA dependency worked the other way round (APCI_NUMA default y
and depends on the relevant architecture specific symbols). That would
be more user friendly I think because all the NUMA options would
be in one place.



-

From: Ravikiran G Thirumalai
Date: Wednesday, June 20, 2007 - 11:25 am

arch/x86_64/Kconfig:706:error: found recursive dependency: PCI -> ACPI ->
X86_64_ACPI_NUMA -> PCI
 -> USB_ARCH_HAS_OHCI -> USB_ARCH_HAS_HCD -> MOUSE_APPLETOUCH -> USB ->
USB_SISUSBVGAmake[1]: *** [menuconfig] Error 1

I guess we have to change PCI to 'depends on' as well.
Else, select PM?

I am OK with either approach.

Thanks,
Kiran

Paper over 'select' inadequacies.  

Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>

Index: linux-2.6.22-rc5/arch/x86_64/Kconfig
===================================================================
--- linux-2.6.22-rc5.orig/arch/x86_64/Kconfig	2007-06-18 16:02:19.571323415 -0700
+++ linux-2.6.22-rc5/arch/x86_64/Kconfig	2007-06-20 11:34:29.845354250 -0700
@@ -366,6 +366,7 @@ config X86_64_ACPI_NUMA
        depends on NUMA
        select ACPI 
 	select PCI
+  	select PM
        select ACPI_NUMA
        default y
        help
-

From: Andreas Herrmann
Date: Thursday, June 21, 2007 - 11:36 am

Yep, this one-liner solves all acpi related compile errors/warnings
that I've reported.

Additional it resolves anoter problem that results from
dependencies between CONFIG_PNP and CONFIG_ACPI.

One problem arises:
If PM is deselected, the PNP menu is empty on x86_64.
So I suggest to add the attached patch to hide the PNP-menu
if no PM (and thus ACPI) is selected.


Regards,

Andreas

--

Make CONFIG_PNP a menuconfig to hide the submenu if neither
ISA nor ACPI is selected.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>

diff --git a/drivers/pnp/Kconfig b/drivers/pnp/Kconfig
index 1959cef..2291e7d 100644
--- a/drivers/pnp/Kconfig
+++ b/drivers/pnp/Kconfig
@@ -2,11 +2,9 @@
 # Plug and Play configuration
 #
 
-menu "Plug and Play support"
-	depends on HAS_IOMEM
-
-config PNP
+menuconfig PNP
 	bool "Plug and Play support"
+	depends on HAS_IOMEM
 	depends on ISA || ACPI
 	---help---
 	  Plug and Play (PnP) is a standard for peripherals which allows those
@@ -22,15 +20,15 @@ config PNP
 
 	  If unsure, say Y.
 
+if PNP
+
 config PNP_DEBUG
 	bool "PnP Debug Messages"
-	depends on PNP
 	help
 	  Say Y if you want the Plug and Play Layer to print debug messages.
 	  This is useful if you are developing a PnP driver or troubleshooting.
 
 comment "Protocols"
-	depends on PNP
 
 source "drivers/pnp/isapnp/Kconfig"
 
@@ -38,5 +36,4 @@ source "drivers/pnp/pnpbios/Kconfig"
 
 source "drivers/pnp/pnpacpi/Kconfig"
 
-endmenu
-
+endif



-

From: Andreas Herrmann
Date: Friday, June 22, 2007 - 7:39 am

Andi,

are you going to apply Ravikiran's patch?


Regards,

Andreas

-- 
Operating | AMD Saxony Limited Liability Company & Co. KG,
  System  | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 Research | Register Court Dresden: HRA 4896, General Partner authorized
  Center  | to represent: AMD Saxony LLC (Wilmington, Delaware, US)
  (OSRC)  | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy



-

From: Andrew Morton
Date: Friday, June 22, 2007 - 8:55 pm

argh.  (and not just argh-at-your-email-client).

I went through some hair-tearing a few months ago working out why it is so
damn hard to make CONFIG_PM go away and then I fixed it.  And now you're
proposing a change which would reinstate the obnoxious old behaviour.

Is the "bug" which you're "fixing" here caused by the randconfig
inadequacies?  Because randconfig is just busted and simply should auto-run
oldconfig.
-

From: Randy Dunlap
Date: Friday, June 22, 2007 - 9:28 pm

Running oldconfig won't fix randconfigs that have busted "select"
chains (i.e., select does not follow chains like "depends on" does).

However, lots of this bustage isn't due to randconfig at all.
It's due to Jan E.'s menuconfig changes (yes, which I supported).
It seems that Jan didn't realize that a transition of a tristate symbol
to a boolean converts y or m to y (and n to n) and all (?) of the new
menuconfig symbols that control whether a menu is displayed or not
are boolean.  One simple "fix" (or workaround) is to make them
tristate.  Now that seems a little odd for a symbol that just controls
whether a menu is displayed or not, so Satyam Sharma has made some
other suggestions, and of course Andreas Herrmann has produced lots
of patches to fix various issues that he has run into.

I can't tell you the final answer (I proposed the tristate fix).
I don't particularly like some of the patches that change
	depends on XYZ
to
	depends on XYZ && ksymbol
in many lines of kconfig entries.


Not that it answers your question, but we should have hit this
in -mm, not in mainline, but it appears that our testing was a bit
insufficient.

And of course, if someone would fix kconfig to follow "select" chains,
we should bestow an award on them.  :)
(however, I don't know if Roman would merge that patch)

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

Previous thread: [PATCH 11/12] pcmcia/net_pcmcia: all net_pcmcia modules depend on PCMCIA by Andreas Herrmann on Tuesday, June 19, 2007 - 3:52 pm. (5 messages)

Next thread: [PATCH 5/12] acpi: fix compile error with ACPI && !ACPI_SYSTEM by Andreas Herrmann on Tuesday, June 19, 2007 - 3:49 pm. (2 messages)