Skip to contentSkip to conversation

2.6.16-rc6 calls check_acpi_pci() on x86 with ACPI disabled

PATCH In review Herbert Poetzl 2 participants 5 messages Mar 15, 2006 ACPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 2.6.16-rc6 calls check_acpi_pci() on x86 with ACPI disabled

Hi Andrew! Folks!

check_acpi_pci() is called form arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case, so either we need to move the declaration outside the CONFIG_ACPI check, or alternatively move the call in setup.c inside the CONFIG_ACPI one

attached two patches which would do this

best,
Herbert

include/asm/acpi.h renamed from include/asm/acpi.h.orig +6 -4
@@ -103,6 +103,12 @@ __acpi_release_global_lock (unsigned int
         :"=r"(n_hi), "=r"(n_lo)     \
         :"0"(n_hi), "1"(n_lo))
 
+#ifdef CONFIG_X86_IO_APIC
+extern void check_acpi_pci(void);
+#else
+static inline void check_acpi_pci(void) { }
+#endif
+
 #ifdef CONFIG_ACPI 
 extern int acpi_lapic;
 extern int acpi_ioapic;
@@ -128,8 +134,6 @@ extern int acpi_gsi_to_irq(u32 gsi, unsi
 extern int skip_ioapic_setup;
 extern int acpi_skip_timer_override;
 
-extern void check_acpi_pci(void);
-
 static inline void disable_ioapic_setup(void)
 {
 	skip_ioapic_setup = 1;
@@ -142,8 +146,6 @@ static inline int ioapic_setup_disabled(
 
 #else
 static inline void disable_ioapic_setup(void) { }
-static inline void check_acpi_pci(void) { }
-
 #endif
 
 static inline void acpi_noirq_set(void) { acpi_noirq = 1; }

alternatively:

arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c.orig +1 -2
@@ -1599,11 +1599,10 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled)
 		efi_map_memmap();
 
+#ifdef CONFIG_ACPI
 #ifdef CONFIG_X86_IO_APIC
 	check_acpi_pci();	/* Checks more than just ACPI actually */
 #endif
-
-#ifdef CONFIG_ACPI
 	/*
 	 * Parse the ACPI tables for possible boot-time SMP configuration.
 	 */
#1 lore raw
Quoted 7 lines · Herbert Poetzl

Hi Andrew! Folks!

check_acpi_pci() is called form arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case,

Well that's a shame.

Quoted 5 lines

so either we need to move the declaration outside the CONFIG_ACPI check, or alternatively move the call in setup.c inside the CONFIG_ACPI one

attached two patches which would do this

Prefer the first version. But it'll break if CONFIG_X86_IO_APIC && !CONFIG_ACPI

So how's about this?

From: Herbert Poetzl <[email protected]>

check_acpi_pci() is called from arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case.

---

 include/asm/acpi.h |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
include/asm/acpi.h renamed from include/asm/acpi.h~dont-check_acpi_pci-on-x86-with-acpi-disabled +6 -4
diff -puN include/asm/acpi.h~dont-check_acpi_pci-on-x86-with-acpi-disabled include/asm/acpi.h
@@ -103,6 +103,12 @@ __acpi_release_global_lock (unsigned int
         :"=r"(n_hi), "=r"(n_lo)     \
         :"0"(n_hi), "1"(n_lo))
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_X86_IO_APIC)
+extern void check_acpi_pci(void);
+#else
+static inline void check_acpi_pci(void) { }
+#endif
+
 #ifdef CONFIG_ACPI 
 extern int acpi_lapic;
 extern int acpi_ioapic;
@@ -128,8 +134,6 @@ extern int acpi_gsi_to_irq(u32 gsi, unsi
 extern int skip_ioapic_setup;
 extern int acpi_skip_timer_override;
 
-extern void check_acpi_pci(void);
-
 static inline void disable_ioapic_setup(void)
 {
 	skip_ioapic_setup = 1;
@@ -142,8 +146,6 @@ static inline int ioapic_setup_disabled(
 
 #else
 static inline void disable_ioapic_setup(void) { }
-static inline void check_acpi_pci(void) { }
-
 #endif
 
 static inline void acpi_noirq_set(void) { acpi_noirq = 1; }

_

#2 lore raw
Quoted 15 lines · Andrew Morton
Quoted 7 lines · Herbert Poetzl

Hi Andrew! Folks!

check_acpi_pci() is called form arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case,

Well that's a shame.

Quoted 4 lines

so either we need to move the declaration outside the CONFIG_ACPI check, or alternatively move the call in setup.c inside the CONFIG_ACPI one

Quoted 6 lines
Quoted 1 line

attached two patches which would do this

Prefer the first version. But it'll break if CONFIG_X86_IO_APIC && !CONFIG_ACPI

So how's about this?

hmm, well, the comment around the check_acpi_pci() call
says: "Checks more than just ACPI actually", so I didn't want to make it depend on ACPI in the 'first' version, which now would change semantics, but if it is fine to make it depend on ACPI, the second version might be the simpler solution (which should have the same semantic as your version ... I think

maybe the ACPI folks should clarify if this stuff has to be run if ACPI is off, in which case renaming the thing might be a good idea ...

best,
Herbert

Quoted 47 lines

From: Herbert Poetzl <[email protected]>

check_acpi_pci() is called from arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case.

---

 include/asm/acpi.h |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff -puN include/asm/acpi.h~dont-check_acpi_pci-on-x86-with-acpi-disabled include/asm/acpi.h

include/asm/acpi.h renamed from include/asm/acpi.h~dont-check_acpi_pci-on-x86-with-acpi-disabled +6 -4
@@ -103,6 +103,12 @@ __acpi_release_global_lock (unsigned int
         :"=r"(n_hi), "=r"(n_lo)     \
         :"0"(n_hi), "1"(n_lo))
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_X86_IO_APIC)
+extern void check_acpi_pci(void);
+#else
+static inline void check_acpi_pci(void) { }
+#endif
+
 #ifdef CONFIG_ACPI 
 extern int acpi_lapic;
 extern int acpi_ioapic;
@@ -128,8 +134,6 @@ extern int acpi_gsi_to_irq(u32 gsi, unsi
 extern int skip_ioapic_setup;
 extern int acpi_skip_timer_override;
 
-extern void check_acpi_pci(void);
-
 static inline void disable_ioapic_setup(void)
 {
 	skip_ioapic_setup = 1;
@@ -142,8 +146,6 @@ static inline int ioapic_setup_disabled(
 
 #else
 static inline void disable_ioapic_setup(void) { }
-static inline void check_acpi_pci(void) { }
-
 #endif
 
 static inline void acpi_noirq_set(void) { acpi_noirq = 1; }

_

#3 lore raw
Quoted 32 lines · Herbert Poetzl
Quoted 15 lines · Andrew Morton
Quoted 7 lines · Herbert Poetzl

Hi Andrew! Folks!

check_acpi_pci() is called form arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case,

Well that's a shame.

Quoted 4 lines

so either we need to move the declaration outside the CONFIG_ACPI check, or alternatively move the call in setup.c inside the CONFIG_ACPI one

Quoted 6 lines
Quoted 1 line

attached two patches which would do this

Prefer the first version. But it'll break if CONFIG_X86_IO_APIC && !CONFIG_ACPI

So how's about this?

hmm, well, the comment around the check_acpi_pci() call
says: "Checks more than just ACPI actually", so I didn't want to make it depend on ACPI in the 'first' version, which now would change semantics, but if it is fine to make it depend on ACPI, the second version might be the simpler solution (which should have the same semantic as your version ... I think

Quoted 3 lines

maybe the ACPI folks should clarify if this stuff has to be run if ACPI is off, in which case renaming the thing might be a good idea ...

Yes, actually I didn't check closely enough - arch/i386/kernel/acpi/* gets built even if CONFIG_ACPI=n (!)

So the code will actualy compile and link OK if we do:

#ifdef CONFIG_X86_IO_APIC
extern void check_acpi_pci(void);
#else
static inline void check_acpi_pci(void) { }
#endif

But we'd need the acpi guys to tell us what's actually intended here, please. Does it make sense to be calling this function in a non-ACPI kernel?

erk, your patch was against include/asm/... - please don't do that - it doesn't work very well if the patch receiver isn't using a setup-for-i386 tree.

#4 lore raw
Quoted 48 lines · Andrew Morton
Quoted 32 lines · Herbert Poetzl
Quoted 15 lines · Andrew Morton
Quoted 7 lines · Herbert Poetzl

Hi Andrew! Folks!

check_acpi_pci() is called form arch/i386/kernel/setup.c even if CONFIG_ACPI is not defined, but the code in include/asm/acpi.h doesn't provide it in this case,

Well that's a shame.

Quoted 4 lines

so either we need to move the declaration outside the CONFIG_ACPI check, or alternatively move the call in setup.c inside the CONFIG_ACPI one

Quoted 6 lines
Quoted 1 line

attached two patches which would do this

Prefer the first version. But it'll break if CONFIG_X86_IO_APIC && !CONFIG_ACPI

So how's about this?

hmm, well, the comment around the check_acpi_pci() call
says: "Checks more than just ACPI actually", so I didn't want to make it depend on ACPI in the 'first' version, which now would change semantics, but if it is fine to make it depend on ACPI, the second version might be the simpler solution (which should have the same semantic as your version ... I think

Quoted 3 lines

maybe the ACPI folks should clarify if this stuff has to be run if ACPI is off, in which case renaming the thing might be a good idea ...

Yes, actually I didn't check closely enough - arch/i386/kernel/acpi/* gets built even if CONFIG_ACPI=n (!)

So the code will actualy compile and link OK if we do:

#ifdef CONFIG_X86_IO_APIC
extern void check_acpi_pci(void);
#else
static inline void check_acpi_pci(void) { }
#endif

Quoted 3 lines

But we'd need the acpi guys to tell us what's actually intended here, please. Does it make sense to be calling this function in a non-ACPI kernel?

yes, maybe it needs to be split into two parts, one generic and the other acpi related ...

Quoted 4 lines

erk, your patch was against include/asm/...
- please don't do that -
it doesn't work very well if the patch receiver isn't using a setup-for-i386 tree.

oops, sorry, I should know that, but I just forgot about that ... will try to remember next time ...

best,
Herbert