[PATCH] Fix pciehp driver on non ACPI systems

[PATCH] Fix pciehp driver on non ACPI systems

[Anton Blanchard]

Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
===================================================================
--- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
+++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
@@ -38,10 +38,14 @@
 
 #include "../pci.h"
 #include "pciehp.h"
+
+#ifdef CONFIG_ACPI
 #include <acpi/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/actypes.h>
 #include <linux/pci-acpi.h>
+#endif
+
 #ifdef DEBUG
 #define DBG_K_TRACE_ENTRY      ((unsigned int)0x00000001)	/* On function entry */
 #define DBG_K_TRACE_EXIT       ((unsigned int)0x00000002)	/* On function exit */

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Greg KH]

On Thu, Apr 06, 2006 at 08:17:31PM +1000, Anton Blanchard wrote:
> 
> Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
> ===================================================================
> --- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
> +++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
> @@ -38,10 +38,14 @@
>  
>  #include "../pci.h"
>  #include "pciehp.h"
> +
> +#ifdef CONFIG_ACPI
>  #include <acpi/acpi.h>
>  #include <acpi/acpi_bus.h>
>  #include <acpi/actypes.h>
>  #include <linux/pci-acpi.h>
> +#endif

Shouldn't the ACPI headers handle it if CONFIG_ACPI is not enabled?  All
other header files work that way, and we shouldn't have to add this to
the .c files.

thanks,

greg k-h

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Randy.Dunlap]

On Thu, 6 Apr 2006 09:05:27 -0700 Greg KH wrote:

> On Thu, Apr 06, 2006 at 08:17:31PM +1000, Anton Blanchard wrote:
> > 
> > Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.
> > 
> > Signed-off-by: Anton Blanchard <anton@samba.org>
> > ---
> > 
> > Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
> > ===================================================================
> > --- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
> > +++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
> > @@ -38,10 +38,14 @@
> >  
> >  #include "../pci.h"
> >  #include "pciehp.h"
> > +
> > +#ifdef CONFIG_ACPI
> >  #include <acpi/acpi.h>
> >  #include <acpi/acpi_bus.h>
> >  #include <acpi/actypes.h>
> >  #include <linux/pci-acpi.h>
> > +#endif
> 
> Shouldn't the ACPI headers handle it if CONFIG_ACPI is not enabled?  All
> other header files work that way, and we shouldn't have to add this to
> the .c files.

maybe the C file could just #include <linux/acpi.h> ?

---
~Randy

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Sam Ravnborg]

On Thu, Apr 06, 2006 at 10:41:13AM -0700, Randy.Dunlap wrote:
> On Thu, 6 Apr 2006 09:05:27 -0700 Greg KH wrote:
> 
> > On Thu, Apr 06, 2006 at 08:17:31PM +1000, Anton Blanchard wrote:
> > > 
> > > Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.
> > > 
> > > Signed-off-by: Anton Blanchard <anton@samba.org>
> > > ---
> > > 
> > > Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
> > > ===================================================================
> > > --- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
> > > +++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
> > > @@ -38,10 +38,14 @@
> > >  
> > >  #include "../pci.h"

When one introdues relative apths like the above this is a good sign
that the header file ought to move to a common place somewhere in
include/.

	Sam

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Greg KH]

On Thu, Apr 06, 2006 at 07:46:44PM +0200, Sam Ravnborg wrote:
> On Thu, Apr 06, 2006 at 10:41:13AM -0700, Randy.Dunlap wrote:
> > On Thu, 6 Apr 2006 09:05:27 -0700 Greg KH wrote:
> > 
> > > On Thu, Apr 06, 2006 at 08:17:31PM +1000, Anton Blanchard wrote:
> > > > 
> > > > Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.
> > > > 
> > > > Signed-off-by: Anton Blanchard <anton@samba.org>
> > > > ---
> > > > 
> > > > Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
> > > > ===================================================================
> > > > --- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
> > > > +++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
> > > > @@ -38,10 +38,14 @@
> > > >  
> > > >  #include "../pci.h"
> 
> When one introdues relative apths like the above this is a good sign
> that the header file ought to move to a common place somewhere in
> include/.

No, this is a pci-core only header file.  I really don't want to have
these in include/linux/pci.h as no one other than the pci core, or pci
hotplug drivers need to use it.

I guess I could create,
include/linux/pci-core-only-dont-use-unless-you-really-know-what-you-are-doing.h
but that might be a bit rude :)

thanks,

greg k-h

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Sam Ravnborg]

On Thu, Apr 06, 2006 at 10:57:04AM -0700, Greg KH wrote:
> > > > >  #include "../pci.h"
> > 
> > When one introdues relative apths like the above this is a good sign
> > that the header file ought to move to a common place somewhere in
> > include/.
> 
> No, this is a pci-core only header file.  I really don't want to have
> these in include/linux/pci.h as no one other than the pci core, or pci
> hotplug drivers need to use it.

But that hold true for other stuff in include/* also.

The guideline is (my understanding):
- Use .h files only when declarations are shared by more than one .c
  file
- Put the .h file in same dir as the .c files, iff the .c files are all
  in same dir (and include using #include "file.h")
- For bigger subsystems create an include/<subsystem> dir for shared .h
  files (and include using #include <file.h>)
- For smaller subsystems create an include/linux/<subsystem> dir for
  shared .h files (and include using #include <file.h>)

And then we also have:
- For Greg's pci-core keep the shared .h file with the .c files
  (and include using #include "../file.h")

See why this sticks out a bit.

Not that I imply the above guidlines are strictly followed - but thats
the best I have seen.

	Sam

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Greg KH]

On Thu, Apr 06, 2006 at 10:41:13AM -0700, Randy.Dunlap wrote:
> On Thu, 6 Apr 2006 09:05:27 -0700 Greg KH wrote:
> 
> > On Thu, Apr 06, 2006 at 08:17:31PM +1000, Anton Blanchard wrote:
> > > 
> > > Wrap some ACPI specific headers. ACPI hasnt taken over the whole world yet.
> > > 
> > > Signed-off-by: Anton Blanchard <anton@samba.org>
> > > ---
> > > 
> > > Index: kernel/drivers/pci/hotplug/pciehp_hpc.c
> > > ===================================================================
> > > --- kernel.orig/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:01:32.000000000 -0500
> > > +++ kernel/drivers/pci/hotplug/pciehp_hpc.c	2006-04-06 05:09:48.501122395 -0500
> > > @@ -38,10 +38,14 @@
> > >  
> > >  #include "../pci.h"
> > >  #include "pciehp.h"
> > > +
> > > +#ifdef CONFIG_ACPI
> > >  #include <acpi/acpi.h>
> > >  #include <acpi/acpi_bus.h>
> > >  #include <acpi/actypes.h>
> > >  #include <linux/pci-acpi.h>
> > > +#endif
> > 
> > Shouldn't the ACPI headers handle it if CONFIG_ACPI is not enabled?  All
> > other header files work that way, and we shouldn't have to add this to
> > the .c files.
> 
> maybe the C file could just #include <linux/acpi.h> ?

Would that solve this issue?  I'm guessing that they are being included
as it needs something in those headers...

thanks,

greg k-h

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Dave Hansen]

On Thu, 2006-04-06 at 11:27 -0700, Greg KH wrote:
> Would that solve this issue?  I'm guessing that they are being included
> as it needs something in those headers... 

There's another #ifdef CONFIG_ACPI area in the .c file.  I doubt
anything else in there really needs ACPI.  Maybe the #ifdef'd area could
go into its own _acpi.c file?

-- Dave

Re: [PATCH] Fix pciehp driver on non ACPI systems

[Greg KH]

On Thu, Apr 06, 2006 at 08:14:22PM +0200, Sam Ravnborg wrote:
> On Thu, Apr 06, 2006 at 10:57:04AM -0700, Greg KH wrote:
> > > > > >  #include "../pci.h"
> > > 
> > > When one introdues relative apths like the above this is a good sign
> > > that the header file ought to move to a common place somewhere in
> > > include/.
> > 
> > No, this is a pci-core only header file.  I really don't want to have
> > these in include/linux/pci.h as no one other than the pci core, or pci
> > hotplug drivers need to use it.
> 
> But that hold true for other stuff in include/* also.
> 
> The guideline is (my understanding):
> - Use .h files only when declarations are shared by more than one .c
>   file
> - Put the .h file in same dir as the .c files, iff the .c files are all
>   in same dir (and include using #include "file.h")
> - For bigger subsystems create an include/<subsystem> dir for shared .h
>   files (and include using #include <file.h>)
> - For smaller subsystems create an include/linux/<subsystem> dir for
>   shared .h files (and include using #include <file.h>)
> 
> And then we also have:
> - For Greg's pci-core keep the shared .h file with the .c files
>   (and include using #include "../file.h")

Ok, sometimes I feel special, but never that "special" :)  If you note,
USB also does this for its core files, so there is precidence...

Anyway, is include/linux/pci/pci.h really necessary for just one file?
I guess I could put the msi stuff in there, but again, I really don't
want any driver including it, like they have tried to do so in the
past...

thanks,

greg k-h